8171130: jshell tool: /edit adds empty statement to brace terminated snippet
authorrfield
Thu, 19 Jan 2017 11:17:11 -0800
changeset 43264 7b06e19184de
parent 43263 ca999fb7b46d
child 43265 4ec472ee5135
8171130: jshell tool: /edit adds empty statement to brace terminated snippet 8173007: JShell Tests: ToolFormatTest takes too long Reviewed-by: jlahoda
langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java
langtools/test/jdk/jshell/ExternalEditorTest.java
langtools/test/jdk/jshell/ToolFormatTest.java
--- a/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Thu Jan 19 11:12:02 2017 -0800
+++ b/langtools/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java	Thu Jan 19 11:17:11 2017 -0800
@@ -2191,13 +2191,22 @@
                 case ASSIGNMENT_SUBKIND:
                 case OTHER_EXPRESSION_SUBKIND:
                 case TEMP_VAR_EXPRESSION_SUBKIND:
-                case STATEMENT_SUBKIND:
                 case UNKNOWN_SUBKIND:
                     if (!src.endsWith(";")) {
                         src = src + ";";
                     }
                     srcSet.add(src);
                     break;
+                case STATEMENT_SUBKIND:
+                    if (src.endsWith("}")) {
+                        // Could end with block or, for example, new Foo() {...}
+                        // so, we need deeper analysis to know if it needs a semicolon
+                        src = analysis.analyzeCompletion(src).source();
+                    } else if (!src.endsWith(";")) {
+                        src = src + ";";
+                    }
+                    srcSet.add(src);
+                    break;
                 default:
                     srcSet.add(src);
                     break;
--- a/langtools/test/jdk/jshell/ExternalEditorTest.java	Thu Jan 19 11:12:02 2017 -0800
+++ b/langtools/test/jdk/jshell/ExternalEditorTest.java	Thu Jan 19 11:17:11 2017 -0800
@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Testing external editor.
- * @bug 8143955 8080843 8163816 8143006 8169828
+ * @bug 8143955 8080843 8163816 8143006 8169828 8171130
  * @modules jdk.jshell/jdk.internal.jshell.tool
  * @build ReplToolTesting CustomEditor EditorTestBase
  * @run testng ExternalEditorTest
@@ -50,6 +50,7 @@
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.fail;
 
 public class ExternalEditorTest extends EditorTestBase {
@@ -119,6 +120,28 @@
         super.testEditor(defaultStartup, args, t);
     }
 
+    @Test
+    public void testStatementSemicolonAddition() {
+        testEditor(
+                a -> assertCommand(a, "if (true) {}", ""),
+                a -> assertCommand(a, "if (true) {} else {}", ""),
+                a -> assertCommand(a, "Object o", "o ==> null"),
+                a -> assertCommand(a, "if (true) o = new Object() { int x; }", ""),
+                a -> assertCommand(a, "if (true) o = new Object() { int y; }", ""),
+                a -> assertCommand(a, "System.err.flush()", ""), // test still ; for expression statement
+                a -> assertEditOutput(a, "/ed", "", () -> {
+                    assertEquals(getSource(),
+                            "if (true) {}\n" +
+                            "if (true) {} else {}\n" +
+                            "Object o;\n" +
+                            "if (true) o = new Object() { int x; };\n" +
+                            "if (true) o = new Object() { int y; };\n" +
+                            "System.err.flush();\n");
+                    exit();
+                })
+        );
+    }
+
     private static boolean isWindows() {
         return System.getProperty("os.name").startsWith("Windows");
     }
--- a/langtools/test/jdk/jshell/ToolFormatTest.java	Thu Jan 19 11:12:02 2017 -0800
+++ b/langtools/test/jdk/jshell/ToolFormatTest.java	Thu Jan 19 11:17:11 2017 -0800
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8148316 8148317 8151755 8152246 8153551 8154812 8157261 8163840 8166637 8161969
+ * @bug 8148316 8148317 8151755 8152246 8153551 8154812 8157261 8163840 8166637 8161969 8173007
  * @summary Tests for output customization
  * @library /tools/lib
  * @modules jdk.compiler/com.sun.tools.javac.api
@@ -38,8 +38,6 @@
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import org.testng.annotations.Test;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
@@ -140,6 +138,66 @@
         );
     }
 
+    public void testSetFormatSelectorSample() {
+        test(
+                (a) -> assertCommandOutputStartsWith(a, "/set mode ate -quiet",
+                            "|  Created new feedback mode: ate"),
+                (a) -> assertCommand(a, "/set feedback ate", ""),
+
+                (a) -> assertCommand(a, "/set format ate display '---replaced,modified,added-primary---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++replaced,modified,added-primary+++' replaced,modified,added-primary", ""),
+                (a) -> assertCommand(a, "\"replaced,modified,added-primary\"", "+++replaced,modified,added-primary+++"),
+
+                (a) -> assertCommand(a, "/set format ate display '---added-primary,update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++added-primary,update+++' added-primary,update", ""),
+                (a) -> assertCommand(a, "\"added-primary,update\"", "+++added-primary,update+++"),
+
+
+                (a) -> assertCommand(a, "/set format ate display '---method-replaced-primary---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++method-replaced-primary+++' method-replaced-primary", ""),
+                (a) -> assertCommand(a, "\"method-replaced-primary\"", "---method-replaced-primary---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---method-replaced-update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++method-replaced-update+++' method-replaced-update", ""),
+                (a) -> assertCommand(a, "\"method-replaced-update\"", "---method-replaced-update---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---method-added-update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++method-added-update+++' method-added-update", ""),
+                (a) -> assertCommand(a, "\"method-added-update\"", "---method-added-update---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---method-added---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++method-added+++' method-added", ""),
+                (a) -> assertCommand(a, "\"method-added\"", "---method-added---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---class-modified,added-primary,update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++class-modified,added-primary,update+++' class-modified,added-primary,update", ""),
+                (a) -> assertCommand(a, "\"class-modified,added-primary,update\"", "---class-modified,added-primary,update---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---class-modified,added-primary---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++class-modified,added-primary+++' class-modified,added-primary", ""),
+                (a) -> assertCommand(a, "\"class-modified,added-primary\"", "---class-modified,added-primary---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---class-modified,added-update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++class-modified,added-update+++' class-modified,added-update", ""),
+                (a) -> assertCommand(a, "\"class-modified,added-update\"", "---class-modified,added-update---"),
+
+                (a) -> assertCommand(a, "/set format ate display '---replaced,added---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++replaced,added+++' replaced,added", ""),
+                (a) -> assertCommand(a, "\"replaced,added\"", "+++replaced,added+++"),
+
+                (a) -> assertCommand(a, "/set format ate display '---replaced-primary,update---'", ""),
+                (a) -> assertCommand(a, "/set format ate display '+++replaced-primary,update+++' replaced-primary,update", ""),
+                (a) -> assertCommand(a, "\"replaced-primary,update\"", "---replaced-primary,update---"),
+
+                (a) -> assertCommandOutputStartsWith(a, "/set feedback normal", "|  Feedback mode: normal")
+        );
+    }
+
+    // This test is exhaustive and takes to long for routine testing -- disabled.
+    // A sampling of these has been added (above: testSetFormatSelectorSample).
+    // See 8173007
+    // Save for possible future deep testing or debugging
+    @Test(enabled = false)
     public void testSetFormatSelector() {
         List<ReplTest> tests = new ArrayList<>();
         tests.add((a) -> assertCommandOutputStartsWith(a, "/set mode ate -quiet",
@@ -202,6 +260,11 @@
                     tests.add((a) -> assertCommand(a, "/set format ate display '" + no  + "'", ""));
                     tests.add((a) -> assertCommand(a, "/set format ate display '" + yes + "' " + select, ""));
                     tests.add((a) -> assertCommand(a, "\"" + select + "\"", expect));
+             /**** for sample generation ****
+             System.err.println("                (a) -> assertCommand(a, \"/set format ate display '" + no  + "'\", \"\"),");
+             System.err.println("                (a) -> assertCommand(a, \"/set format ate display '" + yes + "' " + select + "\", \"\"),");
+             System.err.println("                (a) -> assertCommand(a, \"\\\"" + select + "\\\"\", \"" + expect + "\"),\n");
+             ****/
                 }
             }
         }