8145263: JShell: Fix the format of SourceCodeAnalysis#documentation

Reviewed-by: rfield, jlahoda

langtools/src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java

@@ -116,6 +116,7 @@ import javax.lang.model.SourceVersion;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.PackageElement;
 import javax.lang.model.element.QualifiedNameable;
+import javax.lang.model.element.TypeParameterElement;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.ArrayType;
 import javax.lang.model.type.ExecutableType;
@@ -132,6 +133,7 @@ import javax.tools.ToolProvider;
 import static jdk.jshell.Util.REPL_DOESNOTMATTER_CLASS_NAME;
 import static java.util.stream.Collectors.joining;
 import static jdk.jshell.SourceCodeAnalysis.Completeness.DEFINITELY_INCOMPLETE;
+import static jdk.jshell.TreeDissector.printType;
 
 /**
  * The concrete implementation of SourceCodeAnalysis.
@@ -1185,7 +1187,7 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
             proc.debug(ex, "SourceCodeAnalysisImpl.element2String(..., " + el + ")");
         }
 
-        return Util.expunge(elementHeader(el));
+        return Util.expunge(elementHeader(sourceCache.originalTask, el, !hasSyntheticParameterNames(el)));
     }
 
     private boolean hasSyntheticParameterNames(Element el) {
@@ -1248,7 +1250,7 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
                 topLevelName2Signature2Method.put(binaryName, cache = createMethodCache(binaryName));
             }
 
-            String handle = elementHeader(method, false);
+            String handle = elementHeader(originalTask, method, false);
 
             return cache.getOrDefault(handle, method);
         }
@@ -1276,7 +1278,7 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
                     Element currentMethod = trees.getElement(getCurrentPath());
 
                     if (currentMethod != null) {
-                        signature2Method.put(elementHeader(currentMethod, false), currentMethod);
+                        signature2Method.put(elementHeader(originalTask, currentMethod, false), currentMethod);
                     }
 
                     return null;
@@ -1331,39 +1333,79 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
         return availableSources = result;
     }
 
-    private String elementHeader(Element el) {
+    private String elementHeader(AnalyzeTask at, Element el) {
-        return elementHeader(el, true);
+        return elementHeader(at, el, true);
     }
 
-    private String elementHeader(Element el, boolean includeParameterNames) {
+    private String elementHeader(AnalyzeTask at, Element el, boolean includeParameterNames) {
         switch (el.getKind()) {
-            case ANNOTATION_TYPE: case CLASS: case ENUM: case INTERFACE:
+            case ANNOTATION_TYPE: case CLASS: case ENUM: case INTERFACE: {
-                return ((TypeElement) el).getQualifiedName().toString();
+                TypeElement type = (TypeElement)el;
+                String fullname = type.getQualifiedName().toString();
+                Element pkg = at.getElements().getPackageOf(el);
+                String name = pkg == null ? fullname :
+                        proc.maps.fullClassNameAndPackageToClass(fullname, ((PackageElement)pkg).getQualifiedName().toString());
+
+                return name + typeParametersOpt(at, type.getTypeParameters());
+            }
+            case TYPE_PARAMETER: {
+                TypeParameterElement tp = (TypeParameterElement)el;
+                String name = tp.getSimpleName().toString();
+
+                List<? extends TypeMirror> bounds = tp.getBounds();
+                boolean boundIsObject = bounds.isEmpty() ||
+                        bounds.size() == 1 && at.getTypes().isSameType(bounds.get(0), Symtab.instance(at.getContext()).objectType);
+
+                return boundIsObject
+                        ? name
+                        : name + " extends " + bounds.stream()
+                                .map(bound -> printType(at, proc, bound))
+                                .collect(joining(" & "));
+            }
             case FIELD:
-                return elementHeader(el.getEnclosingElement()) + "." + el.getSimpleName() + ":" + el.asType();
+                return elementHeader(at, el.getEnclosingElement()) + "." + el.getSimpleName() + ":" + el.asType();
             case ENUM_CONSTANT:
-                return elementHeader(el.getEnclosingElement()) + "." + el.getSimpleName();
+                return elementHeader(at, el.getEnclosingElement()) + "." + el.getSimpleName();
             case EXCEPTION_PARAMETER: case LOCAL_VARIABLE: case PARAMETER: case RESOURCE_VARIABLE:
                 return el.getSimpleName() + ":" + el.asType();
-            case CONSTRUCTOR: case METHOD:
+            case CONSTRUCTOR: case METHOD: {
                 StringBuilder header = new StringBuilder();
-                header.append(elementHeader(el.getEnclosingElement()));
+
-                if (el.getKind() == ElementKind.METHOD) {
+                boolean isMethod = el.getKind() == ElementKind.METHOD;
-                    header.append(".");
+                ExecutableElement method = (ExecutableElement) el;
-                    header.append(el.getSimpleName());
+
+                if (isMethod) {
+                    // return type
+                    header.append(printType(at, proc, method.getReturnType())).append(" ");
+                } else {
+                    // type parameters for the constructor
+                    String typeParameters = typeParametersOpt(at, method.getTypeParameters());
+                    if (!typeParameters.isEmpty()) {
+                        header.append(typeParameters).append(" ");
                     }
+                }
+
+                // receiver type
+                String clazz = elementHeader(at, el.getEnclosingElement());
+                header.append(clazz);
+
+                if (isMethod) {
+                    //method name with type parameters
+                    (clazz.isEmpty() ? header : header.append("."))
+                            .append(typeParametersOpt(at, method.getTypeParameters()))
+                            .append(el.getSimpleName());
+                }
+
+                // arguments
                 header.append("(");
                 String sep = "";
-                ExecutableElement method = (ExecutableElement) el;
                 for (Iterator<? extends VariableElement> i = method.getParameters().iterator(); i.hasNext();) {
                     VariableElement p = i.next();
                     header.append(sep);
                     if (!i.hasNext() && method.isVarArgs()) {
-                        header.append(unwrapArrayType(p.asType()));
+                        header.append(printType(at, proc, unwrapArrayType(p.asType()))).append("...");
-                        header.append("...");
-
                     } else {
-                        header.append(p.asType());
+                        header.append(printType(at, proc, p.asType()));
                     }
                     if (includeParameterNames) {
                         header.append(" ");
@@ -1372,7 +1414,17 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
                     sep = ", ";
                 }
                 header.append(")");
+
+                // throws
+                List<? extends TypeMirror> thrownTypes = method.getThrownTypes();
+                if (!thrownTypes.isEmpty()) {
+                    header.append(" throws ")
+                            .append(thrownTypes.stream()
+                                    .map(type -> printType(at, proc, type))
+                                    .collect(joining(", ")));
+                }
                 return header.toString();
+            }
             default:
                 return el.toString();
         }
@@ -1383,6 +1435,12 @@ class SourceCodeAnalysisImpl extends SourceCodeAnalysis {
         }
         return arrayType;
     }
+    private String typeParametersOpt(AnalyzeTask at, List<? extends TypeParameterElement> typeParameters) {
+        return typeParameters.isEmpty() ? ""
+                : typeParameters.stream()
+                        .map(tp -> elementHeader(at, tp))
+                        .collect(joining(", ", "<", ">"));
+    }
 
     @Override
     public String analyzeType(String code, int cursor) {

langtools/test/jdk/jshell/CompletionSuggestionTest.java

@@ -23,8 +23,8 @@
 
 /*
  * @test
- * @bug 8131025 8141092 8153761
+ * @bug 8131025 8141092 8153761 8145263
- * @summary Test Completion
+ * @summary Test Completion and Documentation
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
  *          jdk.jdeps/com.sun.tools.javap
@@ -43,6 +43,8 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Set;
 import java.util.HashSet;
+import java.util.function.BiFunction;
+import java.util.function.Function;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 
@@ -304,35 +306,35 @@ public class CompletionSuggestionTest extends KullaTesting {
     public void testDocumentation() throws Exception {
         dontReadParameterNamesFromClassFile();
         assertDocumentation("System.getProperty(|",
-                "java.lang.System.getProperty(java.lang.String key)",
+                "String System.getProperty(String key)",
-                "java.lang.System.getProperty(java.lang.String key, java.lang.String def)");
+                "String System.getProperty(String key, String def)");
         assertEval("char[] chars = null;");
         assertDocumentation("new String(chars, |",
-                "java.lang.String(char[] arg0, int arg1, int arg2)");
+                "String(char[], int, int)");
         assertDocumentation("String.format(|",
-                "java.lang.String.format(java.lang.String arg0, java.lang.Object... arg1)",
+                "String String.format(String, Object...)",
-                "java.lang.String.format(java.util.Locale arg0, java.lang.String arg1, java.lang.Object... arg2)");
+                "String String.format(java.util.Locale, String, Object...)");
-        assertDocumentation("\"\".getBytes(\"\"|", "java.lang.String.getBytes(int arg0, int arg1, byte[] arg2, int arg3)",
+        assertDocumentation("\"\".getBytes(\"\"|", "void String.getBytes(int, int, byte[], int)",
-                                                    "java.lang.String.getBytes(java.lang.String arg0)",
+                                                    "byte[] String.getBytes(String) throws java.io.UnsupportedEncodingException",
-                                                    "java.lang.String.getBytes(java.nio.charset.Charset arg0)");
+                                                    "byte[] String.getBytes(java.nio.charset.Charset)");
-        assertDocumentation("\"\".getBytes(\"\" |", "java.lang.String.getBytes(int arg0, int arg1, byte[] arg2, int arg3)",
+        assertDocumentation("\"\".getBytes(\"\" |", "void String.getBytes(int, int, byte[], int)",
-                                                     "java.lang.String.getBytes(java.lang.String arg0)",
+                                                     "byte[] String.getBytes(String) throws java.io.UnsupportedEncodingException",
-                                                     "java.lang.String.getBytes(java.nio.charset.Charset arg0)");
+                                                     "byte[] String.getBytes(java.nio.charset.Charset)");
     }
 
     public void testMethodsWithNoArguments() throws Exception {
         dontReadParameterNamesFromClassFile();
         assertDocumentation("System.out.println(|",
-                "java.io.PrintStream.println()",
+                "void java.io.PrintStream.println()",
-                "java.io.PrintStream.println(boolean arg0)",
+                "void java.io.PrintStream.println(boolean)",
-                "java.io.PrintStream.println(char arg0)",
+                "void java.io.PrintStream.println(char)",
-                "java.io.PrintStream.println(int arg0)",
+                "void java.io.PrintStream.println(int)",
-                "java.io.PrintStream.println(long arg0)",
+                "void java.io.PrintStream.println(long)",
-                "java.io.PrintStream.println(float arg0)",
+                "void java.io.PrintStream.println(float)",
-                "java.io.PrintStream.println(double arg0)",
+                "void java.io.PrintStream.println(double)",
-                "java.io.PrintStream.println(char[] arg0)",
+                "void java.io.PrintStream.println(char[])",
-                "java.io.PrintStream.println(java.lang.String arg0)",
+                "void java.io.PrintStream.println(String)",
-                "java.io.PrintStream.println(java.lang.Object arg0)");
+                "void java.io.PrintStream.println(Object)");
     }
 
     public void testErroneous() {
@@ -472,14 +474,14 @@ public class CompletionSuggestionTest extends KullaTesting {
 
     public void testDocumentationOfUserDefinedMethods() {
         assertEval("void f() {}");
-        assertDocumentation("f(|", "f()");
+        assertDocumentation("f(|", "void f()");
         assertEval("void f(int i) {}");
-        assertDocumentation("f(|", "f()", "f(int i)");
+        assertDocumentation("f(|", "void f()", "void f(int i)");
         assertEval("<T> void f(T... ts) {}", DiagCheck.DIAG_WARNING, DiagCheck.DIAG_OK);
-        assertDocumentation("f(|", "f()", "f(int i)", "f(T... ts)");
+        assertDocumentation("f(|", "void f()", "void f(int i)", "void <T>f(T... ts)");
         assertEval("class A {}");
         assertEval("void f(A a) {}");
-        assertDocumentation("f(|", "f()", "f(int i)", "f(T... ts)", "f(A a)");
+        assertDocumentation("f(|", "void f()", "void f(int i)", "void <T>f(T... ts)", "void f(A a)");
     }
 
     public void testDocumentationOfUserDefinedConstructors() {
@@ -489,25 +491,25 @@ public class CompletionSuggestionTest extends KullaTesting {
                 ste(MAIN_SNIPPET, VALID, VALID, true, null),
                 ste(a, VALID, OVERWRITTEN, false, MAIN_SNIPPET)));
         assertDocumentation("new A(|", "A()", "A(int i)");
-        assertEval("class A<T> { A(T t) {} A(int i) {}}",
+        assertEval("class A<T> { A(T a) {} A(int i) {} <U> A(T t, U u) {}}",
                 ste(MAIN_SNIPPET, VALID, VALID, true, null),
                 ste(a2, VALID, OVERWRITTEN, false, MAIN_SNIPPET));
-        assertDocumentation("new A(|", "A(T t)", "A(int i)");
+        assertDocumentation("new A(|", "A<T>(T a)", "A<T>(int i)", "<U> A<T>(T t, U u)");
     }
 
     public void testDocumentationOfOverriddenMethods() throws Exception {
         dontReadParameterNamesFromClassFile();
         assertDocumentation("\"\".wait(|",
-            "java.lang.Object.wait(long arg0)",
+            "void Object.wait(long) throws InterruptedException",
-            "java.lang.Object.wait(long arg0, int arg1)",
+            "void Object.wait(long, int) throws InterruptedException",
-            "java.lang.Object.wait()");
+            "void Object.wait() throws InterruptedException");
         assertEval("class Base {void method() {}}");
         Snippet e = classKey(assertEval("class Extend extends Base {}"));
-        assertDocumentation("new Extend().method(|", "Base.method()");
+        assertDocumentation("new Extend().method(|", "void Base.method()");
         assertEval("class Extend extends Base {void method() {}}",
                 ste(MAIN_SNIPPET, VALID, VALID, true, null),
                 ste(e, VALID, OVERWRITTEN, false, MAIN_SNIPPET));
-        assertDocumentation("new Extend().method(|", "Extend.method()");
+        assertDocumentation("new Extend().method(|", "void Extend.method()");
     }
 
     public void testDocumentationOfInvisibleMethods() {
@@ -534,13 +536,67 @@ public class CompletionSuggestionTest extends KullaTesting {
         assertEval("void method(int n, Object o) { }");
         assertEval("void method(Object n, int o) { }");
         assertDocumentation("method(primitive,|",
-                "method(int n, java.lang.Object o)",
+                "void method(int n, Object o)",
-                "method(java.lang.Object n, int o)");
+                "void method(Object n, int o)");
         assertDocumentation("method(boxed,|",
-                "method(int n, java.lang.Object o)",
+                "void method(int n, Object o)",
-                "method(java.lang.Object n, int o)");
+                "void method(Object n, int o)");
         assertDocumentation("method(object,|",
-                "method(java.lang.Object n, int o)");
+                "void method(Object n, int o)");
+    }
+
+    public void testDocumentationWithGenerics() {
+        class TestDocumentationWithGenerics {
+            private final Function<Integer, String> codeFacotry;
+            private final BiFunction<String, Integer, String> evalFormatter;
+            private final BiFunction<String, Integer, String> docFormatter;
+            int count;
+
+            TestDocumentationWithGenerics(
+                    Function<Integer, String> codeFactory,
+                    BiFunction<String, Integer, String> evalFormatter,
+                    BiFunction<String, Integer, String> documentationFormatter) {
+                this.codeFacotry = codeFactory;
+                this.evalFormatter = evalFormatter;
+                this.docFormatter = documentationFormatter;
+            }
+
+            void assertDoc(String generics) {
+                assertDoc(generics, generics);
+            }
+
+            void assertDoc(String generics, String expectedGenerics) {
+                assertEval(evalFormatter.apply(generics, count));
+                assertDocumentation(codeFacotry.apply(count), docFormatter.apply(expectedGenerics, count));
+                count++;
+            }
+        }
+
+        TestDocumentationWithGenerics[] tests = {
+            new TestDocumentationWithGenerics(
+                    i -> "f" + i + "(|",
+                    (g, i) -> "<" + g + "> void f" + i + "() {}",
+                    (g, i) -> "void <" + g + ">f" + i + "()"
+            ),
+            new TestDocumentationWithGenerics(
+                    i -> "new C" + i + "().f(|",
+                    (g, i) -> "class C" + i + "<" + g + "> { void f() {} }",
+                    (g, i) -> "void C" + i + "<" + g + ">.f()"
+            )
+        };
+
+        Arrays.stream(tests).forEach(t -> {
+                t.assertDoc("T");
+                t.assertDoc("T extends Object",
+                        "T");
+                t.assertDoc("T extends String");
+                t.assertDoc("T extends java.lang.String",
+                        "T extends String");
+                t.assertDoc("T extends Number & Comparable<T>");
+                t.assertDoc("T extends java.io.Serializable & CharSequence");
+                t.assertDoc("K, D, M extends java.util.Map<K, D>",
+                        "K, D, M extends java.util.Map<K,D>");
+        });
     }
 
     public void testVarArgs() {