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

groovy · Feb 5, 2019 · a958ca1 · a958ca1
1 parent 2c2ed17
commit a958ca1
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 10 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
@@ -557,6 +557,47 @@ final class FieldCompletionTests extends CompletionTestSuite {
 
     @Test // https://github.com/groovy/groovy-eclipse/issues/803
     void testClosure13() {
+        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.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.owner.zzz //'))
+                break
+            case 'zzz : String - B':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.zzz //'))
+                break
+            case 'zzz : String - C':
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'zzz //'))
+                break
+            }
+        }
+    }
+
+    @Test
+    void testClosure13a() {
         String contents = '''\
             class A {
               String zzz
@@ -584,10 +625,10 @@ final class FieldCompletionTests extends CompletionTestSuite {
         proposals.each { ICompletionProposal proposal ->
             switch (proposal.displayString) {
             case 'zzz : String - A':
-              //applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.owner.zzz //'))
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'zzz //'))
                 break
             case 'zzz : String - B':
-              //applyProposalAndCheck(proposal, contents.replace('zz //', 'owner.delegate.zzz //'))
+                applyProposalAndCheck(proposal, contents.replace('zz //', 'delegate.zzz //')) // TODO: "owner.delegate.zzz"
                 break
             case 'zzz : String - C':
                 applyProposalAndCheck(proposal, contents.replace('zz //', 'delegate.zzz //'))

diff --git a/...ehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java b/...ehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java
@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.regex.Pattern;
@@ -281,18 +282,18 @@ private void proposalCreatorOuterLoop(Collection<IGroovyProposal> groovyProposal
         if (closureStrategy >= Closure.OWNER_FIRST) {
             ClassNode enclosingType = requestor.currentScope.getOwner();
             if (enclosingType != null && !enclosingType.equals(completionType)) {
-                List<IGroovyProposal> ownerProposals = new ArrayList<>(); // keep proposals separate
+                Collection<IGroovyProposal> ownerProposals = new ArrayList<>(); // keep proposals separate
                 proposalCreatorInnerLoop(ownerProposals, creators, requestor, context, options, enclosingType, isStatic, isPrimary);
 
+                requestor.currentScope = enclosingType.getNodeMetaData("outer.scope");
+                if (requestor.currentScope != null) { // TODO: add another "owner." qualifier
+                    proposalCreatorOuterLoop(ownerProposals, creators, requestor, context, options, requestor.currentScope.getDelegate(), isStatic, isPrimary);
+                }
+
                 // if "delegate" and/or "owner" qualifiers are required, add them now
                 setClosureQualifiers(groovyProposals, ownerProposals, closureStrategy);
 
                 groovyProposals.addAll(ownerProposals);
-
-                requestor.currentScope = enclosingType.getNodeMetaData("outer.scope");
-                if (requestor.currentScope != null) { // TODO: add another "owner." qualifier
-                    proposalCreatorOuterLoop(groovyProposals, creators, requestor, context, options, requestor.currentScope.getDelegate(), isStatic, isPrimary);
-                }
             }
         }
     }
@@ -388,13 +389,21 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro
             throw new IllegalStateException("unexpected node type: " + node.getClass());
         };
 
+        BiConsumer<IGroovyProposal, String> addQualifier = (p, q) -> {
+            AbstractGroovyProposal agp = (AbstractGroovyProposal) p;
+            if (agp.getRequiredQualifier() != null) {
+                q += "." + agp.getRequiredQualifier();
+            }
+            agp.setRequiredQualifier(q);
+        };
+
         Consumer<IGroovyProposal> reduceRelevance = p -> {
             AbstractGroovyProposal agp = (AbstractGroovyProposal) p;
             agp.setRelevanceMultiplier(agp.getRelevanceMultiplier() * 0.999f);
         };
 
         if (!delegateProposals.isEmpty()) {
-            Consumer<IGroovyProposal> addDelegateQualifier = p -> ((AbstractGroovyProposal) p).setRequiredQualifier("delegate");
+            Consumer<IGroovyProposal> addDelegateQualifier = bind(addQualifier, "delegate");
 
             if (resolveStrategy == Closure.OWNER_FIRST && !ownerProposals.isEmpty()) {
                 Set<String> names = ownerProposals.stream().map(toName).collect(Collectors.toSet());
@@ -406,7 +415,7 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro
         }
 
         if (!ownerProposals.isEmpty()) {
-            Consumer<IGroovyProposal> addOwnerQualifier = p -> ((AbstractGroovyProposal) p).setRequiredQualifier("owner");
+            Consumer<IGroovyProposal> addOwnerQualifier = bind(addQualifier, "owner");
 
             if (resolveStrategy == Closure.DELEGATE_FIRST && !delegateProposals.isEmpty()) {
                 Set<String> names = delegateProposals.stream().map(toName).collect(Collectors.toSet());
@@ -455,6 +464,10 @@ private static VariableScope createTopLevelScope(ClassNode completionType) {
         return new VariableScope(null, completionType, false);
     }
 
+    private static <A, B> Consumer<A> bind(BiConsumer<A, B> consumer, B b) {
+        return (A a) -> consumer.accept(a, b);
+    }
+
     //--------------------------------------------------------------------------
 
     private class ExpressionCompletionRequestor implements ITypeRequestor {