On Tue, 16 Nov 2021 05:36:37 GMT, Jonathan Gibbons <[email protected]> wrote:
> Please review a moderately simple addition to the `JavadocTester` world, to
> be able to modify the behavior of a file manager to throw exceptions as
> needed.
>
> The bulk of the new functionality is in a new class,
> `TestJavaFileManagerBuilder`, which uses the builder pattern to allow a
> client to configure and build a file manager that can provide file objects
> for which the behavior can be overridden for one or more methods. The
> expected use case is to throw an exception instead of calling the underlying
> delegate method.
>
> `JavadocTester` is modified to allow a file manager to be provided when
> invoking `javadoc`. This requires some minor changes to the outermost javadoc
> tool classes, `Main` and `Start`. Rather than add more `static` methods to
> `Main`, instance methods are now provided to make it easier to configure the
> values that will be passed to `Start`. In `Start`, it was previously
> assumed that either the default file manager was being used or that _all_
> paths would be configured directly in the file manager. The latter part of
> that assumption is reduced so that path-setting options (e.g.
> `--source-path`, `--class-path` etc) can be passed to the provided file
> manager. (Previously, they were silently ignored.) It is an error to pass
> path-like options as javadoc options to a file manager that does not support
> them. However, since none of the changes are visible to any public API, this
> should not be an issue.
>
> A new test is provided, with a few simple test cases. One is for direct use
> of the new file manager mechanism (without using `javadoc`). The other two
> illustrate how the feature can be used inside a `JavadocTester` call of
> `javadoc`. Given that the feature is a relatively simple combination of
> predicates and proxies, it's not clear that we need a significant body of
> test cases.
Initial comments below.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
line 48:
> 46: doclet.exception.read.resource=Error reading system resource: {0}\n\
> 47: \t({1})
> 48: doclet.internal.exception=An internal exception has occurred.\n\
Out of curiosity, how did you notice that trailing space?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Main.java line 119:
> 117:
> 118: /**
> 119: * Sets the file manager to be used to when running javadoc.
Suggestion:
* Sets the file manager to be used when running javadoc.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Main.java line 134:
> 132: * Runs javadoc with preconfigured values and a given set of
> arguments.
> 133: * Any errors will be reported to the error stream, or to {@link
> System#err}
> 134: * if not error stream has been specified with {@code setStreams}.
Suggestion:
* if no error stream has been specified with {@code setStreams}.
test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 381:
> 379: : jdk.javadoc.internal.tool.Main.execute(args,
> outOut.pw); // default
> 380: */
> 381: jdk.javadoc.internal.tool.Main main = new
> jdk.javadoc.internal.tool.Main();
Do we need this commented-out code?
test/langtools/jdk/javadoc/lib/javadoc/tester/TestJavaFileManagerBuilder.java
line 66:
> 64: * by method and predicate and then dynamically build the set of methods
> to be used for
> 65: * a file object by filtering the methods by their applicable predicate.
> 66: *
Suggestion:
-------------
Changes requested by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6404
