From ec8b6acff39953ca60309c6f1db7ae4c93404da9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Walln=C3=B6fer?= Date: Thu, 20 Jan 2022 16:17:08 +0000 Subject: [PATCH] 8234682: The order of @param in the generated docs should match the method signature Reviewed-by: prappo --- .../doclets/toolkit/taglets/ParamTaglet.java | 117 ++++++++---------- .../testParamTaglet/TestParamTaglet.java | 53 ++++++-- .../javadoc/doclet/testParamTaglet/pkg/C.java | 38 +++++- .../doclet/testParamTaglet/pkg/Parent.java | 9 +- 4 files changed, 139 insertions(+), 78 deletions(-) diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java index 2552eb137a7..2af114d322b 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java @@ -165,54 +165,29 @@ public class ParamTaglet extends BaseTaglet implements InheritableTaglet { private Content getTagletOutput(ParamKind kind, Element holder, TagletWriter writer, List formalParameters, List paramTags) { Content result = writer.getOutputInstance(); - Set alreadyDocumented = new HashSet<>(); - if (!paramTags.isEmpty()) { - result.add( - processParamTags(holder, kind, paramTags, - getRankMap(writer.configuration().utils, formalParameters), writer, alreadyDocumented) - ); - } - if (alreadyDocumented.size() != formalParameters.size()) { - //Some parameters are missing corresponding @param tags. - //Try to inherit them. - result.add(getInheritedTagletOutput(kind, holder, - writer, formalParameters, alreadyDocumented)); - } + result.add(processParamTags(holder, kind, paramTags, formalParameters, writer)); return result; } /** - * Loop through each individual parameter, despite not having a - * corresponding param tag, try to inherit it. + * Try to get the inherited taglet documentation for a specific parameter. */ private Content getInheritedTagletOutput(ParamKind kind, Element holder, - TagletWriter writer, List formalParameters, - Set alreadyDocumented) { + TagletWriter writer, Element param, int rank, + boolean isFirst) { Utils utils = writer.configuration().utils; Content result = writer.getOutputInstance(); - if ((!alreadyDocumented.contains(null)) && utils.isExecutableElement(holder)) { - for (int i = 0; i < formalParameters.size(); i++) { - if (alreadyDocumented.contains(String.valueOf(i))) { - continue; - } - // This parameter does not have any @param documentation. - // Try to inherit it. - Input input = new DocFinder.Input(writer.configuration().utils, holder, this, - Integer.toString(i), kind == ParamKind.TYPE_PARAMETER); - DocFinder.Output inheritedDoc = DocFinder.search(writer.configuration(), input); - if (inheritedDoc.inlineTags != null && !inheritedDoc.inlineTags.isEmpty()) { - Element e = formalParameters.get(i); - String lname = kind != ParamKind.TYPE_PARAMETER - ? utils.getSimpleName(e) - : utils.getTypeName(e.asType(), false); - Content content = processParamTag(inheritedDoc.holder, kind, writer, - (ParamTree) inheritedDoc.holderTag, - lname, - alreadyDocumented.isEmpty()); - result.add(content); - } - alreadyDocumented.add(String.valueOf(i)); - } + Input input = new DocFinder.Input(writer.configuration().utils, holder, this, + Integer.toString(rank), kind == ParamKind.TYPE_PARAMETER); + DocFinder.Output inheritedDoc = DocFinder.search(writer.configuration(), input); + if (inheritedDoc.inlineTags != null && !inheritedDoc.inlineTags.isEmpty()) { + String lname = kind != ParamKind.TYPE_PARAMETER + ? utils.getSimpleName(param) + : utils.getTypeName(param.asType(), false); + Content content = processParamTag(inheritedDoc.holder, kind, writer, + (ParamTree) inheritedDoc.holderTag, + lname, isFirst); + result.add(content); } return result; } @@ -225,23 +200,15 @@ public class ParamTaglet extends BaseTaglet implements InheritableTaglet { * * @param paramTags the array of {@code @param DocTree} to convert. * @param writer the TagletWriter that will write this tag. - * @param alreadyDocumented the set of exceptions that have already - * been documented. - * @param rankMap a {@link java.util.Map} which holds ordering - * information about the parameters. - * @param rankMap a {@link java.util.Map} which holds a mapping - of a rank of a parameter to its name. This is - used to ensure that the right name is used - when parameter documentation is inherited. * @return the Content representation of this {@code @param DocTree}. */ - private Content processParamTags(Element e, ParamKind kind, - List paramTags, Map rankMap, TagletWriter writer, - Set alreadyDocumented) { + private Content processParamTags(Element e, ParamKind kind, List paramTags, + List formalParameters, TagletWriter writer) { + Map documented = new HashMap<>(); Messages messages = writer.configuration().getMessages(); - Content result = writer.getOutputInstance(); + CommentHelper ch = writer.configuration().utils.getCommentHelper(e); if (!paramTags.isEmpty()) { - CommentHelper ch = writer.configuration().utils.getCommentHelper(e); + Map rankMap = getRankMap(writer.configuration().utils, formalParameters); for (ParamTree dt : paramTags) { String name = ch.getParameterName(dt); String paramName = kind == ParamKind.TYPE_PARAMETER ? "<" + name + ">" : name; @@ -250,23 +217,45 @@ public class ParamTaglet extends BaseTaglet implements InheritableTaglet { case PARAMETER -> "doclet.Parameters_warn"; case TYPE_PARAMETER -> "doclet.TypeParameters_warn"; case RECORD_COMPONENT -> "doclet.RecordComponents_warn"; - default -> throw new IllegalArgumentException(kind.toString()); }; messages.warning(ch.getDocTreePath(dt), key, paramName); } String rank = rankMap.get(name); - if (rank != null && alreadyDocumented.contains(rank)) { - String key = switch (kind) { - case PARAMETER -> "doclet.Parameters_dup_warn"; - case TYPE_PARAMETER -> "doclet.TypeParameters_dup_warn"; - case RECORD_COMPONENT -> "doclet.RecordComponents_dup_warn"; - default -> throw new IllegalArgumentException(kind.toString()); - }; - messages.warning(ch.getDocTreePath(dt), key, paramName); + if (rank != null) { + if (documented.containsKey(rank)) { + String key = switch (kind) { + case PARAMETER -> "doclet.Parameters_dup_warn"; + case TYPE_PARAMETER -> "doclet.TypeParameters_dup_warn"; + case RECORD_COMPONENT -> "doclet.RecordComponents_dup_warn"; + }; + messages.warning(ch.getDocTreePath(dt), key, paramName); + } else { + documented.put(rank, dt); + } } + } + } + // Document declared parameters for which taglet documentation is available + // (either directly or inherited) in order of their declaration. + Content result = writer.getOutputInstance(); + for (int i = 0; i < formalParameters.size(); i++) { + ParamTree dt = documented.get(String.valueOf(i)); + if (dt != null) { result.add(processParamTag(e, kind, writer, dt, - name, alreadyDocumented.isEmpty())); - alreadyDocumented.add(rank); + ch.getParameterName(dt), result.isEmpty())); + } else if (writer.configuration().utils.isExecutableElement(e)) { + result.add(getInheritedTagletOutput(kind, e, writer, + formalParameters.get(i), i, result.isEmpty())); + } + } + if (paramTags.size() > documented.size()) { + // Generate documentation for remaining taglets that do not match a declared parameter. + // These are erroneous but we generate them anyway. + for (ParamTree dt : paramTags) { + if (!documented.containsValue(dt)) { + result.add(processParamTag(e, kind, writer, dt, + ch.getParameterName(dt), result.isEmpty())); + } } } return result; diff --git a/test/langtools/jdk/javadoc/doclet/testParamTaglet/TestParamTaglet.java b/test/langtools/jdk/javadoc/doclet/testParamTaglet/TestParamTaglet.java index b6b6a7a78d4..200af37a948 100644 --- a/test/langtools/jdk/javadoc/doclet/testParamTaglet/TestParamTaglet.java +++ b/test/langtools/jdk/javadoc/doclet/testParamTaglet/TestParamTaglet.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 4802275 4967243 8026567 8239804 + * @bug 4802275 4967243 8026567 8239804 8234682 * @summary Make sure param tags are still printed even though they do not * match up with a real parameters. * Make sure inheritDoc cannot be used in an invalid param tag. @@ -48,22 +48,51 @@ public class TestParamTaglet extends JavadocTester { "-sourcepath", testSrc, "pkg"); checkExit(Exit.ERROR); - + checkOutput(Output.OUT, true, + "warning: no @param for param1", + "error: @param name not found", + "warning: @param \"b\" has already been specified"); checkOutput("pkg/C.html", true, - //Regular param tags. + // Regular param tags. """
Parameters:
param1 - testing 1 2 3.
-
param2 - testing 1 2 3.""", - //Param tags that don't match with any real parameters. +
param2 - testing 1 2 3.
+ """, + // Param tags that don't match with any real parameters. + // {@inheritDoc} misuse does not cause doclet to throw exception. + // Param is printed with nothing inherited. """
Parameters:
p1 - testing 1 2 3.
-
p2 - testing 1 2 3.""", - //{@inherit} doc misuse does not cause doclet to throw exception. - // Param is printed with nothing inherited. - //XXX: in the future when Configuration is available during doc inheritence, - //print a warning for this mistake. - "inheritBug -"); +
p2 - testing 1 2 3.
+
inheritBug -
+ """, + """ +
Parameters:
+
i - an int
+
d - a double
+
b - a boolean
+
x - does not exist
+
x - duplicate
+
b - another duplicate
+ """, + """ +
Type Parameters:
+
T2 - type 2
+
Parameters:
+
t1 - param 1
+
t3 - param 3
+ """); + checkOutput("pkg/C.Point.html", true, + """ +
Record Components:
+
y - the y coordinate
+ """); + checkOutput("pkg/C.Nested.html", true, + """ +
Type Parameters:
+
T1 - type 1
+ """); } } diff --git a/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/C.java b/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/C.java index a50a8d7c6b9..ca5bda9eb60 100644 --- a/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/C.java +++ b/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/C.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -36,6 +36,42 @@ public class C extends Parent { * @param p2 testing 1 2 3. * @param inheritBug {@inheritDoc} */ + @Override public void nonMatchingParams(int param1, int param2) {} + /** + * Overriding method with missing/additional/duplicate param tags in non-declaration order. + * + * @param x does not exist + * @param b a boolean + * @param i an int + * @param x duplicate + * @param b another duplicate + */ + @Override + public void unorderedParams(int i, double d, boolean b) {} + + /** + * Generic method with mixed/missing param tags. + * + * @param t1 param 1 + * @param type 2 + * @param t3 param 3 + */ + public static void genericMethod(T1 t1, T2 t2, T3 t3) {} + + /** + * A partially documented point. + * + * @param y the y coordinate + */ + public static record Point(int x, int y) {} + + /** + * Generic class with missing param tags. + * + * @param type 1 + */ + public static class Nested {} + } diff --git a/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/Parent.java b/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/Parent.java index f0b380b19bd..133c7038cb5 100644 --- a/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/Parent.java +++ b/test/langtools/jdk/javadoc/doclet/testParamTaglet/pkg/Parent.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,4 +29,11 @@ public class Parent { * Just a dummy method that is here for inheritDoc testing purposes. */ public void nonMatchingParams(int param1, int param2) {} + + /** + * Base method with partially documented params. + * + * @param d a double + */ + public void unorderedParams(int i, double d, boolean b) {} }