On Fri, 14 Jan 2022 16:35:56 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

>> Please review a change in how documentation from `@param` tags is generated. 
>> 
>> The old code generates parameter documentation for each `@param` in the 
>> order in which the tags occur in the comment, then adds documentation from 
>> inherited `@param` tags for undocumented parameters. 
>> 
>> The new code always generates documentation in the order in which actual 
>> parameters are declared in the code, using local or inherited `@param` tags 
>> as appropriate. Any `@param` tags that do not have a matching parameter are 
>> added afterwards.
>> 
>> Note that `@param` is not just used for parameters of executable members but 
>> also type parameters and record components. The second commit of this PR 
>> fixes a `ClassCastException` for these uses that was caused by the first 
>> commit and adds a few tests for it.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8234682: Review feedback

1. As you pointed elsewhere, `rank` is actually a position of a particular 
parameter. Later, we could rename `rank` into `position` or something similarly 
appropriate.
2. Although "ranks" precede this patch, my gut feeling is that rank being 
String rather than Integer, is a bad idea. But as long as "ranks" are not 
compared for order (only for equality), we are okay; alphanumeric order differs 
from that of numeric: "1", "11", "2" vs 1, 2, 11. We should revisit "ranks" 
later.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java
 line 211:

> 209:         CommentHelper ch = 
> writer.configuration().utils.getCommentHelper(e);
> 210:         if (!paramTags.isEmpty()) {
> 211:             Map<String,String> rankMap = 
> getRankMap(writer.configuration().utils, formalParameters);

Suggestion:

            Map<String, String> rankMap = 
getRankMap(writer.configuration().utils, formalParameters);

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java
 line 229:

> 227:                             case PARAMETER -> 
> "doclet.Parameters_dup_warn";
> 228:                             case TYPE_PARAMETER -> 
> "doclet.TypeParameters_dup_warn";
> 229:                             case RECORD_COMPONENT -> 
> "doclet.RecordComponents_dup_warn";

Suggestion:

                            case PARAMETER        -> 
"doclet.Parameters_dup_warn";
                            case TYPE_PARAMETER   -> 
"doclet.TypeParameters_dup_warn";
                            case RECORD_COMPONENT -> 
"doclet.RecordComponents_dup_warn";

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java
 line 230:

> 228:      * @param alreadyDocumented the set of exceptions that have already
> 229:      *        been documented.
> 230:      * @param rankMap a {@link java.util.Map} which holds ordering

It's ironic that a method whose job, at least in part, is to warn about 
duplicated `@param` tags had one of its `@param` tags duplicated. Was it an 
Easter egg of sorts?

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

Marked as reviewed by prappo (Reviewer).

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

Reply via email to