SONARJAVA-6070 S1133: Fix FP for deprecated code with documented migration paths (#5529)

Author: asya-vorobeva

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

@@ -10,7 +10,6 @@
 398
 ],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/OptionalSslConnectionFactory.java": [
-37,
 93
 ],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Request.java": [
@@ -48,25 +47,13 @@
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java": [
 187
 ],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java": [
-174,
-186,
-198,
-210
-],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java": [
-500
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java": [
 599,
 667,
 717,
 752,
 765
 ],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java": [
-132
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java": [
 140,
 144,

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

@@ -10,7 +10,6 @@
 398
 ],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/OptionalSslConnectionFactory.java": [
-37,
 93
 ],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/Request.java": [
@@ -48,25 +47,13 @@
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java": [
 187
 ],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessHandler.java": [
-174,
-186,
-198,
-210
-],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java": [
-500
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java": [
 599,
 667,
 717,
 752,
 765
 ],
-"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java": [
-132
-],
 "org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java": [
 140,
 144,

its/ruling/src/test/resources/guava/java-S1133.json

@@ -1,20 +1,4 @@
 {
-"com.google.guava:guava:src/com/google/common/base/CharMatcher.java": [
-925
-],
-"com.google.guava:guava:src/com/google/common/base/Converter.java": [
-366
-],
-"com.google.guava:guava:src/com/google/common/base/Objects.java": [
-131,
-149,
-165,
-190,
-203
-],
-"com.google.guava:guava:src/com/google/common/cache/LoadingCache.java": [
-132
-],
 "com.google.guava:guava:src/com/google/common/collect/ArrayTable.java": [
 375,
 489
@@ -133,16 +117,9 @@
 80
 ],
 "com.google.guava:guava:src/com/google/common/collect/Iterators.java": [
-117,
 189,
 1252
 ],
-"com.google.guava:guava:src/com/google/common/collect/MapConstraint.java": [
-56
-],
-"com.google.guava:guava:src/com/google/common/collect/MapConstraints.java": [
-53
-],
 "com.google.guava:guava:src/com/google/common/collect/MapMaker.java": [
 205,
 332,
@@ -162,51 +139,18 @@
 "com.google.guava:guava:src/com/google/common/collect/Ordering.java": [
 176
 ],
-"com.google.guava:guava:src/com/google/common/collect/Range.java": [
-447
-],
-"com.google.guava:guava:src/com/google/common/collect/Sets.java": [
-518
-],
 "com.google.guava:guava:src/com/google/common/collect/UnmodifiableIterator.java": [
 46
 ],
 "com.google.guava:guava:src/com/google/common/collect/UnmodifiableListIterator.java": [
 42,
 52
 ],
-"com.google.guava:guava:src/com/google/common/hash/BloomFilter.java": [
-150
-],
 "com.google.guava:guava:src/com/google/common/io/ByteArrayDataOutput.java": [
 48
 ],
-"com.google.guava:guava:src/com/google/common/io/InputSupplier.java": [
-34
-],
-"com.google.guava:guava:src/com/google/common/io/LittleEndianDataOutputStream.java": [
-74
-],
-"com.google.guava:guava:src/com/google/common/io/OutputSupplier.java": [
-34
-],
-"com.google.guava:guava:src/com/google/common/reflect/TypeToken.java": [
-415,
-428
-],
-"com.google.guava:guava:src/com/google/common/util/concurrent/FutureFallback.java": [
-41
-],
 "com.google.guava:guava:src/com/google/common/util/concurrent/Futures.java": [
-427,
-498,
 734,
-1007,
-1057,
-1824,
-1882
-],
-"com.google.guava:guava:src/com/google/common/util/concurrent/MoreExecutors.java": [
-282
+1824
 ]
 }

its/ruling/src/test/resources/sonar-server/java-S1133.json

@@ -1,7 +1,4 @@
 {
-"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/component/DefaultRubyComponentService.java": [
-54
-],
 "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/index/IssueDoc.java": [
 141,
 168,
@@ -21,9 +18,5 @@
 ],
 "org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/ui/PageDecorations.java": [
 33
-],
-"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/user/UserSession.java": [
-119,
-143
 ]
 }

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

@@ -95,75 +95,84 @@ public void oldMethod2() { // Compliant
   }
 
   /**
-   * @deprecated Replaced by newMethod()
+   * @deprecated Scheduled for removal in version 2.0
    */
   public void oldMethod3() { // Compliant
   }
 
   /**
-   * @deprecated Scheduled for removal in version 2.0
+   * @deprecated Use newApi() instead.
    */
   public void oldMethod4() { // Compliant
   }
 
   /**
-   * @deprecated Use newApi() instead.
+   * @deprecated See {@link NewApi#betterMethod}
    */
-  public void oldMethod5() { // Compliant
+  public void oldMethod5() { // Noncompliant
   }
 
   /**
-   * @deprecated See {@link NewApi#betterMethod}
+   * @deprecated Prefer using modernMethod()
    */
-  public void oldMethod6() { // Compliant
+  public void oldMethod6() { // Noncompliant
   }
 
   /**
-   * @deprecated Prefer using modernMethod()
+   * @deprecated Migrate to the new API
    */
-  public void oldMethod7() { // Compliant
+  public void oldMethod7() { // Noncompliant
   }
 
   /**
-   * @deprecated Migrate to the new API
+   * @deprecated Removed in version 3.0
    */
   public void oldMethod8() { // Compliant
   }
 
   /**
-   * @deprecated Removed in version 3.0
+   * @deprecated To be removed in future releases
    */
   public void oldMethod9() { // Compliant
   }
 
   /**
    * @deprecated To be removed in future releases
    */
-  public void oldMethod10() { // Compliant
+  @Deprecated
+  public void methodWithDeprecatedAnnotationAndTag() { // Compliant
   }
 
   /**
    * @deprecated deprecated since version 1.5, use newMethod() instead
    */
-  public void oldMethod11() { // Compliant
+  public void oldMethod10() { // Compliant
   }
 
   /**
    * @deprecated This is old and not useful
    */
-  public void oldMethod12() { // Noncompliant
+  public void oldMethod11() { // Noncompliant
   }
 
   /**
    * @deprecated
    */
+  public void oldMethod12() { // Noncompliant
+  }
+
+  /**
+   * Some javadoc
+   * @deprecated This method is outdated
+   */
   public void oldMethod13() { // Noncompliant
   }
 
   /**
    * Some javadoc
    * @deprecated This method is outdated
    */
+  @Deprecated
   public void oldMethod14() { // Noncompliant
   }
 

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

@@ -27,6 +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;
 
@@ -40,7 +41,7 @@ public List<Tree.Kind> nodesToVisit() {
 
   @Override
   public void visitNode(Tree tree) {
-    if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTagWithoutLegitimateDocumentation(tree)) {
+    if ((hasDeprecatedAnnotation(tree) && !hasJavadocDeprecatedTag(tree)) || hasJavadocDeprecatedTagWithoutLegitimateDocumentation(tree)) {
       reportIssue(reportTreeForDeprecatedTree(tree), "Do not forget to remove this deprecated code someday.");
     }
   }

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

@@ -43,14 +43,6 @@ public class DeprecatedCheckerHelper {
   private static final Kind[] CLASS_KINDS = PublicApiChecker.classKinds();
   private static final Kind[] METHOD_KINDS = PublicApiChecker.methodKinds();
 
-  private static final Set<String> MIGRATION_GUIDANCE_KEYWORDS = Set.of(
-    "replaced by",
-    "see ",
-    "prefer",
-    "migrate to",
-    "{@link"
-  );
-
   private static final Set<String> REMOVAL_TIMELINE_TERMS = Set.of(
     "will be removed in",
     "removed in version",
@@ -108,8 +100,7 @@ private static String extractDeprecatedTagContent(String javadoc) {
 
   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);
+    return lowerContent.contains("use") && (lowerContent.contains("instead") || lowerContent.contains("new"));
   }
 
   private static boolean hasRemovalTimeline(String deprecatedContent) {

java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java

@@ -60,7 +60,7 @@ void hasLegitimateDeprecationDocumentation(String javadoc, String description, b
 
   private static Stream<Arguments> legitimateDeprecationTestCases() {
     return Stream.of(
-      // Migration guidance - should be legitimate (true)
+      // Migration guidance should be legitimate (true)
       Arguments.of(
         """
         /**
@@ -77,38 +77,6 @@ private static Stream<Arguments> legitimateDeprecationTestCases() {
         "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(
         """
         /**