Fix for #1124: don't remove semicolon followed by '(' or '{' or '['

groovy · Jun 16, 2020 · 3211f8f · 3211f8f
1 parent 6b1cc50
commit 3211f8f
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 33 deletions.
diff --git a/...t/src/org/codehaus/groovy/eclipse/refactoring/test/formatter/SemicolonRemoverTests.groovy b/...t/src/org/codehaus/groovy/eclipse/refactoring/test/formatter/SemicolonRemoverTests.groovy
@@ -1,11 +1,11 @@
 /*
- * Copyright 2009-2017 the original author or authors.
+ * Copyright 2009-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *     https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -108,6 +108,7 @@ final class SemicolonRemoverTests {
         assertContentChangedFromTo('def a = 10;', 'def a = 10')
         assertContentChangedFromTo('def a = {};', 'def a = {}')
         assertContentChangedFromTo('def a = [];', 'def a = []')
+        assertContentChangedFromTo('def a = x;;', 'def a = x')
     }
 
     @Test
@@ -128,6 +129,22 @@ final class SemicolonRemoverTests {
         assertContentChangedFromTo('def a = 1;\ndef b = 2;', 'def a = 1\ndef b = 2')
     }
 
+    @Test
+    void testClosureOnNextLine1() {
+        assertContentUnchanged '''\
+            def a = m();
+            { -> print a }
+            '''
+    }
+
+    @Test
+    void testClosureOnNextLine2() {
+        assertContentUnchanged '''\
+            def b = '123';
+            { -> b = 123 }
+            '''
+    }
+
     @Test
     void testSelection_ifNothingIsSelected_theWholeDocumentShouldBeFormatted() {
         assertSelectedContentChangedFromTo(null, 'a = [{ 1; }, { 2; }];', 'a = [{ 1 }, { 2 }]')

diff --git a/...e.refactoring/src/org/codehaus/groovy/eclipse/refactoring/formatter/SemicolonRemover.java b/...e.refactoring/src/org/codehaus/groovy/eclipse/refactoring/formatter/SemicolonRemover.java
@@ -1,11 +1,11 @@
 /*
- * Copyright 2009-2019 the original author or authors.
+ * Copyright 2009-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *     https://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -15,9 +15,6 @@
  */
 package org.codehaus.groovy.eclipse.refactoring.formatter;
 
-import java.util.Arrays;
-import java.util.List;
-
 import groovyjarjarantlr.Token;
 import org.codehaus.groovy.antlr.GroovyTokenTypeBridge;
 import org.codehaus.groovy.eclipse.core.GroovyCore;
@@ -30,18 +27,18 @@
 import org.eclipse.text.edits.TextEdit;
 
 /**
- * Removes trailing semicolons as they are optional in Groovy.
+ * Removes trailing semicolons as they are mostly optional in Groovy.
  */
 public class SemicolonRemover extends GroovyFormatter {
 
-    private final GroovyDocumentScanner scanner;
     private final MultiTextEdit edits;
+    private final GroovyDocumentScanner scanner;
 
-    public SemicolonRemover(ITextSelection sel, IDocument doc) {
+    public SemicolonRemover(final ITextSelection sel, final IDocument doc) {
         this(sel, doc, new MultiTextEdit());
     }
 
-    public SemicolonRemover(ITextSelection sel, IDocument doc, MultiTextEdit edits) {
+    public SemicolonRemover(final ITextSelection sel, final IDocument doc, final MultiTextEdit edits) {
         super(sel, doc);
         this.edits = edits;
         this.scanner = new GroovyDocumentScanner(doc);
@@ -50,12 +47,14 @@ public SemicolonRemover(ITextSelection sel, IDocument doc, MultiTextEdit edits)
     @Override
     public TextEdit format() {
         try {
-            List<Token> tokens = scanner.getTokens(selection);
-            for (Token token : tokens) {
-                Token nextToken = scanner.getNextToken(token);
-
-                if (isSemicolon(token) && (nextToken == null || isDelimiter(nextToken)))
-                    addSemicolonRemoval(token);
+            for (Token token : scanner.getTokens(selection)) {
+                if (isOptionalSemicolon(token)) {
+                    TextEdit removeSemicolon = new DeleteEdit(scanner.getOffset(token), 1);
+                    try {
+                        edits.addChild(removeSemicolon);
+                    } catch (MalformedTreeException ignore) {
+                    }
+                }
             }
         } catch (BadLocationException e) {
             GroovyCore.logException("Cannot perform semicolon removal.", e);
@@ -66,22 +65,23 @@ public TextEdit format() {
         return edits;
     }
 
-    private boolean isSemicolon(Token token) {
-        return token != null && token.getType() == GroovyTokenTypeBridge.SEMI;
-    }
-
-    private boolean isDelimiter(Token token) {
-        List<Integer> delimiterTypes = Arrays.asList(GroovyTokenTypeBridge.RCURLY, GroovyTokenTypeBridge.NLS, GroovyTokenTypeBridge.EOF);
-        return token != null && delimiterTypes.contains(token.getType());
-    }
-
-    private void addSemicolonRemoval(Token semicolonToken) throws BadLocationException {
-        int semicolonOffset = scanner.getOffset(semicolonToken);
-        TextEdit deleteSemicolon = new DeleteEdit(semicolonOffset, 1);
-        try {
-            edits.addChild(deleteSemicolon);
-        } catch (MalformedTreeException e) {
-            GroovyCore.logWarning("Ignoring conflicting edit: " + deleteSemicolon, e);
+    private boolean isOptionalSemicolon(Token token) throws BadLocationException {
+        if (token != null && token.getType() == GroovyTokenTypeBridge.SEMI) {
+            token = scanner.getNextToken(token);
+            if (token != null) {
+                int type = token.getType();
+                if (type == GroovyTokenTypeBridge.NLS) {
+                    while ((token = scanner.getNextToken(token)) != null &&
+                        (type = token.getType()) == GroovyTokenTypeBridge.NLS) {
+                    }
+                    // semicolon may prevent treating next expression as method call argument
+                    if (type != GroovyTokenTypeBridge.LPAREN && type != GroovyTokenTypeBridge.LCURLY && type != GroovyTokenTypeBridge.LBRACK) {
+                        type = GroovyTokenTypeBridge.NLS;
+                    }
+                }
+                return type == GroovyTokenTypeBridge.RCURLY || type == GroovyTokenTypeBridge.SEMI || type == GroovyTokenTypeBridge.NLS || type == GroovyTokenTypeBridge.EOF;
+            }
         }
+        return false;
     }
 }