From 3bff642a209cba5dc1565ee8886a515973b169c0 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 5 Apr 2017 21:17:24 -0500 Subject: [PATCH] Fix for issue #278: enum constant annotations not handled properly --- .../core/tests/basic/GroovySimpleTest.java | 85 +++---------------- .../groovy/antlr/AntlrParserPlugin.java | 37 +++----- .../groovy/antlr/AntlrParserPlugin.java | 45 +++------- .../groovy/antlr/AntlrParserPlugin.java | 67 ++++++--------- .../groovy/antlr/AntlrParserPlugin.java | 21 ++--- .../groovy/antlr/AntlrParserPlugin.java | 21 ++--- .../ast/GroovyCompilationUnitDeclaration.java | 4 +- .../tests/CodeSelectTypesTests.java | 22 ++--- .../test/ui/SemanticHighlightingTests.groovy | 18 ++++ 9 files changed, 107 insertions(+), 213 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTest.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTest.java index a024cec715..cb082a9e56 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTest.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTest.java @@ -1230,32 +1230,18 @@ public void testEnumPositions_GRE1072() { " GREEN,\n"+ " BLUE\n"+ "}\n", - },""); + }); GroovyCompilationUnitDeclaration decl = getCUDeclFor("X.groovy"); - FieldDeclaration fDecl = null; + FieldDeclaration fDecl = grabField(decl, "RED"); + assertEquals("RED sourceStart>sourceEnd:30>32 declSourceStart>declSourceEnd:15>32 modifiersSourceStart=30 endPart1Position:30", stringifyFieldDecl(fDecl)); - fDecl = grabField(decl,"RED"); - if (GroovyUtils.GROOVY_LEVEL<18) { - assertEquals("RED sourceStart>sourceEnd:30>32 declSourceStart>declSourceEnd:30>31 modifiersSourceStart=0 endPart1Position:30",stringifyFieldDecl(fDecl)); - } else { - assertEquals("RED sourceStart>sourceEnd:30>32 declSourceStart>declSourceEnd:15>31 modifiersSourceStart=30 endPart1Position:30",stringifyFieldDecl(fDecl)); + fDecl = grabField(decl, "GREEN"); + assertEquals("GREEN sourceStart>sourceEnd:37>41 declSourceStart>declSourceEnd:37>41 modifiersSourceStart=37 endPart1Position:37", stringifyFieldDecl(fDecl)); - } - - fDecl = grabField(decl,"GREEN"); - if (GroovyUtils.GROOVY_LEVEL<18) { - assertEquals("GREEN sourceStart>sourceEnd:37>41 declSourceStart>declSourceEnd:37>40 modifiersSourceStart=0 endPart1Position:37",stringifyFieldDecl(fDecl)); - } else { - assertEquals("GREEN sourceStart>sourceEnd:37>41 declSourceStart>declSourceEnd:37>40 modifiersSourceStart=37 endPart1Position:37",stringifyFieldDecl(fDecl)); - } - fDecl = grabField(decl,"BLUE"); - if (GroovyUtils.GROOVY_LEVEL<18) { - assertEquals("BLUE sourceStart>sourceEnd:46>49 declSourceStart>declSourceEnd:46>48 modifiersSourceStart=0 endPart1Position:46",stringifyFieldDecl(fDecl)); - } else { - assertEquals("BLUE sourceStart>sourceEnd:46>49 declSourceStart>declSourceEnd:46>48 modifiersSourceStart=46 endPart1Position:46",stringifyFieldDecl(fDecl)); - } + fDecl = grabField(decl, "BLUE"); + assertEquals("BLUE sourceStart>sourceEnd:46>49 declSourceStart>declSourceEnd:46>49 modifiersSourceStart=46 endPart1Position:46", stringifyFieldDecl(fDecl)); } public void testEnumValues_GRE1071() { @@ -1785,8 +1771,8 @@ public void testInvalidScripts_GRE323_1b() { // def x // ^ public void testInvalidScripts_GRE323_2() { - if (GroovyUtils.GROOVY_LEVEL<18) { - runNegativeTest(new String[] { + // command expression syntax now allows this but it looks weird as + runConformTest(new String[] { "One.groovy", "def moo(closure) {\n" + " closure();\n" + @@ -1799,31 +1785,7 @@ public void testInvalidScripts_GRE323_2() { " def secBoardRep = session2.\n" + " def x\n" + "}\n" - }, - "----------\n" + - "1. ERROR in One.groovy (at line 10)\n" + - "\tdef x\n" + - "\t ^\n" + - "Groovy:expecting \'}\', found \'x\' @ line 10, column 7.\n" + - "----------\n"); - } else { - // command expression syntax now allows this but it looks weird as - runConformTest(new String[] { - "One.groovy", - "def moo(closure) {\n" + - " closure();\n" + - "}\n" + - "\n" + - "moo {\n" + - " final session2 = null\n" + - " \n" + - " // Define scenarios\n" + - " def secBoardRep = session2.\n" + - " def x\n" + - "}\n" - }, - ""); - } + }); } // removed surrounding method @@ -1997,8 +1959,7 @@ public void testInvalidScripts_GRE323_5b() { } public void testInvalidScripts_GRE323_6() { - if (GroovyUtils.GROOVY_LEVEL<18) { - runNegativeTest(new String[] { + runConformTest(new String[] { "Six.groovy", "def moo(closure) {\n" + " closure();\n" + @@ -2011,29 +1972,7 @@ public void testInvalidScripts_GRE323_6() { " // Define scenarios\n" + " final y = session2.def x\n" + "}\n" - }, - "----------\n" + - "1. ERROR in Six.groovy (at line 10)\n" + - "\tfinal y = session2.def x\n" + - "\t ^\n" + - "Groovy:expecting \'}\', found \'x\' @ line 10, column 26.\n" + - "----------\n"); - } else { - runConformTest(new String[] { - "Six.groovy", - "def moo(closure) {\n" + - " closure();\n" + - "}\n" + - "\n" + - "moo {\n" + - " final session2 = [\"def\": { println \"DEF\" }]\n" + - " \n" + - " final x = 1\n"+ - " // Define scenarios\n" + - " final y = session2.def x\n" + - "}\n" - },"DEF"); - } + },"DEF"); } public void testBridgeMethods_GRE336() { diff --git a/base/org.codehaus.groovy21/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy21/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index c5a11771ce..5e346c5e99 100644 --- a/base/org.codehaus.groovy21/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy21/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -974,34 +974,25 @@ protected void enumConstantDef(AST node) { ListExpression le = new ListExpression(); le.addExpression(init); init = le; - } + } } } - - // GRECLIPSE: start - /*old{ - EnumHelper.addEnumConstant(classNode, identifier, init); - }new */ - GroovySourceAST groovySourceAST = (GroovySourceAST) node; - int nameStart = locations.findOffset(groovySourceAST.getLine(), groovySourceAST.getColumn()); - int nameEnd = nameStart + identifier.length()-1; - - ClassNode fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue = ClassHelper.make(classNode.getName()); - fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue.setRedirect(classNode); - - FieldNode fn = - // end - EnumHelper.addEnumConstant(fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); - // GRECLIPSE: start + // GRECLIPSE edit + //FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); + //enumField.addAnnotations(annotations); + //configureAST(enumField, node); + ClassNode nonDeclaredTypeOfEnumValue = + ClassHelper.make(classNode.getName()); + nonDeclaredTypeOfEnumValue.setRedirect(classNode); + FieldNode fn = EnumHelper.addEnumConstant(nonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); + fn.setNameStart(locations.findOffset(savedLine, savedColumn)); + fn.setNameEnd(fn.getNameStart() + identifier.length() - 1); + fn.addAnnotations(annotations); configureAST(fn, node); - fn.setNameStart(nameStart); - fn.setNameEnd(nameEnd); - fn.setStart(nameStart); - fn.setEnd(nameEnd); - // end + // GRECLIPSE end enumConstantBeingDef = false; } - + protected void throwsList(AST node, List list) { String name; if (isType(DOT, node)) { diff --git a/base/org.codehaus.groovy22/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy22/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index 6fbf785283..18366fe1e8 100644 --- a/base/org.codehaus.groovy22/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy22/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -979,42 +979,25 @@ protected void enumConstantDef(AST node) { ListExpression le = new ListExpression(); le.addExpression(init); init = le; - } + } } } - - // GRECLIPSE: start - /*old{ - EnumHelper.addEnumConstant(classNode, identifier, init); - -// looks like this is 3 lines in groovy 2.2. now: do we care? - - FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); - enumField.addAnnotations(annotations); - configureAST(enumField, node); - - - }new */ - GroovySourceAST groovySourceAST = (GroovySourceAST) node; - int nameStart = locations.findOffset(groovySourceAST.getLine(), groovySourceAST.getColumn()); - int nameEnd = nameStart + identifier.length()-1; - - ClassNode fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue = ClassHelper.make(classNode.getName()); - fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue.setRedirect(classNode); - - FieldNode fn = - // end - EnumHelper.addEnumConstant(fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); - // GRECLIPSE: start + // GRECLIPSE edit + //FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); + //enumField.addAnnotations(annotations); + //configureAST(enumField, node); + ClassNode nonDeclaredTypeOfEnumValue = + ClassHelper.make(classNode.getName()); + nonDeclaredTypeOfEnumValue.setRedirect(classNode); + FieldNode fn = EnumHelper.addEnumConstant(nonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); + fn.setNameStart(locations.findOffset(savedLine, savedColumn)); + fn.setNameEnd(fn.getNameStart() + identifier.length() - 1); + fn.addAnnotations(annotations); configureAST(fn, node); - fn.setNameStart(nameStart); - fn.setNameEnd(nameEnd); - fn.setStart(nameStart); - fn.setEnd(nameEnd); - // end + // GRECLIPSE end enumConstantBeingDef = false; } - + protected void throwsList(AST node, List list) { String name; if (isType(DOT, node)) { diff --git a/base/org.codehaus.groovy23/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/base/org.codehaus.groovy23/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java index 5e4a2ce9aa..e3d7541525 100644 --- a/base/org.codehaus.groovy23/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java +++ b/base/org.codehaus.groovy23/src/org/codehaus/groovy/antlr/AntlrParserPlugin.java @@ -986,54 +986,37 @@ protected void enumConstantDef(AST node) { ListExpression le = new ListExpression(); le.addExpression(init); init = le; - } + } } } - - // GRECLIPSE: start - /*old{ - EnumHelper.addEnumConstant(classNode, identifier, init); - -// looks like this is 3 lines in groovy 2.2. now: do we care? - - FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); - enumField.addAnnotations(annotations); - configureAST(enumField, node); - - - }new */ - GroovySourceAST groovySourceAST = (GroovySourceAST) node; - int nameStart = locations.findOffset(groovySourceAST.getLine(), groovySourceAST.getColumn()); - int nameEnd = nameStart + identifier.length()-1; - - ClassNode fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue = ClassHelper.make(classNode.getName()); - fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue.setRedirect(classNode); - - FieldNode fn = - // end - EnumHelper.addEnumConstant(fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); - // GRECLIPSE: start + // GRECLIPSE edit + //FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); + //enumField.addAnnotations(annotations); + //configureAST(enumField, node); + ClassNode nonDeclaredTypeOfEnumValue = + ClassHelper.make(classNode.getName()); + nonDeclaredTypeOfEnumValue.setRedirect(classNode); + FieldNode fn = EnumHelper.addEnumConstant(nonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); + fn.setNameStart(locations.findOffset(savedLine, savedColumn)); + fn.setNameEnd(fn.getNameStart() + identifier.length() - 1); + fn.addAnnotations(annotations); configureAST(fn, node); - fn.setNameStart(nameStart); - fn.setNameEnd(nameEnd); - fn.setStart(nameStart); - fn.setEnd(nameEnd); - // end + // GRECLIPSE end enumConstantBeingDef = false; } - + protected void throwsList(AST node, List list) { - String name; - if (isType(DOT, node)) { - name = qualifiedName(node); - } else { - name = identifier(node); - } - ClassNode exception = ClassHelper.make(name); - configureAST(exception, node); - list.add(exception); - AST next = node.getNextSibling(); - if (next!=null) throwsList(next, list); + String name; + if (isType(DOT, node)) { + name = qualifiedName(node); + } else { + name = identifier(node); + } + ClassNode exception = ClassHelper.make(name); + configureAST(exception, node); + list.add(exception); + AST next = node.getNextSibling(); + if (next!=null) throwsList(next, list); } protected void methodDef(AST methodDef) { 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 80fa345263..af659ae981 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 @@ -967,20 +967,15 @@ protected void enumConstantDef(AST node) { } // GRECLIPSE edit //FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); - //enumField.addAnnotations(annotations); - //configureAST(enumField, node); - GroovySourceAST groovySourceAST = (GroovySourceAST) node; - int nameStart = locations.findOffset(groovySourceAST.getLine(), groovySourceAST.getColumn()); - int nameEnd = nameStart + identifier.length() - 1; - ClassNode fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue = ClassHelper.make(classNode.getName()); - fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue.setRedirect(classNode); - FieldNode fn = EnumHelper.addEnumConstant(fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); - configureAST(fn, node); - fn.setNameStart(nameStart); - fn.setNameEnd(nameEnd); - fn.setStart(nameStart); - fn.setEnd(nameEnd); + ClassNode nonDeclaredTypeOfEnumValue = + ClassHelper.make(classNode.getName()); + nonDeclaredTypeOfEnumValue.setRedirect(classNode); + FieldNode enumField = EnumHelper.addEnumConstant(nonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); + enumField.setNameStart(locations.findOffset(savedLine, savedColumn)); + enumField.setNameEnd(enumField.getNameStart() + identifier.length() - 1); // GRECLIPSE end + enumField.addAnnotations(annotations); + configureAST(enumField, node); enumConstantBeingDef = false; } 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 be4a1a5900..d87275534a 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 @@ -1014,20 +1014,15 @@ protected void enumConstantDef(AST node) { } // GRECLIPSE edit //FieldNode enumField = EnumHelper.addEnumConstant(classNode, identifier, init); - //enumField.addAnnotations(annotations); - //configureAST(enumField, node); - GroovySourceAST groovySourceAST = (GroovySourceAST) node; - int nameStart = locations.findOffset(groovySourceAST.getLine(), groovySourceAST.getColumn()); - int nameEnd = nameStart + identifier.length() - 1; - ClassNode fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue = ClassHelper.make(classNode.getName()); - fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue.setRedirect(classNode); - FieldNode fn = EnumHelper.addEnumConstant(fakeNodeToRepresentTheNonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); - configureAST(fn, node); - fn.setNameStart(nameStart); - fn.setNameEnd(nameEnd); - fn.setStart(nameStart); - fn.setEnd(nameEnd); + ClassNode nonDeclaredTypeOfEnumValue = + ClassHelper.make(classNode.getName()); + nonDeclaredTypeOfEnumValue.setRedirect(classNode); + FieldNode enumField = EnumHelper.addEnumConstant(nonDeclaredTypeOfEnumValue, classNode, identifier, init, savedLine, savedColumn); + enumField.setNameStart(locations.findOffset(savedLine, savedColumn)); + enumField.setNameEnd(enumField.getNameStart() + identifier.length() - 1); // GRECLIPSE end + enumField.addAnnotations(annotations); + configureAST(enumField, node); enumConstantBeingDef = false; } diff --git a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java index 4021b99633..de0f2ed507 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java @@ -2363,8 +2363,8 @@ private void fixupSourceLocationsForFieldDeclaration(FieldDeclaration fieldDecla if (isEnumField) { // they have no 'leading' type declaration or modifiers - fieldDeclaration.declarationSourceStart = doc == null ? fieldNode.getNameStart() : doc.sourceStart; - fieldDeclaration.declarationSourceEnd = fieldNode.getNameEnd() - 1; + fieldDeclaration.declarationSourceStart = doc == null ? fieldNode.getStart() : doc.sourceStart; + fieldDeclaration.declarationSourceEnd = fieldNode.getEnd() - 1; } else { fieldDeclaration.declarationSourceStart = doc == null ? fieldNode.getStart() : doc.sourceStart; // the end of the fragment including initializer (and trailing ',') diff --git a/ide-test/org.codehaus.groovy.eclipse.codebrowsing.test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectTypesTests.java b/ide-test/org.codehaus.groovy.eclipse.codebrowsing.test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectTypesTests.java index 2bf440d023..5bf5fe480f 100644 --- a/ide-test/org.codehaus.groovy.eclipse.codebrowsing.test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectTypesTests.java +++ b/ide-test/org.codehaus.groovy.eclipse.codebrowsing.test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectTypesTests.java @@ -124,22 +124,12 @@ public void testSelectAnnotationClass4a() { assertCodeSelect(asList(contents), "EqualsAndHashCode"); } - public void testSelectAnnotationValue() { - String another = "@interface RunWith {\n Class value()\n}\nclass Runner { }"; - String contents = "@RunWith(Runner)\nclass ATest { }"; - assertCodeSelect(asList(another, contents), "Runner"); - } - - public void testSelectAnnotationValue2() { - String another = "enum Foo {\nFOO1, FOO2\n} \n@interface RunWith {\nFoo value();\n}"; - String contents = "@RunWith(Foo.FOO1)\nclass ATest { }"; - assertCodeSelect(asList(another, contents), "Foo"); - } - - public void testSelectAnnotationValue3() { - String another = "enum Foo {\nFOO1, FOO2\n} \n@interface RunWith {\nFoo value();\n}"; - String contents = "@RunWith(Foo.FOO1)\nclass ATest { }"; - assertCodeSelect(asList(another, contents), "FOO1"); + public void testSelectAnnotationClass5() { + if (GroovyUtils.GROOVY_LEVEL < 21) return; + String another = "import java.lang.annotation.*; @Target(ElementType.FIELD) @interface Tag { String value() }"; + String contents = "enum Foo { @Tag('Bar') Baz }"; + assertCodeSelect(asList(another, contents), "Tag"); + assertCodeSelect(asList(another, contents), "Baz"); } // fields diff --git a/ide-test/org.codehaus.groovy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy b/ide-test/org.codehaus.groovy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy index ccbe72fa79..20cadfcd0d 100644 --- a/ide-test/org.codehaus.groovy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy @@ -701,6 +701,24 @@ final class SemanticHighlightingTests extends TestCase { new HighlightedTypedPosition(contents.lastIndexOf('alt'), 3, FIELD)) } + void testEnumAnno() { + EclipseTestSetup.addGroovySource '''\ + import java.lang.annotation.* + @Target(ElementType.FIELD) + @Retention(RetentionPolicy.RUNTIME) + @interface Tag { String value() } + '''.stripIndent() + + String contents = '''\ + enum X { + @Tag('why') Y + } + '''.stripIndent() + + assertHighlighting(contents, + new HighlightedTypedPosition(contents.indexOf('Y'), 1, STATIC_VALUE)) + } + void testAnnoElems() { String contents = '''\ @Grab( module = 'something:anything' )