Note also that unlike the doclet part of javadoc, the tool part of
javadoc is (and has always been) tightly coupled to javac, and directly
uses a number of internal javac interfaces, to modify javac behavior.
For example, this includes providing custom subtypes to override javac
behavior to read files like package.html, to not analyze method bodies,
and even to discard method bodies, which are of no relevance to javadoc.
In other words, the tool part of javadoc is allowed access to javac
internals, whereas we try and restrict the doclet, which is conceptually
a plugin, to the public APIs like Compiler Tree API (com.sun.source.*)
and Language Model API (javax.lang.model.*)
-- Jon
On 1/24/20 8:57 AM, Jonathan Gibbons wrote:
Re: com.sun.tools.javac.main.Option.*
Well, it is deliberate and intentional that we fully delegate to javac
those options that are handled by javac, like path-related options,
--source, --release, --enable-preview etc. This reflects the fact that
we totally rely on the javac front end to read source and class files.
It was actually a big step forward in the JDK 9 rewrite that we
properly delegate through the underlying Option objects, as compared
to tunneling values into the compOpts object, as was done previously,
which bypassed much of javac's checking.
-- Jon
On 1/24/20 8:51 AM, Pavel Rappo wrote:
Hi Jon,
I'm still working through the review but I have to say that degree to
which com.sun.tools.javac.main.Option.* types have percolated to the
javadoc internal interfaces is unsettling. I understand this is
probably due to practical and historical reasons. Maybe we could
address that later.
-Pavel
On 24 Jan 2020, at 02:30, Jonathan Gibbons
<[email protected]> wrote:
Although the underlying problems are different, the general goal of
this cleanup is similar in nature to that of the recent cleanup for
doclet options.
In this case, the effect is not as widespread ... just 6 source
files affected, no tests ... but the changes to the main affected
class are more substantial, although still primarily a refactoring
and just moving code around, with no intentional change in
functionality.
To describe the changes, let me describe the world before this change:
The ToolOption class followed the javac model for options and used
an enum to represent the individual supported options. One problem
of using an enum is that they are implicitly static, and so have
never have any enclosing context. This means that when analyzing
command-line arguments, the enum members need to be given an object
providing the necessary context. In the case of ToolOption, this was
a nested Helper class, which contained a mix of fields containing
the values for some options, most notably those used in Start, and a
map of objects for the values of other options, where the map was
literally, Map<ToolOption,Object>. This led to "clunky" code to
access the values in the map and to cast the result to the correct
type for each value.
In general, while there were some benefits to using the enum (such
as being able to refer to some of the options by their member name),
the cost outweighed the benefits.
The primary change is to invert the nesting relationship between
ToolOption and its Helper, and to rename and refactor the code
accordingly.
To summarize the changes,
1. ToolOption.Helper becomes a new top-level class ToolOptions,
which is the new primary abstraction for the accessing everything to
do with tool options.
2. ToolOption is changed from a top-level enum to a nested class
in ToolOptions, with the members becoming a simple List<ToolOption>.
3. All option values are represented as properly-typed
encapsulated fields of ToolOptions. The fields are encapsulated,
based on the feedback for the doclet options review.
4. The direct use and passing around of the Map jdToolOpts is
replaced by direct use of the new ToolOptions class.
5. ToolOptions uses a new ShowHelper interface to separate out
the functionality for handling options like --help and --verbose.
Previously, Start implemented ToolOption.Help directly; now, it just
uses a local anonymous class instead.
6. ToolOption.java is renamed to ToolOptions.java, to retain
history and to maximize the opportunity to compare the old and new
versions.
There are no significant changes to the high-level option handling
in Start, which continues to do the double scan, to pick up
selection options, like -doclet, -docletpath, -locale, before doing
the main scan. The handling of OptionException could also be
simplified (separately), possibly allowing the ShowHelper class to
be eliminated.
One of the advantages of using the enum (in the old code) was that
it allowed symbolic references to options handled in
Start.preprocess. These references are fixed up by defining string
constants for the names of the handful of options in question, which
is all that is needed.
While the code is generally cleaner for allowing the ToolOption
objects to be inner classes of ToolOptions, it does mean they can
only exist in the context of a ToolOptions object. This has an
impact on a little-used method on the DocumentationTask interface,
to determine if an option name is supported. The body of the
implementing method is moved into ToolOptions, which creates a
temporary minimal ToolOptions object, sufficient to the needs of the
isSupportedOption method.
-- Jon
JBS: https://bugs.openjdk.java.net/browse/JDK-8237803
Webrev: http://cr.openjdk.java.net/~jjg/8237803/webrev/
