SONARJAVA-4107 Filter out SpringData method names from S100 (#4407)

its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java

@@ -178,7 +178,7 @@ public void javaCheckTestSources() throws Exception {
 
     assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs);
     assertThat(newTotal).isEqualTo(knownTotal);
-    assertThat(rulesCausingFPs).hasSize(4);
+    assertThat(rulesCausingFPs).hasSize(5);
     assertThat(rulesNotReporting).hasSize(7);
     assertThat(rulesSilenced).hasSize(69);
 
@@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception {
      * No differences would mean that we find the same issues with and without the bytecode and libraries
      */
     String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences"));
-    assertThat(differences).isEqualTo("Issues differences: 3266");
+    assertThat(differences).isEqualTo("Issues differences: 3268");
   }
 
   private static Path pathFor(String path) {

its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json

@@ -3,7 +3,7 @@
     "ruleKey": "S100",
     "hasTruePositives": true,
     "falseNegatives": 0,
-    "falsePositives": 0
+    "falsePositives": 2
   },
   {
     "ruleKey": "S101",

java-checks-test-sources/src/main/files/non-compiling/filters/SpringFilter.java

@@ -1,7 +1,21 @@
 package filters;
 
+import java.util.List;
+
 public class SpringFilter {
 
+  static class S100 implements Repository<Person, Long> {
+    static class S100_1  {
+      // FP in case of missing semantic - 'Repository' is not resolved
+      List<Person> findByAddress_ZipCode(ZipCode zipCode) { // WithIssue
+        return List.of();
+      }
+    }
+
+    interface Person {}
+    interface ZipCode {}
+  }
+
   static class S1185 {
     abstract static class S1185_1 {
       void bar() {}

java-checks-test-sources/src/main/java/filters/SpringFilter.java

@@ -1,9 +1,11 @@
 package filters;
 
+import java.util.List;
 import javax.servlet.http.HttpServlet;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.data.mongodb.repository.Query;
 import org.springframework.stereotype.Component;
 import org.springframework.stereotype.Repository;
 import org.springframework.stereotype.Service;
@@ -17,6 +19,39 @@
 
 public class SpringFilter {
 
+  static class S100 {
+    static class S100_1 implements org.springframework.data.repository.Repository<Person, Long> {
+      List<Person> findByAddress_ZipCode(ZipCode zipCode) { // NoIssue
+        return List.of();
+      }
+
+      public void invalidName_() {} // WithIssue
+      public void _invalidName() {} // WithIssue
+    }
+
+    static class S100_2 {
+      List<Person> findByAddress_ZipCode(ZipCode zipCode) { // WithIssue
+        return List.of();
+      }
+    }
+
+    static class S100_3 implements PersonRepository {
+      @Override
+      public List<Person> findByAddress_ZipCode(Address address, ZipCode zipCode) {
+        return List.of();
+      }
+    }
+
+    interface PersonRepository extends org.springframework.data.repository.Repository<Person, Long> {
+      @Query("select p from Person where p.address=?1 and p.zipcode=?2")
+      List<Person> findByAddress_ZipCode(Address address, ZipCode zipCode); // NoIssue
+    }
+
+    interface Address {}
+    interface Person {}
+    interface ZipCode {}
+  }
+
   static class S107 {
 
     S107(String p1, String p2, String p3, String p4) {} // NoIssue

java-checks/src/main/java/org/sonar/java/filters/SpringFilter.java

@@ -26,8 +26,9 @@
 import org.sonar.java.checks.ServletInstanceFieldCheck;
 import org.sonar.java.checks.TooManyParametersCheck;
 import org.sonar.java.checks.helpers.ExpressionsHelper;
+import org.sonar.java.checks.naming.BadMethodNameCheck;
 import org.sonar.plugins.java.api.JavaCheck;
-import org.sonar.plugins.java.api.semantic.Symbol.MethodSymbol;
+import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
@@ -60,6 +61,7 @@ public class SpringFilter extends BaseTreeVisitorIssueFilter {
   @Override
   public Set<Class<? extends JavaCheck>> filteredRules() {
     return Set.of(
+      /* S100_ */ BadMethodNameCheck.class,
       /* S107_ */ TooManyParametersCheck.class,
       /* S1185 */ MethodOnlyCallsSuperCheck.class,
       /* S1258 */ AtLeastOneConstructorCheck.class,
@@ -81,17 +83,36 @@ public void visitVariable(VariableTree tree) {
 
   @Override
   public void visitMethod(MethodTree tree) {
-    MethodSymbol symbol = tree.symbol();
+    Symbol.MethodSymbol symbol = tree.symbol();
+    Tree reportTree = tree.simpleName();
     if (tree.is(Tree.Kind.CONSTRUCTOR)) {
       SymbolMetadata ownerMetadata = symbol.owner().metadata();
-      excludeLinesIfTrue(S107_CLASS_ANNOTATION_EXCEPTIONS.stream().anyMatch(ownerMetadata::isAnnotatedWith), tree.simpleName(), TooManyParametersCheck.class);
+      excludeLinesIfTrue(S107_CLASS_ANNOTATION_EXCEPTIONS.stream().anyMatch(ownerMetadata::isAnnotatedWith), reportTree, TooManyParametersCheck.class);
     } else {
       SymbolMetadata methodMetadata = symbol.metadata();
-      excludeLinesIfTrue(S107_METHOD_ANNOTATION_EXCEPTIONS.stream().anyMatch(methodMetadata::isAnnotatedWith), tree.simpleName(), TooManyParametersCheck.class);
+      excludeLinesIfTrue(S107_METHOD_ANNOTATION_EXCEPTIONS.stream().anyMatch(methodMetadata::isAnnotatedWith), reportTree, TooManyParametersCheck.class);
+      excludeLinesIfTrue(isRepositoryPropertyExpression(symbol), reportTree, BadMethodNameCheck.class);
     }
     super.visitMethod(tree);
   }
 
+  /**
+   * This methods requires semantic information to take a decision and filter out issues.
+   * The knowledge of being in a SpringData context can not be inferred from only tokens; it needs to understand the interfaces that the owning class implements.
+   *
+   * As a consequence, in case of missing semantic (degraded environment), S100 (BadMethodNameCheck) will still raise issues on SpringData methods.
+   * These issues will be considered FPs by the user, but can not be eliminated without introducing way too many FN for the rule.
+   *
+   * @param symbol the symbol of the method under analysis
+   * @return true if the method is understood as being a repository property expression, containing an underscore character in the middle of its name. Returns false otherwise.
+   */
+  private static boolean isRepositoryPropertyExpression(Symbol.MethodSymbol symbol) {
+    String name = symbol.name();
+    int underscorePosition = name.indexOf('_');
+    boolean isSeparatorInMethodName = underscorePosition > 0 && underscorePosition < name.length() - 1;
+    return isSeparatorInMethodName && symbol.owner().type().isSubtypeOf("org.springframework.data.repository.Repository");
+  }
+
   private static boolean hasAutowiredField(ClassTree tree) {
     return tree.members().stream()
       .filter(member -> member.is(Tree.Kind.VARIABLE))