diff --git a/cli/src/main/java/dev/sonarcli/cli/SonarCommand.java b/cli/src/main/java/dev/sonarcli/cli/SonarCommand.java index 3d1e8cc..a5a7139 100644 --- a/cli/src/main/java/dev/sonarcli/cli/SonarCommand.java +++ b/cli/src/main/java/dev/sonarcli/cli/SonarCommand.java @@ -100,6 +100,15 @@ public final class SonarCommand implements Runnable { description = "A SonarQube quality-profile XML to drive rule selection.") private String configProfile; + @Option(names = "--test-path", paramLabel = "GLOB", + description = { + "Treat files matching GLOB as test code (additive).", + "Repeatable. Augments the built-in test-path detection", + "(src/test/**, *Test.java, *_test.go, *.spec.ts, ...).", + "Useful when the project's test layout is non-standard,", + "e.g. --test-path 'src/integration/**'."}) + private java.util.List additionalTestPaths = new java.util.ArrayList<>(); + @Spec private CommandSpec spec; @@ -148,7 +157,10 @@ private int analyzeAndReport(FileResolver.ResolvedFiles resolved, PrintWriter ou AnalyzeRequest request = new AnalyzeRequest( resolved.baseDir().toString(), resolved.relativePaths(), - List.of(), resolveProfileRef(), List.of()); + List.of(), resolveProfileRef(), List.of(), + additionalTestPaths != null + ? List.copyOf(additionalTestPaths) + : List.of()); AnalyzeResponse response = rpc.analyze(request); List filtered = response.issues().stream() diff --git a/daemon/src/main/java/dev/sonarcli/daemon/AnalysisService.java b/daemon/src/main/java/dev/sonarcli/daemon/AnalysisService.java index 4c41f63..927c7e7 100644 --- a/daemon/src/main/java/dev/sonarcli/daemon/AnalysisService.java +++ b/daemon/src/main/java/dev/sonarcli/daemon/AnalysisService.java @@ -183,6 +183,31 @@ public AnalyzeResponse analyze(AnalyzeRequest request) { } } + /** + * Glob-match a relative path against any of the patterns the caller passed + * via {@link AnalyzeRequest#additionalTestPaths()}. Uses Java's standard + * {@code glob:} {@link java.nio.file.PathMatcher} so callers can pass + * shapes like {@code src/integration/**} or {@code **/legacy/*Test.java}. + */ + private static boolean matchesAnyGlob(String relativePath, List globs) { + if (globs == null || globs.isEmpty()) { + return false; + } + java.nio.file.Path p = java.nio.file.Path.of(relativePath); + for (String glob : globs) { + if (glob == null || glob.isEmpty()) continue; + try { + if (java.nio.file.FileSystems.getDefault() + .getPathMatcher("glob:" + glob).matches(p)) { + return true; + } + } catch (IllegalArgumentException ignored) { + // A malformed glob from the caller shouldn't crash analysis. + } + } + return false; + } + /** The analysis body; always runs holding {@link #analysisLock}. */ private AnalyzeResponse analyzeLocked(AnalyzeRequest request) { Path baseDir = Path.of(request.baseDir()).toAbsolutePath().normalize(); @@ -220,7 +245,9 @@ private AnalyzeResponse analyzeLocked(AnalyzeRequest request) { "file path escapes the analysis base directory; skipped")); continue; } - inputFiles.add(new FileInputFile(file, baseDir, language.get())); + boolean isTest = TestPathDetector.isTest(relative, language.get()) + || matchesAnyGlob(relative, request.additionalTestPaths()); + inputFiles.add(new FileInputFile(file, baseDir, language.get(), isTest)); presentLanguages.add(language.get()); } diff --git a/daemon/src/main/java/dev/sonarcli/daemon/FileInputFile.java b/daemon/src/main/java/dev/sonarcli/daemon/FileInputFile.java index a620d3c..d34d3fc 100644 --- a/daemon/src/main/java/dev/sonarcli/daemon/FileInputFile.java +++ b/daemon/src/main/java/dev/sonarcli/daemon/FileInputFile.java @@ -24,6 +24,7 @@ public final class FileInputFile implements ClientInputFile { private final Path absolutePath; private final String relativePath; private final SonarLanguage language; + private final boolean isTest; /** * @param file the source file (need not be absolute) @@ -31,11 +32,23 @@ public final class FileInputFile implements ClientInputFile { * @param language the analyzer language for this file */ public FileInputFile(Path file, Path baseDir, SonarLanguage language) { + this(file, baseDir, language, false); + } + + /** + * @param file the source file (need not be absolute) + * @param baseDir directory the file's relative path is computed against + * @param language the analyzer language for this file + * @param isTest {@code true} to mark this file as test scope; the + * analysis engine then skips MAIN-only rules on it + */ + public FileInputFile(Path file, Path baseDir, SonarLanguage language, boolean isTest) { this.absolutePath = Objects.requireNonNull(file, "file").toAbsolutePath().normalize(); Path base = Objects.requireNonNull(baseDir, "baseDir").toAbsolutePath().normalize(); // Path.relativize uses the OS separator; the engine expects '/'. this.relativePath = base.relativize(absolutePath).toString().replace('\\', '/'); this.language = Objects.requireNonNull(language, "language"); + this.isTest = isTest; } @Override @@ -45,7 +58,7 @@ public String getPath() { @Override public boolean isTest() { - return false; + return isTest; } @Override diff --git a/daemon/src/main/java/dev/sonarcli/daemon/TestPathDetector.java b/daemon/src/main/java/dev/sonarcli/daemon/TestPathDetector.java new file mode 100644 index 0000000..88ccd17 --- /dev/null +++ b/daemon/src/main/java/dev/sonarcli/daemon/TestPathDetector.java @@ -0,0 +1,107 @@ +package dev.sonarcli.daemon; + +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +import org.sonarsource.sonarlint.core.commons.api.SonarLanguage; + +/** + * Classifies a source file as production or test code per SonarQube's standard + * {@code sonar.sources} vs {@code sonar.tests} distinction. + * + *

SonarSource rules carry a scope ({@code MAIN}, {@code TEST}, {@code ALL}); + * the analysis engine skips {@code MAIN}-only rules on files marked as test. + * This is how SonarQube proper lets test code keep its conventional looser + * style (e.g. {@code methodUnderTest_scenario_expected} method names, longer + * methods, generic exception handling) without false positives like + * {@code java:S100}. We do not override any rule here — we only correctly + * classify files; the analyzer's own scope metadata does the rest. + * + *

Patterns follow each language's standard build-tool and analyzer + * conventions: + * 

+ *   Java/Kotlin: src/test/**, *Test.{java,kt}, *Tests.{java,kt}, *IT.java
+ *   Scala:       src/test/**, *Test.scala
+ *   Go:          *_test.go
+ *   Python:      tests/**, test/**, test_*.py, *_test.py
+ *   JS/TS:       tests/**, __tests__/**, *.{test,spec}.{js,jsx,ts,tsx}
+ *   PHP:         tests/**, *Test.php
+ *   Ruby:        spec/**, test/**, *_spec.rb, *_test.rb
+ *   CSS/HTML/XML: no test convention; everything is MAIN
+ *
+ */ +public final class TestPathDetector { + + /** Path segments that indicate test scope regardless of language. */ + private static final List COMMON_PATH_PATTERNS = List.of( + Pattern.compile("(^|/)src/test/"), + Pattern.compile("(^|/)tests?/"), + Pattern.compile("(^|/)__tests__/"), + Pattern.compile("(^|/)spec/")); + + /** Per-language filename patterns matched against the file's basename. */ + private static final Map> FILENAME_PATTERNS = Map.ofEntries( + Map.entry(SonarLanguage.JAVA, List.of( + Pattern.compile(".*Test\\.java$"), + Pattern.compile(".*Tests\\.java$"), + Pattern.compile(".*IT\\.java$"))), + Map.entry(SonarLanguage.KOTLIN, List.of( + Pattern.compile(".*Test\\.kt$"), + Pattern.compile(".*Tests\\.kt$"))), + Map.entry(SonarLanguage.SCALA, List.of( + Pattern.compile(".*Test\\.scala$"))), + Map.entry(SonarLanguage.GO, List.of( + Pattern.compile(".*_test\\.go$"))), + Map.entry(SonarLanguage.PYTHON, List.of( + Pattern.compile("test_.*\\.py$"), + Pattern.compile(".*_test\\.py$"))), + Map.entry(SonarLanguage.JS, List.of( + Pattern.compile(".*\\.(test|spec)\\.jsx?$"))), + Map.entry(SonarLanguage.TS, List.of( + Pattern.compile(".*\\.(test|spec)\\.tsx?$"))), + Map.entry(SonarLanguage.PHP, List.of( + Pattern.compile(".*Test\\.php$"))), + Map.entry(SonarLanguage.RUBY, List.of( + Pattern.compile(".*_spec\\.rb$"), + Pattern.compile(".*_test\\.rb$")))); + + private TestPathDetector() { + // utility class + } + + /** + * @param relativePath '/'-separated path relative to the analysis base + * directory (the engine's standard form) + * @param language the file's detected language; may be {@code null} + * @return {@code true} if the path matches a test convention for the + * given language, {@code false} otherwise + */ + public static boolean isTest(String relativePath, SonarLanguage language) { + if (relativePath == null || relativePath.isEmpty()) { + return false; + } + String normalized = relativePath.replace('\\', '/'); + + for (Pattern p : COMMON_PATH_PATTERNS) { + if (p.matcher(normalized).find()) { + return true; + } + } + + if (language != null) { + List patterns = FILENAME_PATTERNS.get(language); + if (patterns != null) { + int slash = normalized.lastIndexOf('/'); + String filename = slash < 0 ? normalized : normalized.substring(slash + 1); + for (Pattern p : patterns) { + if (p.matcher(filename).matches()) { + return true; + } + } + } + } + + return false; + } +} diff --git a/daemon/src/test/java/dev/sonarcli/daemon/TestPathDetectorTest.java b/daemon/src/test/java/dev/sonarcli/daemon/TestPathDetectorTest.java new file mode 100644 index 0000000..08477fd --- /dev/null +++ b/daemon/src/test/java/dev/sonarcli/daemon/TestPathDetectorTest.java @@ -0,0 +1,150 @@ +package dev.sonarcli.daemon; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.sonarsource.sonarlint.core.commons.api.SonarLanguage; + +class TestPathDetectorTest { + + // --- Path-segment based detection (cross-language) ---------------------- + + @Test + void srcTestUnderMavenLayoutIsTest() { + assertTrue(TestPathDetector.isTest( + "daemon/src/test/java/dev/sonarcli/daemon/AnalysisServiceTest.java", + SonarLanguage.JAVA)); + } + + @Test + void topLevelTestsDirIsTest() { + assertTrue(TestPathDetector.isTest("tests/test_thing.py", SonarLanguage.PYTHON)); + assertTrue(TestPathDetector.isTest("tests/scanner.spec.ts", SonarLanguage.TS)); + } + + @Test + void jestUnderscoresDirIsTest() { + assertTrue(TestPathDetector.isTest("src/__tests__/scanner.js", SonarLanguage.JS)); + } + + @Test + void rspecSpecDirIsTest() { + assertTrue(TestPathDetector.isTest("spec/models/order_spec.rb", SonarLanguage.RUBY)); + } + + // --- Per-language filename-based detection ------------------------------ + + @Test + void javaTestSuffixesAreTest() { + assertTrue(TestPathDetector.isTest("foo/bar/AnalysisServiceTest.java", SonarLanguage.JAVA)); + assertTrue(TestPathDetector.isTest("foo/bar/AnalysisServiceTests.java", SonarLanguage.JAVA)); + assertTrue(TestPathDetector.isTest("foo/bar/AnalysisServiceIT.java", SonarLanguage.JAVA)); + } + + @Test + void kotlinTestSuffixesAreTest() { + assertTrue(TestPathDetector.isTest("foo/AnalyzerTest.kt", SonarLanguage.KOTLIN)); + assertTrue(TestPathDetector.isTest("foo/AnalyzerTests.kt", SonarLanguage.KOTLIN)); + } + + @Test + void goUnderscoreTestSuffixIsTest() { + assertTrue(TestPathDetector.isTest("cmd/main_test.go", SonarLanguage.GO)); + } + + @Test + void pythonTestPrefixAndSuffixAreTest() { + assertTrue(TestPathDetector.isTest("a/test_thing.py", SonarLanguage.PYTHON)); + assertTrue(TestPathDetector.isTest("a/thing_test.py", SonarLanguage.PYTHON)); + } + + @Test + void jestAndJasmineSpecsAreTest() { + assertTrue(TestPathDetector.isTest("src/foo.test.js", SonarLanguage.JS)); + assertTrue(TestPathDetector.isTest("src/foo.spec.js", SonarLanguage.JS)); + assertTrue(TestPathDetector.isTest("src/foo.test.jsx", SonarLanguage.JS)); + assertTrue(TestPathDetector.isTest("src/foo.spec.jsx", SonarLanguage.JS)); + assertTrue(TestPathDetector.isTest("src/foo.test.ts", SonarLanguage.TS)); + assertTrue(TestPathDetector.isTest("src/foo.spec.ts", SonarLanguage.TS)); + assertTrue(TestPathDetector.isTest("src/foo.test.tsx", SonarLanguage.TS)); + assertTrue(TestPathDetector.isTest("src/foo.spec.tsx", SonarLanguage.TS)); + } + + @Test + void phpTestSuffixIsTest() { + assertTrue(TestPathDetector.isTest("app/OrderTest.php", SonarLanguage.PHP)); + } + + @Test + void scalaTestSuffixIsTest() { + assertTrue(TestPathDetector.isTest("foo/AnalyzerTest.scala", SonarLanguage.SCALA)); + } + + @Test + void rubySpecAndTestSuffixesAreTest() { + assertTrue(TestPathDetector.isTest("models/order_spec.rb", SonarLanguage.RUBY)); + assertTrue(TestPathDetector.isTest("models/order_test.rb", SonarLanguage.RUBY)); + } + + // --- Negatives: production code stays production ------------------------ + + @Test + void productionJavaPathIsNotTest() { + assertFalse(TestPathDetector.isTest( + "daemon/src/main/java/dev/sonarcli/daemon/AnalysisService.java", + SonarLanguage.JAVA)); + } + + @Test + void productionGoPathIsNotTest() { + assertFalse(TestPathDetector.isTest("cmd/main.go", SonarLanguage.GO)); + } + + @Test + void productionPythonPathIsNotTest() { + assertFalse(TestPathDetector.isTest("app/service.py", SonarLanguage.PYTHON)); + } + + @Test + void javaFileNamedSimilarlyButNotASuffixIsNotTest() { + // 'TestHelper' contains 'Test' but the *suffix* is 'Helper' + assertFalse(TestPathDetector.isTest( + "src/main/java/foo/TestHelper.java", SonarLanguage.JAVA)); + } + + @Test + void htmlXmlCssHaveNoFilenameConventionAndPathSegmentStillWorks() { + // No filename pattern → falls through to path segments + assertFalse(TestPathDetector.isTest("src/main/resources/index.html", SonarLanguage.HTML)); + assertFalse(TestPathDetector.isTest("src/main/resources/config.xml", SonarLanguage.XML)); + assertFalse(TestPathDetector.isTest("src/main/resources/style.css", SonarLanguage.CSS)); + // …but if they ARE under tests/, the path segment marks them as test + assertTrue(TestPathDetector.isTest("tests/fixtures/sample.html", SonarLanguage.HTML)); + } + + // --- Edge cases --------------------------------------------------------- + + @Test + void nullPathIsNotTest() { + assertFalse(TestPathDetector.isTest(null, SonarLanguage.JAVA)); + } + + @Test + void emptyPathIsNotTest() { + assertFalse(TestPathDetector.isTest("", SonarLanguage.JAVA)); + } + + @Test + void nullLanguageStillRespectsCommonPathSegments() { + assertTrue(TestPathDetector.isTest("src/test/java/foo/Bar.java", null)); + assertFalse(TestPathDetector.isTest("src/main/java/foo/Bar.java", null)); + } + + @Test + void windowsBackslashPathsAreNormalised() { + assertTrue(TestPathDetector.isTest( + "daemon\\src\\test\\java\\dev\\sonarcli\\daemon\\AnalysisServiceTest.java", + SonarLanguage.JAVA)); + } +} diff --git a/plugin/agents/sonar-scanner-claude.md b/plugin/agents/sonar-scanner-claude.md index 15dc2ee..b3e3f7f 100644 --- a/plugin/agents/sonar-scanner-claude.md +++ b/plugin/agents/sonar-scanner-claude.md @@ -37,6 +37,28 @@ something unusual; don't guess flag names. Exit codes: `0` clean, `1` issues found (a normal result, not a failure), `2` tool error. +## Non-standard test layouts — `--test-path GLOB` + +The daemon auto-detects standard test paths per language (`src/test/**`, +`tests/**`, `__tests__/**`, `spec/**`, plus per-language filename +conventions like `*Test.java`, `*_test.go`, `test_*.py`, `*.spec.ts`, +`*_spec.rb`). For those, no flag is needed — `Main`-only rules (`java:S100`, +`java:S106`, `java:S1118`, …) already skip the test files correctly. + +**Look at the project layout before scanning.** If you see non-standard +test directories or filename conventions — e.g. `src/integration/`, `e2e/`, +`fixtures/`, `cypress/`, an `acceptance/` tree — pass them as repeatable +`--test-path GLOB` *global* options so the analyzer skips `Main`-only rules +on them too. Globs are standard Java NIO globs against `/`-separated paths. + +```sh +# Augment the built-in detection for a project with cypress e2e + integration: +./bin/sonar --test-path 'src/integration/**' --test-path 'cypress/**' agent-scan analyze . +``` + +Skip this flag when the layout is conventional. Don't over-classify: the +goal is to not flag intentional test conventions, not to silence rules. + ## What to report The stdout summary from `agent-scan` is your top-line: issue count, severity diff --git a/plugin/agents/sonar-scanner-copilot.md b/plugin/agents/sonar-scanner-copilot.md index 3c55a61..de4f586 100644 --- a/plugin/agents/sonar-scanner-copilot.md +++ b/plugin/agents/sonar-scanner-copilot.md @@ -37,6 +37,28 @@ something unusual; don't guess flag names. Exit codes: `0` clean, `1` issues found (a normal result, not a failure), `2` tool error. +## Non-standard test layouts — `--test-path GLOB` + +The daemon auto-detects standard test paths per language (`src/test/**`, +`tests/**`, `__tests__/**`, `spec/**`, plus per-language filename +conventions like `*Test.java`, `*_test.go`, `test_*.py`, `*.spec.ts`, +`*_spec.rb`). For those, no flag is needed — `Main`-only rules (`java:S100`, +`java:S106`, `java:S1118`, …) already skip the test files correctly. + +**Look at the project layout before scanning.** If you see non-standard +test directories or filename conventions — e.g. `src/integration/`, `e2e/`, +`fixtures/`, `cypress/`, an `acceptance/` tree — pass them as repeatable +`--test-path GLOB` *global* options so the analyzer skips `Main`-only rules +on them too. Globs are standard Java NIO globs against `/`-separated paths. + +```sh +# Augment the built-in detection for a project with cypress e2e + integration: +./bin/sonar --test-path 'src/integration/**' --test-path 'cypress/**' agent-scan analyze . +``` + +Skip this flag when the layout is conventional. Don't over-classify: the +goal is to not flag intentional test conventions, not to silence rules. + ## What to report The stdout summary from `agent-scan` is your top-line: issue count, severity diff --git a/plugin/skills/sonar-predictor/SKILL.md b/plugin/skills/sonar-predictor/SKILL.md index abeef0d..c384ec0 100644 --- a/plugin/skills/sonar-predictor/SKILL.md +++ b/plugin/skills/sonar-predictor/SKILL.md @@ -13,6 +13,8 @@ Run `./bin/sonar` from this skill's base directory (the folder with this `SKILL. **Agent invocation pattern — `./bin/sonar agent-scan [scope]`.** A wrapper subcommand that bakes the out-of-context discipline into the tool: it runs the scan with `--format json` redirected to `.sonar-predictor/scan.json` at the project root, adds `.sonar-predictor/` to `.gitignore` on first use (when inside a git repo), and prints a compact summary to stdout — issue count, severity breakdown, file path. The calling agent reports that summary and points its caller at the file; deeper drill-down happens with `jq` on the file, on demand. With no scope argument, `agent-scan` defaults to `check --diff` (the git changeset); pass an explicit scope (`agent-scan analyze src/`, `agent-scan check src/Main.java`, etc.) to scan something else. All flags forward to the underlying CLI. +**Test vs production classification.** The analyzer applies SonarSource's standard `Main` vs `Test` rule scope, so `Main`-only rules (`java:S100` method naming, `java:S106` System.out, `java:S1118` utility-class ctor, ...) skip test files. The daemon auto-detects tests by language-conventional paths and filenames: `src/test/**`, `tests/**`, `__tests__/**`, `spec/**` for any language; plus per-language filename conventions (`*Test.java`, `*_test.go`, `test_*.py`, `*.spec.ts`, `*_spec.rb`, etc.). **When a project's layout is non-standard, an agent passes `--test-path GLOB` (repeatable, global option)** so non-conventional dirs are also treated as tests — e.g., `./bin/sonar --test-path 'src/integration/**' --test-path 'e2e/**' agent-scan analyze .`. Patterns are standard Java NIO globs against `/`-separated paths. Standalone invocations without an agent fall back to the built-in detector. + Exit codes: `0` clean, `1` issues found, `2` tool error. Acting on findings: fix `BUG`/`VULNERABILITY`/`SECURITY_HOTSPOT` and `CRITICAL`/`MAJOR` first. This is a fast first-pass gate, not the release gate — fix the real issues and move on. diff --git a/protocol/src/main/java/dev/sonarcli/protocol/dto/AnalyzeRequest.java b/protocol/src/main/java/dev/sonarcli/protocol/dto/AnalyzeRequest.java index c31abfe..9487fd3 100644 --- a/protocol/src/main/java/dev/sonarcli/protocol/dto/AnalyzeRequest.java +++ b/protocol/src/main/java/dev/sonarcli/protocol/dto/AnalyzeRequest.java @@ -5,18 +5,33 @@ /** * Request payload for {@code Method.ANALYZE}. * - * @param baseDir absolute path the {@code files} are relative to - * @param files file paths to analyze, relative to {@code baseDir} - * @param languageHints optional language keys; empty means auto-detect - * @param profileRef path to a SonarQube quality-profile XML, or {@code null} - * to use each analyzer's default (SonarWay) profile - * @param coverageReports paths to coverage reports to import; may be empty + * @param baseDir absolute path the {@code files} are relative to + * @param files file paths to analyze, relative to {@code baseDir} + * @param languageHints optional language keys; empty means auto-detect + * @param profileRef path to a SonarQube quality-profile XML, or + * {@code null} to use each analyzer's default + * (SonarWay) profile + * @param coverageReports paths to coverage reports to import; may be empty + * @param additionalTestPaths glob patterns (relative to {@code baseDir}) that + * mark extra paths as test code on top of the + * built-in language-aware test-path detection; + * useful when an agent/skill knows the project's + * test layout is non-standard. May be empty. */ public record AnalyzeRequest( String baseDir, List files, List languageHints, String profileRef, - List coverageReports + List coverageReports, + List additionalTestPaths ) { + /** + * Backward-compatible 5-arg constructor — callers that don't override + * test-path classification fall through to the built-in detector only. + */ + public AnalyzeRequest(String baseDir, List files, List languageHints, + String profileRef, List coverageReports) { + this(baseDir, files, languageHints, profileRef, coverageReports, List.of()); + } }