On Tue, 11 Feb 2025 14:00:42 GMT, Nizar Benalla <nbena...@openjdk.org> wrote:
>> Some javadoc snippets can match multiple links to the same content, leading
>> to different results in different javadoc runs.
>> This patch proposes emitting an error when such cases are encountered.
>>
>> There is a very trivial, unrelated change in `TestGlobalHtml.java` because I
>> noticed some whitespace wasn't right.
>
> Nizar Benalla has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains four additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into snippet-non-rep
> - adjusting error message
> - Merge remote-tracking branch 'upstream/master' into snippet-non-rep
> - emit error when encountering ambigious link
Looks mostly good. I suggest renaming a variable and checking output of
expected message in the test.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java
line 140:
> 138: } else {
> 139: Element e = null;
> 140: String linkEncountered = null;
Since the primary purpose of this variable is now to carry the link target (and
checking for overlapping links is just a side-use) I would prefer something
like `linkTarget` as name.
test/langtools/jdk/javadoc/doclet/ReproducibleSnippet/ReproducibleSnippetTest.java
line 70:
> 68: src.toString(),
> 69: "p");
> 70: checkExit(Exit.ERROR);
The error message should also be checked.
-------------
Changes requested by hannesw (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/23328#pullrequestreview-2623816468
PR Review Comment: https://git.openjdk.org/jdk/pull/23328#discussion_r1959916042
PR Review Comment: https://git.openjdk.org/jdk/pull/23328#discussion_r1959891174
