diff --git a/.please/docs/knowledge/gotchas.md b/.please/docs/knowledge/gotchas.md index 9641573..3492f44 100644 --- a/.please/docs/knowledge/gotchas.md +++ b/.please/docs/knowledge/gotchas.md @@ -22,7 +22,9 @@ Neither service is invoked by a workflow — both analyse every push through their GitHub App. That changes where their configuration lives and when it takes effect. -- **SonarQube Cloud is in Automatic Analysis mode, which reads `.sonarcloud.properties` and ignores `sonar-project.properties`.** The docs are explicit that the two files are different and that a `sonar-project.properties` in an imported project is ignored. Three further constraints: only the copy on the **default branch** applies (a change does not affect the PR that makes it), **wildcards are not allowed** in the values, and where the file and the SonarQube Cloud UI disagree the **file wins** — so an entry that matches nothing cannot be corrected from the UI while it is still there. +- **SonarQube Cloud is in Automatic Analysis mode, which reads `.sonarcloud.properties` and ignores `sonar-project.properties`.** The docs are explicit that the two files are different and that a `sonar-project.properties` in an imported project is ignored. Three further constraints: only the copy on the **default branch** applies (a change does not affect the PR that makes it), the **exclusion values are path patterns, not directory prefixes**, and where the file and the SonarQube Cloud UI disagree the **file wins** — so an entry that matches nothing cannot be corrected from the UI while it is still there. + +- **The [Automatic Analysis page](https://docs.sonarsource.com/sonarqube-cloud/analyzing-source-code/automatic-analysis/)'s "Wildcard patterns are not allowed" is about `sonar.sources` and `sonar.tests`, not the exclusions.** Those two take plain path lists; `sonar.exclusions` and `sonar.cpd.exclusions` take [path patterns](https://docs.sonarsource.com/sonarqube-cloud/managing-your-projects/project-analysis/setting-analysis-scope/excluding-files-based-on-patterns/) matched against whole file paths, where a whole subtree is written `dir/**/*` and a bare `dir/` matches nothing and is discarded in silence. Reading that line as covering every value in the file is what made ee09832's exclusion inert: `main` kept indexing 5,018 lines of generated bundle and reported 359 of its 412 findings inside the directory that was supposed to be excluded, until #26. `scripts/__tests__/generated-bundle-exclusions.test.ts` now fails on any exclusion pattern that matches no committed bundle — a pattern matching nothing is indistinguishable from a working one by reading it. - **Codacy reads `.codacy.yml` (or `.codacy.yaml`), and the first line must be `---`.** Additions are honoured on the PR that makes them; only removals wait for the default branch. Once the file exists, the UI's "Ignored files" settings stop applying. Validate before pushing: `docker run --rm -v "$(pwd)":/src codacy/codacy-analysis-cli validate-configuration --directory /src`. diff --git a/.sonarcloud.properties b/.sonarcloud.properties index dca9717..1bf1495 100644 --- a/.sonarcloud.properties +++ b/.sonarcloud.properties @@ -1,15 +1,22 @@ # SonarQube Cloud runs here in Automatic Analysis mode — no workflow invokes a # scanner, the GitHub App analyses every push on its own. That mode reads this -# file and ignores `sonar-project.properties`, and it accepts no wildcards in -# these values, so paths are written as plain directory prefixes. +# file and ignores `sonar-project.properties`. # -# Two consequences worth knowing before editing: +# Three consequences worth knowing before editing: # - Only the copy on the default branch takes effect. A change here does not # alter the analysis of the pull request that makes it; it applies once # merged. # - Where this file and the SonarQube Cloud UI disagree, this file wins. An # entry that matches nothing therefore cannot be corrected from the UI while # it is still here. +# - The exclusion values below are path *patterns* matched against whole file +# paths, not directory prefixes: `*` matches within one path segment, `**` +# spans directories, and a whole subtree is written `dir/**/*`. A bare +# `dir/` matches no file and is discarded in silence. The Automatic Analysis +# page's "Wildcard patterns are not allowed" is about the plain path lists +# `sonar.sources` and `sonar.tests`; ee09832 read it as covering these too +# and wrote bare prefixes, which left the exclusion inert — `main` still +# indexed 5,018 lines of generated code and reported 359 findings in it. # Generated by `bun run build:skill` from `scripts/*.ts`, committed because the # `npx skills` install channel copies only the skill directory and runs no @@ -17,7 +24,8 @@ # of `scripts/docs.ts` reappears in the bundle, which read as 7.7% duplication # on new code, and the `var` declarations Bun emits cost a reliability rating # that no edit can recover — `bun run build:skill:check` byte-compares the -# bundle against a fresh build, so hand-editing it fails CI. -# `eslint.config.js` ignores the same directory, for the same reason. -sonar.exclusions=skills/spring-docs/scripts/ -sonar.cpd.exclusions=skills/spring-docs/scripts/ +# bundle against a fresh build, so hand-editing it fails CI. `detect.mjs` also +# inlines `fast-xml-parser`, whose entity tables read as first-party source. +# `eslint.config.js` and `.codacy.yml` exclude the same directories. +sonar.exclusions=skills/*/scripts/**/* +sonar.cpd.exclusions=skills/*/scripts/**/* diff --git a/scripts/__tests__/generated-bundle-exclusions.test.ts b/scripts/__tests__/generated-bundle-exclusions.test.ts new file mode 100644 index 0000000..e72d060 --- /dev/null +++ b/scripts/__tests__/generated-bundle-exclusions.test.ts @@ -0,0 +1,122 @@ +/** + * Guards the analyser exclusions that keep `skills//scripts/` out of Sonar, + * Codacy and ESLint. + * + * The exclusion in ee09832 was written as a bare directory prefix + * (`skills/spring-docs/scripts/`). Every analyser matches patterns against the + * whole file path, so that prefix matched nothing and was dropped without a + * warning — SonarCloud kept indexing 5,018 generated lines for two days before + * anyone noticed. A pattern that matches no file looks identical to a pattern + * that works, so assert the match instead of reading the config. + * + * Each reader below takes the *active* setting, never the file's raw text: a + * commented-out exclusion still contains a pattern that would match, so a text + * scan passes while the analyser receives nothing — the same blind spot one + * level down. Each reader therefore drops the file's comment lines before it + * takes anything from them. + * + * `Bun.Glob` stands in for three matchers it is not, so a pass is not proof + * that SonarCloud reads a pattern the same way. What it does catch is the + * failure that actually happened: a pattern that matches no bundle at all. + */ +import { join } from 'node:path' + +import { describe, expect, test } from 'bun:test' + +const ROOT = join(import.meta.dir, '..', '..') + +/** The committed bundles every exclusion below is supposed to cover. */ +const BUNDLES = Array.from(new Bun.Glob('skills/*/scripts/*.mjs').scanSync(ROOT), p => p.replaceAll('\\', '/')) + +/** + * Split on `\r?\n`, not `\n`: the repository carries no `.gitattributes`, so a + * Windows checkout with `core.autocrlf=true` leaves a `\r` on every value, and + * a pattern ending in `\r` matches no bundle — a false failure that reads + * exactly like the misconfiguration this file exists to catch. + */ +async function lines(file: string): Promise { + return (await Bun.file(join(ROOT, file)).text()).split(/\r?\n/) +} + +/** + * The patterns `.sonarcloud.properties` sets for `key`, as a `key=a,b` line. + * + * Split each line on its first `=` and compare the trimmed key, rather than + * matching a prefix: `.properties` allows whitespace around the separator and + * before the key, and a reformatted file should not fail the guard. A + * commented-out setting keeps its `#`, so its key never compares equal. + */ +async function sonarExclusions(key: string): Promise { + for (const line of await lines('.sonarcloud.properties')) { + const separator = line.indexOf('=') + if (separator < 0 || line.slice(0, separator).trim() !== key) + continue + return line.slice(separator + 1).split(',').map(v => v.trim()).filter(Boolean) + } + return [] +} + +/** The `skills/` entries of `.codacy.yml`'s `exclude_paths:` block. */ +async function codacyExclusions(): Promise { + const source = await lines('.codacy.yml') + const start = source.findIndex(l => l.startsWith('exclude_paths:')) + expect(start).toBeGreaterThanOrEqual(0) + + const patterns: string[] = [] + for (const line of source.slice(start + 1)) { + const entry = line.trim() + if (entry.startsWith('- ')) { + // Strip the quotes YAML does not require here; single, double, or none. + patterns.push(entry.slice(2).trim().replace(/^["']|["']$/g, '')) + continue + } + // A comment or a blank line sits inside the block; anything else ends it. + if (entry !== '' && !entry.startsWith('#')) + break + } + return patterns.filter(p => p.startsWith('skills/')) +} + +/** + * The `skills/` ignore patterns in `eslint.config.js`'s `ignores` array. + * + * Read as text with comment lines dropped, not by importing the module. The + * evaluated config would be the stronger source, but reaching it needs either a + * non-literal dynamic `import()` or `allowJs` in `tsconfig.json`, and neither is + * worth a project-wide change here. Dropping `//` lines closes the gap that + * matters: a commented-out ignore no longer supplies a pattern that passes. + */ +async function eslintIgnores(): Promise { + return (await lines('eslint.config.js')) + .map(l => l.trim()) + .filter(l => !l.startsWith('//') && l.startsWith('\'skills/')) + .map(l => l.slice(1, l.indexOf('\'', 1))) +} + +function expectCoversBundles(patterns: string[]): void { + expect(patterns.length).toBeGreaterThan(0) + for (const pattern of patterns) { + const glob = new Bun.Glob(pattern) + for (const bundle of BUNDLES) expect([pattern, bundle, glob.match(bundle)]).toEqual([pattern, bundle, true]) + } +} + +describe('generated bundle exclusions', () => { + test('the bundles the exclusions target are committed', () => { + // A Set, because `Bun.Glob.scanSync` fixes no traversal order. + expect(new Set(BUNDLES)).toEqual(new Set(['skills/spring-docs/scripts/detect.mjs', 'skills/spring-docs/scripts/docs.mjs'])) + }) + + test('.sonarcloud.properties excludes them from issues and duplication', async () => { + expectCoversBundles(await sonarExclusions('sonar.exclusions')) + expectCoversBundles(await sonarExclusions('sonar.cpd.exclusions')) + }) + + test('.codacy.yml excludes them', async () => { + expectCoversBundles(await codacyExclusions()) + }) + + test('eslint.config.js ignores them', async () => { + expectCoversBundles(await eslintIgnores()) + }) +})