Fix for #1512: type annotation visitation

groovy · Sep 18, 2023 · c299939 · c299939
1 parent 240d2d3
commit c299939
Show file tree

Hide file tree

Showing 17 changed files with 209 additions and 74 deletions.
diff --git a/....tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/TypeReferenceSearchTests.java b/....tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/TypeReferenceSearchTests.java
@@ -531,6 +531,26 @@ public void testTypeAnnotations() throws Exception {
         }
     }
 
+    @Test
+    public void testPermittedSubclasses() throws Exception {
+        assumeTrue(isParrotParser() && isAtLeastGroovy(40));
+
+        String firstContents =
+            "package p\n" +
+            "class First {}\n";
+        String secondContents =
+            "package q\n" +
+            "import p.*\n" +
+            "sealed class Second permits First {}\n";
+
+        List<SearchMatch> matches = searchForFirst(firstContents, secondContents, "p", "q");
+        assertEquals(1, matches.size());
+
+        SearchMatch match = matches.get(0);
+        assertEquals("First".length(), match.getLength());
+        assertEquals(secondContents.indexOf("First"), match.getOffset());
+    }
+
     @Test
     public void testConstructorWithDefaultArgsInCompileStatic() throws Exception {
         String firstContents =

diff --git a/base/org.codehaus.groovy40/src/org/apache/groovy/parser/antlr4/AstBuilder.java b/base/org.codehaus.groovy40/src/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -1556,7 +1556,9 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
                                 .map(ClassExpression::new)
                                 .collect(Collectors.toList()));
                 sealedAnnotationNode.setMember("permittedSubclasses", permittedSubclassesListExpression);
+                /* GRECLIPSE edit
                 configureAST(sealedAnnotationNode, ctx.PERMITS());
+                */
                 sealedAnnotationNode.setNodeMetaData("permits", Boolean.TRUE);
             }
             classNode.addAnnotation(sealedAnnotationNode);

diff --git a/base/org.codehaus.groovy40/src/org/codehaus/groovy/ast/ClassNode.java b/base/org.codehaus.groovy40/src/org/codehaus/groovy/ast/ClassNode.java
@@ -1673,6 +1673,11 @@ public List<AnnotationNode> getTypeAnnotations(ClassNode type) {
     }
 
     public void addTypeAnnotation(AnnotationNode annotation) {
+        // GRECLIPSE add -- GROOVY-10937
+        if (!isRedirectNode() && (isResolved() || isPrimaryClassNode())) {
+            throw new GroovyBugError("Adding type annotation @" + annotation.getClassNode().getNameWithoutPackage() + " to non-redirect node: " + getName());
+        }
+        // GRECLIPSE end
         if (annotation != null) {
             if (typeAnnotations == Collections.EMPTY_LIST) {
                 typeAnnotations = new ArrayList<>(3);

diff --git a/base/org.codehaus.groovy50/src/org/apache/groovy/parser/antlr4/AstBuilder.java b/base/org.codehaus.groovy50/src/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -1554,7 +1554,9 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
                                 .map(ClassExpression::new)
                                 .collect(Collectors.toList()));
                 sealedAnnotationNode.setMember("permittedSubclasses", permittedSubclassesListExpression);
+                /* GRECLIPSE edit
                 configureAST(sealedAnnotationNode, ctx.PERMITS());
+                */
                 sealedAnnotationNode.setNodeMetaData("permits", Boolean.TRUE);
             }
             classNode.addAnnotation(sealedAnnotationNode);

diff --git a/base/org.codehaus.groovy50/src/org/codehaus/groovy/ast/ClassNode.java b/base/org.codehaus.groovy50/src/org/codehaus/groovy/ast/ClassNode.java
@@ -1676,8 +1676,8 @@ public List<AnnotationNode> getTypeAnnotations(ClassNode type) {
     }
 
     public void addTypeAnnotation(final AnnotationNode annotation) {
-        if (!isPrimaryClassNode() && !isRedirectNode() && isResolved()) {
-            throw new GroovyBugError("Adding type annotation @" + annotation.getClassNode().getNameWithoutPackage() + " to non-primary, non-redirect node: " + getName());
+        if (!isRedirectNode() && (isResolved() || isPrimaryClassNode())) {
+            throw new GroovyBugError("Adding type annotation @" + annotation.getClassNode().getNameWithoutPackage() + " to non-redirect node: " + getName());
         }
         if (typeAnnotations == Collections.EMPTY_LIST) {
             typeAnnotations = new ArrayList<>(3);

diff --git a/...t.groovy.core/src/org/codehaus/jdt/groovy/integration/internal/GroovyIndexingVisitor.java b/...t.groovy.core/src/org/codehaus/jdt/groovy/integration/internal/GroovyIndexingVisitor.java
@@ -376,8 +376,10 @@ private void visitTypeParameters(final ClassNode type) {
 
     private void visitTypeParameters(final GenericsType[] generics, final String typeName) {
         for (GenericsType generic : generics) {
-            if (generic.getType() != null && generic.getType().getName().charAt(0) != '?') {
+            if (!generic.isPlaceholder() && !generic.isWildcard()) {
                 visitTypeReference(generic.getType(), generic.getType().isAnnotationDefinition(), true);
+            } else {
+                visitAnnotations(generic.getType().getTypeAnnotations());
             }
             if (generic.getLowerBound() != null) {
                 visitTypeReference(generic.getLowerBound(), generic.getLowerBound().isAnnotationDefinition(), true);

diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/core/util/DepthFirstVisitor.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/core/util/DepthFirstVisitor.java
@@ -159,7 +159,6 @@ public void visitImport(ImportNode node) {
     @Override
     public void visitClass(ClassNode node) {
         visitAnnotations(node.getAnnotations());
-        visitAnnotations(node.getTypeAnnotations());
 
         // visit "<clinit>" statements before visitContents
         MethodNode clinit = node.getMethod("<clinit>", Parameter.EMPTY_ARRAY);

diff --git a/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java b/...dt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java
@@ -293,7 +293,6 @@ public void visitJDT(final IType type, final ITypeRequestor requestor) {
                 // visit relocated @AnnotationCollector annotations
                 visitAnnotations(AnnotationCollectorTransform.getMeta(node));
             }
-            visitAnnotations(node.getTypeAnnotations()); // declaration TYPE_USE
 
             // visit name "node"
             TypeLookupResult result = new TypeLookupResult(node, node, node, TypeConfidence.EXACT, scopes.getLast());
@@ -601,7 +600,8 @@ private void visitClassReference(final ClassNode node) {
         VisitStatus status = notifyRequestor(type, requestor, result);
         switch (status) {
         case CONTINUE:
-            if (!type.isGenericsPlaceHolder()) {
+            if (!type.isGenericsPlaceHolder() && (type.isRedirectNode() ||
+                    (!type.isResolved() && !type.isPrimaryClassNode()))) {
                 visitGenericTypes(type.getGenericsTypes());
             }
             visitAnnotations(type.getTypeAnnotations());
@@ -1282,8 +1282,10 @@ private void visitGenericTypes(final GenericsType[] generics) {
         if (generics != null) {
             for (GenericsType gt : generics) {
                 ClassNode type = gt.getType();
-                if (type != null && !type.getUnresolvedName().startsWith("?")) {
+                if (!gt.isWildcard()) {
                     visitClassReference(type);
+                } else {
+                    visitAnnotations(type.getTypeAnnotations());
                 }
                 if (gt.getLowerBound() != null) {
                     visitClassReference(gt.getLowerBound());

diff --git a/....test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectAttributesTests.groovy b/....test/src/org/codehaus/groovy/eclipse/codebrowsing/tests/CodeSelectAttributesTests.groovy
@@ -15,7 +15,9 @@
  */
 package org.codehaus.groovy.eclipse.codebrowsing.tests
 
+import static org.eclipse.jdt.groovy.core.tests.GroovyBundle.isParrotParser
 import static org.eclipse.jdt.groovy.core.util.JavaConstants.AST_LEVEL
+import static org.junit.Assume.assumeTrue
 
 import org.codehaus.groovy.ast.ClassNode
 import org.codehaus.groovy.ast.FieldNode
@@ -50,13 +52,17 @@ final class CodeSelectAttributesTests extends BrowsingTestSuite {
             |'''.stripMargin(), 'A'
 
         String source = '''\
-            |@A(one='1')
+            |@A(one='1', two=C.D)
             |class C {
+            |  public static final String D = ""
             |}
             |'''.stripMargin()
 
         def elem = assertCodeSelect([source], 'one')
         assert elem.inferredElement instanceof MethodNode
+
+        /**/elem = assertCodeSelect([source], 'two')
+        assert elem.inferredElement instanceof MethodNode
     }
 
     @Test // https://github.com/groovy/groovy-eclipse/issues/959
@@ -105,6 +111,33 @@ final class CodeSelectAttributesTests extends BrowsingTestSuite {
         }
     }
 
+    @Test // https://github.com/groovy/groovy-eclipse/issues/1512
+    void testCodeSelectOnAttributeName5() {
+        assumeTrue(isParrotParser())
+
+        addJavaSource '''\
+            |import java.lang.annotation.*;
+            |@Target(ElementType.TYPE_USE)
+            |@interface A {
+            |  String one();
+            |  String two();
+            |}
+            |'''.stripMargin(), 'A'
+
+        String source = '''\
+            |@A(one=C.VALUE)
+            |class C {
+            |  public static final @A(two=C.VALUE) String VALUE = ""
+            |}
+            |'''.stripMargin()
+
+        def elem = assertCodeSelect([source], 'one')
+        assert elem.inferredElement instanceof MethodNode
+
+        /**/elem = assertCodeSelect([source], 'two')
+        assert elem.inferredElement instanceof MethodNode
+    }
+
     @Test
     void testCodeSelectOnAttributeValue1() {
         String source = '''\