diff --git a/java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java b/java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java index 201e3f6ae67..ba83dae9f88 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/naming/BadPackageNameCheck.java +++ b/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 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(); } - } diff --git a/java-checks/src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java b/java-checks/src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java index b605f69b576..f955572cc95 100644 --- a/java-checks/src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java +++ b/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 { } diff --git a/java-checks/src/test/files/checks/PACKAGE/BadQualifiedIdentifierPackageName.java b/java-checks/src/test/files/checks/PACKAGE/BadQualifiedIdentifierPackageName.java index a489b8ff555..c0679952e02 100644 --- a/java-checks/src/test/files/checks/PACKAGE/BadQualifiedIdentifierPackageName.java +++ b/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 { } diff --git a/java-checks/src/test/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java index 9357348c531..638d0d9fd96 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java +++ b/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()); } }