diff --git a/CONTEXT.md b/CONTEXT.md index 5bd892992..6c261800f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -750,9 +750,9 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr `DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.symptom=an assertion compares the return value of a path-returning function (resolveAgentDir, path.join, path.resolve, getPathX, computePathPrefix, etc.) to a HARDCODED forward-slash string literal like '/H/.config/opencode/agent' or 'C:/Users/...' — passes on POSIX (macOS/linux/ubuntu CI incl. gsd-test docker mirror, where path.join emits forward slashes so literal == actual), FAILS on windows-latest CI lane where path.join emits backslashes so literal != actual` `DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.examples=PR #1692 tests/stale-bake-guard.test.cjs resolveAgentDir suite: assert.equal(resolveAgentDir('opencode',{homedir:()=>'/H'}), '/H/.config/opencode/agent') — green on macOS+ubuntu (docker gate PASS 21101/21101), red on test (windows-latest,24) + full test (windows-latest,22, shard 2/3); same root cause as DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT but on the TEST side against a function return, not the production-markdown side` -`DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.detect=any assert*/expect call whose ACTUAL operand is a call to a path-returning fn (path.join, path.resolve, resolveAgentDir, getPathX, computePathPrefix, os.homedir(), path.dirname/basename) AND whose EXPECTED operand is a string literal containing '/' that does NOT first flow through .replace(/\\/g,'/'); the literal-vs-fnCall shape is the tripwire — assert.equal(pathFn(...), '/hardcoded/posix/path') is the violation; assert.equal(String(pathFn(...)).replace(/\\/g,'/'), '/hardcoded/posix/path') is the compliant form` +`DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.detect=any assert*/expect call whose ACTUAL operand is a call to a path-returning fn (path.join, path.resolve, resolveAgentDir, getPathX, computePathPrefix, os.homedir(), path.dirname/basename) AND whose EXPECTED operand is a string literal containing '/' that does NOT first flow through .replace(/\\/g,'/'); the literal-vs-fnCall shape is the tripwire — assert.equal(pathFn(...), '/hardcoded/posix/path') is the violation; assert.equal(String(pathFn(...)).replace(/\\/g,'/'), '/hardcoded/posix/path') is the compliant form; NOW mechanically enforced by the AST ESLint rule local/no-path-literal-in-assert (eslint-rules/no-path-literal-in-assert.cjs, ADR-1703 Phase 1 #1707) — platform-guard-aware (won't flag an assertion control-dependent on a process.platform !== 'win32' guard; eslint-rules/lib/platform-guard.cjs), fn list single-sourced as eslint-rules/lib/portability-vocab.cjs PATH_RETURNING_FNS (drift-guarded vs src/runtime-homes.cts)` `DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.fix-forward=normalize the ACTUAL value to POSIX before comparing: assert.equal(String(pathFn(...)).replace(/\\/g,'/'), '/posix/literal'). Do NOT instead path.join the expected value to match the platform separator — that passes on every platform but masks a malformed backslash-on-POSIX return (both sides wrong together). The .replace is idempotent on POSIX so it is safe unconditionally. For values that are conceptually never paths (null/undefined/numbers), no normalization needed.` -`DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.prevention=run npm run lint:ci (lint-windows-test-portability) before push — enhancement TBD to extend that lint to flag the literal-vs-pathFn assertion shape mechanically; treat the CI windows-latest lane as the only true Windows signal — gsd-test (Mac/Linux only) cannot substitute; ref umbrella DEFECT.WINDOWS-TEST-PORTABILITY and production-side analogue DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT` +`DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT.prevention=enforced at write-time (editor) and in CI by the AST ESLint rule local/no-path-literal-in-assert (error, scoped to tests/**/*.test.cjs in eslint.config.mjs; ADR-1703 Phase 1 #1707); inline suppression is banned out-of-band by tests/portability-rule-disable-ban.test.cjs (zero escape hatches — structure platform-specific code behind a recognized process.platform guard, never opt out); run npm run lint before push; treat the CI windows-latest lane as the only true Windows signal — gsd-test (Mac/Linux only) cannot substitute; ref umbrella DEFECT.WINDOWS-TEST-PORTABILITY and production-side analogue DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT` `DEFECT.PROMPT-INJECTION-SCAN-COLLISION-WITH-TESTS.symptom=scripts/prompt-injection-scan.sh flags a NEW test file as a finding because the test contains real injection payloads as fixtures (strings that match one of the scanner's PATTERNS — see scripts/prompt-injection-scan.sh lines 18-64) to prove the validator under test rejects them; scanner cannot distinguish fixture from real injection; CI security lane fails on the test that ADDS the security validation` `DEFECT.PROMPT-INJECTION-SCAN-COLLISION-WITH-TESTS.examples=PR #1622 commit 4ed208e74 added convertClaudeCommandToWindsurfWorkflow commandName validation with 22 malicious-name fixtures; scanner matched an instruction-override phrase at tests/windsurf-conversion.test.cjs:122; CI security lane failed even though the test is the security control` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c2d14a389..a7a5b0a54 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -817,6 +817,7 @@ The following checks run on every PR in addition to the test suite: | Job | What it checks | How to pass | |-----|----------------|-------------| | `Lint — ESLint` | No source-grep tests (see above), via the `local/no-source-grep` rule | Replace with `runGsdTools()` behavioral tests, or add `// allow-test-rule: ` | +| `Lint — cross-platform portability` | Windows-portability defects in tests, via `local/no-path-literal-in-assert` (more rules land per [ADR-1703](docs/adr/1703-portability-enforcement-architecture.md)) — e.g. a path-returning call asserted against a hardcoded `/`-literal | Normalize the actual: `String(pathFn(...)).replace(/\\/g, '/')`, or structure platform-specific code behind a `process.platform !== 'win32'` guard. **No `eslint-disable`** — see [cross-platform-portability-rules.md](docs/contributing/cross-platform-portability-rules.md) | Run locally before pushing: `npm run lint` (or `npx eslint .`) diff --git a/docs/contributing/cross-platform-portability-rules.md b/docs/contributing/cross-platform-portability-rules.md new file mode 100644 index 000000000..d1d15095d --- /dev/null +++ b/docs/contributing/cross-platform-portability-rules.md @@ -0,0 +1,90 @@ +# Cross-platform portability lint rules + +GSD must run on Windows as well as macOS/Linux. A family of AST-based ESLint rules (the +`local/*` plugin) enforces the `DEFECT.WINDOWS-*` portability classes documented in +[`CONTEXT.md`](../../CONTEXT.md) **at write-time (in your editor) and in CI**, so a +Windows-only defect is caught before it ships — not after it reaches the `windows-latest` CI +lane. The architecture and rationale are in [ADR-1703](../adr/1703-portability-enforcement-architecture.md); +this page is the practical reference + how-to. + +These rules are **hard-fail with zero escape hatches**: there is no `// windows-portability-ok:` +comment and no `eslint-disable` for them (a `tests/portability-rule-disable-ban.test.cjs` check, +running outside ESLint, fails the build if you try). Legitimately platform-specific code must be +*structured* so the rule recognizes it (see "Platform guards" below) — not annotated around. + +## Reference — the rules + +| Rule | Flags | Surface | +|---|---|---| +| `local/no-path-literal-in-assert` | An `assert.equal`/`strictEqual`/`deepEqual`/`deepStrictEqual` or `expect(...).toBe`/`toEqual`/`toStrictEqual` where one operand is a **path-returning function call** and the other is a **hardcoded `/`-string literal** not normalized to POSIX. | `tests/**/*.test.cjs` | + +(More rules land per the epic — see ADR-1703's catalog and [epic #1702](https://github.com/open-gsd/gsd-core/issues/1702).) + +The set of path-returning functions is single-sourced in +[`eslint-rules/lib/portability-vocab.cjs`](../../eslint-rules/lib/portability-vocab.cjs) as +`PATH_RETURNING_FNS` (Node's `path.*`/`os.homedir`/`os.tmpdir` plus the project resolvers such as +`getGlobalConfigDir`, `resolveAgentDir`, `computePathPrefix`, …). A drift-guard test +(`tests/portability-vocab-drift.test.cjs`) parses `src/runtime-homes.cts` and **fails CI if a new +path resolver is added but not registered** in that list. + +## How-to — fix a `no-path-literal-in-assert` violation + +Why it fails on Windows: `path.join('a','b')` returns `a/b` on POSIX but `a\b` on Windows, so +`assert.equal(path.join('a','b'), '/a/b')` passes on your Mac/Linux machine and the docker gate, +then fails only on the `windows-latest` lane. + +**Fix: normalize the ACTUAL operand to POSIX before comparing** — this is idempotent on POSIX +(a no-op when there are no backslashes) and *reveals* a malformed return rather than masking it: + +```js +// ❌ flagged +assert.strictEqual(getGlobalConfigDir('claude'), '/custom/claude'); + +// ✅ compliant +assert.strictEqual(String(getGlobalConfigDir('claude')).replace(/\\/g, '/'), '/custom/claude'); +``` + +Do **not** instead wrap the *expected* literal in `path.join(...)` to match the platform +separator — that passes everywhere but masks a wrong backslash-on-POSIX return (both sides wrong +together). Recognized normalizers: `.replace(/\\/g,'/')`, `.replace(/[\\/]/g,'/')`, +`.replaceAll('\\','/')`, `.replaceAll(path.sep,'/')`, `.split(path.sep).join('/')`, +`toPosixPath(...)`. + +## Platform guards (the only "escape" — by structure, not annotation) + +If an assertion is *genuinely* POSIX-only, gate it behind a Windows platform check the rule +recognizes — it then won't flag the guarded code. Recognized shapes: + +```js +if (process.platform !== 'win32') { + assert.equal(path.join(a, b), '/a/b'); // guarded → not flagged +} + +if (process.platform === 'win32') return; // early-return guard +assert.equal(path.join(a, b), '/a/b'); // → not flagged + +const isWindows = process.platform === 'win32'; // hoisted boolean (any name, binding-resolved) +if (!isWindows) assert.equal(path.join(a, b), '/a/b'); // → not flagged +``` + +The guard is recognized by control-dependence (it must actually dominate the assertion), is +binding-aware (a reassigned or `false`-initialized variable is not trusted), and handles +`os.platform()` and `node:test` skip returns. See +[`eslint-rules/lib/platform-guard.cjs`](../../eslint-rules/lib/platform-guard.cjs). + +## How-to — add a new path resolver + +When you add a function that returns a filesystem path (e.g. in `src/runtime-homes.cts`), add its +name to `PATH_RETURNING_FNS` in `eslint-rules/lib/portability-vocab.cjs`. The drift-guard test +will fail until you do. + +## Known boundaries + +The rule matches by spelling and inspects the direct operand (or a `String()` wrapper): + +- It assumes `path`/`os` are the standard modules and the resolver names are the project's — a + local variable that *shadows* one of those names in a test file is out of scope. +- Deeper wrapping (e.g. `realpathSync(path.join(...))`, `.toLowerCase()` on a path) is not + inspected; assert against the path call directly or its `String(...)` wrap. +- For a genuine explicit-dir *pass-through* assertion (a resolver that returns its input + verbatim), the `String(...).replace(/\\/g,'/')` remedy is a harmless no-op. diff --git a/eslint-rules/lib/platform-guard.cjs b/eslint-rules/lib/platform-guard.cjs new file mode 100644 index 000000000..d3009de27 --- /dev/null +++ b/eslint-rules/lib/platform-guard.cjs @@ -0,0 +1,627 @@ +'use strict'; + +/** + * platform-guard.cjs — precision backbone for no-path-literal-in-assert. + * + * Exported API: + * classifyPlatformTest(node) → 'windows' | 'not-windows' | null + * isWindowsExcludedNode(node, sourceCode) → boolean + * + * Shapes handled by isWindowsExcludedNode: + * + * (A) Consequent of `if () { ... }`: + * if (process.platform !== 'win32') { } + * + * (B) Alternate of `if () { ... } else { }`: + * if (process.platform === 'win32') { ... } else { } + * + * (C) A preceding sibling IfStatement that is a Windows early-return guard, + * making the node unreachable on Windows: + * if (process.platform === 'win32') return; + * if (process.platform === 'win32') return t.skip(...); + * if (process.platform === 'win32') { ...; return; } + * + * (D) Hoisted windows-boolean consumed by (A)/(B)/(C). Both the conventional + * names (isWindows, IS_WINDOWS, isWin, onWindows) AND arbitrary-named + * variables (e.g. `const winFlag = process.platform === 'win32'`) are + * resolved by looking up the variable's initializer in the enclosing scope + * and classifying that expression. Negation (`!winFlag`) is applied after + * the lookup, so `if (!winFlag)` is correctly recognized as a not-windows + * guard when winFlag was initialized to a windows test. + * + * If a shape is genuinely ambiguous, the function returns false so the rule + * errs toward reporting — the fix is to teach this helper, never an opt-out. + */ + +/** + * Identifier names conventionally used for "is this Windows?" booleans. + * @type {Set} + */ +const WINDOWS_BOOL_NAMES = new Set(['isWindows', 'IS_WINDOWS', 'isWin', 'onWindows']); + +/** + * Classify a test expression as a Windows test, not-Windows test, or unrelated. + * + * Recognized forms: + * - `process.platform === 'win32'` → 'windows' + * - `process.platform !== 'win32'` → 'not-windows' + * - `os.platform() === 'win32'` → 'windows' + * - `os.platform() !== 'win32'` → 'not-windows' + * - `isWindows` / `IS_WINDOWS` / etc → 'windows' + * - `!isWindows` / etc → 'not-windows' + * + * @param {import('eslint').Rule.Node} node + * @returns {'windows' | 'not-windows' | null} + */ +function classifyPlatformTest(node) { + if (!node) return null; + + // Binary: X === 'win32' or X !== 'win32' or 'win32' === X etc. + if (node.type === 'BinaryExpression' && (node.operator === '===' || node.operator === '!==')) { + const { left, right, operator } = node; + if (_isPlatformExpr(left) && _isWin32Literal(right)) { + return operator === '===' ? 'windows' : 'not-windows'; + } + if (_isPlatformExpr(right) && _isWin32Literal(left)) { + return operator === '===' ? 'windows' : 'not-windows'; + } + } + + // Bare identifier: isWindows, IS_WINDOWS, isWin, onWindows + if (node.type === 'Identifier' && WINDOWS_BOOL_NAMES.has(node.name)) { + return 'windows'; + } + + // Negated: !isWindows + if ( + node.type === 'UnaryExpression' && + node.operator === '!' && + node.argument.type === 'Identifier' && + WINDOWS_BOOL_NAMES.has(node.argument.name) + ) { + return 'not-windows'; + } + + return null; +} + +/** True if node is `process.platform` or `os.platform()` */ +function _isPlatformExpr(node) { + // process.platform + if ( + node.type === 'MemberExpression' && + !node.computed && + node.object.type === 'Identifier' && + node.object.name === 'process' && + node.property.type === 'Identifier' && + node.property.name === 'platform' + ) { + return true; + } + // os.platform() + if ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + !node.callee.computed && + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'os' && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'platform' + ) { + return true; + } + return false; +} + +/** True if node is the string literal 'win32' */ +function _isWin32Literal(node) { + return node.type === 'Literal' && node.value === 'win32'; +} + +/** + * Returns true when `targetNode` only executes on non-Windows because it is + * control-dependent on one of the recognized Windows-guard shapes. + * + * @param {import('eslint').Rule.Node} targetNode + * @param {import('eslint').SourceCode} sourceCode + * @returns {boolean} + */ +function isWindowsExcludedNode(targetNode, sourceCode) { + // Walk ancestors bottom-up to find a guarding IfStatement. + const ancestors = _getAncestors(targetNode, sourceCode); + + for (let i = ancestors.length - 1; i >= 0; i--) { + const ancestor = ancestors[i]; + + if (ancestor.type !== 'IfStatement') continue; + + const testClassification = _classifyPlatformTestWithHoisting( + ancestor.test, + targetNode, + sourceCode + ); + + if (!testClassification) continue; + + // Determine which branch targetNode is in + const inConsequent = _containsNode(ancestor.consequent, targetNode); + const inAlternate = ancestor.alternate != null && _containsNode(ancestor.alternate, targetNode); + + if (inConsequent && testClassification === 'not-windows') { + // if (platform !== 'win32') { } → excluded + return true; + } + if (inAlternate && testClassification === 'windows') { + // if (platform === 'win32') { … } else { } → excluded + return true; + } + } + + // Check for early-return guards in the same block as the target node + if (_hasEarlyWindowsReturnBefore(targetNode, sourceCode)) return true; + + return false; +} + +/** + * Classify a test expression, resolving hoisted windows-boolean variables. + * + * C3 (binding-aware): for bare Identifier or !Identifier test forms, this + * function resolves the variable's binding in the lexical scope: + * + * 1. If `sourceCode.getScope` is available (real ESLint rule context), use + * it to resolve the NEAREST binding of the identifier, walking scope.upper + * so inner shadows take priority. If a binding is found in-file: + * a. Classify the initializer — not a platform test → return null. + * b. Check for reassignment (any write reference after init) → return null. + * c. Otherwise return the init classification (with negation applied). + * If NO in-file binding exists (global/import), fall through to the name + * heuristic below. + * + * 2. AST-walk fallback (unit-test contexts without live scope): for identifiers + * NOT in WINDOWS_BOOL_NAMES, use _resolveIdentifierInitBindingAware which + * respects inner shadows and reassignment. For names IN WINDOWS_BOOL_NAMES + * with no in-file binding found, apply the name heuristic. + * + * 3. Direct platform expressions (`process.platform === 'win32'`, etc.) are + * classified directly (no change from before). + * + * @param {import('eslint').Rule.Node} testNode — the IfStatement's .test + * @param {import('eslint').Rule.Node} targetNode — the node we are checking + * @param {import('eslint').SourceCode} sourceCode + * @returns {'windows' | 'not-windows' | null} + */ +function _classifyPlatformTestWithHoisting(testNode, targetNode, sourceCode) { + // Step 1: try direct classification of platform expressions + // (BinaryExpression process.platform === 'win32', etc.) + // Do NOT use classifyPlatformTest here for the bare-identifier forms — + // we want binding-aware resolution for those. + const directBinary = _classifyPlatformExprOnly(testNode); + if (directBinary) return directBinary; + + // Extract the identifier and negation flag from the test expression. + let identNode = null; + let negated = false; + + if (testNode.type === 'Identifier') { + identNode = testNode; + negated = false; + } else if ( + testNode.type === 'UnaryExpression' && + testNode.operator === '!' && + testNode.argument.type === 'Identifier' + ) { + identNode = testNode.argument; + negated = true; + } + + if (!identNode) return null; + + const identName = identNode.name; + + // Step 2: binding-aware resolution via ESLint scope (when available). + if (typeof sourceCode.getScope === 'function') { + const scopeResult = _resolveIdentifierViaScope(identNode, identName, negated, sourceCode); + // scopeResult is one of: + // 'windows' | 'not-windows' — binding found, init classifies as platform test + // null — binding found but doesn't classify (or reassigned) + // 'no-binding' — no in-file binding; fall through to name heuristic + if (scopeResult !== 'no-binding') return scopeResult; + // Fall through: no in-file binding → name heuristic below. + } else { + // AST-walk fallback (unit-test contexts without live scope). + // Use binding-aware AST walk for ALL names. + const astResult = _resolveIdentifierInitBindingAware(identName, identNode, targetNode, sourceCode); + if (astResult !== 'no-binding') { + if (!astResult) return null; + return negated + ? (astResult === 'windows' ? 'not-windows' : 'windows') + : astResult; + } + // No binding found via AST walk → fall through to name heuristic. + } + + // Step 3: name heuristic — only for globally-recognized Windows bool names + // that have no in-file binding (imported/global constants like `isWindows` + // imported from a test helper). + if (WINDOWS_BOOL_NAMES.has(identName)) { + return negated ? 'not-windows' : 'windows'; + } + + return null; +} + +/** + * Classify a BinaryExpression or os.platform() call as a platform test. + * Does NOT handle bare Identifiers or !Identifier — those need binding-aware + * resolution (handled above in _classifyPlatformTestWithHoisting). + * + * @param {import('eslint').Rule.Node} node + * @returns {'windows' | 'not-windows' | null} + */ +function _classifyPlatformExprOnly(node) { + if (!node) return null; + if (node.type === 'BinaryExpression' && (node.operator === '===' || node.operator === '!==')) { + const { left, right, operator } = node; + if (_isPlatformExpr(left) && _isWin32Literal(right)) { + return operator === '===' ? 'windows' : 'not-windows'; + } + if (_isPlatformExpr(right) && _isWin32Literal(left)) { + return operator === '===' ? 'windows' : 'not-windows'; + } + } + return null; +} + +/** + * Resolve an identifier's binding via ESLint scope analysis. + * + * Walks `scope.upper` from the identifier's immediate scope to find the NEAREST + * binding (so inner shadows take priority over outer declarations). + * + * @param {import('eslint').Rule.Node} identNode + * @param {string} identName + * @param {boolean} negated + * @param {import('eslint').SourceCode} sourceCode + * @returns {'windows' | 'not-windows' | null | 'no-binding'} + */ +function _resolveIdentifierViaScope(identNode, identName, negated, sourceCode) { + let scope; + try { + scope = sourceCode.getScope(identNode); + } catch (_) { + return 'no-binding'; + } + if (!scope) return 'no-binding'; + + // Walk scope chain from innermost to outermost; take the NEAREST binding. + let s = scope; + while (s) { + const variable = s.variables.find(v => v.name === identName); + if (variable) { + // Found an in-file binding (the NEAREST one wins — inner shadow beats outer). + const defs = variable.defs; + if (!defs || defs.length === 0) { + // Binding exists but no declarator (e.g. function parameter) — no init. + return null; + } + const decl = defs[0].node; // VariableDeclarator + if (!decl || !decl.init) { + // No initializer (e.g. `let w;`) → not a platform test. + return null; + } + // Classify the initializer. + const cls = classifyPlatformTest(decl.init); + if (!cls) return null; // init is not a platform test + + // Check for reassignment: any write reference that is NOT the initialization. + const isReassigned = variable.references.some( + ref => ref.isWrite() && !ref.init + ); + if (isReassigned) return null; + + // Valid platform guard binding found. + return negated + ? (cls === 'windows' ? 'not-windows' : 'windows') + : cls; + } + s = s.upper; + } + + // No binding found in any scope — treat as a global/imported name. + return 'no-binding'; +} + +/** + * Binding-aware AST-walk resolver — used as a fallback when + * sourceCode.getScope is not available. + * + * Walks ancestor blocks from innermost to outermost, looking for a + * VariableDeclaration of `name` that precedes `targetNode`. + * + * Key differences from the old _resolveIdentifierInit: + * - Returns 'no-binding' when NO declaration of `name` is found in any + * ancestor block (so the caller can apply the name heuristic). + * - Returns null (not 'no-binding') when a declaration IS found but: + * • its init does not classify as a platform test, OR + * • the variable is reassigned (any ExpressionStatement `name = ...` + * appears before targetNode after the declaration), OR + * • an inner-scope declaration shadows the outer one (inner wins). + * - Stops at the FIRST block that declares `name` (innermost shadow wins). + * + * @param {string} name + * @param {import('eslint').Rule.Node} identNode — the Identifier AST node (for inner-shadow check) + * @param {import('eslint').Rule.Node} targetNode — the assert CallExpression node + * @param {import('eslint').SourceCode} sourceCode + * @returns {'windows' | 'not-windows' | null | 'no-binding'} + */ +function _resolveIdentifierInitBindingAware(name, identNode, targetNode, sourceCode) { + const ancestors = _getAncestors(targetNode, sourceCode); + + for (let i = ancestors.length - 1; i >= 0; i--) { + const block = ancestors[i]; + if (block.type !== 'BlockStatement' && block.type !== 'Program') continue; + + const stmts = block.body; + if (!stmts) continue; + + // Find which direct-child statement contains the targetNode. + let targetIdx = -1; + for (let j = 0; j < stmts.length; j++) { + if (_containsNode(stmts[j], targetNode) || stmts[j] === targetNode) { + targetIdx = j; + break; + } + } + if (targetIdx === -1) continue; + + // Scan all preceding siblings in this block for a declaration of `name`. + let foundDecl = null; + let foundDeclIdx = -1; + for (let j = 0; j < targetIdx; j++) { + const stmt = stmts[j]; + if (stmt.type !== 'VariableDeclaration') continue; + for (const decl of stmt.declarations) { + if ( + decl.type === 'VariableDeclarator' && + decl.id && + decl.id.type === 'Identifier' && + decl.id.name === name + ) { + foundDecl = decl; + foundDeclIdx = j; + break; + } + } + if (foundDecl) break; + } + + if (foundDecl) { + // A binding was found in this block. Innermost shadow wins — stop climbing. + + // No initializer → not a platform guard. + if (!foundDecl.init) return null; + + // Init must classify as a platform test. + const cls = classifyPlatformTest(foundDecl.init); + if (!cls) return null; + + // Check for reassignment: any ExpressionStatement `name = ...` between + // foundDeclIdx and targetIdx. + if (_hasReassignmentBetween(name, stmts, foundDeclIdx + 1, targetIdx)) { + return null; + } + + return cls; + } + + // No declaration found in this block — continue climbing to outer scope. + } + + // No declaration found in any ancestor block. + return 'no-binding'; +} + +/** + * Returns true when any statement in stmts[fromIdx..toIdx) is an assignment + * expression ` = ...` (simple reassignment, not an initializer). + * + * @param {string} name + * @param {Array} stmts + * @param {number} fromIdx — inclusive + * @param {number} toIdx — exclusive + * @returns {boolean} + */ +function _hasReassignmentBetween(name, stmts, fromIdx, toIdx) { + for (let j = fromIdx; j < toIdx; j++) { + const stmt = stmts[j]; + if ( + stmt.type === 'ExpressionStatement' && + stmt.expression.type === 'AssignmentExpression' && + stmt.expression.left.type === 'Identifier' && + stmt.expression.left.name === name + ) { + return true; + } + } + return false; +} + +/** + * Legacy alias kept for _isWindowsEarlyReturn's call to + * _classifyPlatformTestWithHoisting, which uses stmt (the IfStatement) as + * the "targetNode" to look up hoisting context. No callers outside that path. + * + * @param {string} name + * @param {import('eslint').Rule.Node} targetNode + * @param {import('eslint').SourceCode} sourceCode + * @returns {'windows' | 'not-windows' | null} + */ +function _resolveIdentifierInit(name, targetNode, sourceCode) { + const result = _resolveIdentifierInitBindingAware(name, null, targetNode, sourceCode); + if (result === 'no-binding') return null; + return result; +} + +/** + * True when there is a preceding sibling statement (before targetNode in ANY + * enclosing block — function body, nested block, or Program) that is an + * IfStatement whose consequence is a Windows-only early return — making + * targetNode unreachable on Windows. + * + * Recognized patterns: + * if (windowsTest) return; + * if (windowsTest) return ; + * if (windowsTest) { …; return; } — block with a return + * + * C2 fix: climbs ALL ancestor blocks, not just the innermost one. + * An early-return guard in a function body before a nested if-block that + * contains targetNode is equally valid (control cannot reach targetNode on Windows + * because the outer return fired first). + */ +function _hasEarlyWindowsReturnBefore(targetNode, sourceCode) { + const ancestors = _getAncestors(targetNode, sourceCode); + + // Walk ALL ancestor blocks bottom-up (innermost first). + for (let i = ancestors.length - 1; i >= 0; i--) { + const block = ancestors[i]; + if (block.type !== 'BlockStatement' && block.type !== 'Program') continue; + + const stmts = block.body; + if (!stmts) continue; + + // Find targetNode's position in this block's statements. + // targetNode might be nested inside a statement; we need the direct-child index. + let targetStmtIdx = -1; + for (let j = 0; j < stmts.length; j++) { + if (_containsNode(stmts[j], targetNode) || stmts[j] === targetNode) { + targetStmtIdx = j; + break; + } + } + if (targetStmtIdx === -1) continue; + + // Scan preceding siblings in this block for a Windows early-return guard. + for (let j = 0; j < targetStmtIdx; j++) { + const stmt = stmts[j]; + if (_isWindowsEarlyReturn(stmt, sourceCode, block)) return true; + } + + // No guard found in this block — continue climbing to outer blocks. + // (Unlike the IfStatement-branch check, an early-return in an outer block + // before the nested block that contains targetNode is equally protective.) + } + + return false; +} + +/** + * True when `stmt` is `if () return;` / `if () return ;` + * / `if () { …; return; }` with no `else`. + */ +function _isWindowsEarlyReturn(stmt, sourceCode, _block) { + if (stmt.type !== 'IfStatement') return false; + if (stmt.alternate != null) return false; // has else → not a simple guard + + const testClass = _classifyPlatformTestWithHoisting(stmt.test, stmt, sourceCode); + if (testClass !== 'windows') return false; + + // Consequent must contain a return statement + const consequent = stmt.consequent; + if (!consequent) return false; + + if (consequent.type === 'ReturnStatement') return true; + + if (consequent.type === 'BlockStatement') { + // Only direct-child ReturnStatements are checked. Nested/conditional returns + // (e.g. inside inner if-blocks) are intentionally NOT treated as guards — + // this is the sound conservative choice: we only suppress the report when + // we are certain execution cannot continue on Windows. + return consequent.body.some(s => s.type === 'ReturnStatement'); + } + + return false; +} + +/** + * Get the ancestor chain for `node` using the sourceCode API. + * Returns an array from outermost to innermost (not including node itself). + */ +function _getAncestors(node, sourceCode) { + // ESLint 8+: sourceCode.getAncestors(node) + if (sourceCode.getAncestors) { + try { + return sourceCode.getAncestors(node); + } catch (_) { + // Fallback: not always available outside a rule handler + } + } + // Fallback: traverse the AST manually (used in unit tests) + return _findAncestors(sourceCode.ast, node); +} + +/** + * Find the ancestor chain by walking the AST. + * Returns array from root to immediate parent of target. + * Skips `parent` and other cycle-inducing keys. + */ +function _findAncestors(root, target) { + const chain = []; + function walk(node, ancestors) { + if (!node || typeof node !== 'object') return false; + if (node === target) { + chain.push(...ancestors); + return true; + } + for (const key of Object.keys(node)) { + if (SKIP_KEYS.has(key)) continue; + const child = node[key]; + if (Array.isArray(child)) { + for (const item of child) { + if (item && typeof item === 'object' && item.type) { + if (walk(item, [...ancestors, node])) return true; + } + } + } else if (child && typeof child === 'object' && child.type) { + if (walk(child, [...ancestors, node])) return true; + } + } + return false; + } + walk(root, []); + return chain; +} + +/** + * Keys to skip when traversing an AST node to avoid circular parent refs. + * ESLint attaches `parent` to every node, which creates cycles. + */ +const SKIP_KEYS = new Set(['parent', 'tokens', 'comments']); + +/** + * Returns true when `container` node contains `target` node (by identity). + * Skips `parent` and other non-AST keys to avoid circular reference loops. + */ +function _containsNode(container, target) { + if (!container || typeof container !== 'object') return false; + if (container === target) return true; + for (const key of Object.keys(container)) { + if (SKIP_KEYS.has(key)) continue; + const child = container[key]; + if (Array.isArray(child)) { + for (const item of child) { + if (item && typeof item === 'object' && item.type) { + if (_containsNode(item, target)) return true; + } + } + } else if (child && typeof child === 'object' && child.type) { + if (_containsNode(child, target)) return true; + } + } + return false; +} + +module.exports = { + classifyPlatformTest, + isWindowsExcludedNode, +}; diff --git a/eslint-rules/lib/portability-vocab.cjs b/eslint-rules/lib/portability-vocab.cjs new file mode 100644 index 000000000..017068e37 --- /dev/null +++ b/eslint-rules/lib/portability-vocab.cjs @@ -0,0 +1,337 @@ +'use strict'; + +/** + * portability-vocab.cjs — single source of truth for path-related portability. + * + * PATH_RETURNING_FNS: canonical list of function calls (Node builtins and + * project resolvers) that return a filesystem path. The drift-guard test + * (tests/portability-vocab-drift.test.cjs) enforces completeness against + * src/runtime-homes.cts's exported path-returning functions. + * + * EXTEND THIS LIST when adding a new path resolver to the codebase. + * The drift-guard test will fail if you forget. + * + * ── Known boundaries ───────────────────────────────────────────────────────── + * + * Matching is by spelling: `path`, `os`, and the project resolver names below + * are assumed to refer to the standard Node modules / project resolver exports. + * A local variable that shadows one of these names (e.g. `const path = …`) is + * out of scope — the helpers treat it as the real module. + * + * isPosixNormalizerCall inspects only the DIRECT argument of the call node; + * deeper nesting (e.g. `String(path.join(...)).toLowerCase().replace(/\\/g,'/')`) + * is not covered — only the outermost call and one level of String() cast are + * visible to the rule. + */ + +/** + * Canonical set of function names (dotted or bare) that return filesystem paths. + * + * Format: + * - "path.join" → MemberExpression: object=Identifier{path}, property=Identifier{join} + * - "os.homedir" → MemberExpression: object=Identifier{os}, property=Identifier{homedir} + * - "getGlobalDir" → Identifier callee with that name + */ +const PATH_RETURNING_FNS = [ + // ── Node built-ins ────────────────────────────────────────────────────────── + 'path.join', + 'path.resolve', + 'path.dirname', + 'path.basename', + 'path.normalize', + 'path.relative', + 'os.homedir', + 'os.tmpdir', + + // ── Project resolvers (src/runtime-homes.cts exports + install.js helpers) ── + // Add bare function names here; dotted forms (e.g. obj.resolveX) are not used + // in the test corpus because these are module-level exports, not methods. + 'resolveAgentDir', + 'getGlobalConfigDir', + 'getGlobalSkillsBase', + 'getGlobalSkillDir', + 'getGlobalSkillDisplayPath', + 'resolveSkillsBaseFromDescriptor', + 'resolveConfigHomeFromDescriptor', + 'resolveKimiGlobalDir', + 'resolveAntigravityGlobalDir', + 'getGlobalDir', + 'getConfigDirFromHome', + 'resolveKiloConfigPath', + 'resolveOpencodeConfigPath', + 'computePathPrefix', + 'expandHome', + 'getPathX', + 'normalizeInstallRelativePath', + 'toPosixPath', +]; + +/** + * Returns true when `node` is a CallExpression whose callee matches one of the + * PATH_RETURNING_FNS entries. + * + * Handles two call shapes: + * - Dotted: path.join(…) → callee is MemberExpression{object: Identifier, property: Identifier} + * - Bare: getGlobalDir() → callee is Identifier + * + * @param {import('eslint').Rule.Node} node - AST node to inspect + * @returns {boolean} + */ +function isPathReturningCall(node) { + if (!node || node.type !== 'CallExpression') return false; + const callee = node.callee; + + // Dotted call: path.join, os.homedir, etc. + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.object.type === 'Identifier' && + callee.property.type === 'Identifier' + ) { + const dotted = `${callee.object.name}.${callee.property.name}`; + if (PATH_RETURNING_FNS.includes(dotted)) return true; + } + + // Bare call: getGlobalConfigDir(), resolveKimiGlobalDir(), etc. + if (callee.type === 'Identifier') { + if (PATH_RETURNING_FNS.includes(callee.name)) return true; + } + + return false; +} + +/** + * Returns true when `node` is a string literal (or a template literal with no + * expressions) whose value contains '/' and does NOT look like a URL. + * + * URL exclusion: value starts with 'http://' or 'https://'. + * + * @param {import('eslint').Rule.Node} node + * @returns {boolean} + */ +function isPosixSlashStringLiteral(node) { + if (!node) return false; + + // Plain string literal + if (node.type === 'Literal' && typeof node.value === 'string') { + const v = node.value; + if (!v.includes('/')) return false; + if (v.startsWith('http://') || v.startsWith('https://')) return false; + return true; + } + + // Template literal with no expressions (static): `some/path` + if (node.type === 'TemplateLiteral' && node.expressions.length === 0) { + const v = node.quasis[0]?.value?.cooked ?? ''; + if (!v.includes('/')) return false; + if (v.startsWith('http://') || v.startsWith('https://')) return false; + return true; + } + + return false; +} + +/** + * Returns true when `node` is a CallExpression that normalizes its first + * argument to POSIX-style slashes. + * + * Recognized shapes: + * 1. .replace(/\\/g, '/') — regex /\\/g with replacement '/' + * 2. .replace(/[\\/]/g, '/') — regex /[\\/]/g with replacement '/' + * 3. .replaceAll('\\', '/') — literal backslash to slash + * 4. .replaceAll(path.sep, '/') — path.sep to slash + * 5. .split(path.sep).join('/') — split-join idiom + * 6. toPosixPath() — explicit wrapper + * + * Note: for replace(), we REQUIRE the 'g' flag on the regex AND the regex + * source must actually target backslashes (source `\\` or `[\\/]`). + * A regex like /foo/g or /\//g does NOT qualify. + * + * @param {import('eslint').Rule.Node} node + * @returns {boolean} + */ +function isPosixNormalizerCall(node) { + if (!node || node.type !== 'CallExpression') return false; + const callee = node.callee; + + // toPosixPath() + if (callee.type === 'Identifier' && callee.name === 'toPosixPath') return true; + + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' + ) { + const method = callee.property.name; + const args = node.arguments; + + // .replace(regex, '/') + // REQUIRE: g flag + regex source must target backslashes: `\\` or `[\\/]` + if (method === 'replace' && args.length >= 2) { + const regexArg = args[0]; + const replacementArg = args[1]; + if ( + regexArg.type === 'Literal' && + regexArg.regex != null && + regexArg.regex.flags.includes('g') && + _isBackslashTargetingRegex(regexArg.regex.pattern) && + _isSlashReplacement(replacementArg) + ) { + return true; + } + } + + // .replaceAll(sep, '/') + if (method === 'replaceAll' && args.length >= 2) { + const sepArg = args[0]; + const replacementArg = args[1]; + if (_isSlashReplacement(replacementArg)) { + // replaceAll('\\', '/') or replaceAll('\\\\', '/') or replaceAll(path.sep, '/') + if (_isBackslashLiteral(sepArg)) return true; + if (_isPathSep(sepArg)) return true; + } + } + + // .split(path.sep).join('/') + // The callee is .join — check the object for .split(path.sep) + if (method === 'join' && args.length >= 1 && _isSlashReplacement(args[0])) { + const splitCall = callee.object; + if ( + splitCall.type === 'CallExpression' && + splitCall.callee.type === 'MemberExpression' && + !splitCall.callee.computed && + splitCall.callee.property.type === 'Identifier' && + splitCall.callee.property.name === 'split' && + splitCall.arguments.length >= 1 && + _isPathSep(splitCall.arguments[0]) + ) { + return true; + } + } + } + + return false; +} + +/** True if node is the replacement '/' string literal */ +function _isSlashReplacement(node) { + return node && node.type === 'Literal' && node.value === '/'; +} + +/** + * True if regexPattern (the raw regex source string, as stored in the AST's + * `.regex.pattern` field) actually targets backslashes. + * + * Accepted: exactly `\\` (two-char, two backslashes: matches one backslash) + * exactly `[\\/]` (five-char: backslash-or-forward-slash charset) + * Rejected: `foo`, `\/` (forward-slash only), anything else. + * + * @param {string} pattern — the AST `.regex.pattern` string + * @returns {boolean} + */ +function _isBackslashTargetingRegex(pattern) { + // Pattern `\\` (two backslash chars in the regex) — matches a single backslash + if (pattern === '\\\\') return true; + // Pattern `[\\/]` (backslash-or-forward-slash charset) — five chars + if (pattern === '[\\\\/]') return true; + return false; +} + +/** True if node is a backslash literal ('\\' or '\\\\') */ +function _isBackslashLiteral(node) { + if (!node || node.type !== 'Literal') return false; + return node.value === '\\' || node.value === '\\\\'; +} + +/** True if node is path.sep */ +function _isPathSep(node) { + return ( + node && + node.type === 'MemberExpression' && + !node.computed && + node.object.type === 'Identifier' && + node.object.name === 'path' && + node.property.type === 'Identifier' && + node.property.name === 'sep' + ); +} + +/** + * If `node` is `String()`, return ``; otherwise return `node` as-is. + * Allows the rule to see through String() casts on path expressions. + * + * @param {import('eslint').Rule.Node} node + * @returns {import('eslint').Rule.Node} + */ +function unwrapString(node) { + if ( + node && + node.type === 'CallExpression' && + node.callee.type === 'Identifier' && + node.callee.name === 'String' && + node.arguments.length === 1 + ) { + return node.arguments[0]; + } + return node; +} + +/** + * If `node` is a method-call chain of the form `.replace(...)`, + * `.replaceAll(...)`, or `.split(...).join(...)` that is + * NOT a valid POSIX normalizer (i.e. `isPosixNormalizerCall(node)` is false), + * return the receiver (the `.object` of the callee MemberExpression). + * + * This lets the rule detect: + * `path.join(a,b).replace(/foo/g, '/')` → not a normalizer, but the + * receiver `path.join(a,b)` IS a path-returning call → violation. + * + * Only peels ONE layer. The caller is responsible for checking the peeled node. + * Returns `null` when `node` is already a valid normalizer or is not a method chain. + * + * @param {import('eslint').Rule.Node} node + * @returns {import('eslint').Rule.Node | null} + */ +function unwrapNonNormalizerMethodChain(node) { + if (!node || node.type !== 'CallExpression') return null; + // If it IS a valid normalizer, do NOT peel — the caller already handled that. + if (isPosixNormalizerCall(node)) return null; + + const callee = node.callee; + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' + ) { + const method = callee.property.name; + // String-mutation methods that commonly wrap path calls + if (method === 'replace' || method === 'replaceAll') { + return callee.object; + } + // .split(...).join(...) — callee.object is the .split() result; + // peel to the .split()'s receiver + if (method === 'join') { + const splitCall = callee.object; + if ( + splitCall && + splitCall.type === 'CallExpression' && + splitCall.callee.type === 'MemberExpression' && + !splitCall.callee.computed && + splitCall.callee.property.type === 'Identifier' && + splitCall.callee.property.name === 'split' + ) { + return splitCall.callee.object; + } + } + } + return null; +} + +module.exports = { + PATH_RETURNING_FNS, + isPathReturningCall, + isPosixSlashStringLiteral, + isPosixNormalizerCall, + unwrapString, + unwrapNonNormalizerMethodChain, +}; diff --git a/eslint-rules/no-path-literal-in-assert.cjs b/eslint-rules/no-path-literal-in-assert.cjs new file mode 100644 index 000000000..e7d01ad4a --- /dev/null +++ b/eslint-rules/no-path-literal-in-assert.cjs @@ -0,0 +1,190 @@ +'use strict'; + +/** + * no-path-literal-in-assert + * + * Flag assertion calls where a path-returning function (path.join, path.resolve, + * getGlobalConfigDir, …) is compared to a hardcoded POSIX-slash string literal. + * These assertions FAIL on Windows because path.join emits backslashes. + * + * Triggers on: + * assert.equal|strictEqual|deepEqual|deepStrictEqual(actual, expected) + * expect(actual).toBe|toEqual|toStrictEqual(expected) + * + * Out of scope — intentionally NOT reported: + * assert.notEqual|notStrictEqual(actual, expected) + * expect(actual).not.toBe|not.toEqual|not.toStrictEqual(expected) + * A path-vs-POSIX-literal INEQUALITY passes on Windows regardless of separator + * differences, so it does not exhibit the portability-defect shape this rule + * targets. + * + * Suppressed when: + * - The path operand is wrapped by a POSIX normalizer (replace/replaceAll/toPosixPath/…) + * - The assertion is inside a Windows-excluded block (platform guard, early-return, + * hoisted isWindows) as detected by platform-guard.cjs + * + * DEFECT category: DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT + * + * ── Known boundaries ─────────────────────────────────────────────────────────── + * + * (a) Name-based matching only. The rule recognises `path`, `os`, and the + * project resolver names listed in PATH_RETURNING_FNS by spelling alone. If + * a test file declares a LOCAL variable named `path` that shadows the real + * `path` module, that shadow is out of scope — the rule will still treat a + * `path.join(...)` call as path-returning. + * + * (b) Shallow operand inspection. Only the direct first/second argument of the + * assert call is inspected, plus one level of `String()` cast and one + * level of non-normalizer method-chain peeling (`.replace()`, `.replaceAll()`, + * `.split().join()`). Deeper wrapping — e.g. `.toLowerCase()` applied after + * a path call, or `fs.realpathSync(path.join(...))` — is NOT detected as a + * path-returning expression and will not trigger the rule. + * + * (c) Harmless no-op remedy. For explicit dir-pass-through assertions (where the + * path really does contain forward-slashes even on Windows), wrapping with + * `String().replace(/\\\\/g, '/')` is the correct suppression; on POSIX + * systems where `\\` never appears, the replace is a no-op and has zero cost. + */ + +const { + isPathReturningCall, + isPosixSlashStringLiteral, + isPosixNormalizerCall, + unwrapString, + unwrapNonNormalizerMethodChain, +} = require('./lib/portability-vocab.cjs'); + +const { isWindowsExcludedNode } = require('./lib/platform-guard.cjs'); + +/** @type {import('eslint').Rule.RuleModule} */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow path-returning calls compared to hardcoded POSIX-slash literals in assertions (fails on Windows)', + category: 'Portability', + }, + schema: [], + messages: { + pathLiteral: + "Path-returning call compared to a hardcoded '/'-literal (DEFECT.WINDOWS-PATH-LITERAL-IN-ASSERT): " + + "fails on Windows where path.join emits '\\\\'. " + + "Normalize the actual: String().replace(/\\\\\\\\/g, '/') or .replaceAll(path.sep, '/').", + }, + }, + + create(context) { + const sourceCode = context.sourceCode ?? context.getSourceCode(); + + /** assert.equal / assert.strictEqual / assert.deepEqual / assert.deepStrictEqual */ + const ASSERT_EQUALITY_METHODS = new Set([ + 'equal', + 'strictEqual', + 'deepEqual', + 'deepStrictEqual', + ]); + + /** expect(actual).(expected) */ + const EXPECT_MATCHERS = new Set(['toBe', 'toEqual', 'toStrictEqual']); + + /** + * Returns true when `pathNode` represents a path call and `literalNode` is + * a POSIX slash literal, AND the path call is NOT already normalized. + * + * `rawPathNode` is the operand as-is (before unwrapping) — we check it for + * normalizer wrapping before stripping String(). + * + * Lookup order: + * 1. If rawPathNode IS a valid POSIX normalizer → no violation. + * 2. Unwrap String() cast → check if inner call is a path call. + * 3. If rawPathNode is a non-normalizer method chain (e.g. .replace(/foo/g,'/')) + * peel one layer to find if the receiver is a path-returning call. + */ + function isViolation(rawPathNode, rawLiteralNode) { + // Is the path-side already wrapped by a POSIX normalizer? + if (isPosixNormalizerCall(rawPathNode)) return false; + + // Unwrap String() cast to see the inner call + const pathNode = unwrapString(rawPathNode); + + if (isPathReturningCall(pathNode)) { + if (!isPosixSlashStringLiteral(rawLiteralNode)) return false; + return true; + } + + // C1: if rawPathNode is a non-normalizer method chain (.replace, .replaceAll, + // .split().join()) wrapping a path call, that is still a violation — the method + // chain does not perform a valid POSIX normalization. + const peeled = unwrapNonNormalizerMethodChain(rawPathNode); + if (peeled != null) { + const innerPath = unwrapString(peeled); + if (isPathReturningCall(innerPath) && isPosixSlashStringLiteral(rawLiteralNode)) { + return true; + } + } + + return false; + } + + return { + CallExpression(node) { + const callee = node.callee; + + // ── assert.(actual, expected) ────────────────────────────── + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.object.type === 'Identifier' && + callee.object.name === 'assert' && + callee.property.type === 'Identifier' && + ASSERT_EQUALITY_METHODS.has(callee.property.name) + ) { + const args = node.arguments; + if (args.length < 2) return; + const actual = args[0]; + const expected = args[1]; + // Ignore 3rd arg (message) + + const violated = + isViolation(actual, expected) || + isViolation(expected, actual); + + if (violated && !isWindowsExcludedNode(node, sourceCode)) { + context.report({ node, messageId: 'pathLiteral' }); + } + return; + } + + // ── expect(actual).(expected) ───────────────────────────── + // Shape: CallExpression{ callee: MemberExpression{ object: CallExpression{callee: Identifier{expect}}, property: Identifier{} } } + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' && + EXPECT_MATCHERS.has(callee.property.name) && + callee.object.type === 'CallExpression' && + callee.object.callee.type === 'Identifier' && + callee.object.callee.name === 'expect' && + callee.object.arguments.length === 1 + ) { + const actual = callee.object.arguments[0]; // the arg to expect(...) + const matcherArgs = node.arguments; + if (matcherArgs.length < 1) return; + const expected = matcherArgs[0]; + + const violated = + isViolation(actual, expected) || + isViolation(expected, actual); + + if (violated && !isWindowsExcludedNode(node, sourceCode)) { + context.report({ node, messageId: 'pathLiteral' }); + } + return; + } + }, + }; + }, +}; + +module.exports = rule; diff --git a/eslint.config.mjs b/eslint.config.mjs index b8344707c..743c1339d 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -15,6 +15,7 @@ import noElapsedAssertion from './eslint-rules/no-elapsed-assertion.cjs'; import noRawRmsyncInTests from './eslint-rules/no-raw-rmsync-in-tests.cjs'; import noTautologicalAssert from './eslint-rules/no-tautological-assert.cjs'; import noAdhocMarkdownParsing from './eslint-rules/no-adhoc-markdown-parsing.cjs'; +import noPathLiteralInAssert from './eslint-rules/no-path-literal-in-assert.cjs'; const localPlugin = { rules: { @@ -24,6 +25,7 @@ const localPlugin = { 'no-raw-rmsync-in-tests': noRawRmsyncInTests, 'no-tautological-assert': noTautologicalAssert, 'no-adhoc-markdown-parsing': noAdhocMarkdownParsing, + 'no-path-literal-in-assert': noPathLiteralInAssert, }, }; @@ -265,6 +267,8 @@ export default tseslint.config( 'local/no-tautological-assert': 'error', // Ban source-grep pattern in tests — use require() + behavior assertions instead 'local/no-source-grep': 'error', + // Ban path-returning calls compared to hardcoded POSIX-slash literals (fails on Windows) + 'local/no-path-literal-in-assert': 'error', // Ban raw setTimeout sync + elapsed/duration-style assertions via no-restricted-syntax 'no-restricted-syntax': [ 'error', diff --git a/tests/bug-3126-global-skills-base-runtime-path.test.cjs b/tests/bug-3126-global-skills-base-runtime-path.test.cjs index fe801df04..2a67ddc30 100644 --- a/tests/bug-3126-global-skills-base-runtime-path.test.cjs +++ b/tests/bug-3126-global-skills-base-runtime-path.test.cjs @@ -93,18 +93,18 @@ describe('bug #3126: runtime-homes getGlobalConfigDir — defaults', () => { describe('bug #3126: runtime-homes env-var overrides', () => { test('claude respects CLAUDE_CONFIG_DIR (was missing in old code)', () => { withEnv('CLAUDE_CONFIG_DIR', '/custom/claude', () => { - assert.strictEqual(getGlobalConfigDir('claude'), '/custom/claude'); + assert.strictEqual(String(getGlobalConfigDir('claude')).replace(/\\/g, '/'), '/custom/claude'); }); }); test('cursor respects CURSOR_CONFIG_DIR', () => { withEnv('CURSOR_CONFIG_DIR', '/custom/cursor', () => { - assert.strictEqual(getGlobalConfigDir('cursor'), '/custom/cursor'); + assert.strictEqual(String(getGlobalConfigDir('cursor')).replace(/\\/g, '/'), '/custom/cursor'); }); }); test('opencode respects OPENCODE_CONFIG_DIR', () => { withEnv('OPENCODE_CONFIG_DIR', '/custom/opencode', () => { withEnv('XDG_CONFIG_HOME', undefined, () => { - assert.strictEqual(getGlobalConfigDir('opencode'), '/custom/opencode'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/custom/opencode'); }); }); }); @@ -208,7 +208,7 @@ describe('bug #3126: runtime-homes getGlobalSkillDir', () => { describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-path precedence', () => { // ── explicitDir override ────────────────────────────────────────────────── test('explicitDir absolute path is returned as-is (claude)', () => { - assert.strictEqual(getGlobalConfigDir('claude', '/tmp/x'), '/tmp/x'); + assert.strictEqual(String(getGlobalConfigDir('claude', '/tmp/x')).replace(/\\/g, '/'), '/tmp/x'); }); test('explicitDir with tilde is expanded (opencode)', () => { @@ -220,7 +220,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat test('explicitDir wins even when OPENCODE_CONFIG_DIR is also set', () => { withEnv('OPENCODE_CONFIG_DIR', '/should/not/win', () => { - assert.strictEqual(getGlobalConfigDir('opencode', '/explicit/wins'), '/explicit/wins'); + assert.strictEqual(String(getGlobalConfigDir('opencode', '/explicit/wins')).replace(/\\/g, '/'), '/explicit/wins'); }); }); @@ -229,7 +229,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat withEnv('OPENCODE_CONFIG_DIR', undefined, () => { withEnv('XDG_CONFIG_HOME', undefined, () => { withEnv('OPENCODE_CONFIG', '/home/u/cfg/opencode.json', () => { - assert.strictEqual(getGlobalConfigDir('opencode'), '/home/u/cfg'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/home/u/cfg'); }); }); }); @@ -238,7 +238,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat test('opencode: OPENCODE_CONFIG_DIR takes precedence over OPENCODE_CONFIG', () => { withEnv('OPENCODE_CONFIG_DIR', '/dir/wins', () => { withEnv('OPENCODE_CONFIG', '/file/loses.json', () => { - assert.strictEqual(getGlobalConfigDir('opencode'), '/dir/wins'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/dir/wins'); }); }); }); @@ -247,7 +247,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat withEnv('OPENCODE_CONFIG_DIR', undefined, () => { withEnv('OPENCODE_CONFIG', '/cfg/opencode.json', () => { withEnv('XDG_CONFIG_HOME', '/xdg/should/lose', () => { - assert.strictEqual(getGlobalConfigDir('opencode'), '/cfg'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/cfg'); }); }); }); @@ -271,7 +271,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat withEnv('KILO_CONFIG_DIR', undefined, () => { withEnv('XDG_CONFIG_HOME', undefined, () => { withEnv('KILO_CONFIG', '/home/u/cfg/kilo.json', () => { - assert.strictEqual(getGlobalConfigDir('kilo'), '/home/u/cfg'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/home/u/cfg'); }); }); }); @@ -280,7 +280,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat test('kilo: KILO_CONFIG_DIR takes precedence over KILO_CONFIG', () => { withEnv('KILO_CONFIG_DIR', '/dir/wins', () => { withEnv('KILO_CONFIG', '/file/loses.json', () => { - assert.strictEqual(getGlobalConfigDir('kilo'), '/dir/wins'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/dir/wins'); }); }); }); @@ -289,7 +289,7 @@ describe('getGlobalConfigDir — explicitDir override and opencode/kilo file-pat withEnv('KILO_CONFIG_DIR', undefined, () => { withEnv('KILO_CONFIG', '/cfg/kilo.json', () => { withEnv('XDG_CONFIG_HOME', '/xdg/should/lose', () => { - assert.strictEqual(getGlobalConfigDir('kilo'), '/cfg'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/cfg'); }); }); }); diff --git a/tests/bug-kimi-path-layout-local-guard.test.cjs b/tests/bug-kimi-path-layout-local-guard.test.cjs index db97faa77..fd9c33ebe 100644 --- a/tests/bug-kimi-path-layout-local-guard.test.cjs +++ b/tests/bug-kimi-path-layout-local-guard.test.cjs @@ -102,12 +102,12 @@ describe('Kimi runtime homes', () => { test('KIMI_CONFIG_DIR can select the brand-specific ~/.kimi-code root', () => { withEnv({ KIMI_CONFIG_DIR: '/tmp/custom-kimi-code', XDG_CONFIG_HOME: undefined }, () => { - assert.strictEqual(getGlobalConfigDir('kimi'), '/tmp/custom-kimi-code'); + assert.strictEqual(String(getGlobalConfigDir('kimi')).replace(/\\/g, '/'), '/tmp/custom-kimi-code'); assert.strictEqual( getGlobalSkillsBase('kimi'), path.join('/tmp/custom-kimi-code', 'skills'), ); - assert.strictEqual(getGlobalDir('kimi'), '/tmp/custom-kimi-code'); + assert.strictEqual(String(getGlobalDir('kimi')).replace(/\\/g, '/'), '/tmp/custom-kimi-code'); }); }); diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 2e87d5833..2144a9227 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -207,7 +207,7 @@ describe('getGlobalConfigDir — explicit configDir overrides env for all runtim const savedHome = process.env.HERMES_HOME; process.env.HERMES_HOME = '~/from-env'; try { - assert.strictEqual(getGlobalConfigDir('hermes', '/explicit/hermes'), '/explicit/hermes'); + assert.strictEqual(String(getGlobalConfigDir('hermes', '/explicit/hermes')).replace(/\\/g, '/'), '/explicit/hermes'); } finally { if (savedHome !== undefined) process.env.HERMES_HOME = savedHome; else delete process.env.HERMES_HOME; @@ -218,7 +218,7 @@ describe('getGlobalConfigDir — explicit configDir overrides env for all runtim const saved = process.env.KILO_CONFIG_DIR; process.env.KILO_CONFIG_DIR = '~/from-env'; try { - assert.strictEqual(getGlobalConfigDir('kilo', '/explicit/kilo'), '/explicit/kilo'); + assert.strictEqual(String(getGlobalConfigDir('kilo', '/explicit/kilo')).replace(/\\/g, '/'), '/explicit/kilo'); } finally { if (saved !== undefined) process.env.KILO_CONFIG_DIR = saved; else delete process.env.KILO_CONFIG_DIR; diff --git a/tests/no-path-literal-in-assert.rule.test.cjs b/tests/no-path-literal-in-assert.rule.test.cjs new file mode 100644 index 000000000..96490197c --- /dev/null +++ b/tests/no-path-literal-in-assert.rule.test.cjs @@ -0,0 +1,402 @@ +'use strict'; + +/** + * no-path-literal-in-assert.rule.test.cjs + * + * RuleTester unit tests for the local/no-path-literal-in-assert ESLint rule. + * Mirrors the style of tests/eslint-rules.test.cjs. + * + * Rule: report when a path-returning call (path.join, getGlobalConfigDir, …) + * is compared to a hardcoded POSIX-slash literal in an assert.*() or + * expect(…).() assertion — a DEFECT that fails on Windows. + * + * VALID (no report) when: + * - the path operand is wrapped by a POSIX normalizer (.replace, .replaceAll, toPosixPath, etc.) + * - the assertion is inside a Windows-excluded block (process.platform !== 'win32' guard, + * early-return guard, hoisted isWindows guard) + * - both operands are path calls (no slash literal involved) + * - the string literal has no slash (file-name only) + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const { RuleTester } = require('eslint'); + +const noPathLiteralInAssert = require('../eslint-rules/no-path-literal-in-assert.cjs'); + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'commonjs', + }, +}); + +// ─── module shape ───────────────────────────────────────────────────────────── + +describe('no-path-literal-in-assert rule module', () => { + test('exports meta and create', () => { + assert.strictEqual(typeof noPathLiteralInAssert.meta, 'object'); + assert.strictEqual(typeof noPathLiteralInAssert.create, 'function'); + assert.strictEqual(noPathLiteralInAssert.meta.type, 'problem'); + assert.ok(noPathLiteralInAssert.meta.messages.pathLiteral); + }); +}); + +// ─── INVALID cases (violation expected) ─────────────────────────────────────── + +describe('no-path-literal-in-assert invalid cases', () => { + test('invalid: assert.strictEqual(path.join(a,b), "/x/y")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.strictEqual(path.join(a, b), '/x/y');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: assert.equal(getGlobalConfigDir("claude"), "/custom/claude")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal(getGlobalConfigDir('claude'), '/custom/claude');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: assert.deepStrictEqual(path.resolve(x), "/a/b")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.deepStrictEqual(path.resolve(x), '/a/b');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: reversed operands — assert.equal("/x/y", path.join(a,b))', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal('/x/y', path.join(a, b));`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: expect(path.resolve(x)).toBe("/a/b")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `expect(path.resolve(x)).toBe('/a/b');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: multi-line assert.strictEqual', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: ` + assert.strictEqual( + path.join(base, 'dir'), + '/home/user/dir' + ); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: assert.equal(os.homedir(), "/home/user")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal(os.homedir(), '/home/user');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: assert.equal(os.tmpdir(), "/tmp")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal(os.tmpdir(), '/tmp');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: expect(getGlobalConfigDir("claude")).toEqual("/custom/claude")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `expect(getGlobalConfigDir('claude')).toEqual('/custom/claude');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('invalid: assert.deepEqual(path.normalize(x), "/a/b/c")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.deepEqual(path.normalize(x), '/a/b/c');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); +}); + +// ─── VALID cases (no violation expected) ───────────────────────────────────── + +describe('no-path-literal-in-assert valid cases', () => { + test('valid: normalized with String(path.join).replace(/\\\\/g, "/")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(String(path.join(a, b)).replace(/\\\\/g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: normalized with String(path.join).replace(/[\\\\/]/g, "/")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(String(path.join(a, b)).replace(/[\\\\/]/g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: normalized with path.join().replaceAll(path.sep, "/")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(path.join(a, b).replaceAll(path.sep, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: assert.equal("foo", "foo") — no path call, no slash difference', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal('foo', 'foo');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: assert.equal(path.basename(p), "file.txt") — string has no slash', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(path.basename(p), 'file.txt');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: both operands are path calls — no slash literal', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(path.join(a, b), path.join(c, d));`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: guarded by if (process.platform !== "win32") { ... }', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: ` + if (process.platform !== 'win32') { + assert.equal(path.join(a, b), '/x/y'); + } + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: early-return guard — if (process.platform === "win32") return; assert.equal(path.join(a,b), "/x/y")', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: ` + if (process.platform === 'win32') return; + assert.equal(path.join(a, b), '/x/y'); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: hoisted isWindows guard — const isWindows = process.platform === "win32"; if (!isWindows) assert.equal(...)', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: ` + const isWindows = process.platform === 'win32'; + if (!isWindows) assert.equal(path.join(a, b), '/x/y'); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: assert.ok(path.join(a,b)) — not an equality assert', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.ok(path.join(a, b));`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: URL string starting with https:// is not flagged', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(getUrl(), 'https://example.com/path');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: normalized with toPosixPath(path.join(a,b))', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(toPosixPath(path.join(a, b)), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); +}); + +// ─── C1: isPosixNormalizerCall must require backslash-targeting regex ───────── +// The fix: .replace(/foo/g,'/') and .replace(/\//g,'/') must NOT suppress the rule. +// Before the fix, ANY .replace(//g, '/') was accepted as a normalizer +// and would suppress the violation even when the regex did not target backslashes. + +describe('C1 — isPosixNormalizerCall backslash-targeting requirement', () => { + test('C1 INVALID: path.join().replace(/foo/g, "/") is NOT a POSIX normalizer — flagged', () => { + // Before C1 fix: was NOT flagged (any regex with g flag was accepted as normalizer). + // After C1 fix: IS flagged (/foo/g does not target backslashes → not a normalizer; + // rule now peels the non-normalizer method chain and finds path.join inside). + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal(path.join(a,b).replace(/foo/g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('C1 INVALID: path.join().replace(/\\//g, "/") targets forward-slash only — flagged', () => { + // Before C1 fix: was NOT flagged. + // After C1 fix: IS flagged (/\//g matches forward-slash only, not backslash → not a normalizer). + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [], + invalid: [ + { + code: `assert.equal(path.join(a,b).replace(/\\//g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'pathLiteral' }], + }, + ], + }); + }); + + test('C1 VALID: path.join().replace(/\\\\/g, "/") IS a proper POSIX normalizer — NOT flagged', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(path.join(a,b).replace(/\\\\/g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('C1 VALID: path.join().replace(/[\\\\/]/g, "/") IS a proper POSIX normalizer — NOT flagged', () => { + ruleTester.run('no-path-literal-in-assert', noPathLiteralInAssert, { + valid: [ + { + code: `assert.equal(path.join(a,b).replace(/[\\\\/]/g, '/'), '/x/y');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); +}); diff --git a/tests/platform-guard.unit.test.cjs b/tests/platform-guard.unit.test.cjs new file mode 100644 index 000000000..837d4cd0e --- /dev/null +++ b/tests/platform-guard.unit.test.cjs @@ -0,0 +1,381 @@ +'use strict'; + +/** + * platform-guard.unit.test.cjs + * + * Unit tests for eslint-rules/lib/platform-guard.cjs. + * Verifies classifyPlatformTest and isWindowsExcludedNode shapes + * using espree to parse code snippets into ASTs. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const espree = require('espree'); +const { Linter } = require('eslint'); + +const { classifyPlatformTest, isWindowsExcludedNode } = require('../eslint-rules/lib/platform-guard.cjs'); + +const PARSE_OPTIONS = { + ecmaVersion: 2022, + sourceType: 'script', + range: true, + loc: true, + tokens: true, + comment: true, +}; + +function parse(code) { + return espree.parse(code, PARSE_OPTIONS); +} + +// ─── classifyPlatformTest ───────────────────────────────────────────────────── + +describe('classifyPlatformTest', () => { + test('process.platform === "win32" → windows', () => { + const ast = parse(`process.platform === 'win32'`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('"win32" === process.platform (reversed) → windows', () => { + const ast = parse(`'win32' === process.platform`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('process.platform !== "win32" → not-windows', () => { + const ast = parse(`process.platform !== 'win32'`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'not-windows'); + }); + + test('os.platform() === "win32" → windows', () => { + const ast = parse(`os.platform() === 'win32'`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('os.platform() !== "win32" → not-windows', () => { + const ast = parse(`os.platform() !== 'win32'`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'not-windows'); + }); + + test('isWindows identifier → windows', () => { + const ast = parse(`isWindows`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('IS_WINDOWS identifier → windows', () => { + const ast = parse(`IS_WINDOWS`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('isWin identifier → windows', () => { + const ast = parse(`isWin`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('onWindows identifier → windows', () => { + const ast = parse(`onWindows`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'windows'); + }); + + test('!isWindows → not-windows', () => { + const ast = parse(`!isWindows`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), 'not-windows'); + }); + + test('unrelated expression → null', () => { + const ast = parse(`x === 'linux'`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), null); + }); + + test('bare identifier "result" → null', () => { + const ast = parse(`result`); + const node = ast.body[0].expression; + assert.strictEqual(classifyPlatformTest(node), null); + }); +}); + +// ─── isWindowsExcludedNode via Linter ──────────────────────────────────────── +// We use the Linter + a custom rule to get real sourceCode with ancestors. + +/** + * Build a mini rule that collects data about CallExpression nodes named + * "assert.equal" and checks isWindowsExcludedNode on them. + */ +function buildCollectorRule() { + return { + create(context) { + const sourceCode = context.sourceCode; + const results = []; + return { + CallExpression(node) { + if ( + node.callee.type === 'MemberExpression' && + node.callee.object.name === 'assert' && + node.callee.property.name === 'equal' + ) { + results.push(isWindowsExcludedNode(node, sourceCode)); + } + }, + 'Program:exit'() { + context.report({ node: sourceCode.ast, messageId: 'result', data: { results: JSON.stringify(results) } }); + }, + }; + }, + meta: { type: 'suggestion', schema: [], messages: { result: '{{results}}' } }, + }; +} + +function runCollector(code) { + const linter = new Linter({ configType: 'flat' }); + const messages = linter.verify( + code, + [{ plugins: { t: { rules: { collector: buildCollectorRule() } } }, rules: { 't/collector': 'warn' }, languageOptions: { ecmaVersion: 2022, sourceType: 'script' } }], + { filename: 'tests/x.test.cjs' } + ); + // The rule emits exactly one message (Program:exit) with results as JSON + const msg = messages.find(m => m.ruleId === 't/collector'); + if (!msg) return []; + return JSON.parse(msg.message); +} + +describe('isWindowsExcludedNode — if (!windows) { assert } shape', () => { + test('assert inside if (process.platform !== "win32") block → excluded=true', () => { + const code = ` + if (process.platform !== 'win32') { + assert.equal(path.join(a, b), '/x/y'); + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('assert inside else of if (process.platform === "win32") block → excluded=true', () => { + const code = ` + if (process.platform === 'win32') { + doWindows(); + } else { + assert.equal(path.join(a, b), '/x/y'); + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('assert NOT inside any guard → excluded=false', () => { + const code = `assert.equal(path.join(a, b), '/x/y');`; + const results = runCollector(code); + assert.deepStrictEqual(results, [false]); + }); +}); + +describe('isWindowsExcludedNode — early-return guard shape', () => { + test('if (process.platform === "win32") return; assert.equal(...) → excluded=true', () => { + const code = ` + function test() { + if (process.platform === 'win32') return; + assert.equal(path.join(a, b), '/x/y'); + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('if (process.platform === "win32") return t.skip(); assert.equal → excluded=true', () => { + const code = ` + function test(t) { + if (process.platform === 'win32') return t.skip('no windows'); + assert.equal(path.join(a, b), '/x/y'); + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('guard appears AFTER the assert → excluded=false', () => { + const code = ` + function test() { + assert.equal(path.join(a, b), '/x/y'); + if (process.platform === 'win32') return; + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false]); + }); +}); + +describe('isWindowsExcludedNode — hoisted isWindows guard shape', () => { + test('const isWindows = process.platform === "win32"; if (!isWindows) assert.equal → excluded=true', () => { + const code = ` + const isWindows = process.platform === 'win32'; + if (!isWindows) assert.equal(path.join(a, b), '/x/y'); + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('const isWindows = …; if (isWindows) {} else { assert.equal } → excluded=true', () => { + const code = ` + const isWindows = process.platform === 'win32'; + if (isWindows) { doWindowsThing(); } else { assert.equal(path.join(a,b), '/x/y'); } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); +}); + +describe('isWindowsExcludedNode — arbitrary-named hoisted boolean (real hoisting)', () => { + test('const winFlag = process.platform === "win32"; if (!winFlag) assert.equal → excluded=true', () => { + const code = ` + function test() { + const winFlag = process.platform === 'win32'; + if (!winFlag) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('const winFlag = process.platform === "win32"; if (winFlag) return; assert.equal → excluded=true', () => { + const code = ` + function test() { + const winFlag = process.platform === 'win32'; + if (winFlag) return; + assert.equal(path.join(a, b), '/x/y'); + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('unrelated variable used as guard is NOT excluded', () => { + const code = ` + function test() { + const debugMode = true; + if (!debugMode) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false]); + }); + + test('winFlag declared AFTER the assert is NOT a guard → excluded=false', () => { + const code = ` + function test() { + assert.equal(path.join(a, b), '/x/y'); + const winFlag = process.platform === 'win32'; + if (!winFlag) { doSomething(); } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false]); + }); +}); + +// ─── C2: early-return guard must climb ancestor blocks ──────────────────────── + +describe('C2 — early-return guard climbs ancestor blocks', () => { + test('C2: early-return in outer block before nested if-block → excluded=true', () => { + // The guard is in the function body; assert.equal is inside a nested if-block. + // Before the fix, _hasEarlyWindowsReturnBefore only checked the innermost block. + const code = ` + function test() { + if (process.platform === 'win32') return; + if (cond) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true]); + }); + + test('C2: no early-return at all → excluded=false', () => { + const code = ` + function test() { + if (cond) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false]); + }); +}); + +// ─── C3: binding-aware identifier classification ────────────────────────────── + +describe('C3 — binding-aware identifier classification', () => { + test('C3 case 1: const isWindows = false; if (!isWindows) assert.equal → excluded=false (FLAGGED)', () => { + // isWindows is in WINDOWS_BOOL_NAMES but its binding is `false`, not a platform test. + const code = ` + function test() { + const isWindows = false; + if (!isWindows) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false], 'const isWindows = false should not be treated as a platform guard'); + }); + + test('C3 case 2: let w = platform===win32; w = false; if (!w) assert.equal → excluded=false (FLAGGED)', () => { + // w starts as a platform test but is reassigned — should NOT be trusted. + const code = ` + function test() { + let w = process.platform === 'win32'; + w = false; + if (!w) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false], 'reassigned variable should not be treated as a platform guard'); + }); + + test('C3 case 3: inner shadow const w = false wins over outer const w = platform test → excluded=false (FLAGGED)', () => { + // The inner const w = false shadows the outer const w = platform test. + const code = ` + function test() { + const w = process.platform === 'win32'; + { + const w = false; + if (!w) { + assert.equal(path.join(a, b), '/x/y'); + } + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [false], 'inner shadow (w=false) should win over outer platform test'); + }); + + test('C3 case 4: const w = platform test; if (!w) assert.equal → excluded=true (genuine guard)', () => { + // The canonical case — should still work. + const code = ` + function test() { + const w = process.platform === 'win32'; + if (!w) { + assert.equal(path.join(a, b), '/x/y'); + } + } + `; + const results = runCollector(code); + assert.deepStrictEqual(results, [true], 'genuine platform guard should suppress the report'); + }); +}); diff --git a/tests/portability-rule-disable-ban.test.cjs b/tests/portability-rule-disable-ban.test.cjs new file mode 100644 index 000000000..44effb105 --- /dev/null +++ b/tests/portability-rule-disable-ban.test.cjs @@ -0,0 +1,199 @@ +'use strict'; + +/** + * portability-rule-disable-ban.test.cjs + * + * Out-of-band disable-ban scan (ADR-1703). + * + * ESLint inline suppression of portability rules is banned. This test runs + * OUTSIDE ESLint so it cannot itself be eslint-disabled. + * + * PROTECTED_RULES grows as later phases add rules. Each new portability rule + * in the `local/` namespace should be appended to this list. + * + * Hard-fails on: + * (a) Any `eslint-disable*` comment that NAMES a protected portability rule. + * (b) Any BLANKET `eslint-disable*` comment (no rule list) — these suppress + * every rule including the protected ones. + * + * NOTE: This file itself is excluded from the scan by absolute path. It + * references the disable keyword only inside regex/string data structures to + * avoid being detected as a real directive. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const espree = require('espree'); +const { globSync } = require('glob'); + +// ── Protected portability rules (grows with each ADR-1703 phase) ────────────── +const PROTECTED_RULES = [ + 'no-path-literal-in-assert', + // Future phases: add new local/ portability rules here. +]; + +// ── Detect disable directives via the comment text ─────────────────────────── + +// The three directive forms ESLint recognises (built as concatenated strings so +// this source file contains NO real disable directive of its own). +const D = 'eslint-' + 'disable'; +const DN = 'eslint-' + 'disable-next-line'; +const DL = 'eslint-' + 'disable-line'; +const DISABLE_PREFIXES = [DN, DL, D]; // longest first so prefix-match is greedy + +/** + * Classify a comment node. Returns: + * 'blanket' — a disable with NO rule list (suppresses everything) + * 'named' — a disable that lists at least one protected portability rule + * null — not a disable directive, or a non-portability named disable + */ +function classifyComment(commentValue) { + const txt = commentValue.trim(); + for (const prefix of DISABLE_PREFIXES) { + if (txt.startsWith(prefix)) { + // Text after the directive keyword + const rest = txt.slice(prefix.length).trim(); + // Blanket: nothing after the keyword, or only a prose comment (starts with --) + if (!rest || rest.startsWith('--')) { + return 'blanket'; + } + // Named: rest is a comma-separated rule list (possibly with -- prose) + const ruleList = rest.split('--')[0]; // strip trailing prose + const rules = ruleList.split(',').map(r => r.trim()).filter(Boolean); + for (const rule of rules) { + for (const protected_ of PROTECTED_RULES) { + if (rule === 'local/' + protected_ || rule === protected_) { + return 'named'; + } + } + } + return null; // named disable but not for a protected rule + } + } + return null; +} + +// ── Collect test files ──────────────────────────────────────────────────────── + +const SELF_ABS = __filename; + +function collectTestFiles() { + return globSync('tests/**/*.test.cjs', { cwd: path.join(__dirname, '..') }) + .map(rel => path.join(__dirname, '..', rel)) + .filter(absPath => absPath !== SELF_ABS); +} + +// ── Scan ────────────────────────────────────────────────────────────────────── + +function scanFile(absPath) { + let src; + try { + src = fs.readFileSync(absPath, 'utf-8'); + } catch (err) { + throw new Error(`Could not read ${absPath}: ${err.message}`); + } + + let ast; + try { + ast = espree.parse(src, { + comment: true, + ecmaVersion: 2022, + loc: true, + range: true, + tolerant: true, + }); + } catch (parseErr) { + // C5: fail CLOSED on parse error — a file that fails to parse must FAIL the + // test with its path, not be silently skipped. Silent skip is a false-green: + // an unparseable test file could contain a real disable directive. + throw new Error(`Parse error in ${absPath}: ${parseErr.message}`); + } + + const blanket = []; + const named = []; + + for (const cmt of ast.comments || []) { + const kind = classifyComment(cmt.value); + if (!kind) continue; + const line = cmt.loc ? cmt.loc.start.line : '?'; + const entry = { file: absPath, line, text: cmt.value.trim() }; + if (kind === 'blanket') blanket.push(entry); + else if (kind === 'named') named.push(entry); + } + + return { blanket, named }; +} + +// ── C5: parse-error fail-closed ─────────────────────────────────────────────── + +describe('C5 — scanFile fails closed on parse error', () => { + test('C5: scanFile throws on parse error instead of silently returning empty result', () => { + // Inject a parse error deterministically by monkeypatching espree.parse. + // This is the cross-platform approach (works under root/Docker too). + const origParse = espree.parse; + try { + espree.parse = () => { throw new SyntaxError('injected parse error for C5 test'); }; + // Create a minimal real file to scan (use this test file itself, which exists). + assert.throws( + () => scanFile(__filename), + (err) => { + return err instanceof Error && + err.message.includes('injected parse error for C5 test'); + }, + 'scanFile must throw on parse error, not silently return empty result' + ); + } finally { + espree.parse = origParse; + } + }); +}); + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('portability-rule disable-ban (ADR-1703)', () => { + const testFiles = collectTestFiles(); + + test('test file enumeration finds at least 10 test files', () => { + assert.ok( + testFiles.length >= 10, + `Expected at least 10 test files, got ${testFiles.length}`, + ); + }); + + test('no test file contains a named eslint-disable for a portability rule (category a)', () => { + const offenders = []; + for (const absPath of testFiles) { + const { named } = scanFile(absPath); + for (const o of named) { + offenders.push(`${path.relative(path.join(__dirname, '..'), o.file)}:${o.line} — ${o.text}`); + } + } + assert.deepStrictEqual( + offenders, + [], + 'Found inline disable directives suppressing protected portability rules.\n' + + 'These MUST be removed — the rule exists to enforce cross-platform safety:\n\n' + + offenders.map(s => ' ' + s).join('\n'), + ); + }); + + test('no test file contains a blanket eslint-disable (category b — suppresses all rules including portability)', () => { + const offenders = []; + for (const absPath of testFiles) { + const { blanket } = scanFile(absPath); + for (const o of blanket) { + offenders.push(`${path.relative(path.join(__dirname, '..'), o.file)}:${o.line} — ${o.text}`); + } + } + assert.deepStrictEqual( + offenders, + [], + 'Found blanket eslint-disable directives in test files.\n' + + 'Blanket disables suppress ALL rules including portability rules and are banned.\n' + + 'Replace with targeted per-rule disables for non-portability rules, or remove:\n\n' + + offenders.map(s => ' ' + s).join('\n'), + ); + }); +}); diff --git a/tests/portability-vocab-drift.test.cjs b/tests/portability-vocab-drift.test.cjs new file mode 100644 index 000000000..bf93ab1bf --- /dev/null +++ b/tests/portability-vocab-drift.test.cjs @@ -0,0 +1,339 @@ +'use strict'; + +/** + * portability-vocab-drift.test.cjs + * + * Drift-guard: ensure that every exported function from src/runtime-homes.cts + * that returns a filesystem path is listed in PATH_RETURNING_FNS + * (eslint-rules/lib/portability-vocab.cjs). + * + * Method: + * 1. Parse src/runtime-homes.cts with @typescript-eslint/parser. + * 2. Collect `export function ` declarations where the body contains + * a path-building expression (path.join, path.dirname, os.homedir, + * expandTilde, or returns something with "Dir" / "Path" / "Base" in its name). + * 3. Assert each collected name is in PATH_RETURNING_FNS or is listed in + * IGNORED_NON_PATH_EXPORTS (with a reason comment per entry). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const tsParser = require('@typescript-eslint/parser'); + +const { PATH_RETURNING_FNS } = require('../eslint-rules/lib/portability-vocab.cjs'); + +// Functions exported from runtime-homes.cts that do NOT return a filesystem +// path and therefore are intentionally excluded from PATH_RETURNING_FNS. +const IGNORED_NON_PATH_EXPORTS = new Set([ + // resolveConfigHomeFromDescriptor: internal/exported but delegates to path-returning helpers; + // it IS included in PATH_RETURNING_FNS under its bare name (no object prefix needed). + // detectAntigravityDirAmbiguity: returns an object (AntigravityAmbiguity), not a path string. + 'detectAntigravityDirAmbiguity', +]); + +describe('portability-vocab drift guard', () => { + test('PATH_RETURNING_FNS is a non-empty array', () => { + assert.ok(Array.isArray(PATH_RETURNING_FNS)); + assert.ok(PATH_RETURNING_FNS.length > 0); + }); + + test('PATH_RETURNING_FNS includes the Node builtins', () => { + const builtins = ['path.join', 'path.resolve', 'path.dirname', 'path.basename', 'path.normalize', 'path.relative', 'os.homedir', 'os.tmpdir']; + for (const fn of builtins) { + assert.ok(PATH_RETURNING_FNS.includes(fn), `Expected PATH_RETURNING_FNS to include builtin "${fn}"`); + } + }); + + test('every path-returning export from runtime-homes.cts is in PATH_RETURNING_FNS or IGNORED', () => { + const srcPath = path.join(__dirname, '..', 'src', 'runtime-homes.cts'); + const src = fs.readFileSync(srcPath, 'utf-8'); + + // Parse with @typescript-eslint/parser (handles TypeScript syntax) + const ast = tsParser.parse(src, { + jsx: false, + loc: true, + range: true, + comment: true, + tokens: false, + }); + + // Collect exported function names whose body looks path-returning: + // - body contains a call to path.join / path.dirname / os.homedir / expandTilde + // - OR function name ends with Dir, Path, Base, Home, or starts with resolve/get + const pathReturningExports = []; + + function bodyText(node) { + // Slice the source for the function body + if (node.range) return src.slice(node.range[0], node.range[1]); + return ''; + } + + function looksPathReturning(funcNode, name) { + const body = bodyText(funcNode.body ?? funcNode); + const pathBuilders = [ + 'path.join', 'path.resolve', 'path.dirname', 'path.basename', + 'path.normalize', 'path.relative', 'os.homedir', 'os.tmpdir', + 'expandTilde', 'expandTildeWithHome', 'resolveConfigHome', + ]; + if (pathBuilders.some(p => body.includes(p))) return true; + // Name heuristic: resolveXxx / getXxxDir / getXxxPath / getXxxBase + if (/^(resolve|get)[A-Z]/.test(name) && /Dir|Path|Base|Home|Skills/.test(name)) return true; + return false; + } + + // Build a lookup from top-level declaration names to their function nodes, + // to resolve `export { name }` specifier exports (C4). + const topLevelFunctionNodes = new Map(); // name → funcNode + for (const node of ast.body) { + // function (...) { ... } (non-exported declaration) + if ( + node.type === 'FunctionDeclaration' && + node.id + ) { + topLevelFunctionNodes.set(node.id.name, node); + } + // const = () => ... (non-exported const arrow/function) + if (node.type === 'VariableDeclaration') { + for (const decl of node.declarations) { + if ( + decl.type === 'VariableDeclarator' && + decl.id && + decl.id.type === 'Identifier' && + decl.init && + (decl.init.type === 'ArrowFunctionExpression' || + decl.init.type === 'FunctionExpression') + ) { + topLevelFunctionNodes.set(decl.id.name, decl.init); + } + } + } + // export function / export const — also register in the map + if ( + node.type === 'ExportNamedDeclaration' && + node.declaration && + node.declaration.type === 'FunctionDeclaration' && + node.declaration.id + ) { + topLevelFunctionNodes.set(node.declaration.id.name, node.declaration); + } + if ( + node.type === 'ExportNamedDeclaration' && + node.declaration && + node.declaration.type === 'VariableDeclaration' + ) { + for (const decl of node.declaration.declarations) { + if ( + decl.type === 'VariableDeclarator' && + decl.id && + decl.id.type === 'Identifier' && + decl.init && + (decl.init.type === 'ArrowFunctionExpression' || + decl.init.type === 'FunctionExpression') + ) { + topLevelFunctionNodes.set(decl.id.name, decl.init); + } + } + } + } + + for (const node of ast.body) { + // export function (...) { ... } + if ( + node.type === 'ExportNamedDeclaration' && + node.declaration && + node.declaration.type === 'TSDeclareFunction' === false && + (node.declaration.type === 'FunctionDeclaration') && + node.declaration.id + ) { + const name = node.declaration.id.name; + if (looksPathReturning(node.declaration, name)) { + pathReturningExports.push(name); + } + } + + // export const = ( | ) + if ( + node.type === 'ExportNamedDeclaration' && + node.declaration && + node.declaration.type === 'VariableDeclaration' + ) { + for (const decl of node.declaration.declarations) { + if ( + decl.type === 'VariableDeclarator' && + decl.id && + decl.id.type === 'Identifier' && + decl.init && + (decl.init.type === 'ArrowFunctionExpression' || + decl.init.type === 'FunctionExpression') + ) { + const name = decl.id.name; + if (looksPathReturning(decl.init, name)) { + pathReturningExports.push(name); + } + } + } + } + + // export { name1, name2 } — specifier exports (C4) + // Resolve each specifier to its in-file function/const declaration. + if ( + node.type === 'ExportNamedDeclaration' && + !node.declaration && + node.source == null && // not a re-export from another module + Array.isArray(node.specifiers) + ) { + for (const specifier of node.specifiers) { + if ( + specifier.type === 'ExportSpecifier' && + specifier.local && + specifier.local.type === 'Identifier' + ) { + const name = specifier.local.name; + const exportedName = + specifier.exported && specifier.exported.type === 'Identifier' + ? specifier.exported.name + : name; + const funcNode = topLevelFunctionNodes.get(name); + if (funcNode && looksPathReturning(funcNode, exportedName)) { + pathReturningExports.push(exportedName); + } + } + } + } + } + + // Verify we found at least a few (guards against parser silently failing) + assert.ok( + pathReturningExports.length >= 3, + `Expected at least 3 path-returning exports, got ${pathReturningExports.length}: [${pathReturningExports.join(', ')}]` + ); + + const vocabSet = new Set(PATH_RETURNING_FNS); + const missing = []; + for (const name of pathReturningExports) { + if (!vocabSet.has(name) && !IGNORED_NON_PATH_EXPORTS.has(name)) { + missing.push(name); + } + } + + assert.deepStrictEqual( + missing, + [], + `These path-returning exports from runtime-homes.cts are missing from PATH_RETURNING_FNS:\n ${missing.join('\n ')}\n\nEither add them to PATH_RETURNING_FNS in eslint-rules/lib/portability-vocab.cjs or add them to IGNORED_NON_PATH_EXPORTS with a reason.` + ); + }); + + test('export const arrow-function returning path.join would be required in PATH_RETURNING_FNS', () => { + // Simulate parsing a snippet with `export const myArrowResolver = (x) => path.join(home, x)` + // and verify the drift-guard collector would pick it up (i.e. it's NOT silently bypassed). + const snippetSrc = ` + export const myArrowResolver = (x) => path.join('/home', x); + `; + const ast = tsParser.parse(snippetSrc, { + jsx: false, + loc: true, + range: true, + comment: true, + tokens: false, + }); + + const collected = []; + for (const node of ast.body) { + if ( + node.type === 'ExportNamedDeclaration' && + node.declaration && + node.declaration.type === 'VariableDeclaration' + ) { + for (const decl of node.declaration.declarations) { + if ( + decl.type === 'VariableDeclarator' && + decl.id && + decl.id.type === 'Identifier' && + decl.init && + (decl.init.type === 'ArrowFunctionExpression' || + decl.init.type === 'FunctionExpression') + ) { + const name = decl.id.name; + const bodyTxt = snippetSrc.slice(decl.init.range[0], decl.init.range[1]); + if (['path.join', 'path.resolve', 'path.dirname'].some(p => bodyTxt.includes(p))) { + collected.push(name); + } + } + } + } + } + + assert.deepStrictEqual(collected, ['myArrowResolver'], + 'Arrow-function path export should be collected by the drift guard, requiring it in PATH_RETURNING_FNS'); + + // Confirm PATH_RETURNING_FNS does NOT already contain this fictional name + // (so the test demonstrates a missing entry would be caught, not silently pass). + assert.ok( + !PATH_RETURNING_FNS.includes('myArrowResolver'), + 'myArrowResolver should not be in PATH_RETURNING_FNS (it is a test fixture name)', + ); + }); + + test('C4: export { name } specifier form — path resolver would be required in PATH_RETURNING_FNS', () => { + // Demonstrate that the drift guard now handles `export { mySpecifierResolver }` where + // the function is declared separately (not inline in the export statement). + const snippetSrc = ` + function mySpecifierResolver(x) { + return path.join('/home', x); + } + export { mySpecifierResolver }; + `; + const ast = tsParser.parse(snippetSrc, { + jsx: false, + loc: true, + range: true, + comment: true, + tokens: false, + }); + + // Replicate the drift-guard's specifier-resolution logic (C4 addition). + const topLevelFns = new Map(); + for (const node of ast.body) { + if (node.type === 'FunctionDeclaration' && node.id) { + topLevelFns.set(node.id.name, node); + } + } + + const collected = []; + for (const node of ast.body) { + if ( + node.type === 'ExportNamedDeclaration' && + !node.declaration && + node.source == null && + Array.isArray(node.specifiers) + ) { + for (const specifier of node.specifiers) { + if ( + specifier.type === 'ExportSpecifier' && + specifier.local && + specifier.local.type === 'Identifier' + ) { + const localName = specifier.local.name; + const funcNode = topLevelFns.get(localName); + if (funcNode) { + const bodyTxt = snippetSrc.slice(funcNode.range[0], funcNode.range[1]); + if (['path.join', 'path.resolve', 'path.dirname'].some(p => bodyTxt.includes(p))) { + collected.push(localName); + } + } + } + } + } + } + + assert.deepStrictEqual(collected, ['mySpecifierResolver'], + 'export { name } specifier form should be detected by drift guard, requiring entry in PATH_RETURNING_FNS'); + + assert.ok( + !PATH_RETURNING_FNS.includes('mySpecifierResolver'), + 'mySpecifierResolver should not be in PATH_RETURNING_FNS (it is a test fixture name)', + ); + }); +}); diff --git a/tests/runtime-homes-descriptor-drive.test.cjs b/tests/runtime-homes-descriptor-drive.test.cjs index 074bd321a..6c04af418 100644 --- a/tests/runtime-homes-descriptor-drive.test.cjs +++ b/tests/runtime-homes-descriptor-drive.test.cjs @@ -178,7 +178,7 @@ describe('descriptor-driven equivalence: env-var overrides', () => { process.env['COPILOT_CONFIG_DIR'] = '/custom/copilot-dir'; process.env['COPILOT_HOME'] = '/should/not/win'; try { - assert.strictEqual(getGlobalConfigDir('copilot'), '/custom/copilot-dir'); + assert.strictEqual(String(getGlobalConfigDir('copilot')).replace(/\\/g, '/'), '/custom/copilot-dir'); } finally { restoreEnvKeys(saved); } @@ -188,7 +188,7 @@ describe('descriptor-driven equivalence: env-var overrides', () => { const saved = clearAllEnvKeys(); process.env['COPILOT_HOME'] = '/custom/copilot-home'; try { - assert.strictEqual(getGlobalConfigDir('copilot'), '/custom/copilot-home'); + assert.strictEqual(String(getGlobalConfigDir('copilot')).replace(/\\/g, '/'), '/custom/copilot-home'); } finally { restoreEnvKeys(saved); } @@ -219,7 +219,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { const saved = clearAllEnvKeys(); process.env['OPENCODE_CONFIG'] = '/home/u/cfg/opencode.json'; try { - assert.strictEqual(getGlobalConfigDir('opencode'), '/home/u/cfg'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/home/u/cfg'); } finally { restoreEnvKeys(saved); } @@ -230,7 +230,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { process.env['OPENCODE_CONFIG_DIR'] = '/dir/wins'; process.env['OPENCODE_CONFIG'] = '/file/loses.json'; try { - assert.strictEqual(getGlobalConfigDir('opencode'), '/dir/wins'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/dir/wins'); } finally { restoreEnvKeys(saved); } @@ -241,7 +241,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { process.env['OPENCODE_CONFIG'] = '/cfg/opencode.json'; process.env['XDG_CONFIG_HOME'] = '/xdg/should/lose'; try { - assert.strictEqual(getGlobalConfigDir('opencode'), '/cfg'); + assert.strictEqual(String(getGlobalConfigDir('opencode')).replace(/\\/g, '/'), '/cfg'); } finally { restoreEnvKeys(saved); } @@ -272,7 +272,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { const saved = clearAllEnvKeys(); process.env['KILO_CONFIG'] = '/home/u/cfg/kilo.json'; try { - assert.strictEqual(getGlobalConfigDir('kilo'), '/home/u/cfg'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/home/u/cfg'); } finally { restoreEnvKeys(saved); } @@ -283,7 +283,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { process.env['KILO_CONFIG_DIR'] = '/dir/wins'; process.env['KILO_CONFIG'] = '/file/loses.json'; try { - assert.strictEqual(getGlobalConfigDir('kilo'), '/dir/wins'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/dir/wins'); } finally { restoreEnvKeys(saved); } @@ -294,7 +294,7 @@ describe('descriptor-driven equivalence: xdg runtimes (opencode, kilo)', () => { process.env['KILO_CONFIG'] = '/cfg/kilo.json'; process.env['XDG_CONFIG_HOME'] = '/xdg/should/lose'; try { - assert.strictEqual(getGlobalConfigDir('kilo'), '/cfg'); + assert.strictEqual(String(getGlobalConfigDir('kilo')).replace(/\\/g, '/'), '/cfg'); } finally { restoreEnvKeys(saved); } @@ -735,10 +735,10 @@ describe('descriptor-driven equivalence: generic-agents-root kimi probe hit/miss describe('descriptor-driven equivalence: explicitDir short-circuit', () => { test('explicitDir absolute path returned as-is (any runtime)', () => { - assert.strictEqual(getGlobalConfigDir('claude', '/tmp/explicit'), '/tmp/explicit'); - assert.strictEqual(getGlobalConfigDir('opencode', '/tmp/explicit'), '/tmp/explicit'); - assert.strictEqual(getGlobalConfigDir('kimi', '/tmp/explicit'), '/tmp/explicit'); - assert.strictEqual(getGlobalConfigDir('grok', '/tmp/explicit'), '/tmp/explicit'); + assert.strictEqual(String(getGlobalConfigDir('claude', '/tmp/explicit')).replace(/\\/g, '/'), '/tmp/explicit'); + assert.strictEqual(String(getGlobalConfigDir('opencode', '/tmp/explicit')).replace(/\\/g, '/'), '/tmp/explicit'); + assert.strictEqual(String(getGlobalConfigDir('kimi', '/tmp/explicit')).replace(/\\/g, '/'), '/tmp/explicit'); + assert.strictEqual(String(getGlobalConfigDir('grok', '/tmp/explicit')).replace(/\\/g, '/'), '/tmp/explicit'); }); test('explicitDir with ~ is expanded', () => { @@ -750,7 +750,7 @@ describe('descriptor-driven equivalence: explicitDir short-circuit', () => { test('explicitDir wins even when env var is set', () => { withEnv({ CLAUDE_CONFIG_DIR: '/should/not/win' }, () => { - assert.strictEqual(getGlobalConfigDir('claude', '/explicit/wins'), '/explicit/wins'); + assert.strictEqual(String(getGlobalConfigDir('claude', '/explicit/wins')).replace(/\\/g, '/'), '/explicit/wins'); }); }); }); @@ -769,7 +769,7 @@ describe('descriptor-driven equivalence: grok (not in registry)', () => { test('grok: GROK_AGENTS_HOME override', () => { withEnv({ GROK_AGENTS_HOME: '/custom/grok-agents' }, () => { - assert.strictEqual(getGlobalConfigDir('grok'), '/custom/grok-agents'); + assert.strictEqual(String(getGlobalConfigDir('grok')).replace(/\\/g, '/'), '/custom/grok-agents'); }); }); @@ -794,7 +794,7 @@ describe('descriptor-driven equivalence: unknown runtime fallback', () => { test('unknown runtime → CLAUDE_CONFIG_DIR if set', () => { withEnv({ CLAUDE_CONFIG_DIR: '/custom/claude-for-unknown' }, () => { - assert.strictEqual(getGlobalConfigDir('no-such-runtime'), '/custom/claude-for-unknown'); + assert.strictEqual(String(getGlobalConfigDir('no-such-runtime')).replace(/\\/g, '/'), '/custom/claude-for-unknown'); }); }); });