From ca14164d66ab56d74e291542e83080a34143d3df Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Mon, 5 Feb 2018 16:35:50 -0600 Subject: [PATCH] Fix for issue #455: enable argument guessing for constructor completions --- .../tests/CompletionTestSuite.groovy | 6 +- .../tests/ConstructorCompletionTests.groovy | 17 ++++- .../tests/GuessingCompletionTests.groovy | 23 ++++++ .../GroovyExtendedCompletionContext.java | 74 +++++++++---------- .../ConstructorCompletionProcessor.java | 3 +- ...ementAndExpressionCompletionProcessor.java | 16 +--- .../requestor/ContentAssistContext.java | 12 +++ 7 files changed, 88 insertions(+), 63 deletions(-) diff --git a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/CompletionTestSuite.groovy b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/CompletionTestSuite.groovy index fa9035cfcf..5323521a47 100644 --- a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/CompletionTestSuite.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/CompletionTestSuite.groovy @@ -380,10 +380,8 @@ abstract class CompletionTestSuite extends GroovyEclipseTestSuite { protected void checkProposalChoices(String contents, String completion, String replacementString, String[][] expectedChoices) { ICompletionProposal proposal = checkUniqueProposal(contents, completion, completion, replacementString) - proposal.replacementString // instantiate the guesses - - ICompletionProposal[][] choices = proposal.choices - assertEquals(expectedChoices.length, choices.length) + List choices = proposal.choices + assertEquals(expectedChoices.length, choices.size()) for (int i = 0; i < expectedChoices.length; i += 1) { assertEquals(expectedChoices[i].length, choices[i].length) diff --git a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/ConstructorCompletionTests.groovy b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/ConstructorCompletionTests.groovy index e656960ea1..b50b362156 100644 --- a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/ConstructorCompletionTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/ConstructorCompletionTests.groovy @@ -41,6 +41,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { void testConstructorCompletion1() { String contents = 'class YYY { YYY() { } }\nnew YY\nkkk' String expected = 'class YYY { YYY() { } }\nnew YYY()\nkkk' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -48,6 +49,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { void testConstructorCompletion2() { String contents = 'class YYY { YYY() { } }\nnew YY()\nkkk' // trailing parens String expected = 'class YYY { YYY() { } }\nnew YYY()\nkkk' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -55,6 +57,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { void testConstructorCompletion3() { String contents = 'class YYY { YYY(x) { } }\nnew YY\nkkk' String expected = 'class YYY { YYY(x) { } }\nnew YYY(x)\nkkk' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -62,6 +65,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { void testConstructorCompletion4() { String contents = 'class YYY { YYY(x, y) { } }\nnew YY\nkkk' String expected = 'class YYY { YYY(x, y) { } }\nnew YYY(x, y)\nkkk' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -69,6 +73,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { void testConstructorCompletionWithGenerics1() { String contents = 'List list = new ArrayL' String expected = 'List list = new ArrayList()' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new ArrayL'), 'ArrayList()') } @@ -78,6 +83,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { ' Aaa() {\n@Override int foo() {\nnew YY\n}\n}\nint foo() {\n }\n}' String expected = 'class YYY { YYY() { } }\nenum F {\n' + ' Aaa() {\n@Override int foo() {\nnew YYY()\n}\n}\nint foo() {\n }\n}' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -87,6 +93,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { ' Aaa {\n@Override int foo() {\nnew YY\n}\n}\nint foo() {\n }\n}' String expected = 'class YYY { YYY() { } }\nenum F {\n' + ' Aaa {\n@Override int foo() {\nnew YYY()\n}\n}\nint foo() {\n }\n}' + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY') } @@ -106,6 +113,7 @@ final class ConstructorCompletionTests extends CompletionTestSuite { String expected = '''\ def a = new java.text.Annotation(value) '''.stripIndent() + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'Anno'), 'Annotation') } @@ -115,10 +123,11 @@ final class ConstructorCompletionTests extends CompletionTestSuite { def a = new Anno '''.stripIndent() String expected = '''\ - import java.text.Annotation - - def a = new Annotation(value) - '''.stripIndent() + |import java.text.Annotation + | + |def a = new Annotation(value) + |'''.stripMargin() + setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false') checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new Anno'), 'Annotation') } diff --git a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/GuessingCompletionTests.groovy b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/GuessingCompletionTests.groovy index 9cb05be6b4..c036d1934f 100644 --- a/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/GuessingCompletionTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.codeassist.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/GuessingCompletionTests.groovy @@ -130,4 +130,27 @@ final class GuessingCompletionTests extends CompletionTestSuite { |pack.Util.util(DAYS) |'''.stripMargin())*/ } + + @Test + void testCtorParamGuessing() { + addGroovySource '''\ + class C { + C(java.lang.String string, java.util.concurrent.TimeUnit units) { + } + } + '''.stripIndent(), 'C', 'p' + + String contents = '''\ + import static java.util.concurrent.TimeUnit.MILLISECONDS as MILLIS + String s = '' + new p.C + '''.stripIndent() + + ICompletionProposal proposal = checkUniqueProposal(contents, 'new p.C', 'C', '(s, MILLIS)') + List choices = proposal.choices + + assert choices.size() == 2 + assert choices[0]*.displayString == ['s', '""'] + assert choices[1]*.displayString == ['MILLIS', 'DAYS', 'HOURS', 'MINUTES', 'SECONDS', 'MILLISECONDS', 'MICROSECONDS', 'NANOSECONDS', 'null'] + } } diff --git a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/completions/GroovyExtendedCompletionContext.java b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/completions/GroovyExtendedCompletionContext.java index 624738d373..ea23fca68f 100644 --- a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/completions/GroovyExtendedCompletionContext.java +++ b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/completions/GroovyExtendedCompletionContext.java @@ -43,34 +43,6 @@ public class GroovyExtendedCompletionContext extends InternalExtendedCompletionContext { - private static class PropertyVariant extends SourceField implements IField { - private final IMethod baseMethod; - - PropertyVariant(IMethod method) { - super((JavaElement) method.getParent(), toFieldName(method)); - baseMethod = method; - } - - @Override - public boolean exists() { - return true; - } - - @Override - public String getTypeSignature() throws JavaModelException { - return baseMethod.getReturnType(); - } - - @Override - public int getFlags() throws JavaModelException { - return baseMethod.getFlags(); - } - } - - private static String toFieldName(IMethod method) { - return ProposalUtils.createMockFieldName(method.getElementName()); - } - private final ContentAssistContext context; private final VariableScope currentScope; @@ -89,6 +61,11 @@ public GroovyExtendedCompletionContext(ContentAssistContext context, VariableSco this.visibleElements = new HashMap<>(); } + @Override + public boolean canUseDiamond(String[] parameterTypes, char[] fullyQualifiedTypeName) { + return true; + } + @Override public IJavaElement getEnclosingElement() { if (enclosingElement == null) { @@ -101,21 +78,12 @@ public IJavaElement getEnclosingElement() { enclosingElement = context.unit; } } - return enclosingElement; } @Override public IJavaElement[] getVisibleElements(String typeSignature) { - // let's not work with parameterized sigs - typeSignature = Signature.getTypeErasure(typeSignature); - - IJavaElement[] elements = visibleElements.get(typeSignature); - if (elements == null) { - elements = computeVisibleElements(typeSignature); - visibleElements.put(typeSignature, elements); - } - return elements; + return visibleElements.computeIfAbsent(Signature.getTypeErasure(typeSignature), this::computeVisibleElements); } private IJavaElement[] computeVisibleElements(String typeSignature) { @@ -178,7 +146,7 @@ private IJavaElement[] computeVisibleElements(String typeSignature) { return visibleElements.values().toArray(new IJavaElement[0]); } - public void addFields(ClassNode targetType, Map visibleElements, IType type) + protected void addFields(ClassNode targetType, Map visibleElements, IType type) throws JavaModelException { for (IField field : type.getFields()) { ClassNode fieldTypeClassNode = toClassNode(field.getTypeSignature()); @@ -245,4 +213,32 @@ protected ClassNode resolve(String fullyQualifiedTypeName) { return VariableScope.OBJECT_CLASS_NODE; } } + + private static String toFieldName(IMethod method) { + return ProposalUtils.createMockFieldName(method.getElementName()); + } + + private static class PropertyVariant extends SourceField implements IField { + private final IMethod baseMethod; + + PropertyVariant(IMethod method) { + super((JavaElement) method.getParent(), toFieldName(method)); + baseMethod = method; + } + + @Override + public boolean exists() { + return true; + } + + @Override + public String getTypeSignature() throws JavaModelException { + return baseMethod.getReturnType(); + } + + @Override + public int getFlags() throws JavaModelException { + return baseMethod.getFlags(); + } + } } diff --git a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/ConstructorCompletionProcessor.java b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/ConstructorCompletionProcessor.java index cc7a7a14d0..c2637660cd 100644 --- a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/ConstructorCompletionProcessor.java +++ b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/ConstructorCompletionProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 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. @@ -57,6 +57,7 @@ public List generateProposals(IProgressMonitor monitor) { char[] constructorText; int constructorStart; switch (context.location) { case CONSTRUCTOR: + context.extend(getJavaContext().getCoreContext(), null); constructorText = context.fullCompletionExpression.replaceFirst("^new\\s+", "").toCharArray(); constructorStart = context.completionLocation - CharOperation.lastSegment(constructorText, '.').length; break; diff --git a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java index 91d9cb0ce6..1a0ee8a73d 100644 --- a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java +++ b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java @@ -41,7 +41,6 @@ import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist; -import org.codehaus.groovy.eclipse.codeassist.completions.GroovyExtendedCompletionContext; import org.codehaus.groovy.eclipse.codeassist.creators.AbstractProposalCreator; import org.codehaus.groovy.eclipse.codeassist.creators.CategoryProposalCreator; import org.codehaus.groovy.eclipse.codeassist.creators.FieldProposalCreator; @@ -53,12 +52,10 @@ import org.codehaus.groovy.eclipse.codeassist.requestor.ContentAssistLocation; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.jdt.core.CompletionContext; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.ISourceRange; import org.eclipse.jdt.core.ISourceReference; import org.eclipse.jdt.core.JavaModelException; -import org.eclipse.jdt.groovy.core.util.ReflectionUtils; import org.eclipse.jdt.groovy.search.AccessorSupport; import org.eclipse.jdt.groovy.search.ITypeRequestor; import org.eclipse.jdt.groovy.search.TypeInferencingVisitorFactory; @@ -67,7 +64,6 @@ import org.eclipse.jdt.groovy.search.TypeLookupResult.TypeConfidence; import org.eclipse.jdt.groovy.search.VariableScope; import org.eclipse.jdt.groovy.search.VariableScope.VariableInfo; -import org.eclipse.jdt.internal.codeassist.InternalCompletionContext; import org.eclipse.jdt.internal.core.SearchableEnvironment; import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal; import org.eclipse.jdt.ui.text.java.JavaContentAssistInvocationContext; @@ -454,7 +450,7 @@ public List generateProposals(IProgressMonitor monitor) { GroovyContentAssist.logError("Exception accessing proposal provider registry", e); } - fillInExtendedContext(requestor); + getContext().extend(getJavaContext().getCoreContext(), requestor.currentScope); // extra filtering and sorting provided by third parties try { @@ -507,16 +503,6 @@ private void proposalCreatorLoop(ContentAssistContext context, ExpressionComplet } } - private void fillInExtendedContext(ExpressionCompletionRequestor requestor) { - JavaContentAssistInvocationContext javaContext = getJavaContext(); - CompletionContext coreContext = javaContext.getCoreContext(); - if (coreContext != null && !coreContext.isExtended()) { - // must use reflection to set the fields - ReflectionUtils.setPrivateField(InternalCompletionContext.class, "isExtended", coreContext, true); - ReflectionUtils.setPrivateField(InternalCompletionContext.class, "extendedContext", coreContext, new GroovyExtendedCompletionContext(getContext(), requestor.currentScope)); - } - } - protected VariableScope createTopLevelScope(ClassNode completionType) { VariableScope scope = new VariableScope(null, completionType, false); return scope; diff --git a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/requestor/ContentAssistContext.java b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/requestor/ContentAssistContext.java index e8b589cdaf..60bc8d83ed 100644 --- a/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/requestor/ContentAssistContext.java +++ b/ide/org.codehaus.groovy.eclipse.codeassist.completion/src/org/codehaus/groovy/eclipse/codeassist/requestor/ContentAssistContext.java @@ -23,14 +23,18 @@ import org.codehaus.groovy.ast.AnnotatedNode; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist; +import org.codehaus.groovy.eclipse.codeassist.completions.GroovyExtendedCompletionContext; import org.codehaus.jdt.groovy.model.GroovyCompilationUnit; +import org.eclipse.jdt.core.CompletionContext; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.JavaModelException; +import org.eclipse.jdt.groovy.core.util.ReflectionUtils; import org.eclipse.jdt.groovy.search.ITypeRequestor.VisitStatus; import org.eclipse.jdt.groovy.search.TypeInferencingVisitorFactory; import org.eclipse.jdt.groovy.search.TypeLookupResult; import org.eclipse.jdt.groovy.search.VariableScope; +import org.eclipse.jdt.internal.codeassist.InternalCompletionContext; import org.eclipse.jdt.ui.PreferenceConstants; public class ContentAssistContext { @@ -126,6 +130,14 @@ public ContentAssistContext( this.completionEnd = completionEnd; } + public final void extend(CompletionContext that, VariableScope scope) { + if (that != null && !that.isExtended()) { + if (scope == null) scope = getPerceivedCompletionScope(); + ReflectionUtils.setPrivateField(InternalCompletionContext.class, "isExtended", that, Boolean.TRUE); + ReflectionUtils.setPrivateField(InternalCompletionContext.class, "extendedContext", that, new GroovyExtendedCompletionContext(this, scope)); + } + } + public IType getEnclosingType() { try { IJavaElement element = unit.getElementAt(completionLocation);