diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/ASTNodeSourceLocationsTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/ASTNodeSourceLocationsTests.java index 42912faa28..1b2bdf9ce4 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/ASTNodeSourceLocationsTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/locations/ASTNodeSourceLocationsTests.java @@ -16,392 +16,214 @@ package org.eclipse.jdt.core.groovy.tests.locations; import static org.eclipse.jdt.groovy.core.tests.GroovyBundle.isParrotParser; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; import groovy.lang.GroovyClassLoader; import org.codehaus.groovy.ast.ASTNode; -import org.codehaus.groovy.ast.ClassCodeVisitorSupport; -import org.codehaus.groovy.ast.ClassNode; -import org.codehaus.groovy.ast.ImportNode; -import org.codehaus.groovy.ast.ModuleNode; import org.codehaus.groovy.ast.expr.BinaryExpression; +import org.codehaus.groovy.ast.expr.BitwiseNegationExpression; import org.codehaus.groovy.ast.expr.CastExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.ListExpression; import org.codehaus.groovy.ast.expr.MapEntryExpression; import org.codehaus.groovy.ast.expr.MapExpression; +import org.codehaus.groovy.ast.expr.NotExpression; +import org.codehaus.groovy.ast.expr.PostfixExpression; +import org.codehaus.groovy.ast.expr.PrefixExpression; +import org.codehaus.groovy.ast.expr.RangeExpression; +import org.codehaus.groovy.ast.expr.TernaryExpression; +import org.codehaus.groovy.ast.expr.UnaryMinusExpression; +import org.codehaus.groovy.ast.expr.UnaryPlusExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.AssertStatement; +import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.ErrorCollector; import org.codehaus.groovy.control.SourceUnit; -import org.eclipse.jdt.core.groovy.tests.builder.BuilderTestSuite; -import org.eclipse.jdt.groovy.core.util.GroovyUtils; -import org.junit.Ignore; +import org.codehaus.groovy.eclipse.core.util.VisitCompleteException; +import org.eclipse.jdt.groovy.core.util.DepthFirstVisitor; +import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; - -/** - * Test the source locations of ASTNodes to ensure they are correct, especially - * look into the changes that we force into them. - */ -public final class ASTNodeSourceLocationsTests extends BuilderTestSuite { - - private static class StartAndEnd { - final int start; - final int end; - public StartAndEnd(int start, int end) { - this.start = start; - this.end = end; - } - - public StartAndEnd(ASTNode node) { - this(node.getStart(), node.getEnd()); - } - - boolean isOK(ASTNode node) { - return node.getStart() == start && node.getEnd() == end; - } - - @Override - public String toString() { - return "[start=" + start + ", end=" + end + "]"; - } - } - - private static abstract class AbstractSLocTester extends ClassCodeVisitorSupport { - List allCollectedNodes = new ArrayList<>(); - - void doTest(ModuleNode module, StartAndEnd...sae) { - for (ClassNode c : (Iterable) module.getClasses()) { - this.visitClass(c); - } - assertStartAndEnds(sae); - } - - void assertStartAndEnds(StartAndEnd...sae) { - assertEquals("Wrong number expressions found", sae.length, allCollectedNodes.size()); - ASTNode[] bexprs = allCollectedNodes.toArray(new ASTNode[0]); - List problemIndices = new ArrayList<>(); - for (int i = 0; i < bexprs.length; i++) { - if (! sae[i].isOK(bexprs[i])) { - problemIndices.add(i); - } - } - - if (problemIndices.size() > 0) { - StringBuilder sb = new StringBuilder(); - for (Integer integer : problemIndices) { - int val = integer.intValue(); - sb.append("Expected slocs at " + sae[val] + " for expression " + bexprs[val] + - "but instead found: " + new StartAndEnd(bexprs[val]) + "\n"); - } - fail(sb.toString()); - } - } - } - - private static class ImportStatementSLocTester extends AbstractSLocTester { - @Override - public void visitImports(ModuleNode module) { - for (ImportNode node : GroovyUtils.getAllImportNodes(module)) { - allCollectedNodes.add(node); - } - } - } - - private static class BinaryExpressionSLocTester extends AbstractSLocTester { - @Override - public void visitBinaryExpression(BinaryExpression expression) { - super.visitBinaryExpression(expression); - allCollectedNodes.add(expression); - } - } - - private static class MapExpressionSLocTester extends AbstractSLocTester { - @Override - public void visitMapExpression(MapExpression expression) { - super.visitMapExpression(expression); - allCollectedNodes.add(expression); - } - } - - private static class MapEntryExpressionSLocTester extends AbstractSLocTester { - @Override - public void visitMapEntryExpression(MapEntryExpression expression) { - super.visitMapEntryExpression(expression); - allCollectedNodes.add(expression); - } +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public final class ASTNodeSourceLocationsTests { + + @Parameters(name = "{index}. {0}") + public static Object[][] params() { + return new Object[][] { + {" x ", "x", VariableExpression.class}, + {" (x) ", isParrotParser() ? "(x)" : "x", VariableExpression.class}, + + {" !x ", "!x", NotExpression.class}, + {" (!x) ", isParrotParser() ? "(!x)" : "(!x) ", NotExpression.class}, + + {" ++x ", "++x", PrefixExpression.class}, + {" (++x) ", isParrotParser() ? "(++x)" : "(++x) ", PrefixExpression.class}, + + {" --x ", "--x", PrefixExpression.class}, + {" (--x) ", isParrotParser() ? "(--x)" : "(--x) ", PrefixExpression.class}, + + {" x++ ", "x++", PostfixExpression.class}, + {" (x++) ", isParrotParser() ? "(x++)" : "(x++) ", PostfixExpression.class}, + + {" x-- ", "x--", PostfixExpression.class}, + {" (x--) ", isParrotParser() ? "(x--)" : "(x--) ", PostfixExpression.class}, + + {" +x ", "+x", UnaryPlusExpression.class}, + {" (+x) ", isParrotParser() ? "(+x)" : "(+x) ", UnaryPlusExpression.class}, + + {" -x ", "-x", UnaryMinusExpression.class}, + {" (-x) ", isParrotParser() ? "(-x)" : "(-x) ", UnaryMinusExpression.class}, + + {" ~x ", "~x", BitwiseNegationExpression.class}, + {" (~x) ", isParrotParser() ? "(~x)" : "(~x) ", BitwiseNegationExpression.class}, + + {"def n = 1; ", "1", ConstantExpression.class}, + {"def n = -1; ", "-1", ConstantExpression.class}, + {"def n = +1; ", "+1", ConstantExpression.class}, + {"def n = 1.2; ", "1.2", ConstantExpression.class}, + {"def n = -1.2; ", "-1.2", ConstantExpression.class}, + {"def n = +1.2; ", "+1.2", ConstantExpression.class}, + {"def n = 1.2d;", "1.2d", ConstantExpression.class}, + {"def n = -1.2d;", "-1.2d", ConstantExpression.class}, + {"def n = +1.2d;", "+1.2d", ConstantExpression.class}, + {"def n = (1) ", isParrotParser() ? "(1)" : "1", ConstantExpression.class}, + + {"def map = [:]\n", "def map = [:]", BinaryExpression.class}, + {"def map;\n" + "map = [:]\n", "map = [:]", BinaryExpression.class}, + {"def foo = [1, 2] as Set\n", "def foo = [1, 2] as Set", BinaryExpression.class}, + {"def foo;\n" + "foo == [1, 2] as Set\n", "foo == [1, 2] as Set", BinaryExpression.class}, + {"def foo;\n" + "(foo == [1, 2] as Set)\n", "(foo == [1, 2] as Set)", BinaryExpression.class}, + {"def foo;\n" + "((foo == [1, 2] as Set))\n", "((foo == [1, 2] as Set))", BinaryExpression.class}, + {"[:] + [:] + [:]\n", "[:] + [:]", BinaryExpression.class}, + {"[:] + [:] + [:]\n", "[:] + [:] + [:]", BinaryExpression.class}, + {"[:] << [:] + [:]\n", "[:] + [:]", BinaryExpression.class}, + {"[:] << [:] + [:]\n", "[:] << [:] + [:]", BinaryExpression.class}, + {"def x = a == b ", "a == b", BinaryExpression.class}, + {"def x = ( a == b )", "( a == b )", BinaryExpression.class}, + {"def x = ( a == b ) ", isParrotParser() ? "( a == b )" : "( a == b ) ", BinaryExpression.class}, + {"def x = ( a ) == b\n", "( a ) == b", BinaryExpression.class}, + {"def x = c == ( d )\n", "c == ( d )", BinaryExpression.class}, + {"def x = a[b]\n", "a[b]", BinaryExpression.class}, + /*{"def x = (a)[b]\n", "(a)[b]", BinaryExpression.class}, parsed as cast of list*/ + {"def x = a[(b)]\n", "a[(b)]", BinaryExpression.class}, + {"def x = (a[b])\n", "(a[b])", BinaryExpression.class}, + {"Map m\n m['k']\n", "m['k']", BinaryExpression.class}, + {"def x = ( a ) instanceof b\n", "( a ) instanceof b", BinaryExpression.class}, + {"def x = ( a instanceof b )\n", "( a instanceof b )", BinaryExpression.class}, + {"def x = ( ( a ) instanceof b )\n", "( ( a ) instanceof b )", BinaryExpression.class}, + + {"def x = a ? b : c\n", "a ? b : c", TernaryExpression.class}, + {"def x = (a) ? b : c\n", "(a) ? b : c", TernaryExpression.class}, + {"def x = a ? (b) : c\n", "a ? (b) : c", TernaryExpression.class}, + {"def x = a ? b : (c)\n", "a ? b : (c)", TernaryExpression.class}, + {"def x = (a ? b : c)\n", "(a ? b : c)", TernaryExpression.class}, + {"def x = ((a) ? b : c)\n", "((a) ? b : c)", TernaryExpression.class}, + {"def x = (a ? (b) : c)\n", "(a ? (b) : c)", TernaryExpression.class}, + {"def x = (a ? b : (c))\n", "(a ? b : (c))", TernaryExpression.class}, + {"def x = (1) ? 2 : (3)\n", "(1) ? 2 : (3)", TernaryExpression.class}, + {"def x = (1) ?: (3)\n", "(1) ?: (3)", TernaryExpression.class}, + + {"def r = 1..2\n", "1..2", RangeExpression.class}, + {"def r = (1)..2\n", "(1)..2", RangeExpression.class}, + {"def r = 1..(2)\n", "1..(2)", RangeExpression.class}, + {"def r = (1..2)\n", "(1..2)", RangeExpression.class}, + {"def r = a..b\n", "a..b", RangeExpression.class}, + {"def r = (a)..b\n", "(a)..b", RangeExpression.class}, + {"def r = a..(b)\n", "a..(b)", RangeExpression.class}, + {"def r = (a..b)\n", "(a..b)", RangeExpression.class}, + + {"(Set) foo", "(Set) foo", CastExpression.class}, + {"def x = ( Set ) foo ", "( Set ) foo", CastExpression.class}, + {"foo as Set", "foo as Set", CastExpression.class}, + {"def x = foo as Set ", "foo as Set", CastExpression.class}, + {"def x = foo as Set\r\n", "foo as Set", CastExpression.class}, + + {"[]", "[]", ListExpression.class}, + {"[ ]", "[ ]", ListExpression.class}, + {"def list = []", "[]", ListExpression.class}, + {"def list = [ ]", "[ ]", ListExpression.class}, + {"def list = [ 1, 2,\n'3' ]", "[ 1, 2,\n'3' ]", ListExpression.class}, + + // GRECLIPSE-768 -- empty maps + {"[:]", "[:]", MapExpression.class}, + {"[ : ]", "[ : ]", MapExpression.class}, + {"def map = [:]", "[:]", MapExpression.class}, + {"def map = [ : ]", "[ : ]", MapExpression.class}, + + {"[a:b]", "a:b", MapEntryExpression.class}, + {"[a : b]", "a : b", MapEntryExpression.class}, + {"[a : b ]", "a : b", MapEntryExpression.class}, + {"[a : b, c : d]", "a : b", MapEntryExpression.class}, + {"[a : b, c : d]", "c : d", MapEntryExpression.class}, + {"def m = [a : b, c : d]", "a : b", MapEntryExpression.class}, + {"def m = [a : b, c : d]", "c : d", MapEntryExpression.class}, + {"def m = [a : b] << [ c : d]", "a : b", MapEntryExpression.class}, + {"def m = [a : b] << [ c : d]", "c : d", MapEntryExpression.class}, + {"def m = [a : b, e : [ c : d]]", "a : b", MapEntryExpression.class}, + {"def m = [a : b, e : [ c : d]]", "c : d", MapEntryExpression.class}, + {"def m = [a : b, e : [ c : d]]", "e : [ c : d]", MapEntryExpression.class}, + + {"def meth() {\n then:\n assert x == 9\n}", "assert x == 9", AssertStatement.class}, // GRECLIPSE-1270 + + // TODO: package, imports (incl. aliases), annotations, anon. inners, + }; } - private static class CastExpressionSLocTester extends AbstractSLocTester { - @Override - public void visitCastExpression(CastExpression expression) { - super.visitCastExpression(expression); - allCollectedNodes.add(expression); - } - } + //-------------------------------------------------------------------------- - private static class AssertStatementSLocTester extends AbstractSLocTester { - @Override - public void visitAssertStatement(AssertStatement statement) { - super.visitAssertStatement(statement); - allCollectedNodes.add(statement); - } - } + @Parameter(0) + public String source; - private static void checkBinaryExprSLocs(String contents, AbstractSLocTester tester, String... exprStrings) throws Exception { - StartAndEnd[] points = convertToPoints(contents, exprStrings); - ModuleNode module = createModuleNodeFor(contents); - tester.doTest(module, points); - } + @Parameter(1) + public String target; - private static void checkBinaryExprSLocsReverse(String contents, String...exprStrings) throws Exception { - StartAndEnd[] points = convertToPoints(contents, exprStrings); - List list = Arrays.asList(points); - Collections.reverse(list); - points = list.toArray(points); - ModuleNode module = createModuleNodeFor(contents); - BinaryExpressionSLocTester tester = new BinaryExpressionSLocTester(); - tester.doTest(module, points); - } + @Parameter(2) + public Class targetType; - private static StartAndEnd[] convertToPoints(String contents, String[] exprStrings) { - StartAndEnd[] points = new StartAndEnd[exprStrings.length]; - int prevEnd = 0; - int prevStart = 0; - for (int i = 0; i < exprStrings.length; i++) { - int start = contents.indexOf(exprStrings[i], prevEnd); - if (start == -1) { - // now try prevStart - start = contents.indexOf(exprStrings[i], prevStart); - if (start == -1) { - // now try from the beginning - start = contents.indexOf(exprStrings[i]); - if (start == -1) { - fail("Could not find exprString"); - } - } - } - int end = start + exprStrings[i].length(); - points[i] = new StartAndEnd(start, end); - prevStart = start; - prevEnd = end + 1; - } - return points; - } - - private static ModuleNode createModuleNodeFor(String contents) throws Exception { - SourceUnit sourceUnit = new SourceUnit("Foo", contents, new CompilerConfiguration(), new GroovyClassLoader(), new ErrorCollector(new CompilerConfiguration())); + @Test + public void testSourceLocations() throws Exception { + CompilerConfiguration config = new CompilerConfiguration(); + SourceUnit sourceUnit = new SourceUnit("TestUnit", source, config, new GroovyClassLoader(), new ErrorCollector(config)); sourceUnit.parse(); sourceUnit.completePhase(); sourceUnit.convert(); - return sourceUnit.getAST(); - } - - //-------------------------------------------------------------------------- - - @Test - public void testBinaryExpr1() throws Exception { - checkBinaryExprSLocs( - "def map = [:]\n" + - "map = [:]", - new BinaryExpressionSLocTester(), - "def map = [:]", "map = [:]"); - } - - @Test - public void testBinaryExpr2() throws Exception { - checkBinaryExprSLocs( - "def foo = [1, 2] as Set\n" + - "foo == [1, 2] as Set", - new BinaryExpressionSLocTester(), - "def foo = [1, 2] as Set", - "foo == [1, 2] as Set"); - } - - @Test - public void testBinaryExpr3() throws Exception { - checkBinaryExprSLocs( - "(foo == [1, 2] as Set)", - new BinaryExpressionSLocTester(), - "(foo == [1, 2] as Set)"); - } - - @Test - public void testBinaryExpr4() throws Exception { - checkBinaryExprSLocs( - "((foo == [1, 2] as Set))", - new BinaryExpressionSLocTester(), - "((foo == [1, 2] as Set))"); - } - @Test - public void testBinaryExpr5() throws Exception { - checkBinaryExprSLocsReverse( - "[:] + [:] + [:]", - "[:] + [:] + [:]", - "[:] + [:]"); - } - - @Test - public void testBinaryExpr6() throws Exception { - checkBinaryExprSLocsReverse( - "[:] << [:] + [:]", - "[:] << [:] + [:]", - "[:] + [:]"); - } - - @Test - public void testBinaryExpr7() throws Exception { - checkBinaryExprSLocs( - " a = b ", - new BinaryExpressionSLocTester(), - isParrotParser() ? "a = b" : "a = b "); - } - - @Test - public void testMapExpression1() throws Exception { - checkBinaryExprSLocs( - "[:]", - new MapExpressionSLocTester(), - "[:]"); - } - - @Test - public void testMapExpression2() throws Exception { - checkBinaryExprSLocs( - "[ : ]", - new MapExpressionSLocTester(), - "[ : ]"); - } - - @Test - public void testMapExpression3() throws Exception { - checkBinaryExprSLocs( - "def x = [:]", - new MapExpressionSLocTester(), - "[:]"); - } - - @Test @Ignore("fails because we are not smart about how we extend slocs for empty map expressions") - public void testMapExpression4() throws Exception { - checkBinaryExprSLocs( - "def x = [ : ]", - new MapExpressionSLocTester(), - "[ : ]"); - } - - @Test - public void testMapEntryExpression1() throws Exception { - checkBinaryExprSLocs( - "[a:b]", - new MapEntryExpressionSLocTester(), - "a:b"); - } - - @Test - public void testMapEntryExpression2() throws Exception { - checkBinaryExprSLocs( - "[a : b]", - new MapEntryExpressionSLocTester(), - "a : b"); - } - - @Test // has extra whitespace at end, but should not - public void testMapEntryExpression3() throws Exception { - checkBinaryExprSLocs( - "[a : b ]", - new MapEntryExpressionSLocTester(), - "a : b"); - } - - @Test - public void testMapEntryExpression4() throws Exception { - checkBinaryExprSLocs( - "[a : b, c : d]", - new MapEntryExpressionSLocTester(), - "a : b", "c : d"); - } - - @Test - public void testMapEntryExpression5() throws Exception { - checkBinaryExprSLocs( - "def x = [a : b, c : d]", - new MapEntryExpressionSLocTester(), - "a : b", "c : d"); - } - - @Test - public void testMapEntryExpression6() throws Exception { - checkBinaryExprSLocs( - "def x = [a : b] << [ c : d]", - new MapEntryExpressionSLocTester(), - "a : b", "c : d"); - } - - @Test - public void testMapEntryExpression7() throws Exception { - checkBinaryExprSLocs( - "def x = [a : b, e : [ c : d]]", - new MapEntryExpressionSLocTester(), - "a : b", "c : d", "e : [ c : d]"); - } - - @Test - public void testCastExpression1() throws Exception { - checkBinaryExprSLocs( - "def x = ( Set ) foo ", - new CastExpressionSLocTester(), - "( Set ) foo"); - } - - @Test - public void testCastExpression2() throws Exception { - checkBinaryExprSLocs( - "foo as Set", - new CastExpressionSLocTester(), - "foo as Set"); - } - - @Test - public void testCastExpression3() throws Exception { - checkBinaryExprSLocs( - "def x = foo as Set", - new CastExpressionSLocTester(), - "foo as Set"); - } - - @Test - public void testCastExpression4() throws Exception { - checkBinaryExprSLocs( - "def x = foo as Set ", - new CastExpressionSLocTester(), - "foo as Set"); - } + final int offset = source.indexOf(target), length = target.length(); + Assume.assumeTrue(offset >= 0); + + try { + new DepthFirstVisitor() { + @Override + protected void visitExpression(Expression expression) { + if (expression.getEnd() > 0 && targetType.isInstance(expression)) { + if (expression.getStart() == offset && expression.getLength() == length) { + throw new VisitCompleteException("found expected expression"); + } + } + super.visitExpression(expression); + } - @Test - public void testImportStatement1() throws Exception { - checkBinaryExprSLocs( - "import javax.swing.*\n" + - "import javax.swing.JFrame\n" + - "import javax.applet.*;\n" + - "import javax.applet.Applet;\n", - new ImportStatementSLocTester(), - "import javax.swing.*", - "import javax.swing.JFrame", - "import javax.applet.*", - "import javax.applet.Applet"); - } + @Override + protected void visitStatement(Statement statement) { + if (statement.getEnd() > 0 && targetType.isInstance(statement)) { + if (statement.getStart() == offset && statement.getLength() == length) { + throw new VisitCompleteException("found expected statement"); + } + } + super.visitStatement(statement); + } + }.visitModule(sourceUnit.getAST()); - @Test // GRECLIPSE-1270 - public void testAssertStatement1() throws Exception { - checkBinaryExprSLocs( - "def meth() {\n then:\n assert x == 9\n}", - new AssertStatementSLocTester(), - "assert x == 9"); + Assert.fail("Expected to find " + targetType.getSimpleName() + " at " + offset + ".." + (offset + length)); + } catch (VisitCompleteException complete) { + // success + } } } diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java index 4daa4bb556..c4be1e5e27 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java @@ -15,8 +15,6 @@ */ package org.eclipse.jdt.groovy.core.tests.xform; -import static org.eclipse.jdt.groovy.core.tests.GroovyBundle.isParrotParser; - import org.eclipse.jdt.groovy.core.tests.basic.GroovyCompilerTestSuite; import org.junit.Ignore; import org.junit.Test; @@ -48,7 +46,7 @@ public void testTypeChecked1() { "----------\n" + "2. ERROR in Foo.groovy (at line 5)\n" + "\tprintln \"Did you spot the error in this ${message.toUppercase()}?\"\n" + - "\t " + (isParrotParser() ? " ^^^^^^^^^^^^^^^^^^^^^" : "^^^^^^^^^^^^^^^^^^^^^^^") + "\n" + + "\t ^^^^^^^^^^^^^^^^^^^^^\n" + "Groovy:[Static type checking] - Cannot find matching method java.lang.String#toUppercase(). Please check if the declared type is correct and if the method exists.\n" + "----------\n"); } diff --git a/base/org.codehaus.groovy24/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy24/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index d302227025..0f543c33a3 100644 --- a/base/org.codehaus.groovy24/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy24/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -1799,9 +1799,7 @@ protected Statement forStatement(AST forNode) { forParameter.setNameEnd(forParameter.getEnd()); if (type.getStart() > 0) { // set start of parameter node to start of parameter type - forParameter.setStart(type.getStart()); - forParameter.setLineNumber(type.getLineNumber()); - forParameter.setColumnNumber(type.getColumnNumber()); + setSourceStart(forParameter, type); } // GRECLIPSE end } @@ -2134,10 +2132,8 @@ protected Expression expression(AST node) { } protected Expression expression(AST node, boolean convertToConstant) { - // GRECLIPSE add - // error recovery for missing brackets - // determine if this error handling strategy is the right one to use - if (node == null) { + // GRECLIPSE add -- error recovery for missing brackets, missing statements and do + if (node == null || node.getType() == EMPTY_STAT || node.getType() == UNUSED_DO) { return new ConstantExpression("ERROR"); } // GRECLIPSE end @@ -2150,12 +2146,14 @@ protected Expression expression(AST node, boolean convertToConstant) { VariableExpression ve = (VariableExpression) expression; if (!ve.isThisExpression() && !ve.isSuperExpression()) { expression = new ConstantExpression(ve.getName()); + // GRECLIPSE add + configureAST(expression, node); + // GRECLIPSE end } } - // GRECLIPSE add -- expressionSwitch > blockExpression can return NULL for an empty GString expression - if (expression != ConstantExpression.NULL) - // GRECLIPSE end + /* GRECLIPSE edit -- each case of expressionSwitch does this already configureAST(expression, node); + */ return expression; } @@ -2280,6 +2278,9 @@ protected Expression expressionSwitch(AST node) { case LNOT: NotExpression notExpression = new NotExpression(expression(node.getFirstChild())); configureAST(notExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(notExpression, notExpression.getExpression()); + // GRECLIPSE end return notExpression; case UNARY_MINUS: @@ -2288,6 +2289,9 @@ protected Expression expressionSwitch(AST node) { case BNOT: BitwiseNegationExpression bitwiseNegationExpression = new BitwiseNegationExpression(expression(node.getFirstChild())); configureAST(bitwiseNegationExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(bitwiseNegationExpression, bitwiseNegationExpression.getExpression()); + // GRECLIPSE end return bitwiseNegationExpression; case UNARY_PLUS: @@ -2461,7 +2465,12 @@ protected Expression expressionSwitch(AST node) { return binaryExpression(Types.KEYWORD_IN, node); case ANNOTATION: - return new AnnotationConstantExpression(annotation(node)); + // GRECLIPSE edit + //return new AnnotationConstantExpression(annotation(node)); + expression = new AnnotationConstantExpression(annotation(node)); + configureAST(expression, node); + return expression; + // GRECLIPSE end case CLOSURE_LIST: return closureListExpression(node); @@ -2474,12 +2483,9 @@ protected Expression expressionSwitch(AST node) { return anonymousInnerClassDef(node); default: - // GRECLIPSE add - return - // GRECLIPSE end unknownAST(node); } - //return null; + return null; } private TupleExpression tupleExpression(AST node) { @@ -2545,15 +2551,10 @@ protected Expression ternaryExpression(AST ternaryNode) { booleanExpression.setSourcePosition(base); ret = new TernaryExpression(booleanExpression, left, right); } - // GRECLIPSE edit -- sloc was for operator only; TODO include surrounding parentheses + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions //configureAST(ret, ternaryNode); - ret.setStart(base.getStart()); - ret.setLineNumber(base.getLineNumber()); - ret.setColumnNumber(base.getColumnNumber()); - Expression rhs = ((TernaryExpression) ret).getFalseExpression(); - ret.setEnd(rhs.getEnd()); - ret.setLastLineNumber(rhs.getLastLineNumber()); - ret.setLastColumnNumber(rhs.getLastColumnNumber()); + setSourceStart(ret, base); + setSourceEnd(ret, ((TernaryExpression) ret).getFalseExpression()); // GRECLIPSE end return ret; } @@ -2580,7 +2581,11 @@ protected Expression rangeExpression(AST rangeNode, boolean inclusive) { Expression left = expression(node); Expression right = expression(node.getNextSibling()); RangeExpression rangeExpression = new RangeExpression(left, right, inclusive); - configureAST(rangeExpression, rangeNode); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(rangeExpression, rangeNode); + setSourceStart(rangeExpression, left); + setSourceEnd(rangeExpression, right); + // GRECLIPSE end return rangeExpression; } @@ -2639,9 +2644,6 @@ protected Expression listExpression(AST listNode) { return listExpression; } - /** - * Typically only used for map constructors I think? - */ protected Expression mapExpression(AST mapNode) { List expressions = new ArrayList(); AST elist = mapNode.getFirstChild(); @@ -2664,12 +2666,23 @@ protected Expression mapExpression(AST mapNode) { } MapExpression mapExpression = new MapExpression(expressions); configureAST(mapExpression, mapNode); - // GRECLIPSE add - // GRECLIPSE-768 check to see if the sloc is definitely wrong for empty map expressions - // if so, make an educated guess that the text looks like '[:]' + // GRECLIPSE-768 -- fix for empty map expressions if (expressions.isEmpty() && mapExpression.getLength() <= 1) { - mapExpression.setEnd(mapExpression.getStart() + 3); - mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + if (getController() != null) { + try (Reader r = getController().getSource().getReader()) { + r.skip(mapExpression.getStart()); + while (r.read() != ']') { + mapExpression.setEnd(mapExpression.getEnd() + 1); + } + int[] row_col = locations.getRowCol(mapExpression.getEnd()); + mapExpression.setLastColumnNumber(row_col[1]); + mapExpression.setLastLineNumber(row_col[0]); + } catch (Exception e) { + // make an educated guess that map looks like '[:]' + mapExpression.setEnd(mapExpression.getStart() + 3); + mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + } + } } // GRECLIPSE end return mapExpression; @@ -2689,17 +2702,11 @@ protected MapEntryExpression mapEntryExpression(AST node) { AST valueNode = keyNode.getNextSibling(); Expression valueExpression = expression(valueNode); MapEntryExpression mapEntryExpression = new MapEntryExpression(keyExpression, valueExpression); - // GRECLIPSE add - // GRECLIPSE-768 MapEntryExpression have the slocs of the ':' only; fix that here. - mapEntryExpression.setStart(keyExpression.getStart()); - mapEntryExpression.setLineNumber(keyExpression.getLineNumber()); - mapEntryExpression.setColumnNumber(keyExpression.getColumnNumber()); - mapEntryExpression.setEnd(valueExpression.getEnd()); - mapEntryExpression.setLastLineNumber(valueExpression.getLastLineNumber()); - mapEntryExpression.setLastColumnNumber(valueExpression.getLastColumnNumber()); - // potentially re-set the sloc if the 'real' one is more correct + // GRECLIPSE edit -- sloc for node only covers the ':'; must include the expressions + //configureAST(mapEntryExpression, node); + setSourceStart(mapEntryExpression, keyExpression); + setSourceEnd(mapEntryExpression, valueExpression); // GRECLIPSE end - configureAST(mapEntryExpression, node); return mapEntryExpression; } } @@ -2715,7 +2722,11 @@ protected Expression instanceofExpression(AST node) { Expression rightExpression = new ClassExpression(type); configureAST(rightExpression, rightNode); BinaryExpression binaryExpression = new BinaryExpression(leftExpression, makeToken(Types.KEYWORD_INSTANCEOF, node), rightExpression); - configureAST(binaryExpression, node); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(binaryExpression, node); + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); + // GRECLIPSE end return binaryExpression; } @@ -2841,32 +2852,8 @@ protected Expression binaryExpression(int type, AST node) { BinaryExpression binaryExpression = new BinaryExpression(leftExpression, token, rightExpression); // GRECLIPSE edit -- sloc for node only covers the operator; must include the left and right expressions //configureAST(binaryExpression, node); - Long offsets = leftExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets >> 32); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setStart(offset); - binaryExpression.setLineNumber(row_col[0]); - binaryExpression.setColumnNumber(row_col[1]); - } else { - binaryExpression.setStart(leftExpression.getStart()); - binaryExpression.setLineNumber(leftExpression.getLineNumber()); - binaryExpression.setColumnNumber(leftExpression.getColumnNumber()); - } - offsets = rightExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets & 0xFFFFFFFF); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setEnd(offset); - binaryExpression.setLastLineNumber(row_col[0]); - binaryExpression.setLastColumnNumber(row_col[1]); - } else { - binaryExpression.setEnd(rightExpression.getEnd()); - binaryExpression.setLastLineNumber(rightExpression.getLastLineNumber()); - binaryExpression.setLastColumnNumber(rightExpression.getLastColumnNumber()); - } + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); // GRECLIPSE end return binaryExpression; } @@ -2875,6 +2862,9 @@ protected Expression prefixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PrefixExpression prefixExpression = new PrefixExpression(makeToken(token, node), expression); configureAST(prefixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceEnd(prefixExpression, expression); + // GRECLIPSE end return prefixExpression; } @@ -2882,6 +2872,9 @@ protected Expression postfixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PostfixExpression postfixExpression = new PostfixExpression(expression, makeToken(token, node)); configureAST(postfixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceStart(postfixExpression, expression); + // GRECLIPSE end return postfixExpression; } @@ -3043,13 +3036,9 @@ protected Expression methodCallExpression(AST methodCallNode) { ret.setNameEnd(name.getEnd() - 1); // in the case of command expressions, the slocs are incorrect for the start of the method if (!implicitThis && methodCallNode.getText().equals("")) { - ret.setStart(objectExpression.getStart()); - ret.setLineNumber(objectExpression.getLineNumber()); - ret.setColumnNumber(objectExpression.getColumnNumber()); - ret.setEnd(arguments.getEnd()); - ret.setLastLineNumber(arguments.getLastLineNumber()); - ret.setLastColumnNumber(arguments.getLastColumnNumber()); - } + setSourceStart(ret, objectExpression); + setSourceEnd(ret, arguments); + } else // GRECLIPSE end configureAST(ret, methodCallNode); return ret; @@ -3323,6 +3312,9 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_BIG_DECIMAL: ConstantExpression constantExpression = new ConstantExpression(Numbers.parseDecimal("-" + text)); configureAST(constantExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantExpression, expression(node)); + // GRECLIPSE end return constantExpression; case NUM_BIG_INT: @@ -3330,17 +3322,24 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_LONG: ConstantExpression constantLongExpression = new ConstantExpression(Numbers.parseInteger("-" + text)); configureAST(constantLongExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantLongExpression, expression(node)); + // GRECLIPSE end return constantLongExpression; default: UnaryMinusExpression unaryMinusExpression = new UnaryMinusExpression(expression(node)); configureAST(unaryMinusExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(unaryMinusExpression, unaryMinusExpression.getExpression()); + // GRECLIPSE end return unaryMinusExpression; } } protected Expression unaryPlusExpression(AST unaryPlusExpr) { AST node = unaryPlusExpr.getFirstChild(); + /* GRECLIPSE edit switch (node.getType()) { case NUM_DOUBLE: case NUM_FLOAT: @@ -3355,6 +3354,24 @@ protected Expression unaryPlusExpression(AST unaryPlusExpr) { configureAST(unaryPlusExpression, unaryPlusExpr); return unaryPlusExpression; } + */ + UnaryPlusExpression unaryPlusExpression = new UnaryPlusExpression(expression(node)); + configureAST(unaryPlusExpression, unaryPlusExpr); + switch (node.getType()) { + case NUM_DOUBLE: + case NUM_FLOAT: + case NUM_BIG_DECIMAL: + case NUM_BIG_INT: + case NUM_INT: + case NUM_LONG: + setSourceStart(unaryPlusExpression.getExpression(), unaryPlusExpression); + return unaryPlusExpression.getExpression(); + + default: + setSourceEnd(unaryPlusExpression, unaryPlusExpression.getExpression()); + return unaryPlusExpression; + } + // GRECLIPSE end } protected ConstantExpression decimalExpression(AST node) { @@ -3686,34 +3703,16 @@ protected void configureAST(ASTNode node, AST ast) { final int offset = locations.findOffset(ast.getLine(), ast.getColumn()); if (ast instanceof GroovySourceAST) { - int last_row = ((GroovySourceAST) ast).getLineLast(); - int last_col = ((GroovySourceAST) ast).getColumnLast(); - if ((ast.getType() == UNARY_MINUS || ast.getType() == UNARY_PLUS) && - node instanceof ConstantExpression) { // extend for literal - last_row = ((GroovySourceAST) ast.getFirstChild()).getLineLast(); - last_col = ((GroovySourceAST) ast.getFirstChild()).getColumnLast(); - } + final int last_row = ((GroovySourceAST) ast).getLineLast(); + final int last_col = ((GroovySourceAST) ast).getColumnLast(); final int end_offset = locations.findOffset(last_row, last_col); - // GRECLIPSE-768: Only reset the sloc for these kinds of expressions - // if the new sloc is wider than the old. - if ((node instanceof BinaryExpression || - node instanceof TernaryExpression || - node instanceof MapEntryExpression || - node instanceof MapExpression || - node instanceof CastExpression || - node instanceof MethodCall) && - node.getStart() <= offset && node.getEnd() >= end_offset) { - // sloc has already been set and is same as or wider than ast's - return; - } - // GRECLIPSE-829: VariableExpression inside of GStrings contain the // openning '{', but shouldn't. If the new sloc is larger than the - // one being set, then ignore it and don't reset. Also numbers can + // one being set, then save it as node meta-data. Also numbers can // result in an expression node that includes trailing whitespaces. if ((node instanceof VariableExpression || (node instanceof ConstantExpression && ast.getType() == EXPR)) && - node.getEnd() > 0 && offset <= node.getStart() && end_offset >= node.getEnd()) { + node.getEnd() > 0 && offset <= node.getStart() && node.getEnd() <= end_offset) { node.putNodeMetaData("source.offsets", Long.valueOf(((long) offset << 32) | end_offset)); return; } @@ -3737,6 +3736,38 @@ protected void configureAST(AnnotatedNode node, AST ast, AST name0, AST name1) { node.setNameEnd(locations.findOffset(nameStop.getLineLast(), nameStop.getColumnLast()) - 1); } + protected void setSourceStart(ASTNode node, ASTNode first) { + Long offsets = first.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets >> 32); + int[] row_col = locations.getRowCol(offset); + + node.setStart(offset); + node.setLineNumber(row_col[0]); + node.setColumnNumber(row_col[1]); + } else { + node.setStart(first.getStart()); + node.setLineNumber(first.getLineNumber()); + node.setColumnNumber(first.getColumnNumber()); + } + } + + protected void setSourceEnd(ASTNode node, ASTNode last) { + Long offsets = last.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets & 0xFFFFFFFF); + int[] row_col = locations.getRowCol(offset); + + node.setEnd(offset); + node.setLastLineNumber(row_col[0]); + node.setLastColumnNumber(row_col[1]); + } else { + node.setEnd(last.getEnd()); + node.setLastLineNumber(last.getLastLineNumber()); + node.setLastColumnNumber(last.getLastColumnNumber()); + } + } + protected static AnnotationNode makeAnnotationNode(String name) { AnnotationNode node = new AnnotationNode(ClassHelper.make(name)); node.getClassNode().setStart(-1); @@ -3793,8 +3824,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "AST node not implemented yet for type: " + getTokenName(node)); } - // GRECLIPSE edit - protected /*void*/Expression unknownAST(AST node) { + protected void unknownAST(AST node) { if (node.getType() == CLASS_DEF) { throw new ASTRuntimeException(node, "Class definition not expected here. Please define the class at an appropriate place or perhaps try using a block/Closure instead."); @@ -3803,10 +3833,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "Method definition not expected here. Please define the method at an appropriate place or perhaps try using a block/Closure instead."); } - // GRECLIPSE edit - //throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); - return new ConstantExpression("ERROR"); - // GRECLIPSE end + throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); } protected void dumpTree(AST ast) { diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index ffedb5f1ce..c21afc13a2 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -1813,9 +1813,7 @@ protected Statement forStatement(AST forNode) { forParameter.setNameEnd(forParameter.getEnd()); if (type.getStart() > 0) { // set start of parameter node to start of parameter type - forParameter.setStart(type.getStart()); - forParameter.setLineNumber(type.getLineNumber()); - forParameter.setColumnNumber(type.getColumnNumber()); + setSourceStart(forParameter, type); } // GRECLIPSE end } @@ -2147,10 +2145,8 @@ protected Expression expression(AST node) { } protected Expression expression(AST node, boolean convertToConstant) { - // GRECLIPSE add - // error recovery for missing brackets - // determine if this error handling strategy is the right one to use - if (node == null) { + // GRECLIPSE add -- error recovery for missing brackets, missing statements and do + if (node == null || node.getType() == EMPTY_STAT || node.getType() == UNUSED_DO) { return new ConstantExpression("ERROR"); } // GRECLIPSE end @@ -2163,12 +2159,14 @@ protected Expression expression(AST node, boolean convertToConstant) { VariableExpression ve = (VariableExpression) expression; if (!ve.isThisExpression() && !ve.isSuperExpression()) { expression = new ConstantExpression(ve.getName()); + // GRECLIPSE add + configureAST(expression, node); + // GRECLIPSE end } } - // GRECLIPSE add -- expressionSwitch > blockExpression can return NULL for an empty GString expression - if (expression != ConstantExpression.NULL) - // GRECLIPSE end + /* GRECLIPSE edit -- each case of expressionSwitch does this already configureAST(expression, node); + */ return expression; } @@ -2293,6 +2291,9 @@ protected Expression expressionSwitch(AST node) { case LNOT: NotExpression notExpression = new NotExpression(expression(node.getFirstChild())); configureAST(notExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(notExpression, notExpression.getExpression()); + // GRECLIPSE end return notExpression; case UNARY_MINUS: @@ -2301,6 +2302,9 @@ protected Expression expressionSwitch(AST node) { case BNOT: BitwiseNegationExpression bitwiseNegationExpression = new BitwiseNegationExpression(expression(node.getFirstChild())); configureAST(bitwiseNegationExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(bitwiseNegationExpression, bitwiseNegationExpression.getExpression()); + // GRECLIPSE end return bitwiseNegationExpression; case UNARY_PLUS: @@ -2474,7 +2478,12 @@ protected Expression expressionSwitch(AST node) { return binaryExpression(Types.KEYWORD_IN, node); case ANNOTATION: - return new AnnotationConstantExpression(annotation(node)); + // GRECLIPSE edit + //return new AnnotationConstantExpression(annotation(node)); + expression = new AnnotationConstantExpression(annotation(node)); + configureAST(expression, node); + return expression; + // GRECLIPSE end case CLOSURE_LIST: return closureListExpression(node); @@ -2487,12 +2496,9 @@ protected Expression expressionSwitch(AST node) { return anonymousInnerClassDef(node); default: - // GRECLIPSE add - return - // GRECLIPSE end unknownAST(node); } - //return null; + return null; } private TupleExpression tupleExpression(AST node) { @@ -2558,15 +2564,10 @@ protected Expression ternaryExpression(AST ternaryNode) { booleanExpression.setSourcePosition(base); ret = new TernaryExpression(booleanExpression, left, right); } - // GRECLIPSE edit -- sloc was for operator only; TODO include surrounding parentheses + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions //configureAST(ret, ternaryNode); - ret.setStart(base.getStart()); - ret.setLineNumber(base.getLineNumber()); - ret.setColumnNumber(base.getColumnNumber()); - Expression rhs = ((TernaryExpression) ret).getFalseExpression(); - ret.setEnd(rhs.getEnd()); - ret.setLastLineNumber(rhs.getLastLineNumber()); - ret.setLastColumnNumber(rhs.getLastColumnNumber()); + setSourceStart(ret, base); + setSourceEnd(ret, ((TernaryExpression) ret).getFalseExpression()); // GRECLIPSE end return ret; } @@ -2593,7 +2594,11 @@ protected Expression rangeExpression(AST rangeNode, boolean inclusive) { Expression left = expression(node); Expression right = expression(node.getNextSibling()); RangeExpression rangeExpression = new RangeExpression(left, right, inclusive); - configureAST(rangeExpression, rangeNode); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(rangeExpression, rangeNode); + setSourceStart(rangeExpression, left); + setSourceEnd(rangeExpression, right); + // GRECLIPSE end return rangeExpression; } @@ -2652,9 +2657,6 @@ protected Expression listExpression(AST listNode) { return listExpression; } - /** - * Typically only used for map constructors I think? - */ protected Expression mapExpression(AST mapNode) { List expressions = new ArrayList(); AST elist = mapNode.getFirstChild(); @@ -2677,12 +2679,23 @@ protected Expression mapExpression(AST mapNode) { } MapExpression mapExpression = new MapExpression(expressions); configureAST(mapExpression, mapNode); - // GRECLIPSE add - // GRECLIPSE-768 check to see if the sloc is definitely wrong for empty map expressions - // if so, make an educated guess that the text looks like '[:]' + // GRECLIPSE-768 -- fix for empty map expressions if (expressions.isEmpty() && mapExpression.getLength() <= 1) { - mapExpression.setEnd(mapExpression.getStart() + 3); - mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + if (getController() != null) { + try (Reader r = getController().getSource().getReader()) { + r.skip(mapExpression.getStart()); + while (r.read() != ']') { + mapExpression.setEnd(mapExpression.getEnd() + 1); + } + int[] row_col = locations.getRowCol(mapExpression.getEnd()); + mapExpression.setLastColumnNumber(row_col[1]); + mapExpression.setLastLineNumber(row_col[0]); + } catch (Exception e) { + // make an educated guess that map looks like '[:]' + mapExpression.setEnd(mapExpression.getStart() + 3); + mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + } + } } // GRECLIPSE end return mapExpression; @@ -2702,17 +2715,11 @@ protected MapEntryExpression mapEntryExpression(AST node) { AST valueNode = keyNode.getNextSibling(); Expression valueExpression = expression(valueNode); MapEntryExpression mapEntryExpression = new MapEntryExpression(keyExpression, valueExpression); - // GRECLIPSE add - // GRECLIPSE-768 MapEntryExpression have the slocs of the ':' only; fix that here. - mapEntryExpression.setStart(keyExpression.getStart()); - mapEntryExpression.setLineNumber(keyExpression.getLineNumber()); - mapEntryExpression.setColumnNumber(keyExpression.getColumnNumber()); - mapEntryExpression.setEnd(valueExpression.getEnd()); - mapEntryExpression.setLastLineNumber(valueExpression.getLastLineNumber()); - mapEntryExpression.setLastColumnNumber(valueExpression.getLastColumnNumber()); - // potentially re-set the sloc if the 'real' one is more correct + // GRECLIPSE edit -- sloc for node only covers the ':'; must include the expressions + //configureAST(mapEntryExpression, node); + setSourceStart(mapEntryExpression, keyExpression); + setSourceEnd(mapEntryExpression, valueExpression); // GRECLIPSE end - configureAST(mapEntryExpression, node); return mapEntryExpression; } } @@ -2728,7 +2735,11 @@ protected Expression instanceofExpression(AST node) { Expression rightExpression = new ClassExpression(type); configureAST(rightExpression, rightNode); BinaryExpression binaryExpression = new BinaryExpression(leftExpression, makeToken(Types.KEYWORD_INSTANCEOF, node), rightExpression); - configureAST(binaryExpression, node); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(binaryExpression, node); + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); + // GRECLIPSE end return binaryExpression; } @@ -2860,32 +2871,8 @@ protected Expression binaryExpression(int type, AST node) { BinaryExpression binaryExpression = new BinaryExpression(leftExpression, token, rightExpression); // GRECLIPSE edit -- sloc for node only covers the operator; must include the left and right expressions //configureAST(binaryExpression, node); - Long offsets = leftExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets >> 32); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setStart(offset); - binaryExpression.setLineNumber(row_col[0]); - binaryExpression.setColumnNumber(row_col[1]); - } else { - binaryExpression.setStart(leftExpression.getStart()); - binaryExpression.setLineNumber(leftExpression.getLineNumber()); - binaryExpression.setColumnNumber(leftExpression.getColumnNumber()); - } - offsets = rightExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets & 0xFFFFFFFF); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setEnd(offset); - binaryExpression.setLastLineNumber(row_col[0]); - binaryExpression.setLastColumnNumber(row_col[1]); - } else { - binaryExpression.setEnd(rightExpression.getEnd()); - binaryExpression.setLastLineNumber(rightExpression.getLastLineNumber()); - binaryExpression.setLastColumnNumber(rightExpression.getLastColumnNumber()); - } + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); // GRECLIPSE end return binaryExpression; } @@ -2894,6 +2881,9 @@ protected Expression prefixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PrefixExpression prefixExpression = new PrefixExpression(makeToken(token, node), expression); configureAST(prefixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceEnd(prefixExpression, expression); + // GRECLIPSE end return prefixExpression; } @@ -2901,6 +2891,9 @@ protected Expression postfixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PostfixExpression postfixExpression = new PostfixExpression(expression, makeToken(token, node)); configureAST(postfixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceStart(postfixExpression, expression); + // GRECLIPSE end return postfixExpression; } @@ -3064,13 +3057,9 @@ protected Expression methodCallExpression(AST methodCallNode) { ret.setNameEnd(name.getEnd() - 1); // in the case of command expressions, the slocs are incorrect for the start of the method if (!implicitThis && methodCallNode.getText().equals("")) { - ret.setStart(objectExpression.getStart()); - ret.setLineNumber(objectExpression.getLineNumber()); - ret.setColumnNumber(objectExpression.getColumnNumber()); - ret.setEnd(arguments.getEnd()); - ret.setLastLineNumber(arguments.getLastLineNumber()); - ret.setLastColumnNumber(arguments.getLastColumnNumber()); - } + setSourceStart(ret, objectExpression); + setSourceEnd(ret, arguments); + } else // GRECLIPSE end configureAST(ret, methodCallNode); return ret; @@ -3344,6 +3333,9 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_BIG_DECIMAL: ConstantExpression constantExpression = new ConstantExpression(Numbers.parseDecimal("-" + text)); configureAST(constantExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantExpression, expression(node)); + // GRECLIPSE end return constantExpression; case NUM_BIG_INT: @@ -3351,17 +3343,24 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_LONG: ConstantExpression constantLongExpression = new ConstantExpression(Numbers.parseInteger(unaryMinusExpr,"-" + text)); configureAST(constantLongExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantLongExpression, expression(node)); + // GRECLIPSE end return constantLongExpression; default: UnaryMinusExpression unaryMinusExpression = new UnaryMinusExpression(expression(node)); configureAST(unaryMinusExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(unaryMinusExpression, unaryMinusExpression.getExpression()); + // GRECLIPSE end return unaryMinusExpression; } } protected Expression unaryPlusExpression(AST unaryPlusExpr) { AST node = unaryPlusExpr.getFirstChild(); + /* GRECLIPSE edit switch (node.getType()) { case NUM_DOUBLE: case NUM_FLOAT: @@ -3376,6 +3375,24 @@ protected Expression unaryPlusExpression(AST unaryPlusExpr) { configureAST(unaryPlusExpression, unaryPlusExpr); return unaryPlusExpression; } + */ + UnaryPlusExpression unaryPlusExpression = new UnaryPlusExpression(expression(node)); + configureAST(unaryPlusExpression, unaryPlusExpr); + switch (node.getType()) { + case NUM_DOUBLE: + case NUM_FLOAT: + case NUM_BIG_DECIMAL: + case NUM_BIG_INT: + case NUM_INT: + case NUM_LONG: + setSourceStart(unaryPlusExpression.getExpression(), unaryPlusExpression); + return unaryPlusExpression.getExpression(); + + default: + setSourceEnd(unaryPlusExpression, unaryPlusExpression.getExpression()); + return unaryPlusExpression; + } + // GRECLIPSE end } protected ConstantExpression decimalExpression(AST node) { @@ -3723,34 +3740,16 @@ protected void configureAST(ASTNode node, AST ast) { final int offset = locations.findOffset(ast.getLine(), ast.getColumn()); if (ast instanceof GroovySourceAST) { - int last_row = ((GroovySourceAST) ast).getLineLast(); - int last_col = ((GroovySourceAST) ast).getColumnLast(); - if ((ast.getType() == UNARY_MINUS || ast.getType() == UNARY_PLUS) && - node instanceof ConstantExpression) { // extend for literal - last_row = ((GroovySourceAST) ast.getFirstChild()).getLineLast(); - last_col = ((GroovySourceAST) ast.getFirstChild()).getColumnLast(); - } + final int last_row = ((GroovySourceAST) ast).getLineLast(); + final int last_col = ((GroovySourceAST) ast).getColumnLast(); final int end_offset = locations.findOffset(last_row, last_col); - // GRECLIPSE-768: Only reset the sloc for these kinds of expressions - // if the new sloc is wider than the old. - if ((node instanceof BinaryExpression || - node instanceof TernaryExpression || - node instanceof MapEntryExpression || - node instanceof MapExpression || - node instanceof CastExpression || - node instanceof MethodCall) && - node.getStart() <= offset && node.getEnd() >= end_offset) { - // sloc has already been set and is same as or wider than ast's - return; - } - // GRECLIPSE-829: VariableExpression inside of GStrings contain the // openning '{', but shouldn't. If the new sloc is larger than the - // one being set, then ignore it and don't reset. Also numbers can + // one being set, then save it as node meta-data. Also numbers can // result in an expression node that includes trailing whitespaces. if ((node instanceof VariableExpression || (node instanceof ConstantExpression && ast.getType() == EXPR)) && - node.getEnd() > 0 && offset <= node.getStart() && end_offset >= node.getEnd()) { + node.getEnd() > 0 && offset <= node.getStart() && node.getEnd() <= end_offset) { node.putNodeMetaData("source.offsets", Long.valueOf(((long) offset << 32) | end_offset)); return; } @@ -3774,6 +3773,38 @@ protected void configureAST(AnnotatedNode node, AST ast, AST name0, AST name1) { node.setNameEnd(locations.findOffset(nameStop.getLineLast(), nameStop.getColumnLast()) - 1); } + protected void setSourceStart(ASTNode node, ASTNode first) { + Long offsets = first.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets >> 32); + int[] row_col = locations.getRowCol(offset); + + node.setStart(offset); + node.setLineNumber(row_col[0]); + node.setColumnNumber(row_col[1]); + } else { + node.setStart(first.getStart()); + node.setLineNumber(first.getLineNumber()); + node.setColumnNumber(first.getColumnNumber()); + } + } + + protected void setSourceEnd(ASTNode node, ASTNode last) { + Long offsets = last.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets & 0xFFFFFFFF); + int[] row_col = locations.getRowCol(offset); + + node.setEnd(offset); + node.setLastLineNumber(row_col[0]); + node.setLastColumnNumber(row_col[1]); + } else { + node.setEnd(last.getEnd()); + node.setLastLineNumber(last.getLastLineNumber()); + node.setLastColumnNumber(last.getLastColumnNumber()); + } + } + protected static AnnotationNode makeAnnotationNode(String name) { AnnotationNode node = new AnnotationNode(ClassHelper.make(name)); node.getClassNode().setStart(-1); @@ -3830,8 +3861,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "AST node not implemented yet for type: " + getTokenName(node)); } - // GRECLIPSE edit - protected /*void*/Expression unknownAST(AST node) { + protected void unknownAST(AST node) { if (node.getType() == CLASS_DEF) { throw new ASTRuntimeException(node, "Class definition not expected here. Please define the class at an appropriate place or perhaps try using a block/Closure instead."); @@ -3840,10 +3870,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "Method definition not expected here. Please define the method at an appropriate place or perhaps try using a block/Closure instead."); } - // GRECLIPSE edit - //throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); - return new ConstantExpression("ERROR"); - // GRECLIPSE end + throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); } protected void dumpTree(AST ast) { diff --git a/base/org.codehaus.groovy26/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy26/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index 17d3f74bc7..9e2023d94d 100644 --- a/base/org.codehaus.groovy26/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy26/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -1903,9 +1903,7 @@ protected Statement forStatement(AST forNode) { forParameter.setNameEnd(forParameter.getEnd()); if (type.getStart() > 0) { // set start of parameter node to start of parameter type - forParameter.setStart(type.getStart()); - forParameter.setLineNumber(type.getLineNumber()); - forParameter.setColumnNumber(type.getColumnNumber()); + setSourceStart(forParameter, type); } // GRECLIPSE end } @@ -2237,10 +2235,8 @@ protected Expression expression(AST node) { } protected Expression expression(AST node, boolean convertToConstant) { - // GRECLIPSE add - // error recovery for missing brackets - // determine if this error handling strategy is the right one to use - if (node == null) { + // GRECLIPSE add -- error recovery for missing brackets, missing statements and do + if (node == null || node.getType() == EMPTY_STAT || node.getType() == UNUSED_DO) { return new ConstantExpression("ERROR"); } // GRECLIPSE end @@ -2253,12 +2249,14 @@ protected Expression expression(AST node, boolean convertToConstant) { VariableExpression ve = (VariableExpression) expression; if (!ve.isThisExpression() && !ve.isSuperExpression()) { expression = new ConstantExpression(ve.getName()); + // GRECLIPSE add + configureAST(expression, node); + // GRECLIPSE end } } - // GRECLIPSE add -- expressionSwitch > blockExpression can return NULL for an empty GString expression - if (expression != ConstantExpression.NULL) - // GRECLIPSE end + /* GRECLIPSE edit -- each case of expressionSwitch does this already configureAST(expression, node); + */ return expression; } @@ -2383,6 +2381,9 @@ protected Expression expressionSwitch(AST node) { case LNOT: NotExpression notExpression = new NotExpression(expression(node.getFirstChild())); configureAST(notExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(notExpression, notExpression.getExpression()); + // GRECLIPSE end return notExpression; case UNARY_MINUS: @@ -2391,6 +2392,9 @@ protected Expression expressionSwitch(AST node) { case BNOT: BitwiseNegationExpression bitwiseNegationExpression = new BitwiseNegationExpression(expression(node.getFirstChild())); configureAST(bitwiseNegationExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator + setSourceEnd(bitwiseNegationExpression, bitwiseNegationExpression.getExpression()); + // GRECLIPSE end return bitwiseNegationExpression; case UNARY_PLUS: @@ -2564,7 +2568,12 @@ protected Expression expressionSwitch(AST node) { return binaryExpression(Types.KEYWORD_IN, node); case ANNOTATION: - return new AnnotationConstantExpression(annotation(node)); + // GRECLIPSE edit + //return new AnnotationConstantExpression(annotation(node)); + expression = new AnnotationConstantExpression(annotation(node)); + configureAST(expression, node); + return expression; + // GRECLIPSE end case CLOSURE_LIST: return closureListExpression(node); @@ -2577,12 +2586,9 @@ protected Expression expressionSwitch(AST node) { return anonymousInnerClassDef(node); default: - // GRECLIPSE add - return - // GRECLIPSE end unknownAST(node); } - //return null; + return null; } private TupleExpression tupleExpression(AST node) { @@ -2648,15 +2654,10 @@ protected Expression ternaryExpression(AST ternaryNode) { booleanExpression.setSourcePosition(base); ret = new TernaryExpression(booleanExpression, left, right); } - // GRECLIPSE edit -- sloc was for operator only; TODO include surrounding parentheses + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions //configureAST(ret, ternaryNode); - ret.setStart(base.getStart()); - ret.setLineNumber(base.getLineNumber()); - ret.setColumnNumber(base.getColumnNumber()); - Expression rhs = ((TernaryExpression) ret).getFalseExpression(); - ret.setEnd(rhs.getEnd()); - ret.setLastLineNumber(rhs.getLastLineNumber()); - ret.setLastColumnNumber(rhs.getLastColumnNumber()); + setSourceStart(ret, base); + setSourceEnd(ret, ((TernaryExpression) ret).getFalseExpression()); // GRECLIPSE end return ret; } @@ -2683,7 +2684,11 @@ protected Expression rangeExpression(AST rangeNode, boolean inclusive) { Expression left = expression(node); Expression right = expression(node.getNextSibling()); RangeExpression rangeExpression = new RangeExpression(left, right, inclusive); - configureAST(rangeExpression, rangeNode); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(rangeExpression, rangeNode); + setSourceStart(rangeExpression, left); + setSourceEnd(rangeExpression, right); + // GRECLIPSE end return rangeExpression; } @@ -2742,9 +2747,6 @@ protected Expression listExpression(AST listNode) { return listExpression; } - /** - * Typically only used for map constructors I think? - */ protected Expression mapExpression(AST mapNode) { List expressions = new ArrayList(); AST elist = mapNode.getFirstChild(); @@ -2767,12 +2769,23 @@ protected Expression mapExpression(AST mapNode) { } MapExpression mapExpression = new MapExpression(expressions); configureAST(mapExpression, mapNode); - // GRECLIPSE add - // GRECLIPSE-768 check to see if the sloc is definitely wrong for empty map expressions - // if so, make an educated guess that the text looks like '[:]' + // GRECLIPSE-768 -- fix for empty map expressions if (expressions.isEmpty() && mapExpression.getLength() <= 1) { - mapExpression.setEnd(mapExpression.getStart() + 3); - mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + if (getController() != null) { + try (Reader r = getController().getSource().getReader()) { + r.skip(mapExpression.getStart()); + while (r.read() != ']') { + mapExpression.setEnd(mapExpression.getEnd() + 1); + } + int[] row_col = locations.getRowCol(mapExpression.getEnd()); + mapExpression.setLastColumnNumber(row_col[1]); + mapExpression.setLastLineNumber(row_col[0]); + } catch (Exception e) { + // make an educated guess that map looks like '[:]' + mapExpression.setEnd(mapExpression.getStart() + 3); + mapExpression.setLastColumnNumber(mapExpression.getColumnNumber() + 3); + } + } } // GRECLIPSE end return mapExpression; @@ -2792,17 +2805,11 @@ protected MapEntryExpression mapEntryExpression(AST node) { AST valueNode = keyNode.getNextSibling(); Expression valueExpression = expression(valueNode); MapEntryExpression mapEntryExpression = new MapEntryExpression(keyExpression, valueExpression); - // GRECLIPSE add - // GRECLIPSE-768 MapEntryExpression have the slocs of the ':' only; fix that here. - mapEntryExpression.setStart(keyExpression.getStart()); - mapEntryExpression.setLineNumber(keyExpression.getLineNumber()); - mapEntryExpression.setColumnNumber(keyExpression.getColumnNumber()); - mapEntryExpression.setEnd(valueExpression.getEnd()); - mapEntryExpression.setLastLineNumber(valueExpression.getLastLineNumber()); - mapEntryExpression.setLastColumnNumber(valueExpression.getLastColumnNumber()); - // potentially re-set the sloc if the 'real' one is more correct + // GRECLIPSE edit -- sloc for node only covers the ':'; must include the expressions + //configureAST(mapEntryExpression, node); + setSourceStart(mapEntryExpression, keyExpression); + setSourceEnd(mapEntryExpression, valueExpression); // GRECLIPSE end - configureAST(mapEntryExpression, node); return mapEntryExpression; } } @@ -2818,7 +2825,11 @@ protected Expression instanceofExpression(AST node) { Expression rightExpression = new ClassExpression(type); configureAST(rightExpression, rightNode); BinaryExpression binaryExpression = new BinaryExpression(leftExpression, makeToken(Types.KEYWORD_INSTANCEOF, node), rightExpression); - configureAST(binaryExpression, node); + // GRECLIPSE edit -- sloc for node only covers the operator; must include the expressions + //configureAST(binaryExpression, node); + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); + // GRECLIPSE end return binaryExpression; } @@ -2950,32 +2961,8 @@ protected Expression binaryExpression(int type, AST node) { BinaryExpression binaryExpression = new BinaryExpression(leftExpression, token, rightExpression); // GRECLIPSE edit -- sloc for node only covers the operator; must include the left and right expressions //configureAST(binaryExpression, node); - Long offsets = leftExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets >> 32); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setStart(offset); - binaryExpression.setLineNumber(row_col[0]); - binaryExpression.setColumnNumber(row_col[1]); - } else { - binaryExpression.setStart(leftExpression.getStart()); - binaryExpression.setLineNumber(leftExpression.getLineNumber()); - binaryExpression.setColumnNumber(leftExpression.getColumnNumber()); - } - offsets = rightExpression.getNodeMetaData("source.offsets"); - if (offsets != null) { - int offset = (int) (offsets & 0xFFFFFFFF); - int[] row_col = locations.getRowCol(offset); - - binaryExpression.setEnd(offset); - binaryExpression.setLastLineNumber(row_col[0]); - binaryExpression.setLastColumnNumber(row_col[1]); - } else { - binaryExpression.setEnd(rightExpression.getEnd()); - binaryExpression.setLastLineNumber(rightExpression.getLastLineNumber()); - binaryExpression.setLastColumnNumber(rightExpression.getLastColumnNumber()); - } + setSourceStart(binaryExpression, leftExpression); + setSourceEnd(binaryExpression, rightExpression); // GRECLIPSE end return binaryExpression; } @@ -2984,6 +2971,9 @@ protected Expression prefixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PrefixExpression prefixExpression = new PrefixExpression(makeToken(token, node), expression); configureAST(prefixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceEnd(prefixExpression, expression); + // GRECLIPSE end return prefixExpression; } @@ -2991,6 +2981,9 @@ protected Expression postfixExpression(AST node, int token) { Expression expression = expression(node.getFirstChild()); PostfixExpression postfixExpression = new PostfixExpression(expression, makeToken(token, node)); configureAST(postfixExpression, node); + // GRECLIPSE add -- sloc for node only covers the operator; must include the expression + setSourceStart(postfixExpression, expression); + // GRECLIPSE end return postfixExpression; } @@ -3154,13 +3147,9 @@ protected Expression methodCallExpression(AST methodCallNode) { ret.setNameEnd(name.getEnd() - 1); // in the case of command expressions, the slocs are incorrect for the start of the method if (!implicitThis && methodCallNode.getText().equals("")) { - ret.setStart(objectExpression.getStart()); - ret.setLineNumber(objectExpression.getLineNumber()); - ret.setColumnNumber(objectExpression.getColumnNumber()); - ret.setEnd(arguments.getEnd()); - ret.setLastLineNumber(arguments.getLastLineNumber()); - ret.setLastColumnNumber(arguments.getLastColumnNumber()); - } + setSourceStart(ret, objectExpression); + setSourceEnd(ret, arguments); + } else // GRECLIPSE end configureAST(ret, methodCallNode); return ret; @@ -3434,6 +3423,9 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_BIG_DECIMAL: ConstantExpression constantExpression = new ConstantExpression(Numbers.parseDecimal("-" + text)); configureAST(constantExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantExpression, expression(node)); + // GRECLIPSE end return constantExpression; case NUM_BIG_INT: @@ -3441,17 +3433,24 @@ protected Expression unaryMinusExpression(AST unaryMinusExpr) { case NUM_LONG: ConstantExpression constantLongExpression = new ConstantExpression(Numbers.parseInteger(unaryMinusExpr,"-" + text)); configureAST(constantLongExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(constantLongExpression, expression(node)); + // GRECLIPSE end return constantLongExpression; default: UnaryMinusExpression unaryMinusExpression = new UnaryMinusExpression(expression(node)); configureAST(unaryMinusExpression, unaryMinusExpr); + // GRECLIPSE add + setSourceEnd(unaryMinusExpression, unaryMinusExpression.getExpression()); + // GRECLIPSE end return unaryMinusExpression; } } protected Expression unaryPlusExpression(AST unaryPlusExpr) { AST node = unaryPlusExpr.getFirstChild(); + /* GRECLIPSE edit switch (node.getType()) { case NUM_DOUBLE: case NUM_FLOAT: @@ -3466,6 +3465,24 @@ protected Expression unaryPlusExpression(AST unaryPlusExpr) { configureAST(unaryPlusExpression, unaryPlusExpr); return unaryPlusExpression; } + */ + UnaryPlusExpression unaryPlusExpression = new UnaryPlusExpression(expression(node)); + configureAST(unaryPlusExpression, unaryPlusExpr); + switch (node.getType()) { + case NUM_DOUBLE: + case NUM_FLOAT: + case NUM_BIG_DECIMAL: + case NUM_BIG_INT: + case NUM_INT: + case NUM_LONG: + setSourceStart(unaryPlusExpression.getExpression(), unaryPlusExpression); + return unaryPlusExpression.getExpression(); + + default: + setSourceEnd(unaryPlusExpression, unaryPlusExpression.getExpression()); + return unaryPlusExpression; + } + // GRECLIPSE end } protected ConstantExpression decimalExpression(AST node) { @@ -3813,34 +3830,16 @@ protected void configureAST(ASTNode node, AST ast) { final int offset = locations.findOffset(ast.getLine(), ast.getColumn()); if (ast instanceof GroovySourceAST) { - int last_row = ((GroovySourceAST) ast).getLineLast(); - int last_col = ((GroovySourceAST) ast).getColumnLast(); - if ((ast.getType() == UNARY_MINUS || ast.getType() == UNARY_PLUS) && - node instanceof ConstantExpression) { // extend for literal - last_row = ((GroovySourceAST) ast.getFirstChild()).getLineLast(); - last_col = ((GroovySourceAST) ast.getFirstChild()).getColumnLast(); - } + final int last_row = ((GroovySourceAST) ast).getLineLast(); + final int last_col = ((GroovySourceAST) ast).getColumnLast(); final int end_offset = locations.findOffset(last_row, last_col); - // GRECLIPSE-768: Only reset the sloc for these kinds of expressions - // if the new sloc is wider than the old. - if ((node instanceof BinaryExpression || - node instanceof TernaryExpression || - node instanceof MapEntryExpression || - node instanceof MapExpression || - node instanceof CastExpression || - node instanceof MethodCall) && - node.getStart() <= offset && node.getEnd() >= end_offset) { - // sloc has already been set and is same as or wider than ast's - return; - } - // GRECLIPSE-829: VariableExpression inside of GStrings contain the // openning '{', but shouldn't. If the new sloc is larger than the - // one being set, then ignore it and don't reset. Also numbers can + // one being set, then save it as node meta-data. Also numbers can // result in an expression node that includes trailing whitespaces. if ((node instanceof VariableExpression || (node instanceof ConstantExpression && ast.getType() == EXPR)) && - node.getEnd() > 0 && offset <= node.getStart() && end_offset >= node.getEnd()) { + node.getEnd() > 0 && offset <= node.getStart() && node.getEnd() <= end_offset) { node.putNodeMetaData("source.offsets", Long.valueOf(((long) offset << 32) | end_offset)); return; } @@ -3864,6 +3863,38 @@ protected void configureAST(AnnotatedNode node, AST ast, AST name0, AST name1) { node.setNameEnd(locations.findOffset(nameStop.getLineLast(), nameStop.getColumnLast()) - 1); } + protected void setSourceStart(ASTNode node, ASTNode first) { + Long offsets = first.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets >> 32); + int[] row_col = locations.getRowCol(offset); + + node.setStart(offset); + node.setLineNumber(row_col[0]); + node.setColumnNumber(row_col[1]); + } else { + node.setStart(first.getStart()); + node.setLineNumber(first.getLineNumber()); + node.setColumnNumber(first.getColumnNumber()); + } + } + + protected void setSourceEnd(ASTNode node, ASTNode last) { + Long offsets = last.getNodeMetaData("source.offsets"); + if (offsets != null) { + int offset = (int) (offsets & 0xFFFFFFFF); + int[] row_col = locations.getRowCol(offset); + + node.setEnd(offset); + node.setLastLineNumber(row_col[0]); + node.setLastColumnNumber(row_col[1]); + } else { + node.setEnd(last.getEnd()); + node.setLastLineNumber(last.getLastLineNumber()); + node.setLastColumnNumber(last.getLastColumnNumber()); + } + } + protected static AnnotationNode makeAnnotationNode(String name) { AnnotationNode node = new AnnotationNode(ClassHelper.make(name)); node.getClassNode().setStart(-1); @@ -3920,8 +3951,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "AST node not implemented yet for type: " + getTokenName(node)); } - // GRECLIPSE edit - protected /*void*/Expression unknownAST(AST node) { + protected void unknownAST(AST node) { if (node.getType() == CLASS_DEF) { throw new ASTRuntimeException(node, "Class definition not expected here. Please define the class at an appropriate place or perhaps try using a block/Closure instead."); @@ -3930,10 +3960,7 @@ protected void notImplementedYet(AST node) { throw new ASTRuntimeException(node, "Method definition not expected here. Please define the method at an appropriate place or perhaps try using a block/Closure instead."); } - // GRECLIPSE edit - //throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); - return new ConstantExpression("ERROR"); - // GRECLIPSE end + throw new ASTRuntimeException(node, "Unknown type: " + getTokenName(node)); } protected void dumpTree(AST ast) { diff --git a/ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/QuickAssistTests.groovy b/ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/QuickAssistTests.groovy index 1578c6f9e1..842a2b6e28 100644 --- a/ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/QuickAssistTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/QuickAssistTests.groovy @@ -795,6 +795,538 @@ final class QuickAssistTests extends QuickFixTestSuite { @Test void testInlineLocalVariable12() { + String original = '''\ + def x = y[0] + x.intValue() + '''.stripIndent() + + String expected = '''\ + y[0].intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable13() { + String original = '''\ + def x = (Object) y + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((Object) y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable14() { + String original = '''\ + def x = ((Object) y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((Object) y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable15() { + String original = '''\ + def x = y as Object + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y as Object).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable16() { + String original = '''\ + def x = (y as Object) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y as Object).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable17() { + String original = '''\ + def x = y ?: 0 + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y ?: 0).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable18() { + String original = '''\ + def x = (y ?: 0) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y ?: 0).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable19() { + String original = '''\ + def x = y ? 1 : 0 + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y ? 1 : 0).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable20() { + String original = '''\ + def x = (y ? 1 : 0) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y ? 1 : 0).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable21() { + String original = '''\ + def x = (y) ? 1 : 0 + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((y) ? 1 : 0).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable22() { + String original = '''\ + def x = y ? 1 : (0) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y ? 1 : (0)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable23() { + String original = '''\ + def x = (y) ? (1) : (0) + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((y) ? (1) : (0)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable24() { + String original = '''\ + def x = y++ + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y++).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable25() { + String original = '''\ + def x = (y++) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y++).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable26() { + String original = '''\ + def x = y-- + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y--).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable27() { + String original = '''\ + def x = (y--) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (y--).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable28() { + String original = '''\ + def x = ++y + x.intValue() + '''.stripIndent() + + String expected = '''\ + (++y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable29() { + String original = '''\ + def x = (++y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (++y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable30() { + String original = '''\ + def x = --y + x.intValue() + '''.stripIndent() + + String expected = '''\ + (--y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable31() { + String original = '''\ + def x = (--y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (--y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable32() { + String original = '''\ + def x = !y + x.intValue() + '''.stripIndent() + + String expected = '''\ + (!y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable33() { + String original = '''\ + def x = (!y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (!y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable34() { + String original = '''\ + def x = -y + x.intValue() + '''.stripIndent() + + String expected = '''\ + (-y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable35() { + String original = '''\ + def x = (-y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (-y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable36() { + String original = '''\ + def x = +y + x.intValue() + '''.stripIndent() + + String expected = '''\ + (+y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable37() { + String original = '''\ + def x = (+y) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (+y).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable38() { + String original = '''\ + def x = ~/y/ + x.intValue() + '''.stripIndent() + + String expected = '''\ + (~/y/).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable39() { + String original = '''\ + def x = (~/y/) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (~/y/).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable40() { + String original = '''\ + def x = 0..1 + x.intValue() + '''.stripIndent() + + String expected = '''\ + (0..1).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable41() { + String original = '''\ + def x = (0)..1 + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((0)..1).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable42() { + String original = '''\ + def x = 0..(1) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (0..(1)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable43() { + String original = '''\ + def x = (0)..(1) + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((0)..(1)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable44() { + String original = '''\ + def x = (0..1) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (0..1).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable45() { + String original = '''\ + def x = ((0)..1) + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((0)..1).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable46() { + String original = '''\ + def x = (0..(1)) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (0..(1)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable47() { + String original = '''\ + def x = ((0)..(1)) + x.intValue() + '''.stripIndent() + + String expected = '''\ + ((0)..(1)).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable48() { + String original = '''\ + def x = a[1] + x.intValue() + '''.stripIndent() + + String expected = '''\ + a[1].intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable49() { + String original = '''\ + def x = (a[1]) + x.intValue() + '''.stripIndent() + + String expected = '''\ + (a[1]).intValue() + '''.stripIndent() + + assertConversion(original, expected, 'x', new InlineLocalVariableProposal()) + } + + @Test + void testInlineLocalVariable50() { String original = '''\ def x = 1 x.intValue() @@ -810,7 +1342,7 @@ final class QuickAssistTests extends QuickFixTestSuite { } @Test - void testInlineLocalVariable13() { + void testInlineLocalVariable51() { String original = '''\ def x = 1 x.intValue() @@ -836,7 +1368,7 @@ final class QuickAssistTests extends QuickFixTestSuite { } @Test - void testInlineLocalVariable14() { + void testInlineLocalVariable52() { String original = '''\ def x = 1 def y = x = 2 @@ -852,7 +1384,7 @@ final class QuickAssistTests extends QuickFixTestSuite { } @Test @NotYetImplemented - void testInlineLocalVariable15() { + void testInlineLocalVariable53() { String original = '''\ def x = 1, y = 2 def sum = x + y @@ -867,7 +1399,7 @@ final class QuickAssistTests extends QuickFixTestSuite { } @Test @NotYetImplemented - void testInlineLocalVariable16() { + void testInlineLocalVariable54() { String original = '''\ def x = 1, y = 2 def sum = x + y diff --git a/ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickassist/proposals/InlineLocalVariableProposal.java b/ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickassist/proposals/InlineLocalVariableProposal.java index 7711540fbd..42621ff07c 100644 --- a/ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickassist/proposals/InlineLocalVariableProposal.java +++ b/ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickassist/proposals/InlineLocalVariableProposal.java @@ -20,10 +20,18 @@ import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.CodeVisitorSupport; import org.codehaus.groovy.ast.expr.BinaryExpression; +import org.codehaus.groovy.ast.expr.BitwiseNegationExpression; +import org.codehaus.groovy.ast.expr.BooleanExpression; +import org.codehaus.groovy.ast.expr.CastExpression; import org.codehaus.groovy.ast.expr.ClosureListExpression; import org.codehaus.groovy.ast.expr.DeclarationExpression; +import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.ast.expr.PostfixExpression; import org.codehaus.groovy.ast.expr.PrefixExpression; +import org.codehaus.groovy.ast.expr.RangeExpression; +import org.codehaus.groovy.ast.expr.TernaryExpression; +import org.codehaus.groovy.ast.expr.UnaryMinusExpression; +import org.codehaus.groovy.ast.expr.UnaryPlusExpression; import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.eclipse.quickassist.GroovyQuickAssistProposal2; import org.codehaus.groovy.syntax.Types; @@ -80,15 +88,50 @@ public int getRelevance() { } private String getValueExpression() { - String value = context.getNodeText(variableDeclaration.getRightExpression()); - if (variableDeclaration.getRightExpression() instanceof BinaryExpression) { - BinaryExpression valExp = (BinaryExpression) variableDeclaration.getRightExpression(); - if (valExp.getStart() == startOffset(valExp.getLeftExpression()) || - valExp.getEnd() == endOffset(valExp.getRightExpression())) { - value = "(" + value + ")"; + Expression valueExpression = variableDeclaration.getRightExpression(); + + boolean needsParens = false; + if (valueExpression instanceof BinaryExpression) { + BinaryExpression expr = (BinaryExpression) valueExpression; + if (expr.getOperation().getType() != Types.LEFT_SQUARE_BRACKET) + if (expr.getStart() == startOffset(expr.getLeftExpression()) || + expr.getEnd() == endOffset(expr.getRightExpression())) { + needsParens = true; } + } else if (valueExpression instanceof TernaryExpression) { + TernaryExpression expr = (TernaryExpression) valueExpression; + if (expr.getStart() == startOffset(expr.getBooleanExpression()) || + expr.getEnd() == endOffset(expr.getFalseExpression())) { + needsParens = true; + } + } else if (valueExpression instanceof CastExpression) { + CastExpression expr = (CastExpression) valueExpression; + if ((expr.isCoerce() && expr.getStart() == startOffset(expr.getExpression())) || + (!expr.isCoerce() && expr.getEnd() == endOffset(expr.getExpression()))) { + needsParens = true; + } + } else if (valueExpression instanceof RangeExpression) { + RangeExpression expr = (RangeExpression) valueExpression; + if (expr.getStart() == startOffset(expr.getFrom()) || expr.getEnd() == endOffset(expr.getTo())) { + needsParens = true; + } + } else if (valueExpression instanceof PostfixExpression) { + needsParens = (valueExpression.getStart() == startOffset(((PostfixExpression) valueExpression).getExpression())); + } else if (valueExpression instanceof PrefixExpression) { + needsParens = (valueExpression.getEnd() == endOffset(((PrefixExpression) valueExpression).getExpression())); + } else if (valueExpression instanceof BooleanExpression) { + needsParens = (valueExpression.getEnd() == endOffset(((BooleanExpression) valueExpression).getExpression())); + } else if (valueExpression instanceof BitwiseNegationExpression) { + needsParens = (valueExpression.getEnd() == endOffset(((BitwiseNegationExpression) valueExpression).getExpression())); + } else if (valueExpression instanceof UnaryMinusExpression) { + needsParens = (valueExpression.getEnd() == endOffset(((UnaryMinusExpression) valueExpression).getExpression())); + } else if (valueExpression instanceof UnaryPlusExpression) { + needsParens = (valueExpression.getEnd() == endOffset(((UnaryPlusExpression) valueExpression).getExpression())); } - return value; + + String valueString = context.getNodeText(valueExpression); + if (needsParens) valueString = "(" + valueString + ")"; + return valueString; } @Override