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.

Sign up for GitHub

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

Jump to bottom

Per-parameter signature help documentation #2501

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Per-parameter signature help documentation #2501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 18 additions & 4 deletions ...dt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SignatureHelpRequestor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
Expand All @@ -44,9 +46,11 @@
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.JavadocContentAccess;
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;
import org.eclipse.lsp4j.MarkupContent;
import org.eclipse.lsp4j.MarkupKind;
import org.eclipse.lsp4j.ParameterInformation;
import org.eclipse.lsp4j.SignatureHelp;
import org.eclipse.lsp4j.SignatureInformation;
Expand Down Expand Up @@ -151,8 +155,9 @@ public SignatureInformation toSignatureInformation(CompletionProposal methodProp
SignatureInformation $ = new SignatureInformation();
StringBuilder description = descriptionProvider.createMethodProposalDescription(methodProposal);
$.setLabel(description.toString());
String documentation = null;
if (isDescriptionEnabled) {
$.setDocumentation(this.computeJavaDoc(methodProposal));
documentation = this.computeJavaDoc(methodProposal);
}

char[] signature = SignatureUtil.fix83600(methodProposal.getSignature());
Expand All @@ -174,8 +179,17 @@ public SignatureInformation toSignatureInformation(CompletionProposal methodProp
builder.append(parameterTypes[i]);
builder.append(' ');
builder.append(parameterNames[i]);
ParameterInformation parameterInformation = new ParameterInformation(builder.toString());
if (documentation != null) {
Pattern p = Pattern.compile("\\* \\*\\*" + new String(parameterNames[i]) + "\\*\\*[\\s]+([^\n\r]+)");
Matcher m = p.matcher(documentation);
if (m.find()) {
String paramDocs = m.group(1);
parameterInformation.setDocumentation(new MarkupContent(MarkupKind.MARKDOWN, paramDocs));
}
}

parameterInfos.add(new ParameterInformation(builder.toString()));
parameterInfos.add(parameterInformation);
}

$.setParameters(parameterInfos);
Expand Down Expand Up @@ -239,7 +253,7 @@ public String computeJavaDoc(CompletionProposal proposal) {
String javadoc = null;
try {
javadoc = SimpleTimeLimiter.create(JavaLanguageServerPlugin.getExecutorService()).callWithTimeout(() -> {
Reader reader = JavadocContentAccess.getPlainTextContentReader(method);
Reader reader = JavadocContentAccess2.getMarkdownContentReader(method);
return reader == null ? null : CharStreams.toString(reader);
}, 500, TimeUnit.MILLISECONDS);
} catch (UncheckedTimeoutException tooSlow) {
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.ls.filesystem/.classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="src" path="src/"/>
<classpathentry kind="output" path="target/classes"/>
<classpathentry kind="output" path="bin"/>
</classpath>
2 changes: 2 additions & 0 deletions org.eclipse.jdt.ls.tests/projects/eclipse/hello/src/org/sample/MyList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ public interface MyList<E> extends List<E> {

/**
* Test
*
* @param e test
*/
boolean add(E e);

Expand Down
4 changes: 3 additions & 1 deletion ...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3382,7 +3382,9 @@ public void testCompletion_javadocOriginal() throws JavaModelException {
CompletionItem resolvedItem = server.resolveCompletionItem(ci).join();
assertEquals(CompletionItemKind.Method, resolvedItem.getKind());
String documentation = resolvedItem.getDocumentation().getLeft();
assertEquals(" Test ", documentation);
assertEquals(" Test \n" //
+ " * Parameters:\n" //
+ " - e test", documentation);
}

// See https://github.com/redhat-developer/vscode-java/issues/2034
Expand Down
55 changes: 38 additions & 17 deletions ....jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/SignatureHelpHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager;
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
import org.eclipse.lsp4j.MarkupContent;
import org.eclipse.lsp4j.MarkupKind;
import org.eclipse.lsp4j.ParameterInformation;
import org.eclipse.lsp4j.SignatureHelp;
import org.eclipse.lsp4j.SignatureHelpParams;
import org.eclipse.lsp4j.SignatureInformation;
Expand Down Expand Up @@ -94,17 +96,20 @@ public void testSignatureHelp_singleMethod() throws JavaModelException {
StringBuilder buf = new StringBuilder();
buf.append("package test1;\n");
buf.append("public class E {\n");
buf.append(" /** This is a method */\n");
buf.append(" /** This is a method\n");
buf.append(" * @param s documentation */\n");
buf.append(" public int foo(String s) { }\n");
buf.append(" public int bar(String s) { this.foo() }\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);

SignatureHelp help = getSignatureHelp(cu, 4, 39);
SignatureHelp help = getSignatureHelp(cu, 5, 39);
assertNotNull(help);
assertEquals(1, help.getSignatures().size());
assertEquals("foo(String s) : int", help.getSignatures().get(help.getActiveSignature()).getLabel());
assertTrue(help.getSignatures().get(help.getActiveSignature()).getDocumentation().getLeft().length() > 0);
SignatureInformation activeSignature = help.getSignatures().get(help.getActiveSignature());
assertEquals("foo(String s) : int", activeSignature.getLabel());
assertNull(activeSignature.getDocumentation());
assertEquals("documentation", activeSignature.getParameters().get(0).getDocumentation().getRight().getValue());
assertEquals((Integer) 0, help.getActiveParameter());
}

Expand Down Expand Up @@ -354,8 +359,10 @@ public void testSignatureHelp_javadocOriginal() throws JavaModelException {
assertEquals(2, help.getSignatures().size());
SignatureInformation signature = help.getSignatures().get(help.getActiveSignature());
assertTrue(signature.getLabel().equals("add(String e) : boolean"));
String documentation = signature.getDocumentation().getLeft();
assertEquals(" Test ", documentation);
assertNull(signature.getDocumentation());
ParameterInformation paramInfo = signature.getParameters().get(0);
assertEquals(MarkupKind.MARKDOWN, paramInfo.getDocumentation().getRight().getKind());
assertEquals("test", paramInfo.getDocumentation().getRight().getValue());
}

@Test
Expand Down Expand Up @@ -1097,15 +1104,19 @@ public void testSignatureHelpDescriptionDisabled() throws Exception {
buf.append("public class E {\n");
buf.append(" /**\n");
buf.append(" * This is an API.\n");
buf.append(" * @param s documentation\n");
buf.append(" */\n");
buf.append(" public void foo(String s) {\n");
buf.append(" foo(null)\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
SignatureHelp help = getSignatureHelp(cu, 6, 7);
SignatureHelp help = getSignatureHelp(cu, 7, 7);
assertNotNull(help);
assertNull(help.getSignatures().get(help.getActiveSignature()).getDocumentation());
SignatureInformation activeSignature = help.getSignatures().get(help.getActiveSignature());
assertNull(activeSignature.getDocumentation());
ParameterInformation paramInfo = activeSignature.getParameters().get(0);
assertNull(paramInfo.getDocumentation());
}

@Test
Expand All @@ -1117,16 +1128,21 @@ public void testSignatureHelpDescriptionEnabled() throws Exception {
buf.append("public class E {\n");
buf.append(" /**\n");
buf.append(" * This is an API.\n");
buf.append(" * @param s docs for s\n");
buf.append(" */\n");
buf.append(" public void foo(String s) {\n");
buf.append(" foo(null)\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
SignatureHelp help = getSignatureHelp(cu, 6, 7);
SignatureHelp help = getSignatureHelp(cu, 7, 7);
assertNotNull(help);
Either<String, MarkupContent> documentation = help.getSignatures().get(help.getActiveSignature()).getDocumentation();
assertEquals("This is an API.", documentation.getLeft().trim());
SignatureInformation active = help.getSignatures().get(help.getActiveSignature());
Either<String, MarkupContent> documentation = active.getDocumentation();
assertNull(documentation);
ParameterInformation parameterInformation = active.getParameters().get(0);
assertEquals(MarkupKind.MARKDOWN, parameterInformation.getDocumentation().getRight().getKind());
assertEquals("docs for s", parameterInformation.getDocumentation().getRight().getValue());
}

@Test
Expand All @@ -1137,20 +1153,25 @@ public void testSignatureHelp_erasureType() throws Exception {
buf.append("package test1;\n");
buf.append("public class E {\n");
buf.append(" public void foo() {\n");
buf.append(" new V<String>();\n");
buf.append(" new V<String>(null);\n");
buf.append(" }\n");
buf.append("}\n");
buf.append("class V<T> {\n");
buf.append(" /** hi */\n");
buf.append(" public V() {}\n");
buf.append(" private V(String a) {}\n");
buf.append(" /** hi\n");
buf.append(" * @param a documentation */\n");
buf.append(" public V(String a) {}\n");
buf.append(" private V(String a, String b) {}\n");
buf.append("}\n");
ICompilationUnit cu = pack1.createCompilationUnit("E.java", buf.toString(), false, null);
SignatureHelp help = getSignatureHelp(cu, 3, 16);
assertNotNull(help);
assertEquals(1, help.getSignatures().size());
Either<String, MarkupContent> documentation = help.getSignatures().get(help.getActiveSignature()).getDocumentation();
assertEquals("hi", documentation.getLeft().trim());
SignatureInformation active = help.getSignatures().get(help.getActiveSignature());
Either<String, MarkupContent> documentation = active.getDocumentation();
assertNull(documentation);
ParameterInformation parameterInformation = active.getParameters().get(0);
assertEquals(MarkupKind.MARKDOWN, parameterInformation.getDocumentation().getRight().getKind());
assertEquals("documentation", parameterInformation.getDocumentation().getRight().getValue());
}

@Test
Expand Down