SONARJAVA-4647 Improve S120 to report one project-level issue per bad package name

java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java

@@ -16,21 +16,26 @@
  */
 package org.sonar.java.checks.naming;
 
+import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.regex.Pattern;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
 import org.sonar.java.model.PackageUtils;
+import org.sonar.plugins.java.api.InputFileScannerContext;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
-import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
-import org.sonar.plugins.java.api.tree.CompilationUnitTree;
+import org.sonar.plugins.java.api.ModuleScannerContext;
+import org.sonar.plugins.java.api.internal.EndOfAnalysis;
 import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
 
 @DeprecatedRuleKey(ruleKey = "S00120", repositoryKey = "squid")
 @Rule(key = "S120")
-public class BadPackageNameCheck extends BaseTreeVisitor implements JavaFileScanner {
+public class BadPackageNameCheck implements JavaFileScanner, EndOfAnalysis {
 
   private static final String DEFAULT_FORMAT = "^[a-z_]+(\\.[a-z_][a-z0-9_]*)*$";
+  private static final String CACHE_KEY_PREFIX = "java:S120:package:";
 
   @RuleProperty(
     key = "format",
@@ -39,25 +44,49 @@ public class BadPackageNameCheck extends BaseTreeVisitor implements JavaFileScan
   public String format = DEFAULT_FORMAT;
 
   private Pattern pattern = null;
-  private JavaFileScannerContext context;
+  private final Set<String> badPackageNames = new HashSet<>();
+
+  @Override
+  public boolean scanWithoutParsing(InputFileScannerContext context) {
+    var cacheKey = CACHE_KEY_PREFIX + context.getInputFile().key();
+    var bytes = context.getCacheContext().getReadCache().readBytes(cacheKey);
+    if (bytes == null) {
+      return false;
+    }
+    context.getCacheContext().getWriteCache().copyFromPrevious(cacheKey);
+    String name = new String(bytes, StandardCharsets.UTF_8);
+    if (!name.isEmpty()) {
+      handlePackageName(name);
+    }
+    return true;
+  }
 
   @Override
   public void scanFile(JavaFileScannerContext context) {
+    var packageDeclaration = context.getTree().packageDeclaration();
+    String name = packageDeclaration != null ? PackageUtils.packageName(packageDeclaration, ".") : "";
+    if (context.getCacheContext().isCacheEnabled()) {
+      context.getCacheContext().getWriteCache().write(CACHE_KEY_PREFIX + context.getInputFile().key(), name.getBytes(StandardCharsets.UTF_8));
+    }
+    if (!name.isEmpty()) {
+      handlePackageName(name);
+    }
+  }
+
+  private void handlePackageName(String name) {
     if (pattern == null) {
       pattern = Pattern.compile(format, Pattern.DOTALL);
     }
-    this.context = context;
-    scan(context.getTree());
+    if (!pattern.matcher(name).matches()) {
+      badPackageNames.add(name);
+    }
   }
 
   @Override
-  public void visitCompilationUnit(CompilationUnitTree tree) {
-    if (tree.packageDeclaration() != null) {
-      String name = PackageUtils.packageName(tree.packageDeclaration(), ".");
-      if (!pattern.matcher(name).matches()) {
-        context.reportIssue(this, tree.packageDeclaration().packageName(), "Rename this package name to match the regular expression '" + format + "'.");
-      }
+  public void endOfAnalysis(ModuleScannerContext context) {
+    for (String badPackageName : badPackageNames) {
+      context.addIssueOnProject(this, "Rename package \"" + badPackageName + "\" to match the regular expression '" + format + "'.");
     }
+    badPackageNames.clear();
   }
-
 }

java-checks/src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java

@@ -1,5 +1,4 @@
-package PACKAGE; // Noncompliant {{Rename this package name to match the regular expression '^[a-z_]+(\.[a-z_][a-z0-9_]*)*$'.}}
-//      ^^^^^^^
+package PACKAGE;
 
 class BadPackageName {
 }

java-checks/src/test/files/checks/PACKAGE/BadQualifiedIdentifierPackageName.java

@@ -1,4 +1,4 @@
-package com.foo.PACKAGE; // Noncompliant {{Rename this package name to match the regular expression '^[a-z_]+(\.[a-z_][a-z0-9_]*)*$'.}}
+package com.foo.PACKAGE;
 
 class BadQualifiedIdentifierPackageName {
 }

java-checks/src/test/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java

@@ -16,17 +16,41 @@
  */
 package org.sonar.java.checks.naming;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.sensor.cache.ReadCache;
 import org.sonar.java.checks.verifier.CheckVerifier;
+import org.sonar.java.checks.verifier.internal.InternalReadCache;
+import org.sonar.java.checks.verifier.internal.InternalWriteCache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
 
 class BadPackageNameCheckTest {
 
+  private static final String DEFAULT_FORMAT = "^[a-z_]+(\\.[a-z_][a-z0-9_]*)*$";
+  private static final String NONCOMPLIANT_FILE = "src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java";
+
+  private ReadCache readCache;
+  private InternalWriteCache writeCache;
+
+  @BeforeEach
+  void initCaches() {
+    this.readCache = new InternalReadCache();
+    this.writeCache = new InternalWriteCache().bind(readCache);
+  }
+
   @Test
   void test() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java")
+      .onFile(NONCOMPLIANT_FILE)
       .withCheck(new BadPackageNameCheck())
-      .verifyIssues();
+      .verifyIssueOnProject("Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'.");
   }
 
   @Test
@@ -44,15 +68,87 @@ void test3() {
     CheckVerifier.newVerifier()
       .onFile("src/test/files/checks/PACKAGE/BadQualifiedIdentifierPackageName.java")
       .withCheck(new BadPackageNameCheck())
-      .verifyIssues();
+      .verifyIssueOnProject("Rename package \"com.foo.PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'.");
   }
 
   @Test
   void test_without_semantic() {
     CheckVerifier.newVerifier()
-      .onFile("src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java")
+      .onFile(NONCOMPLIANT_FILE)
       .withCheck(new BadPackageNameCheck())
       .withoutSemantic()
-      .verifyIssues();
+      .verifyIssueOnProject("Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'.");
+  }
+
+  @Test
+  void caching() {
+    String expectedMessage = "Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'.";
+
+    CheckVerifier.newVerifier()
+      .onFile(NONCOMPLIANT_FILE)
+      .withCheck(new BadPackageNameCheck())
+      .withCache(readCache, writeCache)
+      .verifyIssueOnProject(expectedMessage);
+
+    var check = spy(new BadPackageNameCheck());
+    var populatedReadCache = new InternalReadCache().putAll(writeCache);
+    var writeCache2 = new InternalWriteCache().bind(populatedReadCache);
+    CheckVerifier.newVerifier()
+      .withCache(populatedReadCache, writeCache2)
+      .addFiles(InputFile.Status.SAME, NONCOMPLIANT_FILE)
+      .withCheck(check)
+      .verifyIssueOnProject(expectedMessage);
+
+    verify(check, times(0)).scanFile(any());
+    verify(check, times(1)).scanWithoutParsing(any());
+    assertThat(writeCache2.getData()).containsExactlyInAnyOrderEntriesOf(writeCache.getData());
+  }
+
+  @Test
+  void caching_default_package() {
+    String defaultPackageFile = mainCodeSourcesPath("DefaultPackage.java");
+
+    CheckVerifier.newVerifier()
+      .onFile(defaultPackageFile)
+      .withCheck(new BadPackageNameCheck())
+      .withCache(readCache, writeCache)
+      .verifyNoIssues();
+
+    var check = spy(new BadPackageNameCheck());
+    var populatedReadCache = new InternalReadCache().putAll(writeCache);
+    var writeCache2 = new InternalWriteCache().bind(populatedReadCache);
+    CheckVerifier.newVerifier()
+      .withCache(populatedReadCache, writeCache2)
+      .addFiles(InputFile.Status.SAME, defaultPackageFile)
+      .withCheck(check)
+      .verifyNoIssues();
+
+    verify(check, times(0)).scanFile(any());
+    verify(check, times(1)).scanWithoutParsing(any());
+    assertThat(writeCache2.getData()).containsExactlyInAnyOrderEntriesOf(writeCache.getData());
+  }
+
+  @Test
+  void caching_no_issue_on_compliant_package() {
+    BadPackageNameCheck check1 = new BadPackageNameCheck();
+    check1.format = "^[a-zA-Z0-9]*$";
+    CheckVerifier.newVerifier()
+      .onFile("src/test/files/checks/PACKAGE/BadPackageName.java")
+      .withCheck(check1)
+      .withCache(readCache, writeCache)
+      .verifyNoIssues();
+
+    var check = spy(new BadPackageNameCheck());
+    check.format = "^[a-zA-Z0-9]*$";
+    var populatedReadCache = new InternalReadCache().putAll(writeCache);
+    var writeCache2 = new InternalWriteCache().bind(populatedReadCache);
+    CheckVerifier.newVerifier()
+      .withCache(populatedReadCache, writeCache2)
+      .addFiles(InputFile.Status.SAME, "src/test/files/checks/PACKAGE/BadPackageName.java")
+      .withCheck(check)
+      .verifyNoIssues();
+
+    verify(check, times(0)).scanFile(any());
+    verify(check, times(1)).scanWithoutParsing(any());
   }
 }