diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1133.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1133.json index 12f2dfe6e5f..3b76a02decb 100644 --- a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1133.json +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S1133.json @@ -24,8 +24,7 @@ 915 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnectionStatistics.java": [ -24, -30 +24 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletRequestHttpWrapper.java": [ 193 diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S1133.json b/its/ruling/src/test/resources/eclipse-jetty/java-S1133.json index 1452df97b15..fc553f0c9dd 100644 --- a/its/ruling/src/test/resources/eclipse-jetty/java-S1133.json +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S1133.json @@ -24,8 +24,7 @@ 915 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnectionStatistics.java": [ -24, -30 +24 ], "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServletRequestHttpWrapper.java": [ 193 diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/DeprecatedTagPresenceCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/DeprecatedTagPresenceCheckSample.java new file mode 100644 index 00000000000..528bdef1383 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/DeprecatedTagPresenceCheckSample.java @@ -0,0 +1,190 @@ +class Foo { + + @Deprecated(forRemoval = true) + public String getName; // Noncompliant + + @Deprecated(forRemoval = false) + public String getName; // Compliant + + @Deprecated + public int foo; // Noncompliant {{Do not forget to remove this deprecated code someday.}} +// ^^^ + + public void foo1() { // Compliant + } + + @Deprecated + public void foo2() { // Noncompliant + } + + /** + * @deprecated + */ + public void foo3() { // Noncompliant + + } + + /** + * @deprecated + */ + @Ignore + @Deprecated + public void foo4() { // Noncompliant +// ^^^^ + } + + @Deprecated + /** + * @deprecated + */ + public void foo5() { // Noncompliant + } + + /* + * @deprecated + */ + @Deprecated + public int foo7() { // Noncompliant + } + + /** + * + */ + @Deprecated + public void foo8() { // Noncompliant + } + + @java.lang.Deprecated // Compliant - no one does this + public void foo9() { + + @Deprecated + int local1 = 0; // Noncompliant + + } + +} + +interface Bar { + + @Deprecated + int foo(); // Noncompliant + +} + +// Test cases for SONARJAVA-6070: Legitimate deprecation documentation should NOT be flagged + +class LegitimateDeprecation { + + /** + * @deprecated Use the new DynEnum instead + */ + public void oldMethod1() { // Compliant + } + + /** + * @deprecated Will be removed in Tomcat 10. + */ + public int getPollerThreadCount() { // Compliant + return 1; + } + + /** + * @deprecated Please use {@link NewClass} instead + */ + public void oldMethod2() { // Compliant + } + + /** + * @deprecated Replaced by newMethod() + */ + public void oldMethod3() { // Compliant + } + + /** + * @deprecated Scheduled for removal in version 2.0 + */ + public void oldMethod4() { // Compliant + } + + /** + * @deprecated Use newApi() instead. + */ + public void oldMethod5() { // Compliant + } + + /** + * @deprecated See {@link NewApi#betterMethod} + */ + public void oldMethod6() { // Compliant + } + + /** + * @deprecated Prefer using modernMethod() + */ + public void oldMethod7() { // Compliant + } + + /** + * @deprecated Migrate to the new API + */ + public void oldMethod8() { // Compliant + } + + /** + * @deprecated Removed in version 3.0 + */ + public void oldMethod9() { // Compliant + } + + /** + * @deprecated To be removed in future releases + */ + public void oldMethod10() { // Compliant + } + + /** + * @deprecated deprecated since version 1.5, use newMethod() instead + */ + public void oldMethod11() { // Compliant + } + + /** + * @deprecated This is old and not useful + */ + public void oldMethod12() { // Noncompliant + } + + /** + * @deprecated + */ + public void oldMethod13() { // Noncompliant + } + + /** + * Some javadoc + * @deprecated This method is outdated + */ + public void oldMethod14() { // Noncompliant + } + + // Use multiline comments + + /** + * Returns the empty iterator. + * + * @deprecated Use + * newApi() instead. + * @since 1.0 + */ + public static void oldMethod15() { // Compliant when having new tag after comment + } + + /** + * Returns the empty iterator. + * + * @deprecated Use + * newApi() instead. + */ + public static void oldMethod16() { // Compliant when having endline after comment + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java index 72643151a99..41d0ac65cec 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java @@ -27,7 +27,7 @@ import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.deprecatedAnnotation; import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.getAnnotationAttributeValue; -import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTag; +import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTagWithoutLegitimateDocumentation; import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree; @Rule(key = "S1133") @@ -40,7 +40,7 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { - if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTag(tree)) { + if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTagWithoutLegitimateDocumentation(tree)) { reportIssue(reportTreeForDeprecatedTree(tree), "Do not forget to remove this deprecated code someday."); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java index 6f7133cac32..a05b6d54eb8 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java @@ -16,8 +16,14 @@ */ package org.sonar.java.checks.helpers; +import java.util.Locale; import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.CheckForNull; + +import org.sonar.java.annotations.VisibleForTesting; import org.sonar.java.ast.visitors.PublicApiChecker; import org.sonar.java.model.expression.AssignmentExpressionTreeImpl; import org.sonar.plugins.java.api.tree.AnnotationTree; @@ -33,15 +39,83 @@ public class DeprecatedCheckerHelper { + private static final String DEPRECATED_TAG = "@deprecated"; private static final Kind[] CLASS_KINDS = PublicApiChecker.classKinds(); private static final Kind[] METHOD_KINDS = PublicApiChecker.methodKinds(); + private static final Set MIGRATION_GUIDANCE_KEYWORDS = Set.of( + "replaced by", + "see ", + "prefer", + "migrate to", + "{@link" + ); + + private static final Set REMOVAL_TIMELINE_TERMS = Set.of( + "will be removed in", + "removed in version", + "to be removed in", + "scheduled for removal", + "deprecated since" + ); + + private static final Pattern DEPRECATED_TAG_CONTENT_PATTERN = Pattern.compile( + DEPRECATED_TAG + "(.*)(?:\\n\\s*\\*\\s*@|$)", + Pattern.DOTALL + ); + private DeprecatedCheckerHelper() { // Helper class, should not be implemented. } public static boolean hasJavadocDeprecatedTag(Tree tree) { - return PublicApiChecker.getApiJavadoc(tree).filter(comment -> comment.contains("@deprecated")).isPresent(); + return PublicApiChecker.getApiJavadoc(tree) + .filter(comment -> comment.contains(DEPRECATED_TAG)) + .isPresent(); + } + + public static boolean hasJavadocDeprecatedTagWithoutLegitimateDocumentation(Tree tree) { + return PublicApiChecker.getApiJavadoc(tree) + .filter(comment -> comment.contains(DEPRECATED_TAG)) + .filter(comment -> !hasLegitimateDeprecationDocumentation(comment)) + .isPresent(); + } + + @VisibleForTesting + static boolean hasLegitimateDeprecationDocumentation(String javadoc) { + String deprecatedSection = extractDeprecatedTagContent(javadoc); + if (deprecatedSection.isEmpty()) { + return false; + } + + // Check for migration guidance indicators or removal timeline + return hasMigrationGuidance(deprecatedSection) || hasRemovalTimeline(deprecatedSection); + } + + private static String extractDeprecatedTagContent(String javadoc) { + // Extract content from @deprecated (including linebreaks) until next javadoc tag or end + // Pattern: @deprecated followed by everything until (newline + whitespaces + * + whitespaces + @) or end + Matcher matcher = DEPRECATED_TAG_CONTENT_PATTERN.matcher(javadoc); + + if (!matcher.find()) { + return ""; + } + + String content = matcher.group(1); + // Clean up javadoc formatting: remove leading asterisks and extra whitespace from continuation lines + return content.replaceAll("(?m)^\\s*\\*\\s*", " ").trim(); + } + + private static boolean hasMigrationGuidance(String deprecatedContent) { + String lowerContent = deprecatedContent.toLowerCase(Locale.ROOT); + return (lowerContent.contains("use") && (lowerContent.contains("instead") || lowerContent.contains("new"))) + || MIGRATION_GUIDANCE_KEYWORDS.stream().anyMatch(lowerContent::contains); + } + + private static boolean hasRemovalTimeline(String deprecatedContent) { + String lowerContent = deprecatedContent.toLowerCase(Locale.ROOT); + return REMOVAL_TIMELINE_TERMS.stream() + .anyMatch(lowerContent::contains); } @CheckForNull diff --git a/java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java b/java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java deleted file mode 100644 index 78dffcf0383..00000000000 --- a/java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java +++ /dev/null @@ -1,72 +0,0 @@ -class Foo { - - @Deprecated(forRemoval = true) - public String getName; // Noncompliant - - @Deprecated(forRemoval = false) - public String getName; // Compliant - - @Deprecated - public int foo; // Noncompliant {{Do not forget to remove this deprecated code someday.}} -// ^^^ - - public void foo1() { // Compliant - } - - @Deprecated - public void foo2() { // Noncompliant - } - - /** - * @deprecated - */ - public void foo3() { // Noncompliant - - } - - /** - * @deprecated - */ - @Ignore - @Deprecated - public void foo4() { // Noncompliant -// ^^^^ - } - - @Deprecated - /** - * @deprecated - */ - public void foo5() { // Noncompliant - } - - /* - * @deprecated - */ - @Deprecated - public int foo7() { // Noncompliant - } - - /** - * - */ - @Deprecated - public void foo8() { // Noncompliant - } - - @java.lang.Deprecated // Compliant - no one does this - public void foo9() { - - @Deprecated - int local1 = 0; // Noncompliant - - } - -} - -interface Bar { - - @Deprecated - int foo(); // Noncompliant - -} diff --git a/java-checks/src/test/java/org/sonar/java/checks/DeprecatedTagPresenceCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/DeprecatedTagPresenceCheckTest.java index bbc2ab9612a..c77ca3bcec1 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/DeprecatedTagPresenceCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/DeprecatedTagPresenceCheckTest.java @@ -19,12 +19,14 @@ import org.junit.jupiter.api.Test; import org.sonar.java.checks.verifier.CheckVerifier; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + class DeprecatedTagPresenceCheckTest { @Test void test() { CheckVerifier.newVerifier() - .onFile("src/test/files/checks/DeprecatedTagPresenceCheck.java") + .onFile(nonCompilingTestSourcesPath("checks/DeprecatedTagPresenceCheckSample.java")) .withCheck(new DeprecatedTagPresenceCheck()) .verifyIssues(); } diff --git a/java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java b/java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java index add3c3f4254..634808be156 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java @@ -16,10 +16,15 @@ */ package org.sonar.java.checks.helpers; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.sonar.plugins.java.api.tree.ClassTree; import org.sonar.plugins.java.api.tree.CompilationUnitTree; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; class DeprecatedCheckerHelperTest { @@ -45,4 +50,205 @@ private ClassTree parseClass(String code) { return (ClassTree) compilationUnitTree.types().get(0); } + @ParameterizedTest(name = "[{index}] {1}") + @MethodSource("legitimateDeprecationTestCases") + void hasLegitimateDeprecationDocumentation(String javadoc, String description, boolean expectedResult) { + assertThat(DeprecatedCheckerHelper.hasLegitimateDeprecationDocumentation(javadoc)) + .as(description) + .isEqualTo(expectedResult); + } + + private static Stream legitimateDeprecationTestCases() { + return Stream.of( + // Migration guidance - should be legitimate (true) + Arguments.of( + """ + /** + * @deprecated Use the new DynEnum instead + */""", + "Migration guidance with 'use instead'", + true + ), + Arguments.of( + """ + /** + * @deprecated Please use {@link NewClass} instead + */""", + "Migration guidance with {@link}", + true + ), + Arguments.of( + """ + /** + * @deprecated Replaced by newMethod() + */""", + "Migration guidance with 'replaced by'", + true + ), + Arguments.of( + """ + /** + * @deprecated See {@link NewApi#betterMethod} + */""", + "Migration guidance with 'see'", + true + ), + Arguments.of( + """ + /** + * @deprecated Prefer using modernMethod() + */""", + "Migration guidance with 'prefer'", + true + ), + Arguments.of( + """ + /** + * @deprecated Migrate to the new API + */""", + "Migration guidance with 'migrate to'", + true + ), + Arguments.of( + """ + /** + * @deprecated Use newApi() instead. + */""", + "Migration guidance with 'use' and 'instead'", + true + ), + Arguments.of( + """ + /** + * @deprecated Use new implementation + */""", + "Migration guidance with 'use' and 'new'", + true + ), + + // Removal timeline - should be legitimate (true) + Arguments.of( + """ + /** + * @deprecated Will be removed in Tomcat 10. + */""", + "Removal timeline with 'will be removed in'", + true + ), + Arguments.of( + """ + /** + * @deprecated Scheduled for removal in version 2.0 + */""", + "Removal timeline with 'scheduled for removal'", + true + ), + Arguments.of( + """ + /** + * @deprecated Removed in version 3.0 + */""", + "Removal timeline with 'removed in version'", + true + ), + Arguments.of( + """ + /** + * @deprecated To be removed in future releases + */""", + "Removal timeline with 'to be removed in'", + true + ), + Arguments.of( + """ + /** + * @deprecated deprecated since version 1.5, use newMethod() instead + */""", + "Removal timeline with 'deprecated since' and migration guidance", + true + ), + + // Multiline with legitimate documentation - should be legitimate (true) + Arguments.of( + """ + /** + * @deprecated Use {@code ImmutableSet.of().iterator()} instead; or for + * Java 7 or later, {@link Collections#emptyIterator}. This method is + * scheduled for removal in May 2016. + */""", + "Multiline with migration guidance and removal timeline", + true + ), + Arguments.of( + """ + /** + * Some description + * @deprecated Use + * newApi() instead. + */""", + "Multiline with migration guidance spanning lines", + true + ), + + // Invalid/vague deprecation - should NOT be legitimate (false) + Arguments.of( + """ + /** + * @deprecated This is old and not useful + */""", + "Vague deprecation without guidance", + false + ), + Arguments.of( + """ + /** + * @deprecated + */""", + "Empty deprecation tag", + false + ), + Arguments.of( + """ + /** + * @deprecated This method is outdated + */""", + "Generic deprecation message", + false + ), + Arguments.of( + """ + /** + * @deprecated Do not use this + */""", + "No migration guidance or timeline", + false + ), + Arguments.of( + """ + /** + * @deprecated Bad method + */""", + "No actionable information", + false + ), + + // Edge cases + Arguments.of( + "/** @deprecated use something */", + "Lowercase 'use' without 'instead' or 'new'", + false + ), + Arguments.of( + "", + "Empty string", + false + ), + Arguments.of( + "/** No deprecated tag */", + "No @deprecated tag", + false + ) + ); + } + }