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

[Pavel Rappo]

On Mon, 17 May 2021 20:40:18 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> Please review a change to overhaul the javadoc support for diagnostics to better leverage the support available in javac. This includes the ability for all javadoc diagnostics to show a "source line and caret" to indicate the position of a reported issue. As a side-effect, it normalizes the formatting of javadoc messages, to be consistent with javac messages.
>> 
>> The primary changes are in the javadoc `Messager` class, and are primarily focussed "downward" on the internal use of the javac `Log.report` method, which is the nexus for reporting methods. There is additional cleanup that could be done in `Messager` in the API it provides to clients, but (for the most part) that is not done in this work.
>> 
>> Additional changes are done to facilitate writing a test for this work, and reflect the current shortcomings of the existing `Doclet` API. Most notably:
>> * changes in `Utils` to allow a user-defined taglet to override a built-in taglet
>> * changes in `TagletManager` and `Workarounds` to allow a user-defined taglet to access internal API, to workaround API that would be useful to provide on `StandardDoclet`
>> 
>> There are a few minor specific cleanup changes in code style and/or improved comments.
>> 
>> There is one primary new test, `TestDiagsLineCaret.java` which exercises different kinds of diagnostics at different positions in a file, to verify that the source line and a caret are produced as appropriate.
>> 
>> There are additional test changes triggered by the slight change in the format of error messages.  Most notably, prefixes like `error -` and `warning -` become `error:` and `warning:`.
>
> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
> 
>   minor cleanup

This is not a review, but preliminary comments to help me get started with the review.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletManager.java line 264:

> 262:                         "jdk.javadoc.internal.doclets.formats.html");
> 263:                 pkgs.forEach(p -> thisModule.addOpens(p, tagletLoaderUnnamedModule));
> 264:             }

What's that and the corresponding WorkArounds.accessInternalAPI about?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 474:

> 472:      */
> 473:     private DiagnosticPosition getDiagnosticPosition(DocTreePath path) {
> 474:         ToolEnvironment toolEnv = getToolEnv();

toolEnv is unused.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ToolEnvironment.java line 147:

> 145:         enter = JavadocEnter.instance(context);
> 146:         names = Names.instance(context);
> 147:         externalizableSym = syms.enterClass(syms.java_base, names.fromString("java.io.Externalizable"));

What was externalizableSym about and why have you deleted it?

test/langtools/jdk/javadoc/doclet/testJavaFX/TestJavaFX.java line 377:

> 375: 
> 376:         // make sure the doclet indeed emits the warning
> 377:         checkOutput(Output.OUT, true, "C.java:31: warning: invalid usage of tag <");

This is interesting: while the line coordinate of an error has changed in this test, those of  [TestSearch.java L676-L679](https://github.com/openjdk/jdk/pull/4074/files#diff-79c641910bccd3bfabdf3912fd24f4858edcd17bdd325555930fd85fba580593L676-L679) from this same PR have not. Mind you, I haven't investigated it yet.

test/langtools/jdk/javadoc/doclet/testJavaFX/TestJavaFX.java line 422:

> 420:                 "--javafx",
> 421:                 "--disable-javafx-strict-checks",
> 422:                 "--no-platform-links",

It just happens so that yesterday I [asked](https://github.com/openjdk/jdk/pull/4051#pullrequestreview-660862882) Hannes about proliferation of `--no-platform-links` in tests and now we have 3 more of those in this PR alone.

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

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