8179856: jshell tool: not suitable for pipeline use
8186708: jshell tool: bad load file garbles message and does not abort
Reviewed-by: jlahoda
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Tue Oct 24 08:37:11 2017 -0700
+++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java Tue Oct 24 20:33:36 2017 -0700
@@ -184,6 +184,7 @@
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 @@
@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 @@
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());
@@ -649,16 +654,6 @@
}
/**
- * 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
*
* @param format printf format
@@ -666,17 +661,17 @@
*/
@Override
public void hard(String format, Object... args) {
- rawout(prefix(format), args);
+ cmdout.printf(prefix(format), args);
}
- /**
+ /**
* Error command output
*
* @param format printf format
* @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 @@
/**
* 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 @@
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 @@
*/
@Override
public void errormsg(String key, Object... args) {
- if (isRunningInteractive()) {
- 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));
+ error(messageFormat(key, args));
}
/**
@@ -847,7 +833,7 @@
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 @@
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 @@
// 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 @@
@Override
public void errormsg(String messageKey, Object... args) {
- startmsg(messageKey, args);
+ JShellTool.this.errormsg(messageKey, args);
}
@Override
@@ -1034,6 +1024,7 @@
shutdownSubscription = state.onShutdown((JShell deadState) -> {
if (deadState == state) {
hardmsg("jshell.msg.terminated");
+ fluffmsg("jshell.msg.terminated.restore");
live = false;
}
});
@@ -1056,10 +1047,6 @@
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 @@
try (IOContext suin = new ScannerIOContext(new StringReader(start))) {
run(suin);
} catch (Exception ex) {
- hardmsg("jshell.err.startup.unexpected.exception", ex);
- ex.printStackTrace(cmdout);
+ errormsg("jshell.err.startup.unexpected.exception", ex);
+ ex.printStackTrace(cmderr);
}
}
@@ -1123,7 +1110,7 @@
String incomplete = "";
while (live) {
String prompt;
- if (isRunningInteractive()) {
+ if (interactive()) {
prompt = testPrompt
? incomplete.isEmpty()
? "\u0005" //ENQ
@@ -1171,7 +1158,7 @@
}
private void addToReplayHistory(String s) {
- if (isRunningInteractive()) {
+ if (!isCurrentlyRunningStartup) {
replayableHistory.add(s);
}
}
@@ -2127,7 +2114,7 @@
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 @@
}
currSrcs = nextSrcs;
} catch (IllegalStateException ex) {
- hardmsg("jshell.msg.resetting");
+ errormsg("jshell.msg.resetting");
resetState();
currSrcs = new LinkedHashSet<>(); // re-process everything
}
@@ -2746,16 +2733,21 @@
private boolean runFile(String filename, String context) {
if (!filename.isEmpty()) {
try {
- Path path = toPathResolvingUserHome(filename);
- Reader reader;
- String resource;
- if (!Files.exists(path) && (resource = getResource(filename)) != null) {
- // Not found as file, but found as resource
- reader = new StringReader(resource);
+ Scanner scanner;
+ if (!interactiveModeBegun && filename.equals("-")) {
+ // - on command line: no interactive later, read from input
+ regenerateOnDeath = false;
+ scanner = new Scanner(cmdin);
} 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 @@
sb.append(a);
}
if (sb.length() > 0) {
- rawout(prefix(sb.toString()));
+ hard(sb.toString());
}
return false;
}
@@ -3175,11 +3167,11 @@
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 @@
} 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 @@
: lineNumber >= 0
? fileName + ":" + lineNumber
: fileName;
- hard(" at %s(%s)", sb, loc);
+ error(" at %s(%s)", sb, loc);
}
}
@@ -3253,9 +3245,9 @@
//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 @@
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 @@
scannerIn.close();
}
- @Override
public int readUserInput() {
return -1;
}
@@ -3657,7 +3648,6 @@
public void close() {
}
- @Override
public int readUserInput() {
return -1;
}
--- a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Tue Oct 24 08:37:11 2017 -0700
+++ b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/resources/l10n.properties Tue Oct 24 20:33:36 2017 -0700
@@ -33,9 +33,8 @@
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 =\
-State engine terminated.\n\
-Restore definitions with: /reload -restore
+jshell.msg.terminated = State engine terminated.
+jshell.msg.terminated.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 @@
/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 @@
\ --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\
--- a/test/langtools/jdk/jshell/StartOptionTest.java Tue Oct 24 08:37:11 2017 -0700
+++ b/test/langtools/jdk/jshell/StartOptionTest.java Tue Oct 24 20:33:36 2017 -0700
@@ -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 @@
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 @@
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");
--- a/test/langtools/jdk/jshell/ToolBasicTest.java Tue Oct 24 08:37:11 2017 -0700
+++ b/test/langtools/jdk/jshell/ToolBasicTest.java Tue Oct 24 20:33:36 2017 -0700
@@ -45,7 +45,6 @@
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 @@
(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() {
--- a/test/langtools/jdk/jshell/ToolProviderTest.java Tue Oct 24 08:37:11 2017 -0700
+++ b/test/langtools/jdk/jshell/ToolProviderTest.java Tue Oct 24 20:33:36 2017 -0700
@@ -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 @@
/*
* @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 @@
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());
+ }
+
}