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