From 9c811c0afb83702a0e7996cbc65d5620fa4dbf7c Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Tue, 25 Aug 2026 10:33:26 +0200 Subject: [PATCH 1/4] SONARJAVA-4647: S120 reports one project-level issue per bad package name Previously, the rule raised an issue on every file within a non-compliant package, producing n issues for n classes in the same package. The rule now collects unique bad package names across all files and reports a single project-level issue per package via endOfAnalysis(). Co-Authored-By: Claude Sonnet 4.6 --- .../checks/naming/BadPackageNameCheck.java | 29 ++++++++++--------- .../PACKAGE/BadPackageNameNoncompliant.java | 3 +- .../BadQualifiedIdentifierPackageName.java | 2 +- .../naming/BadPackageNameCheckTest.java | 8 +++-- 4 files changed, 23 insertions(+), 19 deletions(-) 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..e30204cf00a 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,19 +16,21 @@ */ package org.sonar.java.checks.naming; +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.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_]*)*$"; @@ -39,25 +41,26 @@ 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 void scanFile(JavaFileScannerContext context) { if (pattern == null) { pattern = Pattern.compile(format, Pattern.DOTALL); } - this.context = context; - scan(context.getTree()); - } - - @Override - public void visitCompilationUnit(CompilationUnitTree tree) { - if (tree.packageDeclaration() != null) { - String name = PackageUtils.packageName(tree.packageDeclaration(), "."); + var packageDeclaration = context.getTree().packageDeclaration(); + if (packageDeclaration != null) { + String name = PackageUtils.packageName(packageDeclaration, "."); if (!pattern.matcher(name).matches()) { - context.reportIssue(this, tree.packageDeclaration().packageName(), "Rename this package name to match the regular expression '" + format + "'."); + badPackageNames.add(name); } } } + @Override + public void endOfAnalysis(ModuleScannerContext context) { + for (String badPackageName : badPackageNames) { + context.addIssueOnProject(this, "Rename package \"" + badPackageName + "\" to match the regular expression '" + format + "'."); + } + } } 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..cde7857c320 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 @@ -21,12 +21,14 @@ class BadPackageNameCheckTest { + private static final String DEFAULT_FORMAT = "^[a-z_]+(\\.[a-z_][a-z0-9_]*)*$"; + @Test void test() { CheckVerifier.newVerifier() .onFile("src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java") .withCheck(new BadPackageNameCheck()) - .verifyIssues(); + .verifyIssueOnProject("Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'."); } @Test @@ -44,7 +46,7 @@ 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 @@ -53,6 +55,6 @@ void test_without_semantic() { .onFile("src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java") .withCheck(new BadPackageNameCheck()) .withoutSemantic() - .verifyIssues(); + .verifyIssueOnProject("Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'."); } } From bbc1e64922facbba3f361730cde6d6d71a73489a Mon Sep 17 00:00:00 2001 From: Gitar Date: Tue, 25 Aug 2026 09:20:49 +0000 Subject: [PATCH 2/4] fix: preserve S120 project issue across incremental analysis Co-authored-by: Asya Vorobeva <253306985+asya-vorobeva@users.noreply.github.com> --- .../checks/naming/BadPackageNameCheck.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) 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 e30204cf00a..ebad9b36cab 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,12 +16,14 @@ */ 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.ModuleScannerContext; @@ -33,6 +35,7 @@ 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", @@ -44,16 +47,33 @@ public class BadPackageNameCheck implements JavaFileScanner, EndOfAnalysis { private final Set badPackageNames = new HashSet<>(); @Override - public void scanFile(JavaFileScannerContext context) { - if (pattern == null) { - pattern = Pattern.compile(format, Pattern.DOTALL); + public boolean scanWithoutParsing(InputFileScannerContext context) { + var bytes = context.getCacheContext().getReadCache().readBytes(CACHE_KEY_PREFIX + context.getInputFile().key()); + if (bytes == null) { + return false; } + handlePackageName(context, new String(bytes, StandardCharsets.UTF_8)); + return true; + } + + @Override + public void scanFile(JavaFileScannerContext context) { var packageDeclaration = context.getTree().packageDeclaration(); if (packageDeclaration != null) { String name = PackageUtils.packageName(packageDeclaration, "."); - if (!pattern.matcher(name).matches()) { - badPackageNames.add(name); + if (context.getCacheContext().isCacheEnabled()) { + context.getCacheContext().getWriteCache().write(CACHE_KEY_PREFIX + context.getInputFile().key(), name.getBytes(StandardCharsets.UTF_8)); } + handlePackageName(context, name); + } + } + + private void handlePackageName(InputFileScannerContext context, String name) { + if (pattern == null) { + pattern = Pattern.compile(format, Pattern.DOTALL); + } + if (!pattern.matcher(name).matches()) { + badPackageNames.add(name); } } From a5ed94a445aa328a927573d71e63b6ad159821c0 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Tue, 25 Aug 2026 11:36:55 +0200 Subject: [PATCH 3/4] SONARJAVA-4647: Add caching tests for BadPackageNameCheck Co-Authored-By: Claude Sonnet 4.6 --- .../checks/naming/BadPackageNameCheck.java | 7 +- .../naming/BadPackageNameCheckTest.java | 71 ++++++++++++++++++- 2 files changed, 73 insertions(+), 5 deletions(-) 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 ebad9b36cab..90520581025 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 @@ -52,7 +52,7 @@ public boolean scanWithoutParsing(InputFileScannerContext context) { if (bytes == null) { return false; } - handlePackageName(context, new String(bytes, StandardCharsets.UTF_8)); + handlePackageName(new String(bytes, StandardCharsets.UTF_8)); return true; } @@ -64,11 +64,11 @@ public void scanFile(JavaFileScannerContext context) { if (context.getCacheContext().isCacheEnabled()) { context.getCacheContext().getWriteCache().write(CACHE_KEY_PREFIX + context.getInputFile().key(), name.getBytes(StandardCharsets.UTF_8)); } - handlePackageName(context, name); + handlePackageName(name); } } - private void handlePackageName(InputFileScannerContext context, String name) { + private void handlePackageName(String name) { if (pattern == null) { pattern = Pattern.compile(format, Pattern.DOTALL); } @@ -82,5 +82,6 @@ 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/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/naming/BadPackageNameCheckTest.java index cde7857c320..ac193ca46d9 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,37 @@ */ 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.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; 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()) .verifyIssueOnProject("Rename package \"PACKAGE\" to match the regular expression '" + DEFAULT_FORMAT + "'."); } @@ -52,9 +72,56 @@ void test3() { @Test void test_without_semantic() { CheckVerifier.newVerifier() - .onFile("src/test/files/checks/PACKAGE/BadPackageNameNoncompliant.java") + .onFile(NONCOMPLIANT_FILE) .withCheck(new BadPackageNameCheck()) .withoutSemantic() .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()); + } + + @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()); + } } From cf2b2829843cc03c6062d435dde61e9e36a99873 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Tue, 25 Aug 2026 11:57:46 +0200 Subject: [PATCH 4/4] SONARJAVA-4647: Fix cache misses for default-package and cache-hit files - scanFile now always writes to cache (empty string for default package), so unchanged files without a package declaration are correctly skipped via scanWithoutParsing on subsequent runs - scanWithoutParsing now calls copyFromPrevious to propagate the cache entry to the write cache, preventing it from disappearing after a hit - Empty package name is excluded from badPackageNames in both paths Co-Authored-By: Claude Sonnet 4.6 --- .../checks/naming/BadPackageNameCheck.java | 19 ++++++++----- .../naming/BadPackageNameCheckTest.java | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) 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 90520581025..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 @@ -48,22 +48,27 @@ public class BadPackageNameCheck implements JavaFileScanner, EndOfAnalysis { @Override public boolean scanWithoutParsing(InputFileScannerContext context) { - var bytes = context.getCacheContext().getReadCache().readBytes(CACHE_KEY_PREFIX + context.getInputFile().key()); + var cacheKey = CACHE_KEY_PREFIX + context.getInputFile().key(); + var bytes = context.getCacheContext().getReadCache().readBytes(cacheKey); if (bytes == null) { return false; } - handlePackageName(new String(bytes, StandardCharsets.UTF_8)); + 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(); - if (packageDeclaration != null) { - String name = PackageUtils.packageName(packageDeclaration, "."); - if (context.getCacheContext().isCacheEnabled()) { - context.getCacheContext().getWriteCache().write(CACHE_KEY_PREFIX + context.getInputFile().key(), name.getBytes(StandardCharsets.UTF_8)); - } + 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); } } 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 ac193ca46d9..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 @@ -24,10 +24,12 @@ 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 { @@ -99,6 +101,31 @@ void caching() { 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