RFR: 8159305: Enhance the javadoc tool to support module related options

[Jonathan Gibbons]

On 06/20/2016 04:18 PM, Kumar Srinivasan wrote:
> Hello,
>
> Please review the changes to fix:
> https://bugs.openjdk.java.net/browse/JDK-8159305
>
> The webrev is here:
> http://cr.openjdk.java.net/~ksrini/8159305/webrev.00/
>
> The spec-diff is here for reference:
> http://cr.openjdk.java.net/~ksrini/8159305/spec-diff/overview-summary.html 
>
>
>
> Thanks
> Kumar
>

DocletEnvironment: general  changes will require a CCC.

DocletEnvironment:61

"annotations" is wrong.   You mean "annotation types".  An "annotation" 
is the *use* of an annotation type.

line 101, don't like the "style" of plural type names (like 
"ModuleElements") in plain text. Type names should be in {@code} font.  
Consider using the underlying English words. (module elements, package 
elements, etc). If in doubt, follow whatever style Joe uses in the 269 API.

linbe 142. terse

doclet/package-info:63 plural type names again
Should use code form for any occurrence of an option name

60/61 the name is "included" but the <em> term is "specified". Not good 
to be inconsistent.
89 uses both, I'm not usre if the two are the same or different.


AbstractExecutableMemberWriter, 118, who is "we"?  Could improve 
phrasing, but I accept it's internal API.


AnnotationTypeRequiredMemberImpl, 108, OK, itr's an overloaded method, 
but I simply note that in this override, the parameter is ignored.


ConfigurationImpl 422
replace    DocPath.empty.resolve with plain old "new DocPath"

AbstractDoclet 119
Uugh

Configuration 358
Bad grammar/punctuation.   I'm surprised by the ordering of the decls 
for modules and modulePackages. I would have thought that modules is the 
dominant declaration here, and the modulePackages is derived from 
modules;  instead, the comment makes it seem like it's the other way 
around.  And, according to the code in initModules, the description is 
wrong anyway.

Configuration 392, initModules, similar to preceding comment. I think 
you have the role of modules and modulePackages backwards. Create 
"modules" first, add in the specifiedModules, then add in the packages.

Configuration 420, why the guard, are you worried about calling this 
multiple times?

TypeElementCatalog 103, redundant TypeELementCatalog.this (looks like NB 
error)

Utils is a bit of a mess, taken in conjunction with Configuration. 
There's a no-reason split between stuff in Utils and stuff in Configuration.

DocEnv
funky imports order

DocEnv.ModulePackage  ... an interface? really?   why not just a class 
containing two public final strings?

JavadocTool ... IncludesTable looks big enough to be split into its own 
top level class, and *maybe* should start becoming the basis of a new 
abstraction for all "included" stuff on the command, i.e. superseding 
code from Configuration and Utils.

JavadocTool.IncludesTable.ModulePackage. Over-engineered.

RootDocImpl:
more lack of abstraction with regard to stuff specified on the command line.

ToolOption
add M as an alias for MODULE

New long form options (SHOW_MODULES, etc) use space or = as a separator, 
not :

ToolOption.AccessHelper looks misguided.  Either the functionality here 
should be merged into Helper, or it should possibly be renamed and 
become a member of Helper that supersedes/replaces the showAccess map 
in  Helper.

-- Jon