Hi again,
I figured out and err deserve a null-check test as well. See patch.
I'm wondering, if or how a similar check could be applied as well to
ToolProvider.run(PrintWriter, PrintWriter, String...)
which currently is implemented (hopefully) by each tool having to
repeat the null-checks.
I'm also wondering about the call to flush in run(PrintStream out,
PrintStream err, String... args). It looks like the intention was to
flush the wrapping PrintWriter.
That is not possible without also flushing the underlying PrintStream.
BufferedWriter.flushBuffer would be a more sensible method to call but
is not accessible.
The effect is actually, that the call to PrintWriter.flush will also
call flush of the underlying PrintStream. Should that be documented
more explicitly, for example:
diff -r 7c17199fa37d
src/java.base/share/classes/java/util/spi/ToolProvider.java
--- a/src/java.base/share/classes/java/util/spi/ToolProvider.java
Fri Feb 15 08:21:08 2019 -0500
+++ b/src/java.base/share/classes/java/util/spi/ToolProvider.java
Sat Feb 16 09:17:46 2019 +0100
@@ -107,6 +107,8 @@
* @implNote This implementation wraps the {@code out} and {@code
err}
* streams within {@link PrintWriter}s, and then calls
* {@link #run(PrintWriter, PrintWriter, String[])}.
+ * Both {@code out} and {@code err} streams are flushed before the
method
+ * returns as a side-effect of flushing the wrapping {@link
PrintWriter}s.
*
* @param out a stream to which "expected" output should be
written
*
Regards,
Philippdiff -r 7c17199fa37d src/java.base/share/classes/java/util/spi/ToolProvider.java
--- a/src/java.base/share/classes/java/util/spi/ToolProvider.java Fri Feb 15 08:21:08 2019 -0500
+++ b/src/java.base/share/classes/java/util/spi/ToolProvider.java Sat Feb 16 09:18:32 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -126,8 +126,9 @@
default int run(PrintStream out, PrintStream err, String... args) {
Objects.requireNonNull(out);
Objects.requireNonNull(err);
+ Objects.requireNonNull(args);
for (String arg : args) {
- Objects.requireNonNull(args);
+ Objects.requireNonNull(arg);
}
PrintWriter outWriter = new PrintWriter(out);
diff -r 7c17199fa37d test/jdk/java/util/spi/ToolProviderTest.java
--- a/test/jdk/java/util/spi/ToolProviderTest.java Fri Feb 15 08:21:08 2019 -0500
+++ b/test/jdk/java/util/spi/ToolProviderTest.java Sat Feb 16 09:18:32 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -28,7 +28,6 @@
* @run main/othervm ToolProviderTest
*/
-import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.file.Files;
@@ -46,16 +45,45 @@
void run() throws Exception {
initServices();
+ testNullArgs();
+
System.out.println("test without security manager present:");
- test();
+ testSecurityManager();
System.setSecurityManager(new SecurityManager());
System.out.println("test with security manager present:");
- test();
+ testSecurityManager();
}
- private void test() throws Exception {
+ private static void expectNullPointerException(Runnable test) {
+ try {
+ test.run();
+ throw new Error("NullPointerException not thrown");
+ } catch (NullPointerException e) {
+ // expected
+ }
+ }
+
+ private void testNullArgs() {
+ ToolProvider testProvider = ToolProvider.findFirst("test").get();
+
+ // out null check
+ expectNullPointerException(() -> testProvider.run(null, System.err));
+
+ // err null check
+ expectNullPointerException(() -> testProvider.run(System.out, null));
+
+ // args array null check
+ expectNullPointerException(() ->
+ testProvider.run(System.out, System.err, (String[]) null));
+
+ // args array elements null check
+ expectNullPointerException(() ->
+ testProvider.run(System.out, System.err, (String) null));
+ }
+
+ private void testSecurityManager() throws Exception {
ToolProvider testProvider = ToolProvider.findFirst("test").get();
int rc = testProvider.run(System.out, System.err, "hello test");
if (rc != 0) {
@@ -92,7 +120,7 @@
// system property
System.getProperty("java.home");
if (haveSecurityManager) {
- throw new Error("exception exception not thrown");
+ throw new Error("SecurityException not thrown");
}
} catch (SecurityException e) {
if (!haveSecurityManager) {
