From b33ad90d332b6efa13d15abfbd44b6cbc91f0de1 Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 20 Feb 2024 07:23:35 -0500 Subject: [PATCH] Fix signature help for overloaded methods Signed-off-by: Hope Hadfield --- ...CompletionProposalReplacementProvider.java | 11 ++-- .../contentassist/SignatureHelpRequestor.java | 23 +------- .../handlers/SignatureHelpContext.java | 58 ------------------- .../handlers/SignatureHelpHandler.java | 4 +- .../internal/handlers/SignatureHelpUtils.java | 8 +-- .../handlers/SignatureHelpHandlerTest.java | 27 +++++++++ 6 files changed, 42 insertions(+), 89 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java index e1ff9b34e9..4fd04ddfa4 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java @@ -235,7 +235,7 @@ private String getTextEditText(CompletionProposal proposal, CompletionItem item, if (proposal instanceof GetterSetterCompletionProposal getterSetterProposal) { appendMethodPotentialReplacement(completionBuffer, getterSetterProposal); } else { - appendReplacementString(completionBuffer, proposal); + appendReplacementString(completionBuffer, proposal, false); } break; case CompletionProposal.ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION: @@ -246,7 +246,8 @@ private String getTextEditText(CompletionProposal proposal, CompletionItem item, appendLambdaExpressionReplacement(completionBuffer, proposal); break; default: - appendReplacementString(completionBuffer, proposal); + boolean overloadedMethodProposal = item.getLabelDetails() == null ? false : "(...)".equals(item.getLabelDetails().getDetail()); + appendReplacementString(completionBuffer, proposal, overloadedMethodProposal); break; } return completionBuffer.toString(); @@ -534,7 +535,7 @@ private boolean isInJavadoc() { return context.isInJavadoc(); } - private void appendReplacementString(StringBuilder buffer, CompletionProposal proposal) { + private void appendReplacementString(StringBuilder buffer, CompletionProposal proposal, boolean overloadedMethod) { final boolean completionSnippetsSupported = client.isCompletionSnippetsSupported(); if (!hasArgumentList(proposal)) { String str = null; @@ -578,7 +579,7 @@ private void appendReplacementString(StringBuilder buffer, CompletionProposal pr buffer.append(LPAREN); } - if (hasParameters(proposal)) { + if (hasParameters(proposal) || overloadedMethod) { appendGuessingCompletion(buffer, proposal); } @@ -734,7 +735,7 @@ private final boolean canAutomaticallyAppendSemicolon(CompletionProposal proposa private org.eclipse.lsp4j.TextEdit toRequiredTypeEdit(CompletionProposal typeProposal, char trigger, boolean canUseDiamond) { StringBuilder buffer = new StringBuilder(); - appendReplacementString(buffer, typeProposal); + appendReplacementString(buffer, typeProposal, false); if (compilationUnit == null /*|| getContext() != null && getContext().isInJavadoc()*/) { Range range = toReplacementRange(typeProposal); diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java index 14d471ee8a..e7917435e3 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java @@ -43,7 +43,6 @@ import org.eclipse.jdt.internal.corext.template.java.SignatureUtil; import org.eclipse.jdt.internal.corext.util.JavaModelUtil; import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin; -import org.eclipse.jdt.ls.core.internal.handlers.SignatureHelpUtils; import org.eclipse.jdt.ls.core.internal.javadoc.JavadocContentAccess2; import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager; import org.eclipse.jdt.ls.core.internal.preferences.Preferences; @@ -64,20 +63,18 @@ public final class SignatureHelpRequestor extends CompletionRequestor { private boolean acceptType = false; private String methodName; private boolean isDescriptionEnabled; - private List declaringTypeNames; - public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List declaringTypeName) { - this(aUnit, methodName, declaringTypeName, false); + public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName) { + this(aUnit, methodName, false); } - public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List declaringTypeName, boolean acceptType) { + public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, boolean acceptType) { this.unit = aUnit; setRequireExtendedContext(true); infoProposals = new HashMap<>(); this.acceptType = acceptType; this.methodName = methodName; this.isDescriptionEnabled = isDescriptionEnabled(); - this.declaringTypeNames = declaringTypeName; } public SignatureHelp getSignatureHelp(IProgressMonitor monitor) { @@ -121,20 +118,6 @@ public void accept(CompletionProposal proposal) { if (proposal.getKind() == CompletionProposal.METHOD_REF && !Objects.equals(proposal.getName() == null ? null : new String(proposal.getName()), methodName)) { return; } - if (this.declaringTypeNames != null) { - char[] declarationSignature = proposal.getDeclarationSignature(); - if (declarationSignature != null) { - String proposalTypeSimpleName = SignatureHelpUtils.getSimpleTypeName(String.valueOf(declarationSignature)); - for (String typeName : this.declaringTypeNames) { - String declaringTypeSimpleName = Signature.getSimpleName(typeName); - if (Objects.equals(proposalTypeSimpleName, declaringTypeSimpleName)) { - proposals.putIfAbsent(String.valueOf(proposal.getSignature()), proposal); - return; - } - } - return; - } - } proposals.putIfAbsent(String.valueOf(proposal.getSignature()), proposal); } } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpContext.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpContext.java index a3f6159f87..f4bf5f9bf6 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpContext.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpContext.java @@ -63,11 +63,6 @@ public class SignatureHelpContext { */ private String methodName; - /** - * {@link #declaringTypeNames()} - */ - private List declaringTypeNames; - /** * {@link #arguments()} */ @@ -102,7 +97,6 @@ public void resolve(int triggerOffset, ICompilationUnit unit, IProgressMonitor m } findTargetNode(root, unit, triggerOffset); resolveMethodName(this.targetNode); - resolveDeclaringTypeName(this.targetNode); this.arguments = resolveArguments(this.targetNode); resolveParameterTypes(this.targetNode); guessCompletionOffset(this.targetNode, unit); @@ -323,50 +317,6 @@ private void resolveMethodName(ASTNode node) { } } - /** - * Get the declaring type names of the method-like node. Following names will be added: - *
    - *
  • The declaring type
    • - *
  • All the super types
    • - *
  • All the interfaces
    • - *
- * - * @param node - */ - private void resolveDeclaringTypeName(ASTNode node) { - if (node == null) { - return; - } - - IMethodBinding methodBinding = null; - if (node instanceof MethodInvocation methodInvocation) { - methodBinding = methodInvocation.resolveMethodBinding(); - } else if (node instanceof ClassInstanceCreation classInstanceCreation) { - methodBinding = classInstanceCreation.resolveConstructorBinding(); - } else if (node instanceof SuperMethodInvocation superMethodInvocation) { - methodBinding = superMethodInvocation.resolveMethodBinding(); - } else if (node instanceof SuperConstructorInvocation superConstructorInvocation) { - methodBinding = superConstructorInvocation.resolveConstructorBinding(); - } else if (node instanceof ConstructorInvocation constructorInvocation) { - methodBinding = constructorInvocation.resolveConstructorBinding(); - } - - if (methodBinding != null) { - ITypeBinding declaringType = methodBinding.getDeclaringClass(); - List typeNames = new ArrayList<>(); - for (ITypeBinding mInterface : declaringType.getInterfaces()) { - String unqualifiedName = mInterface.getErasure().getName().replace(";", ""); - typeNames.add(unqualifiedName); - } - while (declaringType != null) { - String unqualifiedName = declaringType.getErasure().getName().replace(";", ""); - typeNames.add(unqualifiedName); - declaringType = declaringType.getSuperclass(); - } - this.declaringTypeNames = typeNames; - } - } - /** * Get the argument list of the input method-like node. * @param node @@ -682,14 +632,6 @@ public String methodName() { return methodName; } - /** - * The declaring type name of the method invocation. It's used to filter methods from - * different types but with same names that provided by the completion engine. - */ - public List declaringTypeNames() { - return declaringTypeNames; - } - /** * The argument nodes parsed from AST. */ diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java index 3d44d75171..8b4b27154d 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java @@ -92,7 +92,7 @@ public SignatureHelp signatureHelp(SignatureHelpParams position, IProgressMonito } IMethod method = getMethod(node); String name = method != null ? method.getElementName() : getMethodName(node, unit, contextInfomation); - SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, name, null); + SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, name); if (offset > -1 && !monitor.isCanceled()) { int pos = contextInfomation[0] + 1; if (method != null) { @@ -115,7 +115,7 @@ public SignatureHelp signatureHelp(SignatureHelpParams position, IProgressMonito SignatureHelp help2 = null; SignatureHelpRequestor collector2 = null; if (contextInfomation[0] + 1 != offset) { - collector2 = new SignatureHelpRequestor(unit, name, null, true); + collector2 = new SignatureHelpRequestor(unit, name, true); unit.codeComplete(offset, collector2, monitor); help2 = collector2.getSignatureHelp(monitor); } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpUtils.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpUtils.java index a1e234ef8f..3b83d231d4 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpUtils.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpUtils.java @@ -72,10 +72,10 @@ public static SignatureHelp getSignatureHelpFromASTNode(ICompilationUnit unit, i } } - SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, context.methodName(), context.declaringTypeNames()); + SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, context.methodName()); unit.codeComplete(context.completionOffset(), collector, monitor); help = collector.getSignatureHelp(monitor); - if (help.getSignatures().isEmpty() && context.secondaryCompletionOffset() > -1) { + if (context.secondaryCompletionOffset() > -1) { unit.codeComplete(context.secondaryCompletionOffset(), collector, monitor); help = collector.getSignatureHelp(monitor); } @@ -135,7 +135,7 @@ private static boolean isMatched(CompletionProposal proposal, SignatureInformati return false; } String[] parameterTypes = Signature.getParameterTypes(String.valueOf(proposal.getSignature())); - + // since the signature information are sorted by the parameter numbers, if the user's code does not // contain argument right now, we can say this is a match. if (context.arguments().isEmpty()) { @@ -255,7 +255,7 @@ public static void fix2097(SignatureHelp help, ASTNode node, SignatureHelpReques if (type == null) { return; } - + IMethod[] methods = type.getMethods(); List infos = new ArrayList<>(); for (IMethod method : methods) { diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java index c9f9b7b66f..46b1b5010e 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java @@ -744,6 +744,9 @@ public void testSignatureHelp_differentDeclaringType() throws Exception { ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null); SignatureHelp help = getSignatureHelp(cu, 7, 15); assertNotNull(help); + // There should really only be one signature in this list, though there are two as a result of + // https://github.com/eclipse-jdtls/eclipse.jdt.ls/pull/3073 + assertEquals(2, help.getSignatures().size()); SignatureInformation signature = help.getSignatures().get(help.getActiveSignature()); assertTrue(signature.getLabel().equals("foo() : String")); } @@ -1238,6 +1241,30 @@ public foo() { assertTrue(l.contains(fromHelpSignatures)); } + @Test + public void testSignatureHelp_overloads() throws JavaModelException { + IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null); + StringBuilder buf = new StringBuilder(); + buf.append("package test1;\n"); + buf.append("public class Car extends Vehicle {\n"); + buf.append(" public void speed(int x, int y) {}\n"); + buf.append(" public static void main(int [] args) {\n"); + buf.append(" Car test = new Car();\n"); + buf.append(" test.speed();\n"); + buf.append(" }\n"); + buf.append("}\n"); + buf.append("class Vehicle extends MovingObject {\n"); + buf.append(" public void speed(int x) {}\n"); + buf.append("}\n"); + buf.append("class MovingObject {\n"); + buf.append(" public void speed(int x, int y, int z) {}\n"); + buf.append("}\n"); + ICompilationUnit cu = pack1.createCompilationUnit("Car.java", buf.toString(), false, null); + SignatureHelp help = getSignatureHelp(cu, 5, 17); + assertNotNull(help); + assertEquals(3, help.getSignatures().size()); + } + private void testAssertEquals(ICompilationUnit cu, int line, int character) { SignatureHelp help = getSignatureHelp(cu, line, character); assertNotNull(help);