diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md index 36387567..c57d7c5b 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md @@ -55,7 +55,7 @@ Every call to `Bun.Glob#scan()` (`glob.scan(...)`) in source MUST pass `{ dot: t ### Automated -- **Archgate rule** ARCH-020/glob-scan-dot: Scans `src/**/*.ts` for `.scan(` calls and reports any whose argument list does not contain `dot:`. Severity: error. +- **Archgate rule** ARCH-020/glob-scan-dot: Parses `src/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` `CallExpression` nodes, reporting any whose argument list has no `dot` key with a `true` (or non-statically-resolvable) value — a literal `dot: false` is flagged. Structural, not text-based, so a comment or string that merely mentions `.scan()` cannot be misreported (see [archgate/cli#513](https://github.com/archgate/cli/issues/513)). Severity: error. ### Manual @@ -64,5 +64,7 @@ Code reviewers MUST verify new `Bun.Glob` scans pass `dot: true` and that any in ## References - [archgate/cli#222](https://github.com/archgate/cli/issues/222) — the dotfile-skipping bug this ADR prevents +- [archgate/cli#513](https://github.com/archgate/cli/issues/513) — the companion rule's regex-over-raw-text false positive on comments/strings, fixed by moving to `ctx.ast()` - [`src/engine/runner.ts`](../../src/engine/runner.ts), [`src/engine/git-files.ts`](../../src/engine/git-files.ts) — canonical `scan({ dot: true })` usage - [ARCH-009: Platform Detection Helper](./ARCH-009-platform-detection-helper.md) — related cross-platform correctness governance +- [ARCH-022: AST-Aware Rule Context](./ARCH-022-ast-aware-rule-context.md) — `ctx.ast()` / `ctx.findAstNodes()`, the structural inspection primitive this rule's companion check now uses diff --git a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts index eb74c3ab..ae87d06c 100644 --- a/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts +++ b/.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts @@ -1,5 +1,120 @@ /// +/** + * ARCH-020 enforcement on top of ctx.ast() / ctx.findAstNodes() (ARCH-022): + * walks the ESTree for real `.scan(...)` CallExpression nodes, so a + * comment or string literal that merely mentions `.scan()` is not a match. + */ + +/** True for `.scan(...)` -- a non-computed `.scan` member call. */ +function isScanCall(node: EsTreeNode): boolean { + if (node.type !== "CallExpression") return false; + const callee = node.callee as EsTreeNode | undefined; + if (callee?.type !== "MemberExpression" || callee.computed === true) { + return false; + } + const property = callee.property as EsTreeNode | undefined; + return property?.type === "Identifier" && property.name === "scan"; +} + +/** + * Does this call's argument list include a `dot` option that isn't + * disqualified? A non-literal value (identifier, expression) can't be + * resolved statically, so it's treated as compliant; a literal value must + * be `true` -- `dot: false` reproduces the exact bug this ADR prevents. + */ +function hasDotOption(call: EsTreeNode): boolean { + const args = (call.arguments as EsTreeNode[] | undefined) ?? []; + return args.some((arg) => { + if (arg.type !== "ObjectExpression") return false; + const properties = (arg.properties as EsTreeNode[] | undefined) ?? []; + return properties.some((prop) => { + if (prop.type !== "Property" || prop.computed === true) return false; + const key = prop.key as + | (EsTreeNode & { name?: unknown; value?: unknown }) + | undefined; + const isDotKey = + key?.type === "Identifier" + ? key.name === "dot" + : key?.type === "Literal" && key.value === "dot"; + if (!isDotKey) return false; + const value = prop.value as + | (EsTreeNode & { value?: unknown }) + | undefined; + return value?.type !== "Literal" || value.value === true; + }); + }); +} + +/** + * Blank comments and string/template literals to spaces, keeping every + * newline, so line numbers computed against the result match the original + * source. Re-locates a call ctx.ast() already found structurally, since + * `loc` is not trustworthy for `"typescript"` (ARCH-022). Does not track + * regex literals, matching `source-positions.ts`. + */ +function blankNonCode(source: string): string { + let out = ""; + let i = 0; + const n = source.length; + while (i < n) { + const ch = source[i]; + const next = source[i + 1]; + if (ch === "/" && next === "/") { + while (i < n && source[i] !== "\n") { + out += " "; + i++; + } + continue; + } + if (ch === "/" && next === "*") { + out += " "; + i += 2; + while (i < n && !(source[i] === "*" && source[i + 1] === "/")) { + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i += 2; + } + continue; + } + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + out += " "; + i++; + while (i < n && source[i] !== quote) { + if (source[i] === "\\" && i + 1 < n) { + out += " "; + i += 2; + continue; + } + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i++; + } + continue; + } + out += ch; + i++; + } + return out; +} + +/** 1-based line of the next `.scan(` at or after `fromIndex` in blanked `code`. */ +function nextScanLine( + code: string, + fromIndex: number +): { line: number | undefined; nextIndex: number } { + const idx = code.indexOf(".scan(", fromIndex); + if (idx === -1) return { line: undefined, nextIndex: fromIndex }; + return { line: code.slice(0, idx).split("\n").length, nextIndex: idx + 6 }; +} + export default { rules: { "glob-scan-dot": { @@ -9,32 +124,49 @@ export default { async check(ctx) { const files = ctx.scopedFiles.filter((f) => f.endsWith(".ts")); - // Capture the argument list of each `.scan( ... )` call. The character - // class `[^)]` spans newlines, so multi-line option objects are - // covered, as long as the args contain no nested `)` (true for glob - // option objects: `{ cwd, dot: true }`). - const callPattern = /\.scan\(([^)]*)\)/gu; - const checks = files.map(async (file) => { - let content: string; + let tree: EsTreeProgram; try { - content = await ctx.readFile(file); + tree = await ctx.ast(file, "typescript"); } catch { return; } - for (const match of content.matchAll(callPattern)) { - const args = match[1]; - if (/\bdot\s*:/u.test(args)) continue; + const scanCalls = ctx + .findAstNodes(tree, "CallExpression") + .filter((node) => isScanCall(node)); + if (scanCalls.length === 0) return; + + // Sort by transpiled loc: not source-accurate for "typescript" + // (ARCH-022), but Bun's transpiler only erases type-only syntax, + // never reorders statements, so relative order is preserved -- + // enough to pair each call with its re-located line below. + scanCalls.sort((a, b) => { + const lineDiff = + (a.loc?.start.line ?? 0) - (b.loc?.start.line ?? 0); + if (lineDiff !== 0) return lineDiff; + return (a.loc?.start.column ?? 0) - (b.loc?.start.column ?? 0); + }); + + let source: string; + try { + source = await ctx.readFile(file); + } catch { + return; + } + const code = blankNonCode(source); - const offset = match.index ?? 0; - const line = content.slice(0, offset).split("\n").length; + let cursor = 0; + for (const call of scanCalls) { + const { line, nextIndex } = nextScanLine(code, cursor); + cursor = nextIndex; + if (hasDotOption(call)) continue; ctx.report.violation({ message: "Bun.Glob#scan() must pass { dot: true } or it silently skips dot-prefixed directories on Windows", file, - line, + ...(line === undefined ? {} : { line }), fix: "Add `dot: true` to the scan options, e.g. `glob.scan({ cwd, dot: true })`", }); } diff --git a/.archgate/adrs/ARCH-022-ast-aware-rule-context.md b/.archgate/adrs/ARCH-022-ast-aware-rule-context.md index b7db07ad..20d2a830 100644 --- a/.archgate/adrs/ARCH-022-ast-aware-rule-context.md +++ b/.archgate/adrs/ARCH-022-ast-aware-rule-context.md @@ -112,6 +112,7 @@ Dispatch on `language` MUST be invisible to rule authors. - **Comment-governance rules become structural** — length, style, and content policies can be written against structured comment tokens (`type`/`value`/`loc`) with original-source-accurate positions instead of fragile line/regex heuristics. - **Failure visibility reuses proven machinery** — no new exit code, reporter branch, or error-boundary design; throw-on-failure rides on `runner.ts`'s per-rule isolation and `reporter.ts`'s exit-code-2 category. - **Incremental adoption** — TS/JS support needs no new capability surface beyond what exists internally, and Python/Ruby can follow independently since the guardrail and failure-semantics design is identical for both. +- **`ctx.findAstNodes(tree, ...types)` retires the hand-rolled `walk(node, visit)` helper each rule file previously had to repeat** — it covers "collect every node of type X" for any language's tree shape by matching the `_type`/`type` discriminant in preorder. A custom walk is still needed when a rule must prune a subtree before collecting (e.g. skip descending into nested function bodies), not merely filter by type. ### Negative diff --git a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md index aea9df4a..d169f370 100644 --- a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md +++ b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.md @@ -72,7 +72,7 @@ The rules engine (`src/engine/`) MUST list files by matching globs **in memory** ## Compliance and Enforcement -- **Automated:** The companion rule `scan-confined-to-fallback-modules` (this ADR) blocks `Bun.Glob#scan()` call sites in `src/engine/` outside `glob-utils.ts`/`git-files.ts`. ARCH-020's `glob-scan-dot` rule covers `dot: true` on the remaining fallbacks. `archgate check` runs both in CI and pre-push. +- **Automated:** The companion rule `scan-confined-to-fallback-modules` (this ADR) parses `src/engine/**/*.ts` via `ctx.ast()` ([ARCH-022](./ARCH-022-ast-aware-rule-context.md)) and walks the ESTree for real `.scan(...)` call sites, blocking any outside `glob-utils.ts`/`git-files.ts` — structural, not text-based, so a comment or string mentioning `.scan(` cannot be misreported. ARCH-020's `glob-scan-dot` rule covers `dot: true` on the remaining fallbacks. `archgate check` runs both in CI and pre-push. - **Manual:** Reviewers of `src/engine/` changes verify new file listings route through `glob-utils.ts` and that sandbox validation precedes the tracked/scan branch. - **Exceptions:** A new scan call site outside the two fallback modules requires updating this ADR (and its rule's allowlist) with justification approved by the maintainer. diff --git a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts index ce110afa..7a35443c 100644 --- a/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts +++ b/.archgate/adrs/ARCH-023-engine-file-listing-via-in-memory-git-tracked-matching.rules.ts @@ -3,13 +3,95 @@ /** * ARCH-023: file listing in src/engine/ must match in memory against the * git-tracked set. Bun.Glob scanning is fallback-only and confined to the - * two modules that implement the fallback. + * two modules that implement the fallback. Walks the ESTree via + * ctx.ast()/ctx.findAstNodes() (ARCH-022) for real `.scan(...)` call sites, + * so a comment or string mentioning `.scan(` is not a match. */ const SCAN_ALLOWED_FILES = new Set([ "src/engine/glob-utils.ts", "src/engine/git-files.ts", ]); +/** True for `.scan(...)` -- a non-computed `.scan` member call. */ +function isScanCall(node: EsTreeNode): boolean { + if (node.type !== "CallExpression") return false; + const callee = node.callee as EsTreeNode | undefined; + if (callee?.type !== "MemberExpression" || callee.computed === true) { + return false; + } + const property = callee.property as EsTreeNode | undefined; + return property?.type === "Identifier" && property.name === "scan"; +} + +/** + * Blank comments and string/template literals to spaces, keeping every + * newline, so line numbers computed against the result match the original + * source. Re-locates a call ctx.ast() already found structurally, since + * `loc` is not trustworthy for `"typescript"` (ARCH-022). Does not track + * regex literals, matching `source-positions.ts`. + */ +function blankNonCode(source: string): string { + let out = ""; + let i = 0; + const n = source.length; + while (i < n) { + const ch = source[i]; + const next = source[i + 1]; + if (ch === "/" && next === "/") { + while (i < n && source[i] !== "\n") { + out += " "; + i++; + } + continue; + } + if (ch === "/" && next === "*") { + out += " "; + i += 2; + while (i < n && !(source[i] === "*" && source[i + 1] === "/")) { + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i += 2; + } + continue; + } + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + out += " "; + i++; + while (i < n && source[i] !== quote) { + if (source[i] === "\\" && i + 1 < n) { + out += " "; + i += 2; + continue; + } + out += source[i] === "\n" ? "\n" : " "; + i++; + } + if (i < n) { + out += " "; + i++; + } + continue; + } + out += ch; + i++; + } + return out; +} + +/** 1-based line of the next `.scan(` at or after `fromIndex` in blanked `code`. */ +function nextScanLine( + code: string, + fromIndex: number +): { line: number | undefined; nextIndex: number } { + const idx = code.indexOf(".scan(", fromIndex); + if (idx === -1) return { line: undefined, nextIndex: fromIndex }; + return { line: code.slice(0, idx).split("\n").length, nextIndex: idx + 6 }; +} + export default { rules: { "scan-confined-to-fallback-modules": { @@ -21,28 +103,48 @@ export default { (f) => f.endsWith(".ts") && !SCAN_ALLOWED_FILES.has(f) ); - // Same call-site detection as ARCH-020's glob-scan-dot rule: capture - // the argument list of each scan call. `[^)]` spans newlines, so - // multi-line option objects are covered. - const callPattern = /\.scan\(([^)]*)\)/gu; - const checks = files.map(async (file) => { - let content: string; + let tree: EsTreeProgram; + try { + tree = await ctx.ast(file, "typescript"); + } catch { + return; + } + + const scanCalls = ctx + .findAstNodes(tree, "CallExpression") + .filter((node) => isScanCall(node)); + if (scanCalls.length === 0) return; + + // Sort by transpiled loc: not source-accurate for "typescript" + // (ARCH-022), but relative order survives transpilation, which + // only erases type-only syntax -- enough to pair each call with + // its re-located line below. + scanCalls.sort((a, b) => { + const lineDiff = + (a.loc?.start.line ?? 0) - (b.loc?.start.line ?? 0); + if (lineDiff !== 0) return lineDiff; + return (a.loc?.start.column ?? 0) - (b.loc?.start.column ?? 0); + }); + + let source: string; try { - content = await ctx.readFile(file); + source = await ctx.readFile(file); } catch { return; } + const code = blankNonCode(source); - for (const match of content.matchAll(callPattern)) { - const offset = match.index ?? 0; - const line = content.slice(0, offset).split("\n").length; + let cursor = 0; + for (const _call of scanCalls) { + const { line, nextIndex } = nextScanLine(code, cursor); + cursor = nextIndex; ctx.report.violation({ message: "Bun.Glob#scan() in src/engine/ is fallback-only and confined to glob-utils.ts/git-files.ts — walking the filesystem per rule re-introduces the traversal cost ARCH-023 eliminates", file, - line, + ...(line === undefined ? {} : { line }), fix: "Use listMatchingFiles() or matchTrackedFiles() from src/engine/glob-utils.ts; if a genuine new fallback is required, update ARCH-023 and its allowlist with maintainer approval", }); }