8179856: jshell tool: not suitable for pipeline use

8186708: jshell tool: bad load file garbles message and does not abort

Reviewed-by: jlahoda

src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java

@@ -184,6 +184,7 @@ public class JShellTool implements MessageHandler {
     private IOContext input = null;
     private boolean regenerateOnDeath = true;
     private boolean live = false;
+    private boolean interactiveModeBegun = false;
     private Options options;
 
     SourceCodeAnalysis analysis;
@@ -499,9 +500,13 @@ public class JShellTool implements MessageHandler {
 
         @Override
         void msg(String key, Object... args) {
-            startmsg(key, args);
+            errormsg(key, args);
         }
 
+        /**
+         * Parse the command line options.
+         * @return the options as an Options object, or null if error
+         */
         @Override
         Options parse(OptionSet options) {
             if (options.has(argHelp)) {
@@ -541,7 +546,7 @@ public class JShellTool implements MessageHandler {
             if (options.has(argStart)) {
                 List<String> sts = options.valuesOf(argStart);
                 if (options.has("no-startup")) {
-                    startmsg("jshell.err.opt.startup.conflict");
+                    msg("jshell.err.opt.startup.conflict");
                     return null;
                 }
                 initialStartup = Startup.fromFileList(sts, "--startup", new InitMessageHandler());
@@ -648,16 +653,6 @@ public class JShellTool implements MessageHandler {
         }
     }
 
-    /**
-     * Base output for command output -- no pre- or post-fix
-     *
-     * @param printf format
-     * @param printf args
-     */
-    void rawout(String format, Object... args) {
-        cmdout.printf(format, args);
-    }
-
     /**
      * Must show command output
      *
@@ -666,7 +661,7 @@ public class JShellTool implements MessageHandler {
      */
     @Override
     public void hard(String format, Object... args) {
-        rawout(prefix(format), args);
+        cmdout.printf(prefix(format), args);
     }
 
    /**
@@ -676,7 +671,7 @@ public class JShellTool implements MessageHandler {
      * @param args printf args
      */
     void error(String format, Object... args) {
-        rawout(prefixError(format), args);
+        (interactiveModeBegun? cmdout : cmderr).printf(prefixError(format), args);
     }
 
     /**
@@ -749,7 +744,8 @@ public class JShellTool implements MessageHandler {
 
     /**
      * Add prefixing/postfixing to embedded newlines in a string,
-     * bracketing with prefix/postfix
+     * bracketing with prefix/postfix.  No prefixing when non-interactive.
+     * Result is expected to be the format for a printf.
      *
      * @param s the string to prefix
      * @param pre the string to prepend to each line
@@ -760,6 +756,10 @@ public class JShellTool implements MessageHandler {
         if (s == null) {
             return "";
         }
+        if (!interactiveModeBegun) {
+            // messages expect to be new-line terminated (even when not prefixed)
+            return s + "%n";
+        }
         String pp = s.replaceAll("\\R", post + pre);
         if (pp.endsWith(post + pre)) {
             // prevent an extra prefix char and blank line when the string
@@ -810,21 +810,7 @@ public class JShellTool implements MessageHandler {
      */
     @Override
     public void errormsg(String key, Object... args) {
-        if (isRunningInteractive()) {
+        error(messageFormat(key, args));
-            rawout(prefixError(messageFormat(key, args)));
-        } else {
-            startmsg(key, args);
-        }
-    }
-
-    /**
-     * Print command-line error using resource bundle look-up, MessageFormat
-     *
-     * @param key the resource key
-     * @param args
-     */
-    void startmsg(String key, Object... args) {
-        cmderr.println(messageFormat(key, args));
     }
 
     /**
@@ -847,7 +833,7 @@ public class JShellTool implements MessageHandler {
                 LinkedHashMap::new));
         for (Entry<String, String> e : a2b.entrySet()) {
             hard("%s", e.getKey());
-            rawout(prefix(e.getValue(), feedback.getPre() + "\t", feedback.getPost()));
+            cmdout.printf(prefix(e.getValue(), feedback.getPre() + "\t", feedback.getPost()));
         }
     }
 
@@ -899,7 +885,10 @@ public class JShellTool implements MessageHandler {
         replayableHistoryPrevious = ReplayableHistory.fromPrevious(prefs);
         // load snippet/command files given on command-line
         for (String loadFile : commandLineArgs.nonOptions()) {
-            runFile(loadFile, "jshell");
+            if (!runFile(loadFile, "jshell")) {
+                // Load file failed -- abort
+                return;
+            }
         }
         // if we survived that...
         if (regenerateOnDeath) {
@@ -909,6 +898,7 @@ public class JShellTool implements MessageHandler {
         // check again, as feedback setting could have failed
         if (regenerateOnDeath) {
             // if we haven't died, and the feedback mode wants fluff, print welcome
+            interactiveModeBegun = true;
             if (feedback.shouldDisplayCommandFluff()) {
                 hardmsg("jshell.msg.welcome", version());
             }
@@ -994,7 +984,7 @@ public class JShellTool implements MessageHandler {
 
         @Override
         public void errormsg(String messageKey, Object... args) {
-            startmsg(messageKey, args);
+            JShellTool.this.errormsg(messageKey, args);
         }
 
         @Override
@@ -1034,6 +1024,7 @@ public class JShellTool implements MessageHandler {
         shutdownSubscription = state.onShutdown((JShell deadState) -> {
             if (deadState == state) {
                 hardmsg("jshell.msg.terminated");
+                fluffmsg("jshell.msg.terminated.restore");
                 live = false;
             }
         });
@@ -1056,10 +1047,6 @@ public class JShellTool implements MessageHandler {
         currentNameSpace = mainNamespace;
     }
 
-    private boolean isRunningInteractive() {
-        return currentNameSpace != null && currentNameSpace == mainNamespace;
-    }
-
     //where -- one-time per run initialization of feedback modes
     private void initFeedback(String initMode) {
         // No fluff, no prefix, for init failures
@@ -1096,8 +1083,8 @@ public class JShellTool implements MessageHandler {
         try (IOContext suin = new ScannerIOContext(new StringReader(start))) {
             run(suin);
         } catch (Exception ex) {
-            hardmsg("jshell.err.startup.unexpected.exception", ex);
+            errormsg("jshell.err.startup.unexpected.exception", ex);
-            ex.printStackTrace(cmdout);
+            ex.printStackTrace(cmderr);
         }
     }
 
@@ -1123,7 +1110,7 @@ public class JShellTool implements MessageHandler {
             String incomplete = "";
             while (live) {
                 String prompt;
-                if (isRunningInteractive()) {
+                if (interactive()) {
                     prompt = testPrompt
                                     ? incomplete.isEmpty()
                                             ? "\u0005" //ENQ
@@ -1171,7 +1158,7 @@ public class JShellTool implements MessageHandler {
     }
 
     private void addToReplayHistory(String s) {
-        if (isRunningInteractive()) {
+        if (!isCurrentlyRunningStartup) {
             replayableHistory.add(s);
         }
     }
@@ -2127,7 +2114,7 @@ public class JShellTool implements MessageHandler {
                         fluff("Wrap debugging on");
                         break;
                     default:
-                        hard("Unknown debugging option: %c", ch);
+                        error("Unknown debugging option: %c", ch);
                         fluff("Use: 0 r g f c d e w");
                         return false;
                 }
@@ -2697,7 +2684,7 @@ public class JShellTool implements MessageHandler {
                     }
                     currSrcs = nextSrcs;
                 } catch (IllegalStateException ex) {
-                    hardmsg("jshell.msg.resetting");
+                    errormsg("jshell.msg.resetting");
                     resetState();
                     currSrcs = new LinkedHashSet<>(); // re-process everything
                 }
@@ -2746,16 +2733,21 @@ public class JShellTool implements MessageHandler {
     private boolean runFile(String filename, String context) {
         if (!filename.isEmpty()) {
             try {
-                Path path = toPathResolvingUserHome(filename);
+                Scanner scanner;
-                Reader reader;
+                if (!interactiveModeBegun && filename.equals("-")) {
-                String resource;
+                    // - on command line: no interactive later, read from input
-                if (!Files.exists(path) && (resource = getResource(filename)) != null) {
+                    regenerateOnDeath = false;
-                    // Not found as file, but found as resource
+                    scanner = new Scanner(cmdin);
-                    reader = new StringReader(resource);
                 } else {
-                    reader = new FileReader(path.toString());
+                    Path path = toPathResolvingUserHome(filename);
+                    String resource;
+                    scanner = new Scanner(
+                            (!Files.exists(path) && (resource = getResource(filename)) != null)
+                            ? new StringReader(resource) // Not found as file, but found as resource
+                            : new FileReader(path.toString())
+                    );
                 }
-                run(new ScannerIOContext(reader));
+                run(new ScannerIOContext(scanner));
                 return true;
             } catch (FileNotFoundException e) {
                 errormsg("jshell.err.file.not.found", context, filename, e.getMessage());
@@ -2841,7 +2833,7 @@ public class JShellTool implements MessageHandler {
                 sb.append(a);
             }
             if (sb.length() > 0) {
-                rawout(prefix(sb.toString()));
+                hard(sb.toString());
             }
             return false;
         }
@@ -3175,11 +3167,11 @@ public class JShellTool implements MessageHandler {
         if (ste.causeSnippet() == null) {
             // main event
             for (Diag d : diagnostics) {
-                hardmsg(d.isError()? "jshell.msg.error" : "jshell.msg.warning");
+                errormsg(d.isError()? "jshell.msg.error" : "jshell.msg.warning");
                 List<String> disp = new ArrayList<>();
                 displayDiagnostics(source, d, disp);
                 disp.stream()
-                        .forEach(l -> hard("%s", l));
+                        .forEach(l -> error("%s", l));
             }
 
             if (ste.status() != Status.REJECTED) {
@@ -3190,7 +3182,7 @@ public class JShellTool implements MessageHandler {
                     } else if (ste.exception() instanceof UnresolvedReferenceException) {
                         printUnresolvedException((UnresolvedReferenceException) ste.exception());
                     } else {
-                        hard("Unexpected execution exception: %s", ste.exception());
+                        error("Unexpected execution exception: %s", ste.exception());
                         return true;
                     }
                 } else {
@@ -3242,7 +3234,7 @@ public class JShellTool implements MessageHandler {
                             : lineNumber >= 0
                                     ? fileName + ":" + lineNumber
                                     : fileName;
-            hard("      at %s(%s)", sb, loc);
+            error("      at %s(%s)", sb, loc);
 
         }
     }
@@ -3253,9 +3245,9 @@ public class JShellTool implements MessageHandler {
     //where
     void printEvalException(EvalException ex) {
         if (ex.getMessage() == null) {
-            hard("%s thrown", ex.getExceptionClassName());
+            error("%s thrown", ex.getExceptionClassName());
         } else {
-            hard("%s thrown: %s", ex.getExceptionClassName(), ex.getMessage());
+            error("%s thrown: %s", ex.getExceptionClassName(), ex.getMessage());
         }
         printStackTrace(ex.getStackTrace());
     }
@@ -3394,7 +3386,7 @@ public class JShellTool implements MessageHandler {
                         resolution, unrcnt, errcnt,
                         name, type, value, unresolved, errorLines);
                 if (!resolutionErrors.trim().isEmpty()) {
-                    hard("    %s", resolutionErrors);
+                    error("    %s", resolutionErrors);
                 }
             } else if (interactive()) {
                 String display = feedback.format(fcase, action, update,
@@ -3625,7 +3617,6 @@ class ScannerIOContext extends NonInteractiveIOContext {
         scannerIn.close();
     }
 
-    @Override
     public int readUserInput() {
         return -1;
     }
@@ -3657,7 +3648,6 @@ class ReloadIOContext extends NonInteractiveIOContext {
     public void close() {
     }
 
-    @Override
     public int readUserInput() {
         return -1;
     }

src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties

@@ -33,9 +33,8 @@ jshell.err.opt.startup.conflict = Conflicting options: both --startup and --no-s
 jshell.err.opt.feedback.one = Only one feedback option (--feedback, -q, -s, or -v) may be used.
 jshell.err.opt.unknown = Unknown option: {0}
 
-jshell.msg.terminated =\
+jshell.msg.terminated = State engine terminated.
-State engine terminated.\n\
+jshell.msg.terminated.restore = Restore definitions with: /reload -restore
-Restore definitions with: /reload -restore
 
 jshell.msg.use.one.of = Use one of: {0}
 jshell.msg.see.classes.etc = See /types, /methods, /vars, or /list
@@ -189,7 +188,7 @@ For more information see:\n\
    /help shortcuts
 
 help.usage = \
-Usage:   jshell <options> <load files>\n\
+Usage:   jshell <option>... <load file>...\n\
 where possible options include:\n\
 \    --class-path <path>   Specify where to find user class files\n\
 \    --module-path <path>  Specify where to find application modules\n\
@@ -213,7 +212,11 @@ where possible options include:\n\
 \    --version             Print version information and exit\n\
 \    --show-version        Print version information and continue\n\
 \    --help                Print this synopsis of standard options and exit\n\
-\    --help-extra, -X      Print help on non-standard options and exit\n
+\    --help-extra, -X      Print help on non-standard options and exit\n\
+A file argument may be a file name, or one of the predefined file names: DEFAULT,\n\
+PRINTING, or JAVASE.\n\
+A load file may also be "-" to indicate standard input, without interactive I/O.\n
+
 help.usage.x = \
 \    --add-exports <module>/<package>   Export specified module-private package to snippets\n\
 \    --execution <spec>                 Specify an alternate execution engine.\n\

test/langtools/jdk/jshell/StartOptionTest.java

@@ -22,7 +22,7 @@
  */
 
 /*
- * @test 8151754 8080883 8160089 8170162 8166581 8172102 8171343 8178023
+ * @test 8151754 8080883 8160089 8170162 8166581 8172102 8171343 8178023 8186708 8179856
  * @summary Testing start-up options.
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -147,7 +147,7 @@ public class StartOptionTest {
         for (String opt : new String[]{"-h", "--help"}) {
             start(s -> {
                 assertTrue(s.split("\n").length >= 7, "Not enough usage lines: " + s);
-                assertTrue(s.startsWith("Usage:   jshell <options>"), "Unexpect usage start: " + s);
+                assertTrue(s.startsWith("Usage:   jshell <option>..."), "Unexpect usage start: " + s);
                 assertTrue(s.contains("--show-version"), "Expected help: " + s);
                 assertFalse(s.contains("Welcome"), "Unexpected start: " + s);
             }, null, null, opt);
@@ -172,6 +172,38 @@ public class StartOptionTest {
               s -> assertEquals(s.trim(), "Unknown option: unknown"), "--unknown");
     }
 
+    /**
+     * Test that input is read with "-" and there is no extra output.
+     * @throws Exception
+     */
+    public void testHypenFile() throws Exception {
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        startWithUserOutput("", "Hello", "", "-");
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        startWithUserOutput("", "Hello", "", "-", "-");
+        Compiler compiler = new Compiler();
+        Path path = compiler.getPath("markload.jsh");
+        compiler.writeToFile(path, "System.out.print(\"===\");");
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        startWithUserOutput("", "===Hello===", "", path.toString(), "-", path.toString());
+        // check that errors go to standard error
+        cmdInStream = new ByteArrayInputStream(") Foobar".getBytes());
+        start(
+                s -> assertEquals(s.trim(), "", "cmdout: empty"),
+                s -> assertEquals(s.trim(), "", "userout: empty"),
+                s -> assertTrue(s.contains("illegal start of expression"),
+                            "cmderr: illegal start of expression"),
+                "-");
+    }
+
+    /**
+     * Test that non-existent load file sends output to stderr and does not start (no welcome).
+     * @throws Exception
+     */
+    public void testUnknownLoadFile() throws Exception {
+        start("", "File 'UNKNOWN' for 'jshell' is not found.", "UNKNOWN");
+    }
+
     public void testStartup() throws Exception {
         Compiler compiler = new Compiler();
         Path p = compiler.getPath("file.txt");

test/langtools/jdk/jshell/ToolBasicTest.java

@@ -45,7 +45,6 @@ import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
 import java.util.Scanner;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
@@ -408,10 +407,6 @@ public class ToolBasicTest extends ReplToolTesting {
                 (a) -> assertCommand(a, "x", "x ==> 20.0"),
                 (a) -> assertCommand(a, "a", "a ==> 10.0")
         );
-        Path unknown = compiler.getPath("UNKNOWN.jar");
-        test(Locale.ROOT, true, new String[]{unknown.toString()},
-                "|  File '" + unknown
-                + "' for 'jshell' is not found.");
     }
 
     public void testReset() {

test/langtools/jdk/jshell/ToolProviderTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
+import java.nio.file.Path;
 import java.util.ServiceLoader;
 import java.util.function.Consumer;
 import javax.tools.Tool;
@@ -34,7 +35,7 @@ import static org.testng.Assert.fail;
 
 /*
  * @test
- * @bug 8170044 8171343
+ * @bug 8170044 8171343 8179856
  * @summary Test ServiceLoader launching of jshell tool
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -97,4 +98,21 @@ public class ToolProviderTest extends StartOptionTest {
                 null, null,
                 "--show-version");
     }
+    /**
+     * Test that input is read with "-" and there is no extra output.
+     * @throws Exception
+     */
+    @Override
+    public void testHypenFile() throws Exception {
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        start("Hello", "", "-");
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        start("Hello", "", "-", "-");
+        Compiler compiler = new Compiler();
+        Path path = compiler.getPath("markload.jsh");
+        compiler.writeToFile(path, "System.out.print(\"===\");");
+        cmdInStream = new ByteArrayInputStream("System.out.print(\"Hello\");\n".getBytes());
+        start("===Hello===", "", path.toString(), "-", path.toString());
+    }
+
 }