Skip to content

Commit 9d666d2

Browse files
asya-vorobevaclaude
asya-vorobeva
and
claude
authored
SONARJAVA-6070 S1133: Fix FP for deprecated code with documented migration paths (#5522)
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 3a4244e commit 9d666d2

File tree

8 files changed

+478
-80
lines changed

8 files changed

+478
-80
lines changed

‎its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1133.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
915
2525
],
2626
"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnectionStatistics.java": [
27-
24,
28-
30
27+
24
2928
],
3029
"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletRequestHttpWrapper.java": [
3130
193

‎its/ruling/src/test/resources/eclipse-jetty/java-S1133.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
915
2525
],
2626
"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnectionStatistics.java": [
27-
24,
28-
30
27+
24
2928
],
3029
"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletRequestHttpWrapper.java": [
3130
193

‎java-checks-test-sources/default/src/main/files/non-compiling/checks/DeprecatedTagPresenceCheckSample.java‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
class Foo {
2+
3+
@Deprecated(forRemoval = true)
4+
public String getName; // Noncompliant
5+
6+
@Deprecated(forRemoval = false)
7+
public String getName; // Compliant
8+
9+
@Deprecated
10+
public int foo; // Noncompliant {{Do not forget to remove this deprecated code someday.}}
11+
// ^^^
12+
13+
public void foo1() { // Compliant
14+
}
15+
16+
@Deprecated
17+
public void foo2() { // Noncompliant
18+
}
19+
20+
/**
21+
* @deprecated
22+
*/
23+
public void foo3() { // Noncompliant
24+
25+
}
26+
27+
/**
28+
* @deprecated
29+
*/
30+
@Ignore
31+
@Deprecated
32+
public void foo4() { // Noncompliant
33+
// ^^^^
34+
}
35+
36+
@Deprecated
37+
/**
38+
* @deprecated
39+
*/
40+
public void foo5() { // Noncompliant
41+
}
42+
43+
/*
44+
* @deprecated
45+
*/
46+
@Deprecated
47+
public int foo7() { // Noncompliant
48+
}
49+
50+
/**
51+
*
52+
*/
53+
@Deprecated
54+
public void foo8() { // Noncompliant
55+
}
56+
57+
@java.lang.Deprecated // Compliant - no one does this
58+
public void foo9() {
59+
60+
@Deprecated
61+
int local1 = 0; // Noncompliant
62+
63+
}
64+
65+
}
66+
67+
interface Bar {
68+
69+
@Deprecated
70+
int foo(); // Noncompliant
71+
72+
}
73+
74+
// Test cases for SONARJAVA-6070: Legitimate deprecation documentation should NOT be flagged
75+
76+
class LegitimateDeprecation {
77+
78+
/**
79+
* @deprecated Use the new DynEnum instead
80+
*/
81+
public void oldMethod1() { // Compliant
82+
}
83+
84+
/**
85+
* @deprecated Will be removed in Tomcat 10.
86+
*/
87+
public int getPollerThreadCount() { // Compliant
88+
return 1;
89+
}
90+
91+
/**
92+
* @deprecated Please use {@link NewClass} instead
93+
*/
94+
public void oldMethod2() { // Compliant
95+
}
96+
97+
/**
98+
* @deprecated Replaced by newMethod()
99+
*/
100+
public void oldMethod3() { // Compliant
101+
}
102+
103+
/**
104+
* @deprecated Scheduled for removal in version 2.0
105+
*/
106+
public void oldMethod4() { // Compliant
107+
}
108+
109+
/**
110+
* @deprecated Use newApi() instead.
111+
*/
112+
public void oldMethod5() { // Compliant
113+
}
114+
115+
/**
116+
* @deprecated See {@link NewApi#betterMethod}
117+
*/
118+
public void oldMethod6() { // Compliant
119+
}
120+
121+
/**
122+
* @deprecated Prefer using modernMethod()
123+
*/
124+
public void oldMethod7() { // Compliant
125+
}
126+
127+
/**
128+
* @deprecated Migrate to the new API
129+
*/
130+
public void oldMethod8() { // Compliant
131+
}
132+
133+
/**
134+
* @deprecated Removed in version 3.0
135+
*/
136+
public void oldMethod9() { // Compliant
137+
}
138+
139+
/**
140+
* @deprecated To be removed in future releases
141+
*/
142+
public void oldMethod10() { // Compliant
143+
}
144+
145+
/**
146+
* @deprecated deprecated since version 1.5, use newMethod() instead
147+
*/
148+
public void oldMethod11() { // Compliant
149+
}
150+
151+
/**
152+
* @deprecated This is old and not useful
153+
*/
154+
public void oldMethod12() { // Noncompliant
155+
}
156+
157+
/**
158+
* @deprecated
159+
*/
160+
public void oldMethod13() { // Noncompliant
161+
}
162+
163+
/**
164+
* Some javadoc
165+
* @deprecated This method is outdated
166+
*/
167+
public void oldMethod14() { // Noncompliant
168+
}
169+
170+
// Use multiline comments
171+
172+
/**
173+
* Returns the empty iterator.
174+
*
175+
* @deprecated Use
176+
* newApi() instead.
177+
* @since 1.0
178+
*/
179+
public static void oldMethod15() { // Compliant when having new tag after comment
180+
}
181+
182+
/**
183+
* Returns the empty iterator.
184+
*
185+
* @deprecated Use
186+
* newApi() instead.
187+
*/
188+
public static void oldMethod16() { // Compliant when having endline after comment
189+
}
190+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.deprecatedAnnotation;
2929
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.getAnnotationAttributeValue;
30-
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTag;
30+
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTagWithoutLegitimateDocumentation;
3131
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree;
3232

3333
@Rule(key = "S1133")
@@ -40,7 +40,7 @@ public List<Tree.Kind> nodesToVisit() {
4040

4141
@Override
4242
public void visitNode(Tree tree) {
43-
if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTag(tree)) {
43+
if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTagWithoutLegitimateDocumentation(tree)) {
4444
reportIssue(reportTreeForDeprecatedTree(tree), "Do not forget to remove this deprecated code someday.");
4545
}
4646
}

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

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,14 @@
1616
*/
1717
package org.sonar.java.checks.helpers;
1818

19+
import java.util.Locale;
1920
import java.util.Optional;
21+
import java.util.Set;
22+
import java.util.regex.Matcher;
23+
import java.util.regex.Pattern;
2024
import javax.annotation.CheckForNull;
25+
26+
import org.sonar.java.annotations.VisibleForTesting;
2127
import org.sonar.java.ast.visitors.PublicApiChecker;
2228
import org.sonar.java.model.expression.AssignmentExpressionTreeImpl;
2329
import org.sonar.plugins.java.api.tree.AnnotationTree;
@@ -33,15 +39,83 @@
3339

3440
public class DeprecatedCheckerHelper {
3541

42+
private static final String DEPRECATED_TAG = "@deprecated";
3643
private static final Kind[] CLASS_KINDS = PublicApiChecker.classKinds();
3744
private static final Kind[] METHOD_KINDS = PublicApiChecker.methodKinds();
3845

46+
private static final Set<String> MIGRATION_GUIDANCE_KEYWORDS = Set.of(
47+
"replaced by",
48+
"see ",
49+
"prefer",
50+
"migrate to",
51+
"{@link"
52+
);
53+
54+
private static final Set<String> REMOVAL_TIMELINE_TERMS = Set.of(
55+
"will be removed in",
56+
"removed in version",
57+
"to be removed in",
58+
"scheduled for removal",
59+
"deprecated since"
60+
);
61+
62+
private static final Pattern DEPRECATED_TAG_CONTENT_PATTERN = Pattern.compile(
63+
DEPRECATED_TAG + "(.*)(?:\\n\\s*\\*\\s*@|$)",
64+
Pattern.DOTALL
65+
);
66+
3967
private DeprecatedCheckerHelper() {
4068
// Helper class, should not be implemented.
4169
}
4270

4371
public static boolean hasJavadocDeprecatedTag(Tree tree) {
44-
return PublicApiChecker.getApiJavadoc(tree).filter(comment -> comment.contains("@deprecated")).isPresent();
72+
return PublicApiChecker.getApiJavadoc(tree)
73+
.filter(comment -> comment.contains(DEPRECATED_TAG))
74+
.isPresent();
75+
}
76+
77+
public static boolean hasJavadocDeprecatedTagWithoutLegitimateDocumentation(Tree tree) {
78+
return PublicApiChecker.getApiJavadoc(tree)
79+
.filter(comment -> comment.contains(DEPRECATED_TAG))
80+
.filter(comment -> !hasLegitimateDeprecationDocumentation(comment))
81+
.isPresent();
82+
}
83+
84+
@VisibleForTesting
85+
static boolean hasLegitimateDeprecationDocumentation(String javadoc) {
86+
String deprecatedSection = extractDeprecatedTagContent(javadoc);
87+
if (deprecatedSection.isEmpty()) {
88+
return false;
89+
}
90+
91+
// Check for migration guidance indicators or removal timeline
92+
return hasMigrationGuidance(deprecatedSection) || hasRemovalTimeline(deprecatedSection);
93+
}
94+
95+
private static String extractDeprecatedTagContent(String javadoc) {
96+
// Extract content from @deprecated (including linebreaks) until next javadoc tag or end
97+
// Pattern: @deprecated followed by everything until (newline + whitespaces + * + whitespaces + @) or end
98+
Matcher matcher = DEPRECATED_TAG_CONTENT_PATTERN.matcher(javadoc);
99+
100+
if (!matcher.find()) {
101+
return "";
102+
}
103+
104+
String content = matcher.group(1);
105+
// Clean up javadoc formatting: remove leading asterisks and extra whitespace from continuation lines
106+
return content.replaceAll("(?m)^\\s*\\*\\s*", " ").trim();
107+
}
108+
109+
private static boolean hasMigrationGuidance(String deprecatedContent) {
110+
String lowerContent = deprecatedContent.toLowerCase(Locale.ROOT);
111+
return (lowerContent.contains("use") && (lowerContent.contains("instead") || lowerContent.contains("new")))
112+
|| MIGRATION_GUIDANCE_KEYWORDS.stream().anyMatch(lowerContent::contains);
113+
}
114+
115+
private static boolean hasRemovalTimeline(String deprecatedContent) {
116+
String lowerContent = deprecatedContent.toLowerCase(Locale.ROOT);
117+
return REMOVAL_TIMELINE_TERMS.stream()
118+
.anyMatch(lowerContent::contains);
45119
}
46120

47121
@CheckForNull

0 commit comments

Comments
 (0)