From a173d9593ba1a415ef949d1e2e11fdc686b3a251 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Thu, 9 Sep 2021 00:11:02 +0200 Subject: [PATCH 1/2] Java: Detect spurious param Javadoc tag of generic classes --- .../Documentation/SpuriousJavadocParam.java | 6 ++++ .../Documentation/SpuriousJavadocParam.qhelp | 6 ++-- .../Documentation/SpuriousJavadocParam.ql | 33 +++++++++++++------ .../SpuriousJavadocParam/Test.java | 14 ++++++++ .../SpuriousJavadocParam/test.expected | 4 +++ 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.java b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.java index aee5e648c429..20513b19a9cb 100644 --- a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.java +++ b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.java @@ -39,6 +39,12 @@ public void html(int ordinate){ ... } */ public void parameterized(T parameter){ ... } +/** + * BAD: The following param tag refers to a non-existent type parameter. + * + * @param The type of the elements. + */ +class Generic { ...} /** * GOOD: A proper Javadoc comment. diff --git a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.qhelp b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.qhelp index 7adc5368fac1..367f03848413 100644 --- a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.qhelp +++ b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.qhelp @@ -6,15 +6,15 @@

-Javadoc comments for public methods and constructors should use the @param tag to describe the available -parameters. If the comment includes any empty, incorrect or outdated parameter names then this will make +Javadoc comments for public methods, constructors and generic classes should use the @param tag to describe the available +parameters and type parameters. If the comment includes any empty, incorrect or outdated parameter names then this will make the documentation more difficult to read.

-

The Javadoc comment for a method or constructor should always use non-empty @param values that match actual parameter or type parameter names.

+

The Javadoc comment for a method, constructor or generic class should always use non-empty @param values that match actual parameter or type parameter names.

diff --git a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.ql b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.ql index 88c100b86931..691811687a79 100644 --- a/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.ql +++ b/java/ql/src/Advisory/Documentation/SpuriousJavadocParam.ql @@ -1,6 +1,7 @@ /** * @name Spurious Javadoc @param tags - * @description Javadoc @param tags that do not match any parameters in the method or constructor are confusing. + * @description Javadoc @param tags that do not match any parameters in the method or constructor or + * any type parameters of the annotated class are confusing. * @kind problem * @problem.severity recommendation * @precision very-high @@ -10,21 +11,33 @@ import java -from Callable callable, ParamTag paramTag, string what, string msg +from Documentable documentable, ParamTag paramTag, string msg where - callable.(Documentable).getJavadoc().getAChild() = paramTag and - (if callable instanceof Constructor then what = "constructor" else what = "method") and + documentable.getJavadoc().getAChild() = paramTag and if exists(paramTag.getParamName()) then - // The tag's value is neither matched by a callable parameter name ... - not callable.getAParameter().getName() = paramTag.getParamName() and - // ... nor by a type parameter name. - not exists(TypeVariable tv | tv.getGenericCallable() = callable | + documentable instanceof Callable and + exists(string what | + if documentable instanceof Constructor then what = "constructor" else what = "method" + | + // The tag's value is neither matched by a callable parameter name ... + not documentable.(Callable).getAParameter().getName() = paramTag.getParamName() and + // ... nor by a type parameter name. + not exists(TypeVariable tv | tv.getGenericCallable() = documentable | + "<" + tv.getName() + ">" = paramTag.getParamName() + ) and + msg = + "@param tag \"" + paramTag.getParamName() + "\" does not match any actual parameter of " + + what + " \"" + documentable.getName() + "()\"." + ) + or + documentable instanceof ClassOrInterface and + not exists(TypeVariable tv | tv.getGenericType() = documentable | "<" + tv.getName() + ">" = paramTag.getParamName() ) and msg = - "@param tag \"" + paramTag.getParamName() + "\" does not match any actual parameter of " + - what + " \"" + callable.getName() + "()\"." + "@param tag \"" + paramTag.getParamName() + + "\" does not match any actual type parameter of type \"" + documentable.getName() + "\"." else // The tag has no value at all. msg = "This @param tag does not have a value." diff --git a/java/ql/test/query-tests/SpuriousJavadocParam/Test.java b/java/ql/test/query-tests/SpuriousJavadocParam/Test.java index 3c3c8ca7d129..841e034c07a6 100644 --- a/java/ql/test/query-tests/SpuriousJavadocParam/Test.java +++ b/java/ql/test/query-tests/SpuriousJavadocParam/Test.java @@ -105,4 +105,18 @@ class SomeClass { */ SomeClass(int i, int j) {} } + + /** + * @param exists + * @param T wrong syntax + * @param does not exist + */ + class GenericClass {} + + /** + * @param exists + * @param T wrong syntax + * @param does not exist + */ + interface GenericInterface {} } diff --git a/java/ql/test/query-tests/SpuriousJavadocParam/test.expected b/java/ql/test/query-tests/SpuriousJavadocParam/test.expected index bb0b241a0768..42bdb93e60c7 100644 --- a/java/ql/test/query-tests/SpuriousJavadocParam/test.expected +++ b/java/ql/test/query-tests/SpuriousJavadocParam/test.expected @@ -8,3 +8,7 @@ | Test.java:91:6:91:12 | @param | This @param tag does not have a value. | | Test.java:97:6:97:12 | @param | This @param tag does not have a value. | | Test.java:104:8:104:14 | @param | @param tag "k" does not match any actual parameter of constructor "SomeClass()". | +| Test.java:111:6:111:12 | @param | @param tag "T" does not match any actual type parameter of type "GenericClass". | +| Test.java:112:6:112:12 | @param | @param tag "" does not match any actual type parameter of type "GenericClass". | +| Test.java:118:6:118:12 | @param | @param tag "T" does not match any actual type parameter of type "GenericInterface". | +| Test.java:119:6:119:12 | @param | @param tag "" does not match any actual type parameter of type "GenericInterface". | From 3c7b39f089ec520b27c598e4f3d6b92a4e68e101 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 13 Sep 2021 15:36:26 +0100 Subject: [PATCH 2/2] Add change note --- java/change-notes/2021-09-13-javadoc-type-parameters.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 java/change-notes/2021-09-13-javadoc-type-parameters.md diff --git a/java/change-notes/2021-09-13-javadoc-type-parameters.md b/java/change-notes/2021-09-13-javadoc-type-parameters.md new file mode 100644 index 000000000000..378228e7c434 --- /dev/null +++ b/java/change-notes/2021-09-13-javadoc-type-parameters.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* The query `java/unknown-javadoc-parameter` now raises alerts for type parameters of generic classes that are documented but don't exist (perhaps due to using the wrong syntax, e.g. `@param T` instead of `@param `).