From 636b29f950698dba227f4ffa915f8754a506a903 Mon Sep 17 00:00:00 2001 From: Snjezana Peco Date: Thu, 6 Jun 2019 19:42:39 +0200 Subject: [PATCH] Signature help returns wrong active parameter Signed-off-by: Snjezana Peco --- .../handlers/SignatureHelpHandler.java | 118 ++++++++++++++---- .../handlers/SignatureHelpHandlerTest.java | 48 ++++++- 2 files changed, 143 insertions(+), 23 deletions(-) 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 aa436ea17a..99b033fc6a 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 @@ -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; @@ -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 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; + } } } } @@ -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 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; @@ -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); @@ -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; } 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 0b55073511..07e7eee82a 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 @@ -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; @@ -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 @@ -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);