Fix for #1413: super types are not searched for extension methods

groovy · Oct 28, 2022 · e3fec3b · e3fec3b
1 parent 8006bcc
commit e3fec3b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 45 deletions.
diff --git a/....core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/CategorySearchTests.java b/....core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/CategorySearchTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2020 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,77 +36,74 @@
 public final class CategorySearchTests extends SearchTestSuite {
 
     //@formatter:off
-    private static String CATEGORY_DEFN =
+    private static String CATEGORY_CLASSES =
+        "class CatTarget {\n" +
+        "  CatTarget self\n" +
+        "}\n" +
         "class Cat {\n" +
-        "  static String doNothing(CatTarget e, msg) {\n" +
+        "  static String printMessage(CatTarget e, msg) {\n" +
         "    print msg\n" +
         "  }\n" +
         "}\n" +
-        "class CatTarget {\n" +
-        "  CatTarget self\n" +
-        "}\n" +
         "class Cat2 {\n" +
-        "  static String doNothing2(CatTarget e, msg) {\n" +
+        "  static String printMessage2(CatTarget e, msg) {\n" +
         "    print msg\n" +
         "  }\n" +
+        "}\n" +
+        "class Cat3 extends Cat {\n" +
         "}";
 
     private static String SIMPLE_CATEGORY =
         "use (Cat) {\n" +
-        "  new CatTarget().doNothing 'jello'\n" +
+        "  new CatTarget().printMessage 'jello'\n" +
         "  def x = new CatTarget()\n" +
-        "  x.doNothing 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
         "  x.self = x\n" +
-        "  x.doNothing 'jello'\n" +
-        "  Cat.doNothing x, 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
+        "  Cat.printMessage x, 'jello'\n" +
         "}";
 
     private static String CATEGORY_WITH_SUBTYPE =
         "class Sub extends CatTarget { }\n" +
         "use (Cat) {\n" +
-        "  new Sub().doNothing 'jello'\n" +
+        "  new Sub().printMessage 'jello'\n" +
         "  def x = new Sub()\n" +
-        "  x.doNothing 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
         "  x.self = x\n" +
-        "  x.doNothing 'jello'\n" +
-        "  Cat.doNothing x, 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
+        "  Cat.printMessage x, 'jello'\n" +
         "}";
 
     private static String CATEGORY_ASSIGNED =
         "def y = Cat\n" +
         "use (y) {\n" +
-        "  new CatTarget().doNothing 'jello'\n" +
+        "  new CatTarget().printMessage 'jello'\n" +
         "  def x = new CatTarget()\n" +
-        "  x.doNothing 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
         "  x.self = x\n" +
-        "  x.doNothing 'jello'\n" +
-        "  y.doNothing x, 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
+        "  y.printMessage x, 'jello'\n" +
         "}";
 
     private static String CATEGORY_MULTIPLE_OUTER =
         "use (Cat) { use (Cat2) {\n" +
-        "  new CatTarget().doNothing 'jello'\n" +
+        "  new CatTarget().printMessage 'jello'\n" +
         "  def x = new CatTarget()\n" +
-        "  x.doNothing 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
         "  x.self = x\n" +
-        "  x.doNothing 'jello'\n" +
-        "  Cat.doNothing x, 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
+        "  Cat.printMessage x, 'jello'\n" +
         "} }";
 
     private static String CATEGORY_MULTIPLE_INNER =
         "use (Cat2) { use (Cat) {\n" +
-        "  new CatTarget().doNothing 'jello'\n" +
+        "  new CatTarget().printMessage 'jello'\n" +
         "  def x = new CatTarget()\n" +
-        "  x.doNothing 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
         "  x.self = x\n" +
-        "  x.doNothing 'jello'\n" +
-        "  Cat.doNothing x, 'jello'\n" +
+        "  x.printMessage 'jello'\n" +
+        "  Cat.printMessage x, 'jello'\n" +
         "} }";
-
-    private static String NO_CATEGORY =
-        "use (Cat) {\n" +
-        "}\n" +
-        "new CatTarget().doNothing 'jello'\n";
     //@formatter:on
 
     @Test
@@ -136,7 +133,15 @@ public void testCategorySearch5() throws Exception {
 
     @Test
     public void testCategorySearch6() throws Exception {
-        doCategorySearchTest(NO_CATEGORY, 0);
+        doCategorySearchTest("use (Cat) {}\n" +
+            "new CatTarget().printMessage('')\n", 0);
+    }
+
+    @Test
+    public void testCategorySearch7() throws Exception {
+        doCategorySearchTest("use (Cat3) {\n" +
+            "  new CatTarget().printMessage('')\n" +
+            "}", 1);
     }
 
     //--------------------------------------------------------------------------
@@ -146,25 +151,25 @@ private void doCategorySearchTest(final String contents, final int numMatches) t
     }
 
     private List<SearchMatch> findMatches(final String contents) throws JavaModelException {
-        GroovyCompilationUnit catUnit = createUnit("Cat", CATEGORY_DEFN);
-        GroovyCompilationUnit unit = createUnit("Other", contents);
+        GroovyCompilationUnit catUnit = createUnit("Cat", CATEGORY_CLASSES);
+        GroovyCompilationUnit unit = createUnit("Script", contents);
         expectingNoProblems();
 
-        IMethod searchFor = (IMethod) catUnit.getElementAt(CATEGORY_DEFN.indexOf("doNothing"));
-        assertEquals("Wrong IJavaElement found: " + searchFor, "doNothing", searchFor.getElementName());
+        IMethod searchFor = (IMethod) catUnit.getElementAt(CATEGORY_CLASSES.indexOf("printMessage"));
+        assertEquals("Wrong IJavaElement found: " + searchFor, "printMessage", searchFor.getElementName());
         return search(SearchPattern.createPattern(searchFor, IJavaSearchConstants.REFERENCES), unit);
     }
 
     private void checkMatches(final List<SearchMatch> matches, final int nExpected, final String contents) {
         assertEquals("Wrong number of matches found:\n" + toString(matches), nExpected, matches.size());
         if (nExpected > 0) {
             Iterator<SearchMatch> it = matches.iterator();
-            Pattern p = Pattern.compile("doNothing");
+            Pattern p = Pattern.compile("printMessage");
             Matcher m = p.matcher(contents);
             while (m.find()) {
                 SearchMatch match = it.next();
                 assertEquals("Wrong starting location for " + toString(match), m.start(), match.getOffset());
-                assertEquals("Wrong length for " + toString(match), "doNothing".length(), match.getLength());
+                assertEquals("Wrong length for " + toString(match), "printMessage".length(), match.getLength());
             }
         }
     }

diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/CategoryTypeLookup.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/CategoryTypeLookup.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -62,7 +62,7 @@ public TypeLookupResult lookupType(final Expression node, final VariableScope sc
 
             if (isMethodPointer || scope.isMethodCall()) {
                 for (ClassNode category : scope.getCategoryNames()) {
-                    for (MethodNode method : category.getMethods(simpleName)) {
+                    for (MethodNode method : category.getDeclaredMethods(simpleName)) {
                         if (isCompatibleCategoryMethod(method, selfType, scope)) {
                             candidates.add(method);
                         }
@@ -74,7 +74,7 @@ public TypeLookupResult lookupType(final Expression node, final VariableScope sc
                     String methodName = kind.createAccessorName(simpleName);
                     if (methodName != null) {
                         for (ClassNode category : scope.getCategoryNames()) {
-                            for (MethodNode method : category.getMethods(methodName)) {
+                            for (MethodNode method : category.getDeclaredMethods(methodName)) {
                                 if (kind.isAccessorKind(method, true) && isCompatibleCategoryMethod(method, selfType, scope) &&
                                         // GROOVY-5245: isPropName() methods cannot be used for bean-style property expressions
                                         (kind != AccessorSupport.ISSER || isDefaultGroovyMethod(method, scope) || isDefaultGroovyStaticMethod(method, scope) || GroovyUtils.getGroovyVersion().getMajor() > 3)) {

diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java
@@ -281,6 +281,11 @@ public Set<ClassNode> getCategoryNames() {
             if (parent.isCategoryBeingDeclared()) {
                 categories = new LinkedHashSet<>(categories);
                 categories.add(parent.categoryBeingDeclared);
+                for (ClassNode superClass = parent.categoryBeingDeclared.getSuperClass();
+                        superClass != OBJECT_CLASS_NODE && superClass != null;
+                        superClass = superClass.getSuperClass()) {
+                    categories.add(superClass);
+                }
             }
         } else {
             categories = scopeNode.getNodeMetaData(DefaultGroovyMethods.class, key -> {

diff --git a/...rc/org/codehaus/groovy/eclipse/codeassist/tests/DefaultGroovyMethodCompletionTests.groovy b/...rc/org/codehaus/groovy/eclipse/codeassist/tests/DefaultGroovyMethodCompletionTests.groovy
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -320,6 +320,13 @@ final class DefaultGroovyMethodCompletionTests extends CompletionTestSuite {
         proposalExists(proposals, 'findAll() : List<T>', 1) // not Collection<T>
     }
 
+    @Test
+    void testNoSuper() {
+        ICompletionProposal[] proposals = createProposalsAtOffset('void m(InputStream s){s.c}\n', 25)
+        proposalExists(proposals, 'closeQuietly()', 0) // from DefaultGroovyMethodsSupport
+        proposalExists(proposals, 'withCloseable', 1)
+    }
+
     @Test
     void testNoExtras() {
         ICompletionProposal[] proposals = createProposalsAtOffset('[].stream().collect()\n', 20)

diff --git a/...deassist/src/org/codehaus/groovy/eclipse/codeassist/creators/CategoryProposalCreator.java b/...deassist/src/org/codehaus/groovy/eclipse/codeassist/creators/CategoryProposalCreator.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2021 the original author or authors.
+ * Copyright 2009-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -71,7 +71,7 @@ public List<IGroovyProposal> findAllProposals(final ClassNode selfType, final Se
         List<IGroovyProposal> proposals = new ArrayList<>();
         for (ClassNode category : categories) {
             boolean isDefaultCategory = isDefaultCategory(category);
-            for (MethodNode method : getAllMethods(category, null)) {
+            for (MethodNode method : category.getMethods()) {
                 // check for DGMs filtered by deprecation or user preference
                 if (isDefaultCategory && (GroovyUtils.isDeprecated(method) || filter.isFiltered(method))) {
                     continue;