8173916: jshell tool: /methods signature confusing/non-standard format
authorrfield
Wed, 08 Feb 2017 13:35:42 -0800
changeset 43759 61535ac55add
parent 43758 868af3718a21
child 43760 bdbe9f1571f0
8173916: jshell tool: /methods signature confusing/non-standard format 8174028: jshell tool: /method /type failed declaration listed (without indication) 8174041: jshell tool: --startup PRINTING references undeclared Locale class Reviewed-by: jlahoda
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java
langtools/src/jdk.jshell/share/classes/jdk/jshell/tool/resources/PRINTING.jsh
langtools/test/jdk/jshell/ReplToolTesting.java
langtools/test/jdk/jshell/ToolReloadTest.java
langtools/test/jdk/jshell/ToolSimpleTest.java
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Feedback.java	Wed Feb 08 13:35:42 2017 -0800
@@ -116,6 +116,13 @@
                 name, type, value, unresolved, errorLines);
     }
 
+    public String format(String field, FormatCase fc, FormatAction fa, FormatWhen fw,
+                    FormatResolve fr, FormatUnresolved fu, FormatErrors fe,
+                    String name, String type, String value, String unresolved, List<String> errorLines) {
+        return mode.format(field, fc, fa, fw, fr, fu, fe,
+                name, type, value, unresolved, errorLines);
+    }
+
     public String truncateVarValue(String value) {
         return mode.truncateVarValue(value);
     }
@@ -463,6 +470,14 @@
         String format(FormatCase fc, FormatAction fa, FormatWhen fw,
                     FormatResolve fr, FormatUnresolved fu, FormatErrors fe,
                     String name, String type, String value, String unresolved, List<String> errorLines) {
+            return format("display", fc, fa, fw, fr, fu, fe,
+                name, type, value, unresolved, errorLines);
+        }
+
+        // Compute the display output given full context and values
+        String format(String field, FormatCase fc, FormatAction fa, FormatWhen fw,
+                    FormatResolve fr, FormatUnresolved fu, FormatErrors fe,
+                    String name, String type, String value, String unresolved, List<String> errorLines) {
             // Convert the context into a bit representation used as selectors for store field formats
             long bits = bits(fc, fa, fw, fr, fu, fe);
             String fname = name==null? "" : name;
@@ -476,7 +491,7 @@
                             fname, ftype, fvalue, funresolved, "*cannot-use-errors-here*", el))
                     .collect(joining());
             return String.format(
-                    format("display", bits),
+                    format(field, bits),
                     fname, ftype, fvalue, funresolved, errors, "*cannot-use-err-here*");
         }
 
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Wed Feb 08 13:35:42 2017 -0800
@@ -36,12 +36,9 @@
 import java.io.PrintStream;
 import java.io.Reader;
 import java.io.StringReader;
-import java.net.URL;
 import java.nio.charset.Charset;
-import java.nio.file.AccessDeniedException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.MessageFormat;
@@ -2624,9 +2621,16 @@
         if (stream == null) {
             return false;
         }
-        stream.forEachOrdered(mk
-                -> hard("  %s %s", mk.name(), mk.signature())
-        );
+        stream.forEachOrdered(meth -> {
+            String sig = meth.signature();
+            int i = sig.lastIndexOf(")") + 1;
+            if (i <= 0) {
+                hard("  %s", meth.name());
+            } else {
+                hard("  %s %s%s", sig.substring(i), meth.name(), sig.substring(0, i));
+            }
+            printSnippetStatus(meth, true);
+        });
         return true;
     }
 
@@ -2658,6 +2662,7 @@
                     break;
             }
             hard("  %s %s", kind, ck.name());
+            printSnippetStatus(ck, true);
         });
         return true;
     }
@@ -2847,7 +2852,8 @@
                         return true;
                     }
                 } else {
-                    new DisplayEvent(ste, false, ste.value(), diagnostics).displayDeclarationAndValue();
+                    new DisplayEvent(ste, FormatWhen.PRIMARY, ste.value(), diagnostics)
+                            .displayDeclarationAndValue();
                 }
             } else {
                 if (diagnostics.isEmpty()) {
@@ -2861,7 +2867,8 @@
                 List<Diag> other = errorsOnly(diagnostics);
 
                 // display update information
-                new DisplayEvent(ste, true, ste.value(), other).displayDeclarationAndValue();
+                new DisplayEvent(ste, FormatWhen.UPDATE, ste.value(), other)
+                        .displayDeclarationAndValue();
             }
         }
         return false;
@@ -2899,10 +2906,7 @@
     }
     //where
     void printUnresolvedException(UnresolvedReferenceException ex) {
-        DeclarationSnippet corralled =  ex.getSnippet();
-        List<Diag> otherErrors = errorsOnly(state.diagnostics(corralled).collect(toList()));
-        new DisplayEvent(corralled, state.status(corralled), FormatAction.USED, true, null, otherErrors)
-                .displayDeclarationAndValue();
+        printSnippetStatus(ex.getSnippet(), false);
     }
     //where
     void printEvalException(EvalException ex) {
@@ -2944,23 +2948,38 @@
         return act;
     }
 
+    void printSnippetStatus(DeclarationSnippet sn, boolean resolve) {
+        List<Diag> otherErrors = errorsOnly(state.diagnostics(sn).collect(toList()));
+        new DisplayEvent(sn, state.status(sn), resolve, otherErrors)
+                .displayDeclarationAndValue();
+    }
+
     class DisplayEvent {
         private final Snippet sn;
         private final FormatAction action;
-        private final boolean update;
+        private final FormatWhen update;
         private final String value;
         private final List<String> errorLines;
         private final FormatResolve resolution;
         private final String unresolved;
         private final FormatUnresolved unrcnt;
         private final FormatErrors errcnt;
+        private final boolean resolve;
 
-        DisplayEvent(SnippetEvent ste, boolean update, String value, List<Diag> errors) {
-            this(ste.snippet(), ste.status(), toAction(ste.status(), ste.previousStatus(), ste.isSignatureChange()), update, value, errors);
+        DisplayEvent(SnippetEvent ste, FormatWhen update, String value, List<Diag> errors) {
+            this(ste.snippet(), ste.status(), false,
+                    toAction(ste.status(), ste.previousStatus(), ste.isSignatureChange()),
+                    update, value, errors);
         }
 
-        DisplayEvent(Snippet sn, Status status, FormatAction action, boolean update, String value, List<Diag> errors) {
+        DisplayEvent(Snippet sn, Status status, boolean resolve, List<Diag> errors) {
+            this(sn, status, resolve, FormatAction.USED, FormatWhen.UPDATE, null, errors);
+        }
+
+        private DisplayEvent(Snippet sn, Status status, boolean resolve,
+                FormatAction action, FormatWhen update, String value, List<Diag> errors) {
             this.sn = sn;
+            this.resolve =resolve;
             this.action = action;
             this.update = update;
             this.value = value;
@@ -2968,6 +2987,12 @@
             for (Diag d : errors) {
                 displayDiagnostics(sn.source(), d, errorLines);
             }
+            if (resolve) {
+                // resolve needs error lines indented
+                for (int i = 0; i < errorLines.size(); ++i) {
+                    errorLines.set(i, "    " + errorLines.get(i));
+                }
+            }
             long unresolvedCount;
             if (sn instanceof DeclarationSnippet && (status == Status.RECOVERABLE_DEFINED || status == Status.RECOVERABLE_NOT_DEFINED)) {
                 resolution = (status == Status.RECOVERABLE_NOT_DEFINED)
@@ -3022,10 +3047,17 @@
         }
 
         private void custom(FormatCase fcase, String name, String type) {
-            String display = feedback.format(fcase, action, (update ? FormatWhen.UPDATE : FormatWhen.PRIMARY),
-                    resolution, unrcnt, errcnt,
-                    name, type, value, unresolved, errorLines);
-            if (interactive()) {
+            if (resolve) {
+                String resolutionErrors = feedback.format("resolve", fcase, action, update,
+                        resolution, unrcnt, errcnt,
+                        name, type, value, unresolved, errorLines);
+                if (!resolutionErrors.trim().isEmpty()) {
+                    hard("    %s", resolutionErrors);
+                }
+            } else if (interactive()) {
+                String display = feedback.format(fcase, action, update,
+                        resolution, unrcnt, errcnt,
+                        name, type, value, unresolved, errorLines);
                 cmdout.print(display);
             }
         }
--- a/langtools/src/jdk.jshell/share/classes/jdk/jshell/tool/resources/PRINTING.jsh	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/jshell/tool/resources/PRINTING.jsh	Wed Feb 08 13:35:42 2017 -0800
@@ -17,5 +17,5 @@
 void println(char s[]) { System.out.println(s); }
 void println(String s) { System.out.println(s); }
 void println(Object obj) { System.out.println(obj); }
-void printf(Locale l, String format, Object... args) { System.out.printf(l, format, args); }
+void printf(java.util.Locale l, String format, Object... args) { System.out.printf(l, format, args); }
 void printf(String format, Object... args) { System.out.printf(format, args); }
--- a/langtools/test/jdk/jshell/ReplToolTesting.java	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/test/jdk/jshell/ReplToolTesting.java	Wed Feb 08 13:35:42 2017 -0800
@@ -72,27 +72,27 @@
     final static List<String> START_UP_CMD_METHOD = Stream.<String>of()
                     .collect(toList());
     final static List<String> PRINTING_CMD_METHOD = Stream.of(
-            "|    print (boolean)void",
-            "|    print (char)void",
-            "|    print (int)void",
-            "|    print (long)void",
-            "|    print (float)void",
-            "|    print (double)void",
-            "|    print (char s[])void",
-            "|    print (String)void",
-            "|    print (Object)void",
-            "|    println ()void",
-            "|    println (boolean)void",
-            "|    println (char)void",
-            "|    println (int)void",
-            "|    println (long)void",
-            "|    println (float)void",
-            "|    println (double)void",
-            "|    println (char s[])void",
-            "|    println (String)void",
-            "|    println (Object)void",
-            "|    printf (Locale,String,Object...)void",
-            "|    printf (String,Object...)void")
+            "|    void print(boolean)",
+            "|    void print(char)",
+            "|    void print(int)",
+            "|    void print(long)",
+            "|    void print(float)",
+            "|    void print(double)",
+            "|    void print(char s[])",
+            "|    void print(String)",
+            "|    void print(Object)",
+            "|    void println()",
+            "|    void println(boolean)",
+            "|    void println(char)",
+            "|    void println(int)",
+            "|    void println(long)",
+            "|    void println(float)",
+            "|    void println(double)",
+            "|    void println(char s[])",
+            "|    void println(String)",
+            "|    void println(Object)",
+            "|    void printf(java.util.Locale,String,Object...)",
+            "|    void printf(String,Object...)")
             .collect(toList());
     final static List<String> START_UP = Collections.unmodifiableList(
             Stream.concat(START_UP_IMPORTS.stream(), START_UP_METHODS.stream())
@@ -152,6 +152,7 @@
         return s -> {
             List<String> lines = Stream.of(s.split("\n"))
                     .filter(l -> !l.isEmpty())
+                    .filter(l -> !l.startsWith("|     ")) // error/unresolved info
                     .collect(Collectors.toList());
             assertEquals(lines.size(), set.size(), message + " : expected: " + set.keySet() + "\ngot:\n" + lines);
             for (String line : lines) {
@@ -664,7 +665,12 @@
 
         @Override
         public String toString() {
-            return String.format("%s %s", name, signature);
+            int i = signature.lastIndexOf(")") + 1;
+            if (i <= 0) {
+                return String.format("%s", name);
+            } else {
+                return String.format("%s %s%s", signature.substring(i), name, signature.substring(0, i));
+            }
         }
     }
 
--- a/langtools/test/jdk/jshell/ToolReloadTest.java	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/test/jdk/jshell/ToolReloadTest.java	Wed Feb 08 13:35:42 2017 -0800
@@ -92,8 +92,8 @@
         test(false, new String[]{"--no-startup"},
                 a -> assertVariable(a, "int", "a"),
                 a -> dropVariable(a, "/dr 1", "int a = 0", "|  dropped variable a"),
-                a -> assertMethod(a, "int b() { return 0; }", "()I", "b"),
-                a -> dropMethod(a, "/drop b", "b ()I", "|  dropped method b()"),
+                a -> assertMethod(a, "int b() { return 0; }", "()int", "b"),
+                a -> dropMethod(a, "/drop b", "int b()", "|  dropped method b()"),
                 a -> assertClass(a, "class A {}", "class", "A"),
                 a -> dropClass(a, "/dr A", "class A", "|  dropped class A"),
                 a -> assertCommand(a, "/reload",
@@ -115,8 +115,8 @@
         test(false, new String[]{"--no-startup"},
                 a -> assertVariable(a, "int", "a"),
                 a -> dropVariable(a, "/dr 1", "int a = 0", "|  dropped variable a"),
-                a -> assertMethod(a, "int b() { return 0; }", "()I", "b"),
-                a -> dropMethod(a, "/drop b", "b ()I", "|  dropped method b()"),
+                a -> assertMethod(a, "int b() { return 0; }", "()int", "b"),
+                a -> dropMethod(a, "/drop b", "int b()", "|  dropped method b()"),
                 a -> assertClass(a, "class A {}", "class", "A"),
                 a -> dropClass(a, "/dr A", "class A", "|  dropped class A"),
                 a -> assertCommand(a, "/reload -quiet",
--- a/langtools/test/jdk/jshell/ToolSimpleTest.java	Wed Feb 08 10:43:16 2017 -0800
+++ b/langtools/test/jdk/jshell/ToolSimpleTest.java	Wed Feb 08 13:35:42 2017 -0800
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513 8170015 8170368 8172102 8172103  8165405 8173073 8173848
+ * @bug 8153716 8143955 8151754 8150382 8153920 8156910 8131024 8160089 8153897 8167128 8154513 8170015 8170368 8172102 8172103  8165405 8173073 8173848 8174041 8173916 8174028
  * @summary Simple jshell tool tests
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -238,8 +238,8 @@
         test(false, new String[]{"--no-startup"},
                 a -> assertVariable(a, "int", "a"),
                 a -> dropVariable(a, "/drop 1", "int a = 0", "|  dropped variable a"),
-                a -> assertMethod(a, "int b() { return 0; }", "()I", "b"),
-                a -> dropMethod(a, "/drop 2", "b ()I", "|  dropped method b()"),
+                a -> assertMethod(a, "int b() { return 0; }", "()int", "b"),
+                a -> dropMethod(a, "/drop 2", "int b()", "|  dropped method b()"),
                 a -> assertClass(a, "class A {}", "class", "A"),
                 a -> dropClass(a, "/drop 3", "class A", "|  dropped class A"),
                 a -> assertImport(a, "import java.util.stream.*;", "", "java.util.stream.*"),
@@ -255,8 +255,8 @@
         test(false, new String[]{"--no-startup"},
                 a -> assertVariable(a, "int", "a"),
                 a -> dropVariable(a, "/drop a", "int a = 0", "|  dropped variable a"),
-                a -> assertMethod(a, "int b() { return 0; }", "()I", "b"),
-                a -> dropMethod(a, "/drop b", "b ()I", "|  dropped method b()"),
+                a -> assertMethod(a, "int b() { return 0; }", "()int", "b"),
+                a -> dropMethod(a, "/drop b", "int b()", "|  dropped method b()"),
                 a -> assertClass(a, "class A {}", "class", "A"),
                 a -> dropClass(a, "/drop A", "class A", "|  dropped class A"),
                 a -> assertCommandCheckOutput(a, "/vars", assertVariables()),
@@ -466,10 +466,50 @@
                 a -> assertCommandCheckOutput(a, "/methods print println printf",
                         s -> checkLineToList(s, printingMethodList)),
                 a -> assertCommandOutputStartsWith(a, "/methods g",
-                        "|    g ()void"),
+                        "|    void g()"),
                 a -> assertCommandOutputStartsWith(a, "/methods f",
-                        "|    f ()int\n" +
-                        "|    f (int)void")
+                        "|    int f()\n" +
+                        "|    void f(int)")
+        );
+    }
+
+    @Test
+    public void testMethodsWithErrors() {
+        test(new String[]{"--no-startup"},
+                a -> assertCommand(a, "double m(int x) { return x; }",
+                        "|  created method m(int)"),
+                a -> assertCommand(a, "GARBAGE junk() { return TRASH; }",
+                        "|  created method junk(), however, it cannot be referenced until class GARBAGE, and variable TRASH are declared"),
+                a -> assertCommand(a, "int w = 5;",
+                        "w ==> 5"),
+                a -> assertCommand(a, "int tyer() { return w; }",
+                        "|  created method tyer()"),
+                a -> assertCommand(a, "String w = \"hi\";",
+                        "w ==> \"hi\""),
+                a -> assertCommand(a, "/methods",
+                        "|    double m(int)\n" +
+                        "|    GARBAGE junk()\n" +
+                        "|       which cannot be referenced until class GARBAGE, and variable TRASH are declared\n" +
+                        "|    int tyer()\n" +
+                        "|       which cannot be invoked until this error is corrected: \n" +
+                        "|          incompatible types: java.lang.String cannot be converted to int\n" +
+                        "|          int tyer() { return w; }\n" +
+                        "|                              ^\n")
+        );
+    }
+
+    @Test
+    public void testTypesWithErrors() {
+        test(new String[]{"--no-startup"},
+                a -> assertCommand(a, "class C extends NONE { int x; }",
+                        "|  created class C, however, it cannot be referenced until class NONE is declared"),
+                a -> assertCommand(a, "class D { void m() { System.out.println(nada); } }",
+                        "|  created class D, however, it cannot be instanciated or its methods invoked until variable nada is declared"),
+                a -> assertCommand(a, "/types",
+                        "|    class C\n" +
+                        "|       which cannot be referenced until class NONE is declared\n" +
+                        "|    class D\n" +
+                        "|       which cannot be instanciated or its methods invoked until variable nada is declared\n")
         );
     }