Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix signature help for overloaded methods #3073

Merged
merged 1 commit into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading