RFR: JDK-8267126: javadoc should show "line and caret" for diagnostics. [v2]

[Jonathan Gibbons]

On Fri, 21 May 2021 17:39:43 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   minor cleanup
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java line 543:
> 
>> 541:      * Returns whether or not to permit dynamically loaded components to access
>> 542:      * part of the javadoc internal API. The flag is the same (hidden) compiler
>> 543:      * option that allows javac plugins and annotation processors to javac internal
> 
> Do you use "allow x to y" here in the sense of "letting x into y"? If so, consider using "allow x into y" instead of "allow x to y". Otherwise it might sound like there's a verb missing: allow to (access? enter?) javac internal API. My non-native English speaker's two cents.

Yes, there was a missing verb. Fixed.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 67:
> 
>> 65:  * of diagnostic messages and to avoid code duplication.
>> 66:  *
>> 67:  * The class is a subtype of javac's Log, and is primarily a transducer between
> 
> Would "adapter" be more suitable than "transducer"?

Both are valid, but agreed that "adapter" is a more common term.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 92:
> 
>> 90:  *     <li>{@code Diagnostic.Kind} -- maps to {@code DiagnosticType} and {@code Set<DiagnosticFlag>}
>> 91:  *     <li>{@code Element} -- maps to {@code DiagnosticSource and {@code DiagnosticPosition}
>> 92:  *     <li>{@code DocTreePath} -- maps to {@code DiagnosticSource and {@code DiagnosticPosition}
> 
> Close `@code` tags.

oops

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 96:
> 
>> 94:  *
>> 95:  * The javac layer deals primarily in pre-localized (key, args) pairs,
>> 96:  * while the javadoc layer, especially the {@code Reporter} interface, deals in localized strings.
> 
> If it is not a typo, consider changing "deal in" to something simpler.

I'll try.  Maybe something based on "operates on" or "uses" or something.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 125:
> 
>> 123:     private ToolEnvironment toolEnv;
>> 124: 
>> 125:     /** The utility class to access the positions of items in doc comments., */
> 
> Remove that trailing comma.

oops

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 332:
> 
>> 330: 
>> 331:     /**
>> 332:      * Print a "notice" message.
> 
> Add "s" to "print" for consistency with doc comment of other methods in this class.

Definitely.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 448:
> 
>> 446:      * such as  "Generating class ...".  Therefore, for now, we detect and report those
>> 447:      * messages directly. (A better solution would be to expose access to the output
>> 448:      * and error streams via {@code DocletEnvironment}.
> 
> Sentence: either remove "(" or add ")".

Add ")" and change DocletEnvironment to Reporter for now, and to delete the ~sentence~ paragraph when we add streams to Reporter.

> test/langtools/jdk/javadoc/doclet/testDiagsLineCaret/MyTaglet.java line 127:
> 
>> 125:             @Override
>> 126:             public Void visitEntity(EntityTree node, Diagnostic.Kind k) {
>> 127:                 if (node.getName().contentEquals("#x1f955")) {
> 
> Is this... a carrot (��)?

https://www.fileformat.info/info/unicode/char/search.htm :-)

> test/langtools/jdk/javadoc/doclet/testMissingComment/TestMissingComment.java line 115:
> 
>> 113:         javadoc("-d", base.resolve("api").toString(),
>> 114:                 "-Xdoclint:missing",
>> 115:                 "--no-platform-links",
> 
> More of those... sigh.

A different solution would be to disable platform links by default (in JavadocTester) and only enables for specific tests.  But that would be a minor paradigm shift of JavadocTester providing default options.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4074