Fix for #803: add multi-level qualifiers for more nested closure cases

groovy · Feb 6, 2019 · 6afb07c · 6afb07c
1 parent a958ca1
commit 6afb07c
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 9 deletions.
diff --git a/...pletion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/FieldCompletionTests.groovy b/...pletion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/FieldCompletionTests.groovy
@@ -628,7 +628,7 @@ final class FieldCompletionTests extends CompletionTestSuite {
                 applyProposalAndCheck(proposal, contents.replace('zz //', 'zzz //'))
                 break
             case 'zzz : String - B':
-                applyProposalAndCheck(proposal, contents.replace('zz //', 'delegate.zzz //')) // TODO: "owner.delegate.zzz"
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.delegate.zzz //'))
                 break
             case 'zzz : String - C':
                 applyProposalAndCheck(proposal, contents.replace('zz //', 'delegate.zzz //'))
@@ -637,6 +637,88 @@ final class FieldCompletionTests extends CompletionTestSuite {
         }
     }
 
+    @Test
+    void testClosure13b() {
+        String contents = '''\
+            class A {
+              String zzz
+              static class B {
+                String zzz
+              }
+              static class C {
+                String zzz
+              }
+              def foo(@DelegatesTo(value=B, strategy=Closure.DELEGATE_FIRST) Closure c) {}
+              def bar(@DelegatesTo(value=C, strategy=Closure.OWNER_FIRST) Closure c) {}
+              void test() {
+                foo {
+                  bar {
+                    zz // delegate is C, owner.delegate is B, owner.owner is A
+                  }
+                }
+              }
+            }
+            '''.stripIndent()
+        ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'zz'))
+
+        proposalExists(proposals, 'zzz', 3)
+
+        proposals.each { ICompletionProposal proposal ->
+            switch (proposal.displayString) {
+            case 'zzz : String - A':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.owner.zzz //'))
+                break
+            case 'zzz : String - B':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'zzz //'))
+                break
+            case 'zzz : String - C':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'delegate.zzz //'))
+                break
+            }
+        }
+    }
+
+    @Test
+    void testClosure13c() {
+        String contents = '''\
+            class A {
+              String zzz
+              static class B {
+                String zzz
+              }
+              static class C {
+                String zzz
+              }
+              def foo(@DelegatesTo(value=B, strategy=Closure.OWNER_FIRST) Closure c) {}
+              def bar(@DelegatesTo(value=C, strategy=Closure.DELEGATE_FIRST) Closure c) {}
+              void test() {
+                foo {
+                  bar {
+                    zz // delegate is C, owner.delegate is B, owner.owner is A
+                  }
+                }
+              }
+            }
+            '''.stripIndent()
+        ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, 'zz'))
+
+        proposalExists(proposals, 'zzz', 3)
+
+        proposals.each { ICompletionProposal proposal ->
+            switch (proposal.displayString) {
+            case 'zzz : String - A':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.zzz //')) // TODO: owner.owner.zzz
+                break
+            case 'zzz : String - B':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.delegate.zzz //'))
+                break
+            case 'zzz : String - C':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'zzz //'))
+                break
+            }
+        }
+    }
+
     @Test
     void testArrayLength1() {
         String contents = 'int[] arr; arr.len'

diff --git a/...ehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java b/...ehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java
@@ -28,6 +28,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -375,7 +376,7 @@ private List<ICompletionProposal> createJavaProposals(Collection<IGroovyProposal
 
     private static void setClosureQualifiers(Collection<IGroovyProposal> delegateProposals, Collection<IGroovyProposal> ownerProposals, int resolveStrategy) {
 
-        Function<IGroovyProposal, String> toName = p -> {
+        Function<IGroovyProposal, String> toName = (p) -> {
             AnnotatedNode node = ((AbstractGroovyProposal) p).getAssociatedNode();
             if (node instanceof FieldNode) {
                 return ((FieldNode) node).getName();
@@ -389,19 +390,24 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro
             throw new IllegalStateException("unexpected node type: " + node.getClass());
         };
 
+        Predicate<IGroovyProposal> isQualified = (p) -> {
+            return ((AbstractGroovyProposal) p).getRequiredQualifier() != null;
+        };
+
         BiConsumer<IGroovyProposal, String> addQualifier = (p, q) -> {
-            AbstractGroovyProposal agp = (AbstractGroovyProposal) p;
-            if (agp.getRequiredQualifier() != null) {
-                q += "." + agp.getRequiredQualifier();
+            if (isQualified.test(p)) {
+                q += "." + ((AbstractGroovyProposal) p).getRequiredQualifier();
             }
-            agp.setRequiredQualifier(q);
+            ((AbstractGroovyProposal) p).setRequiredQualifier(q);
         };
 
-        Consumer<IGroovyProposal> reduceRelevance = p -> {
+        Consumer<IGroovyProposal> reduceRelevance = (p) -> {
             AbstractGroovyProposal agp = (AbstractGroovyProposal) p;
             agp.setRelevanceMultiplier(agp.getRelevanceMultiplier() * 0.999f);
         };
 
+        //
+
         if (!delegateProposals.isEmpty()) {
             Consumer<IGroovyProposal> addDelegateQualifier = bind(addQualifier, "delegate");
 
@@ -419,10 +425,12 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro
 
             if (resolveStrategy == Closure.DELEGATE_FIRST && !delegateProposals.isEmpty()) {
                 Set<String> names = delegateProposals.stream().map(toName).collect(Collectors.toSet());
-                ownerProposals.stream().filter(p -> names.contains(toName.apply(p)))
-                                .forEach(addOwnerQualifier.andThen(reduceRelevance));
+                ownerProposals.stream().filter(isQualified.or(p -> names.contains(toName.apply(p))))
+                                                .forEach(addOwnerQualifier.andThen(reduceRelevance));
             } else if (resolveStrategy == Closure.TO_SELF) {
                 ownerProposals.forEach(addOwnerQualifier.andThen(reduceRelevance));
+            } else {
+                ownerProposals.stream().filter(isQualified).forEach(addOwnerQualifier);
             }
         }
     }