Skip to content

Commit b33ad90

Browse files
hopehadfieldrgrunber
hopehadfield
authored and
rgrunber
committed
Fix signature help for overloaded methods
Signed-off-by: Hope Hadfield <[email protected]>
1 parent 4b2b504 commit b33ad90

File tree

6 files changed

+42
-89
lines changed

6 files changed

+42
-89
lines changed

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ private String getTextEditText(CompletionProposal proposal, CompletionItem item,
235235
if (proposal instanceof GetterSetterCompletionProposal getterSetterProposal) {
236236
appendMethodPotentialReplacement(completionBuffer, getterSetterProposal);
237237
} else {
238-
appendReplacementString(completionBuffer, proposal);
238+
appendReplacementString(completionBuffer, proposal, false);
239239
}
240240
break;
241241
case CompletionProposal.ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION:
@@ -246,7 +246,8 @@ private String getTextEditText(CompletionProposal proposal, CompletionItem item,
246246
appendLambdaExpressionReplacement(completionBuffer, proposal);
247247
break;
248248
default:
249-
appendReplacementString(completionBuffer, proposal);
249+
boolean overloadedMethodProposal = item.getLabelDetails() == null ? false : "(...)".equals(item.getLabelDetails().getDetail());
250+
appendReplacementString(completionBuffer, proposal, overloadedMethodProposal);
250251
break;
251252
}
252253
return completionBuffer.toString();
@@ -534,7 +535,7 @@ private boolean isInJavadoc() {
534535
return context.isInJavadoc();
535536
}
536537

537-
private void appendReplacementString(StringBuilder buffer, CompletionProposal proposal) {
538+
private void appendReplacementString(StringBuilder buffer, CompletionProposal proposal, boolean overloadedMethod) {
538539
final boolean completionSnippetsSupported = client.isCompletionSnippetsSupported();
539540
if (!hasArgumentList(proposal)) {
540541
String str = null;
@@ -578,7 +579,7 @@ private void appendReplacementString(StringBuilder buffer, CompletionProposal pr
578579
buffer.append(LPAREN);
579580
}
580581

581-
if (hasParameters(proposal)) {
582+
if (hasParameters(proposal) || overloadedMethod) {
582583
appendGuessingCompletion(buffer, proposal);
583584
}
584585

@@ -734,7 +735,7 @@ private final boolean canAutomaticallyAppendSemicolon(CompletionProposal proposa
734735
private org.eclipse.lsp4j.TextEdit toRequiredTypeEdit(CompletionProposal typeProposal, char trigger, boolean canUseDiamond) {
735736

736737
StringBuilder buffer = new StringBuilder();
737-
appendReplacementString(buffer, typeProposal);
738+
appendReplacementString(buffer, typeProposal, false);
738739

739740
if (compilationUnit == null /*|| getContext() != null && getContext().isInJavadoc()*/) {
740741
Range range = toReplacementRange(typeProposal);

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java

+3-20
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.eclipse.jdt.internal.corext.template.java.SignatureUtil;
4444
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
4545
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
46-
import org.eclipse.jdt.ls.core.internal.handlers.SignatureHelpUtils;
4746
import org.eclipse.jdt.ls.core.internal.javadoc.JavadocContentAccess2;
4847
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
4948
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
@@ -64,20 +63,18 @@ public final class SignatureHelpRequestor extends CompletionRequestor {
6463
private boolean acceptType = false;
6564
private String methodName;
6665
private boolean isDescriptionEnabled;
67-
private List<String> declaringTypeNames;
6866

69-
public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List<String> declaringTypeName) {
70-
this(aUnit, methodName, declaringTypeName, false);
67+
public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName) {
68+
this(aUnit, methodName, false);
7169
}
7270

73-
public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, List<String> declaringTypeName, boolean acceptType) {
71+
public SignatureHelpRequestor(ICompilationUnit aUnit, String methodName, boolean acceptType) {
7472
this.unit = aUnit;
7573
setRequireExtendedContext(true);
7674
infoProposals = new HashMap<>();
7775
this.acceptType = acceptType;
7876
this.methodName = methodName;
7977
this.isDescriptionEnabled = isDescriptionEnabled();
80-
this.declaringTypeNames = declaringTypeName;
8178
}
8279

8380
public SignatureHelp getSignatureHelp(IProgressMonitor monitor) {
@@ -121,20 +118,6 @@ public void accept(CompletionProposal proposal) {
121118
if (proposal.getKind() == CompletionProposal.METHOD_REF && !Objects.equals(proposal.getName() == null ? null : new String(proposal.getName()), methodName)) {
122119
return;
123120
}
124-
if (this.declaringTypeNames != null) {
125-
char[] declarationSignature = proposal.getDeclarationSignature();
126-
if (declarationSignature != null) {
127-
String proposalTypeSimpleName = SignatureHelpUtils.getSimpleTypeName(String.valueOf(declarationSignature));
128-
for (String typeName : this.declaringTypeNames) {
129-
String declaringTypeSimpleName = Signature.getSimpleName(typeName);
130-
if (Objects.equals(proposalTypeSimpleName, declaringTypeSimpleName)) {
131-
proposals.putIfAbsent(String.valueOf(proposal.getSignature()), proposal);
132-
return;
133-
}
134-
}
135-
return;
136-
}
137-
}
138121
proposals.putIfAbsent(String.valueOf(proposal.getSignature()), proposal);
139122
}
140123
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpContext.java

-58
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,6 @@ public class SignatureHelpContext {
6363
*/
6464
private String methodName;
6565

66-
/**
67-
* {@link #declaringTypeNames()}
68-
*/
69-
private List<String> declaringTypeNames;
70-
7166
/**
7267
* {@link #arguments()}
7368
*/
@@ -102,7 +97,6 @@ public void resolve(int triggerOffset, ICompilationUnit unit, IProgressMonitor m
10297
}
10398
findTargetNode(root, unit, triggerOffset);
10499
resolveMethodName(this.targetNode);
105-
resolveDeclaringTypeName(this.targetNode);
106100
this.arguments = resolveArguments(this.targetNode);
107101
resolveParameterTypes(this.targetNode);
108102
guessCompletionOffset(this.targetNode, unit);
@@ -323,50 +317,6 @@ private void resolveMethodName(ASTNode node) {
323317
}
324318
}
325319

326-
/**
327-
* Get the declaring type names of the method-like node. Following names will be added:
328-
* <ul>
329-
* <li> The declaring type</li>
330-
* <li> All the super types</li>
331-
* <li> All the interfaces</li>
332-
* </ul>
333-
*
334-
* @param node
335-
*/
336-
private void resolveDeclaringTypeName(ASTNode node) {
337-
if (node == null) {
338-
return;
339-
}
340-
341-
IMethodBinding methodBinding = null;
342-
if (node instanceof MethodInvocation methodInvocation) {
343-
methodBinding = methodInvocation.resolveMethodBinding();
344-
} else if (node instanceof ClassInstanceCreation classInstanceCreation) {
345-
methodBinding = classInstanceCreation.resolveConstructorBinding();
346-
} else if (node instanceof SuperMethodInvocation superMethodInvocation) {
347-
methodBinding = superMethodInvocation.resolveMethodBinding();
348-
} else if (node instanceof SuperConstructorInvocation superConstructorInvocation) {
349-
methodBinding = superConstructorInvocation.resolveConstructorBinding();
350-
} else if (node instanceof ConstructorInvocation constructorInvocation) {
351-
methodBinding = constructorInvocation.resolveConstructorBinding();
352-
}
353-
354-
if (methodBinding != null) {
355-
ITypeBinding declaringType = methodBinding.getDeclaringClass();
356-
List<String> typeNames = new ArrayList<>();
357-
for (ITypeBinding mInterface : declaringType.getInterfaces()) {
358-
String unqualifiedName = mInterface.getErasure().getName().replace(";", "");
359-
typeNames.add(unqualifiedName);
360-
}
361-
while (declaringType != null) {
362-
String unqualifiedName = declaringType.getErasure().getName().replace(";", "");
363-
typeNames.add(unqualifiedName);
364-
declaringType = declaringType.getSuperclass();
365-
}
366-
this.declaringTypeNames = typeNames;
367-
}
368-
}
369-
370320
/**
371321
* Get the argument list of the input method-like node.
372322
* @param node
@@ -682,14 +632,6 @@ public String methodName() {
682632
return methodName;
683633
}
684634

685-
/**
686-
* The declaring type name of the method invocation. It's used to filter methods from
687-
* different types but with same names that provided by the completion engine.
688-
*/
689-
public List<String> declaringTypeNames() {
690-
return declaringTypeNames;
691-
}
692-
693635
/**
694636
* The argument nodes parsed from AST.
695637
*/

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public SignatureHelp signatureHelp(SignatureHelpParams position, IProgressMonito
9292
}
9393
IMethod method = getMethod(node);
9494
String name = method != null ? method.getElementName() : getMethodName(node, unit, contextInfomation);
95-
SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, name, null);
95+
SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, name);
9696
if (offset > -1 && !monitor.isCanceled()) {
9797
int pos = contextInfomation[0] + 1;
9898
if (method != null) {
@@ -115,7 +115,7 @@ public SignatureHelp signatureHelp(SignatureHelpParams position, IProgressMonito
115115
SignatureHelp help2 = null;
116116
SignatureHelpRequestor collector2 = null;
117117
if (contextInfomation[0] + 1 != offset) {
118-
collector2 = new SignatureHelpRequestor(unit, name, null, true);
118+
collector2 = new SignatureHelpRequestor(unit, name, true);
119119
unit.codeComplete(offset, collector2, monitor);
120120
help2 = collector2.getSignatureHelp(monitor);
121121
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpUtils.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ public static SignatureHelp getSignatureHelpFromASTNode(ICompilationUnit unit, i
7272
}
7373
}
7474

75-
SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, context.methodName(), context.declaringTypeNames());
75+
SignatureHelpRequestor collector = new SignatureHelpRequestor(unit, context.methodName());
7676
unit.codeComplete(context.completionOffset(), collector, monitor);
7777
help = collector.getSignatureHelp(monitor);
78-
if (help.getSignatures().isEmpty() && context.secondaryCompletionOffset() > -1) {
78+
if (context.secondaryCompletionOffset() > -1) {
7979
unit.codeComplete(context.secondaryCompletionOffset(), collector, monitor);
8080
help = collector.getSignatureHelp(monitor);
8181
}
@@ -135,7 +135,7 @@ private static boolean isMatched(CompletionProposal proposal, SignatureInformati
135135
return false;
136136
}
137137
String[] parameterTypes = Signature.getParameterTypes(String.valueOf(proposal.getSignature()));
138-
138+
139139
// since the signature information are sorted by the parameter numbers, if the user's code does not
140140
// contain argument right now, we can say this is a match.
141141
if (context.arguments().isEmpty()) {
@@ -255,7 +255,7 @@ public static void fix2097(SignatureHelp help, ASTNode node, SignatureHelpReques
255255
if (type == null) {
256256
return;
257257
}
258-
258+
259259
IMethod[] methods = type.getMethods();
260260
List<SignatureInformation> infos = new ArrayList<>();
261261
for (IMethod method : methods) {

‎org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java

+27
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,9 @@ public void testSignatureHelp_differentDeclaringType() throws Exception {
744744
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
745745
SignatureHelp help = getSignatureHelp(cu, 7, 15);
746746
assertNotNull(help);
747+
// There should really only be one signature in this list, though there are two as a result of
748+
// https://github.com/eclipse-jdtls/eclipse.jdt.ls/pull/3073
749+
assertEquals(2, help.getSignatures().size());
747750
SignatureInformation signature = help.getSignatures().get(help.getActiveSignature());
748751
assertTrue(signature.getLabel().equals("foo() : String"));
749752
}
@@ -1238,6 +1241,30 @@ public foo() {
12381241
assertTrue(l.contains(fromHelpSignatures));
12391242
}
12401243

1244+
@Test
1245+
public void testSignatureHelp_overloads() throws JavaModelException {
1246+
IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
1247+
StringBuilder buf = new StringBuilder();
1248+
buf.append("package test1;\n");
1249+
buf.append("public class Car extends Vehicle {\n");
1250+
buf.append(" public void speed(int x, int y) {}\n");
1251+
buf.append(" public static void main(int [] args) {\n");
1252+
buf.append(" Car test = new Car();\n");
1253+
buf.append(" test.speed();\n");
1254+
buf.append(" }\n");
1255+
buf.append("}\n");
1256+
buf.append("class Vehicle extends MovingObject {\n");
1257+
buf.append(" public void speed(int x) {}\n");
1258+
buf.append("}\n");
1259+
buf.append("class MovingObject {\n");
1260+
buf.append(" public void speed(int x, int y, int z) {}\n");
1261+
buf.append("}\n");
1262+
ICompilationUnit cu = pack1.createCompilationUnit("Car.java", buf.toString(), false, null);
1263+
SignatureHelp help = getSignatureHelp(cu, 5, 17);
1264+
assertNotNull(help);
1265+
assertEquals(3, help.getSignatures().size());
1266+
}
1267+
12411268
private void testAssertEquals(ICompilationUnit cu, int line, int character) {
12421269
SignatureHelp help = getSignatureHelp(cu, line, character);
12431270
assertNotNull(help);

0 commit comments

Comments
 (0)