Skip to content

Commit

Permalink
Signature help returns wrong active parameter
Browse files Browse the repository at this point in the history 
Signed-off-by: Snjezana Peco <[email protected]>
  • Loading branch information
@snjeza
snjeza committed Jun 6, 2019
1 parent f51ccd6 commit e21bf40
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 23 deletions.
118 changes: 96 additions & 22 deletions ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.jdt.core.CompletionProposal;
import org.eclipse.jdt.core.Flags;
import org.eclipse.jdt.core.IBuffer;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.IMethod;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.ITypeParameter;
Expand Down Expand Up @@ -78,40 +80,68 @@ public SignatureHelp signatureHelp(TextDocumentPositionParams position, IProgres
if (offset > -1 && !monitor.isCanceled()) {
unit.codeComplete(contextInfomation[0] + 1, collector, monitor);
help = collector.getSignatureHelp(monitor);
if (help != null && help.getSignatures().size() > 0) {
int size = getArgumentsSize(node);
if (!monitor.isCanceled() && help != null) {
SignatureHelp help2 = null;
SignatureHelpRequestor collector2 = null;
if (contextInfomation[0] + 1 != offset) {
collector2 = new SignatureHelpRequestor(unit, offset);
unit.codeComplete(offset, collector2, monitor);
help2 = collector2.getSignatureHelp(monitor);
}
int currentParameter = contextInfomation[1];
char[] signature = getSignature(node);
size = Math.max(currentParameter + 1, size);
int size = currentParameter + 1;
List<SignatureInformation> infos = help.getSignatures();
if (signature != null) {
int activeParameter = currentParameter < 0 ? 0 : currentParameter;
if (node != null) {
for (int i = 0; i < infos.size(); i++) {
if (infos.get(i).getParameters().size() >= size) {
CompletionProposal proposal = collector.getInfoProposals().get(infos.get(i));
char[][] signatureTypes = Signature.getParameterTypes(signature);
char[][] infoTypes = Signature.getParameterTypes(SignatureUtil.fix83600(proposal.getSignature()));
if (Arrays.deepEquals(signatureTypes, infoTypes)) {
IMethod m = JDTUtils.resolveMethod(proposal, unit.getJavaProject());
if (help2 == null || isSameParameters(m, help2, collector2, unit.getJavaProject())) {
help.setActiveSignature(i);
help.setActiveParameter(currentParameter < 0 ? 0 : currentParameter);
help.setActiveParameter(activeParameter);
break;
}
if (size > 0) {
if (infoTypes.length - 1 == signatureTypes.length) {
infoTypes = arrayClone(infoTypes, infoTypes.length - 1);
}

}
}
if (!monitor.isCanceled() && help.getActiveSignature() == null && help2.getSignatures().size() <= 0) {
char[] signature = getSignature(node);
for (int i = 0; i < infos.size(); i++) {
if (infos.get(i).getParameters().size() >= size) {
CompletionProposal proposal = collector.getInfoProposals().get(infos.get(i));
char[][] signatureTypes = Signature.getParameterTypes(signature);
char[][] infoTypes = Signature.getParameterTypes(SignatureUtil.fix83600(proposal.getSignature()));
if (Arrays.deepEquals(signatureTypes, infoTypes)) {
help.setActiveSignature(i);
help.setActiveParameter(currentParameter < 0 ? 0 : currentParameter);
break;
help.setActiveParameter(activeParameter);
}
if (size > 0) {
if (infoTypes.length - 1 == signatureTypes.length) {
infoTypes = arrayClone(infoTypes, infoTypes.length - 1);
}
if (Arrays.deepEquals(signatureTypes, infoTypes)) {
help.setActiveSignature(i);
help.setActiveParameter(activeParameter);
break;
}
}
}
}
}
} else {
for (int i = 0; i < infos.size(); i++) {
if (infos.get(i).getParameters().size() >= size) {
help.setActiveSignature(i);
help.setActiveParameter(currentParameter < 0 ? 0 : currentParameter);
break;
if (!monitor.isCanceled() && help.getActiveSignature() == null) {
for (int i = 0; i < infos.size(); i++) {
CompletionProposal proposal = collector.getInfoProposals().get(infos.get(i));
if (Flags.isVarargs(proposal.getFlags())) {
help.setActiveSignature(i);
char[][] infoTypes = Signature.getParameterTypes(SignatureUtil.fix83600(proposal.getSignature()));
if (infoTypes.length <= activeParameter) {
help.setActiveParameter(infoTypes.length - 1);
} else {
help.setActiveParameter(activeParameter);
}
break;
}
}
}
}
Expand All @@ -123,6 +153,40 @@ public SignatureHelp signatureHelp(TextDocumentPositionParams position, IProgres
return help;
}

private boolean isSameParameters(IMethod m, SignatureHelp help, SignatureHelpRequestor collector, IJavaProject javaProject) throws JavaModelException {
if (m == null || help == null || javaProject == null) {
return false;
}
List<SignatureInformation> infos = help.getSignatures();
for (int i = 0; i < infos.size(); i++) {
CompletionProposal proposal = collector.getInfoProposals().get(infos.get(i));
IMethod method = JDTUtils.resolveMethod(proposal, javaProject);
if (isSameParameters(method, m)) {
return true;
}
}
return false;
}

private boolean isSameParameters(IMethod method1, IMethod method2) {
if (method1 == null || method2 == null) {
return false;
}
String[] params1 = method1.getParameterTypes();
String[] params2 = method2.getParameterTypes();
if (params2.length == params1.length) {
for (int i = 0; i < params2.length; i++) {
String t1 = Signature.getSimpleName(Signature.toString(params2[i]));
String t2 = Signature.getSimpleName(Signature.toString(params1[i]));
if (!t1.equals(t2)) {
return false;
}
}
return true;
}
return false;
}

private static char[][] arrayClone(char[][] src, int length) {
if (src.length < 1) {
return src;
Expand Down Expand Up @@ -193,6 +257,9 @@ private static String resolveTypeSignature(IMethod method, String typeSignature)
String elementTypeName = Signature.toString(elementTypeSignature);
IType type = method.getDeclaringType();
String[][] resolvedElementTypeNames = type.resolveType(elementTypeName);
if (resolvedElementTypeNames == null) {
resolvedElementTypeNames = type.resolveType(elementTypeName.replaceAll("/", "."));
}
if (resolvedElementTypeNames == null || resolvedElementTypeNames.length != 1) {
// check if type parameter
ITypeParameter typeParameter = method.getTypeParameter(elementTypeName);
Expand Down Expand Up @@ -276,7 +343,14 @@ private int[] getContextInfomation(IBuffer buffer, int offset) {
}
// Assuming user are typing current parameter:
if (result[0] + 1 != offset) {
result[1]++;
int i = 1;
while (result[0] + i < offset) {
if (!Character.isWhitespace(buffer.getChar(result[0] + i))) {
result[1]++;
break;
}
i++;
}
}
return result;
}
Expand Down
48 changes: 47 additions & 1 deletion ....jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.eclipse.jdt.ls.core.internal.JsonMessageHelper.getParams;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -125,7 +126,10 @@ public void testSignatureHelp_binary() throws JavaModelException {
SignatureHelp help = getSignatureHelp(cu, 2, 50);
assertNotNull(help);
assertTrue(help.getSignatures().size() >= 10);
assertTrue(help.getSignatures().get(help.getActiveSignature()).getLabel().matches("println\\(\\w+ \\w+\\) : void"));
assertTrue(help.getSignatures().get(help.getActiveSignature()).getLabel().equals("println() : void"));
SignatureHelp help2 = getSignatureHelp(cu, 2, 49);
assertEquals(help.getSignatures().size(), help2.getSignatures().size());
assertEquals(help.getActiveSignature(), help2.getActiveSignature());
}

@Test
Expand Down Expand Up @@ -252,6 +256,48 @@ public void testSignatureHelp_javadoc() throws JavaModelException {
assertTrue(help.getSignatures().get(help.getActiveSignature()).getLabel().matches("substring\\(\\w+ \\w+\\) : String"));
}

@Test
public void testSignatureHelp_varargs() throws JavaModelException {
IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("import java.util.Arrays;\n");
buf.append("public class E {\n");
buf.append(" public static void main(String[] args) {\n");
buf.append(" Arrays.asList(1,2,3);\n");
buf.append(" demo(\"1\", \"2\",\"3\" )\n");
buf.append(" }\n");
buf.append(" public static void demo (String s, String... s2) {\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
SignatureHelp help = getSignatureHelp(cu, 4, 21);
assertNotNull(help);
assertEquals(1, help.getSignatures().size());
assertTrue(help.getSignatures().get(help.getActiveSignature()).getLabel().startsWith("asList(T... "));
help = getSignatureHelp(cu, 5, 19);
assertNotNull(help);
assertEquals(1, help.getSignatures().size());
assertTrue(help.getSignatures().get(help.getActiveSignature()).getLabel().equals("demo(String s, String... s2) : void"));
}

@Test
public void testSignatureHelp_parameterTypes() throws JavaModelException {
IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("public class E {\n");
buf.append(" public static void main(String[] args) {\n");
buf.append(" new RuntimeException(new Exception(),)\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
SignatureHelp help = getSignatureHelp(cu, 3, 40);
assertNotNull(help);
assertEquals(4, help.getSignatures().size());
assertNull(help.getActiveParameter());
}

private SignatureHelp getSignatureHelp(ICompilationUnit cu, int line, int character) {
String payload = createSignatureHelpRequest(cu, line, character);
TextDocumentPositionParams position = getParams(payload);
Expand Down

0 comments on commit e21bf40

Please sign in to comment.