Skip to content

Commit 560700b

Browse files
SONARJAVA-5472 Add S8491: Detect dangling Javadoc comments (#5525)
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6dcc301 commit 560700b

16 files changed

Lines changed: 542 additions & 25 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S8491",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"com.google.guava:guava:src/com/google/common/util/concurrent/Monitor.java": [
3+
1017
4+
]
5+
}

java-checks-common/src/main/java/org/sonar/java/checks/helpers/TreeHelper.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,15 @@
1919
import java.util.IdentityHashMap;
2020
import java.util.Map;
2121
import java.util.Set;
22+
23+
import org.sonar.java.ast.visitors.PublicApiChecker;
2224
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
25+
import org.sonar.plugins.java.api.tree.ClassTree;
2326
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
2427
import org.sonar.plugins.java.api.tree.MethodTree;
2528
import org.sonar.plugins.java.api.tree.Tree;
29+
import org.sonar.plugins.java.api.tree.VariableTree;
30+
2631

2732
public class TreeHelper {
2833
private TreeHelper() {
@@ -48,6 +53,18 @@ public static Tree findClosestParentOfKind(Tree tree, Set<Tree.Kind> nodeKinds)
4853
return null;
4954
}
5055

56+
public static Tree reportTree(Tree tree) {
57+
Tree reportTree = tree;
58+
if (reportTree.is(PublicApiChecker.classKinds())) {
59+
reportTree = ExpressionsHelper.reportOnClassTree((ClassTree) reportTree);
60+
} else if (reportTree.is(PublicApiChecker.methodKinds())) {
61+
reportTree = ((MethodTree) reportTree).simpleName();
62+
} else if (reportTree.is(Tree.Kind.VARIABLE)) {
63+
reportTree = ((VariableTree) reportTree).simpleName();
64+
}
65+
return reportTree;
66+
}
67+
5168
private static class ReachableMethodsFinder extends BaseTreeVisitor {
5269
private final Map<MethodTree, Void> reachableMethods = new IdentityHashMap<>();
5370

java-checks-common/src/test/java/org/sonar/java/checks/helpers/TreeHelperTest.java

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,17 @@
1717
package org.sonar.java.checks.helpers;
1818

1919
import java.util.Set;
20+
import java.util.stream.Stream;
2021
import javax.annotation.Nullable;
2122
import org.junit.jupiter.api.Test;
23+
import org.junit.jupiter.params.ParameterizedTest;
24+
import org.junit.jupiter.params.provider.Arguments;
25+
import org.junit.jupiter.params.provider.MethodSource;
2226
import org.sonar.java.model.statement.BlockTreeImpl;
2327
import org.sonar.plugins.java.api.tree.BlockTree;
2428
import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
2529
import org.sonar.plugins.java.api.tree.ForEachStatement;
30+
import org.sonar.plugins.java.api.tree.IdentifierTree;
2631
import org.sonar.plugins.java.api.tree.IfStatementTree;
2732
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
2833
import org.sonar.plugins.java.api.tree.MethodTree;
@@ -95,4 +100,88 @@ private static Tree assertFoundNode(Tree tree, Set<Tree.Kind> kinds, @Nullable T
95100
}
96101
return actual;
97102
}
103+
104+
@ParameterizedTest(name = "[{index}] {1}")
105+
@MethodSource("reportTreeTestCases")
106+
void reportTree(String code, String description, TreeExtractor treeExtractor, @Nullable String expectedName) {
107+
Tree tree = treeExtractor.extract(code);
108+
Tree reportTree = TreeHelper.reportTree(tree);
109+
110+
if (expectedName != null) {
111+
assertThat(reportTree).isInstanceOf(IdentifierTree.class);
112+
assertThat(((IdentifierTree) reportTree).name()).isEqualTo(expectedName);
113+
} else {
114+
assertThat(reportTree).isSameAs(tree);
115+
}
116+
}
117+
118+
private static Stream<Arguments> reportTreeTestCases() {
119+
return Stream.of(
120+
Arguments.of(
121+
"class MyClass { }",
122+
"Class",
123+
(TreeExtractor) TreeHelperTest::classTree,
124+
"MyClass"
125+
),
126+
Arguments.of(
127+
"interface MyInterface { }",
128+
"Interface",
129+
(TreeExtractor) TreeHelperTest::classTree,
130+
"MyInterface"
131+
),
132+
Arguments.of(
133+
"enum MyEnum { }",
134+
"Enum",
135+
(TreeExtractor) TreeHelperTest::classTree,
136+
"MyEnum"
137+
),
138+
Arguments.of(
139+
"record MyRecord() { }",
140+
"Record",
141+
(TreeExtractor) TreeHelperTest::classTree,
142+
"MyRecord"
143+
),
144+
Arguments.of(
145+
"@interface MyAnnotation { }",
146+
"Annotation",
147+
(TreeExtractor) TreeHelperTest::classTree,
148+
"MyAnnotation"
149+
),
150+
Arguments.of(
151+
newCode("void myMethod() { }"),
152+
"Method",
153+
(TreeExtractor) TreeHelperTest::methodTree,
154+
"myMethod"
155+
),
156+
Arguments.of(
157+
"class A { A() { } }",
158+
"Constructor",
159+
(TreeExtractor) code -> classTree(code).members().get(0),
160+
"A"
161+
),
162+
Arguments.of(
163+
"class A { String myField; }",
164+
"Field",
165+
(TreeExtractor) code -> classTree(code).members().get(0),
166+
"myField"
167+
),
168+
Arguments.of(
169+
newCode("void method() { int localVar = 42; }"),
170+
"Local variable",
171+
(TreeExtractor) code -> methodTree(code).block().body().get(0),
172+
"localVar"
173+
),
174+
Arguments.of(
175+
newCode("void method() { if (true) { } }"),
176+
"Other tree (if statement)",
177+
(TreeExtractor) code -> methodTree(code).block().body().get(0),
178+
null
179+
)
180+
);
181+
}
182+
183+
@FunctionalInterface
184+
private interface TreeExtractor {
185+
Tree extract(String code);
186+
}
98187
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package checks;
2+
3+
class DanglingJavadocCheckSample {
4+
private String name;
5+
6+
/**
7+
* This is a dangling javadoc comment that will be ignored.
8+
*/
9+
/**
10+
* This is the actual javadoc that will be used.
11+
* @param name the name parameter
12+
*/
13+
public void setName(String name) { // Noncompliant {{Remove or merge the dangling Javadoc comment(s).}}
14+
// ^^^^^^^
15+
this.name = name;
16+
}
17+
18+
/**
19+
* Sets the name parameter.
20+
* @param name the name parameter
21+
*/
22+
public void setNameCompliant(String name) { // Compliant
23+
this.name = name;
24+
}
25+
26+
/**
27+
* First dangling comment
28+
*/
29+
/**
30+
* Second dangling comment
31+
*/
32+
/**
33+
* The actual comment for this field
34+
*/
35+
public String multipleJavadocs; // Noncompliant
36+
// ^^^^^^^^^^^^^^^^
37+
/**
38+
* Dangling method comment
39+
*/
40+
/**
41+
* Actual method comment
42+
* @return the value
43+
*/
44+
public int getValue() { // Noncompliant
45+
return 42;
46+
}
47+
}
48+
49+
/**
50+
* Old documentation that is no longer relevant.
51+
*/
52+
/**
53+
* Updated documentation for this class.
54+
*/
55+
class OldUser { // Noncompliant
56+
private String name;
57+
}
58+
59+
/**
60+
* This traditional javadoc will be ignored.
61+
*/
62+
/// This markdown javadoc will be used.
63+
/// Represents a customer entity.
64+
class Customer { // Noncompliant
65+
// ^^^^^^^^
66+
private String customerId;
67+
}
68+
69+
/**
70+
* Updated documentation for this class.
71+
*/
72+
class UserCompliant { // Compliant
73+
private String name;
74+
}
75+
76+
/// Represents a customer entity.
77+
class CustomerCompliant { // Compliant
78+
private String customerId;
79+
}
80+
81+
class NoJavadoc { // Compliant
82+
private String field;
83+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import java.util.Arrays;
20+
import java.util.List;
21+
import org.sonar.check.Rule;
22+
import org.sonar.java.ast.visitors.PublicApiChecker;
23+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
24+
import org.sonar.plugins.java.api.tree.Tree;
25+
26+
import static org.sonar.java.checks.helpers.TreeHelper.reportTree;
27+
28+
@Rule(key = "S8491")
29+
public class DanglingJavadocCheck extends IssuableSubscriptionVisitor {
30+
31+
@Override
32+
public List<Tree.Kind> nodesToVisit() {
33+
return Arrays.asList(PublicApiChecker.apiKinds());
34+
}
35+
36+
@Override
37+
public void visitNode(Tree tree) {
38+
int javadocCount = PublicApiChecker.getApiJavadocsCount(tree);
39+
if (javadocCount > 1) {
40+
reportIssue(reportTree(tree), "Remove or merge the dangling Javadoc comment(s).");
41+
}
42+
}
43+
44+
}

java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.getAnnotationAttributeValue;
3030
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTag;
3131
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTagWithoutLegitimateDocumentation;
32-
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree;
32+
import static org.sonar.java.checks.helpers.TreeHelper.reportTree;
3333

3434
@Rule(key = "S1133")
3535
public class DeprecatedTagPresenceCheck extends IssuableSubscriptionVisitor {
@@ -42,7 +42,7 @@ public List<Tree.Kind> nodesToVisit() {
4242
@Override
4343
public void visitNode(Tree tree) {
4444
if ((hasDeprecatedAnnotation(tree) && !hasJavadocDeprecatedTag(tree)) || hasJavadocDeprecatedTagWithoutLegitimateDocumentation(tree)) {
45-
reportIssue(reportTreeForDeprecatedTree(tree), "Do not forget to remove this deprecated code someday.");
45+
reportIssue(reportTree(tree), "Do not forget to remove this deprecated code someday.");
4646
}
4747
}
4848

@@ -52,12 +52,8 @@ private static boolean hasDeprecatedAnnotation(Tree tree) {
5252
return false;
5353
}
5454
Optional<Boolean> forRemovalValue = getAnnotationAttributeValue(annotation, "forRemoval", Boolean.class);
55-
// If forRemoval was not specified, we consider the deprecated code should be removed in the future.
56-
if (forRemovalValue.isEmpty()) {
57-
return true;
58-
}
59-
// Otherwise, we check the value of forRemoval and return that, so that only @Deprecated(forRemoval = true) is considered.
60-
return Boolean.TRUE.equals(forRemovalValue.get());
55+
// If forRemoval was not specified or equals true, we consider the deprecated code should be removed in the future.
56+
return forRemovalValue.map(Boolean.TRUE::equals).orElse(true);
6157
}
6258

6359
}

java-checks/src/main/java/org/sonar/java/checks/MissingDeprecatedCheck.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import org.sonar.plugins.java.api.tree.VariableTree;
2525
import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
2626

27-
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree;
27+
import static org.sonar.java.checks.helpers.TreeHelper.reportTree;
2828

2929
@DeprecatedRuleKey(ruleKey = "MissingDeprecatedCheck", repositoryKey = "squid")
3030
@Rule(key = "S1123")
@@ -39,10 +39,10 @@ void handleDeprecatedElement(Tree tree, @CheckForNull AnnotationTree deprecatedA
3939
boolean hasDeprecatedAnnotation = deprecatedAnnotation != null;
4040
if (hasDeprecatedAnnotation) {
4141
if (!hasJavadocDeprecatedTag) {
42-
reportIssue(reportTreeForDeprecatedTree(tree), "Add the missing @deprecated Javadoc tag.");
42+
reportIssue(reportTree(tree), "Add the missing @deprecated Javadoc tag.");
4343
}
4444
} else if (hasJavadocDeprecatedTag) {
45-
reportIssue(reportTreeForDeprecatedTree(tree), "Add the missing @Deprecated annotation.");
45+
reportIssue(reportTree(tree), "Add the missing @Deprecated annotation.");
4646
}
4747
}
4848

java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -147,18 +147,6 @@ private static AnnotationTree deprecatedAnnotation(Iterable<AnnotationTree> anno
147147
return null;
148148
}
149149

150-
public static Tree reportTreeForDeprecatedTree(Tree tree) {
151-
Tree reportTree = tree;
152-
if (reportTree.is(PublicApiChecker.classKinds())) {
153-
reportTree = ExpressionsHelper.reportOnClassTree((ClassTree) reportTree);
154-
} else if (reportTree.is(PublicApiChecker.methodKinds())) {
155-
reportTree = ((MethodTree) reportTree).simpleName();
156-
} else if (reportTree.is(Tree.Kind.VARIABLE)) {
157-
reportTree = ((VariableTree) reportTree).simpleName();
158-
}
159-
return reportTree;
160-
}
161-
162150
private static boolean isDeprecated(AnnotationTree tree) {
163151
return tree.annotationType().is(Kind.IDENTIFIER) &&
164152
"Deprecated".equals(((IdentifierTree) tree.annotationType()).name());
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
23+
24+
class DanglingJavadocCheckTest {
25+
26+
@Test
27+
void test() {
28+
CheckVerifier.newVerifier()
29+
.onFile(mainCodeSourcesPath("checks/DanglingJavadocCheckSample.java"))
30+
.withCheck(new DanglingJavadocCheck())
31+
.verifyIssues();
32+
}
33+
34+
}

0 commit comments

Comments
 (0)