Skip to content

Commit

Permalink
Fix signature help for overloaded methods
Browse files Browse the repository at this point in the history
Signed-off-by: Hope Hadfield <[email protected]>
  • Loading branch information
hopehadfield committed Mar 6, 2024
1 parent 4b2b504 commit e1f12d1
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -578,7 +579,7 @@ private void appendReplacementString(StringBuilder buffer, CompletionProposal pr
buffer.append(LPAREN);
}

if (hasParameters(proposal)) {
if (hasParameters(proposal) || overloadedMethod) {
appendGuessingCompletion(buffer, proposal);
}

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -64,20 +63,18 @@ public final class SignatureHelpRequestor extends CompletionRequestor {
private boolean acceptType = false;
private String methodName;
private boolean isDescriptionEnabled;
private List<String> declaringTypeNames;

public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List<String> declaringTypeName) {
this(aUnit, methodName, declaringTypeName, false);
public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName) {
this(aUnit, methodName, false);
}

public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List<String> 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) {
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ public class SignatureHelpContext {
*/
private String methodName;

/**
* {@link #declaringTypeNames()}
*/
private List<String> declaringTypeNames;

/**
* {@link #arguments()}
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -323,50 +317,6 @@ private void resolveMethodName(ASTNode node) {
}
}

/**
* Get the declaring type names of the method-like node. Following names will be added:
* <ul>
* <li> The declaring type</li>
* <li> All the super types</li>
* <li> All the interfaces</li>
* </ul>
*
* @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<String> 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
Expand Down Expand Up @@ -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<String> declaringTypeNames() {
return declaringTypeNames;
}

/**
* The argument nodes parsed from AST.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -255,7 +255,7 @@ public static void fix2097(SignatureHelp help, ASTNode node, SignatureHelpReques
if (type == null) {
return;
}

IMethod[] methods = type.getMethods();
List<SignatureInformation> infos = new ArrayList<>();
for (IMethod method : methods) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit e1f12d1

Please sign in to comment.