diff --git a/scripts/lint-no-source-grep-extras.cjs b/scripts/lint-no-source-grep-extras.cjs deleted file mode 100644 index 10ff4886c..000000000 --- a/scripts/lint-no-source-grep-extras.cjs +++ /dev/null @@ -1,81 +0,0 @@ -'use strict'; - -/** - * Extended detector for the no-source-grep rule (#2982). - * - * The base lint (scripts/lint-no-source-grep.cjs) only catches the - * direct-chain form: readFileSync(...).includes(...). The much more common - * var-binding form escapes it: - * - * const src = fs.readFileSync(p, 'utf8'); - * // ... 50 lines later ... - * assert.ok(src.includes('foo')); // ← still source-grep, lint missed it - * - * This module exposes pure detectors that scan source text and return - * structured violation records. The CLI wrapper (in the base lint) calls - * these for each test file. - * - * Tests assert on the typed VIOLATION enum codes, not on prose messages. - */ - -const VIOLATION = Object.freeze({ - VAR_FROM_READFILE_USED_IN_TEXT_MATCH: 'var_from_readfile_used_in_text_match', - WRAPPED_ASSERT_OK_MATCH: 'wrapped_assert_ok_match', -}); - -const TEXT_MATCH_METHODS = ['includes', 'startsWith', 'endsWith', 'match', 'search']; - -/** - * Single-pass scanner. Tracks variables bound from a readFileSync call, - * then flags any subsequent .( use where method is one of - * TEXT_MATCH_METHODS. - */ -function detectVarBindingViolations(src) { - // Pass 1: collect variables bound from readFileSync. - // Matches: const|let|var = [fs.]readFileSync( - const bindRe = /(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:[A-Za-z_$][\w$.]*\.)?readFileSync\s*\(/g; - const boundVars = new Set(); - let m; - while ((m = bindRe.exec(src)) !== null) { - boundVars.add(m[1]); - } - if (boundVars.size === 0) return []; - - // Pass 2: find .( on any bound var. - const findings = []; - // Build a regex alternation from the bound var names. - const alt = [...boundVars].map((v) => v.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|'); - const useRe = new RegExp( - `\\b(${alt})\\s*\\.\\s*(${TEXT_MATCH_METHODS.join('|')})\\s*\\(`, - 'g', - ); - while ((m = useRe.exec(src)) !== null) { - findings.push({ - kind: VIOLATION.VAR_FROM_READFILE_USED_IN_TEXT_MATCH, - variable: m[1], - method: m[2], - }); - } - return findings; -} - -/** - * Detects assert.ok(.match(/.../)) and assert.ok(.match()) - * which is the same anti-pattern as assert.match but escapes the simpler - * regex used by the base lint. - */ -function detectWrappedAssertOkMatch(src) { - const re = /assert\.ok\s*\(\s*[A-Za-z_$][\w$.]*\.match\s*\(/g; - const findings = []; - let m; - while ((m = re.exec(src)) !== null) { - findings.push({ kind: VIOLATION.WRAPPED_ASSERT_OK_MATCH }); - } - return findings; -} - -function detectAll(src) { - return [...detectVarBindingViolations(src), ...detectWrappedAssertOkMatch(src)]; -} - -module.exports = { detectVarBindingViolations, detectWrappedAssertOkMatch, detectAll, VIOLATION }; diff --git a/scripts/lint-no-source-grep.cjs b/scripts/lint-no-source-grep.cjs deleted file mode 100644 index b64f72968..000000000 --- a/scripts/lint-no-source-grep.cjs +++ /dev/null @@ -1,174 +0,0 @@ -#!/usr/bin/env node -/** - * lint-no-source-grep.cjs - * - * Enforces the "no source-grep tests" rule: - * Tests must NOT read source-code .cjs files with readFileSync to assert string - * presence. That pattern (source-grep theater) proves a literal exists in source, - * not that the runtime behavior is correct. - * - * ALLOWED: - * - require('../gsd-core/bin/lib/foo.cjs') -- runs the module, not text inspection - * - readFileSync on .md / .json / .txt files -- product-content or config output - * - Files annotated: // allow-test-rule: - * - * DISALLOWED (without allow-test-rule): - * - readFileSync where the path argument ends in a .cjs filename literal - * - A path constant (e.g. CONFIG_PATH) assigned to a .cjs lib file, used in readFileSync - * - * Exit 0 = clean. Exit 1 = violations found (with diagnostics). - */ - -'use strict'; - -const fs = require('fs'); -const path = require('path'); - -const TESTS_DIR = path.join(__dirname, '..', 'tests'); -const ALLOW_ANNOTATION = /\/\/\s*allow-test-rule:\s*\S/; - -// Matches constant definitions that hold a .cjs path in a SOURCE directory. -// Requires a source-dir indicator ('bin', 'lib', 'gsd-core') to avoid -// flagging temp files like path.join(tmpDir, 'example.cjs'). -// const CONFIG_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'config-schema.cjs'); -const CJS_PATH_CONST_RE = /(?:const|let|var)\s+(\w+)\s*=\s*path\.join\s*\([^)]*(?:'bin'|"bin"|'lib'|"lib"|'gsd-core'|"gsd-core")[^)]*['"][^'"]*\.cjs['"]/gm; - -// Matches readFileSync with a named variable as first arg -const READ_WITH_CONST_RE = /readFileSync\s*\(\s*([A-Za-z_][A-Za-z0-9_]*)\s*,/gm; - -// Matches readFileSync with an inline path.join(.cjs) as first arg -const READ_WITH_INLINE_CJS_RE = /readFileSync\s*\([^,)]*path\.join\s*\([^)]*(?:'bin'|"bin"|'lib'|"lib"|'gsd-core'|"gsd-core")[^)]*['"][^'"]*\.cjs['"]/; - -/** - * #2962-class violations: raw text matching against process output or file - * content. The rule from CONTRIBUTING.md "Prohibited: Raw Text Matching on - * Test Outputs": tests assert on typed structured fields, never on rendered - * text. Patterns below are the obvious anti-patterns; subtler hidden forms - * (e.g. wrapping the same logic in a parser function) are still forbidden - * by the prose rule but cannot be detected lexically without an AST. - */ -const RAW_MATCH_PATTERNS = [ - { - re: /assert\.(?:match|doesNotMatch)\s*\(\s*[A-Za-z_$][A-Za-z0-9_$]*\.(?:stdout|stderr)\b/, - label: 'assert.match/doesNotMatch on .stdout/.stderr (emit --json from the SUT and assert on typed fields)', - }, - { - re: /\.(?:stdout|stderr)\.(?:includes|startsWith|endsWith)\s*\(/, - label: '.stdout/.stderr substring match (emit --json and assert on typed fields)', - }, - { - re: /readFileSync\s*\([^)]*\)\s*\.(?:includes|startsWith|endsWith)\s*\(/, - label: 'readFileSync(...). (expose an IR from production code; assert on its fields)', - }, -]; - -function setFromMatches(content, re) { - const found = new Set(); - let m; - const cloned = new RegExp(re.source, re.flags); - while ((m = cloned.exec(content)) !== null) found.add(m[1]); - return found; -} - -function check(filepath) { - const content = fs.readFileSync(filepath, 'utf-8'); - const rel = path.relative(path.join(__dirname, '..'), filepath); - - if (ALLOW_ANNOTATION.test(content)) return null; - - const violations = []; - - // Pattern A: readFileSync(path.join(..., 'foo.cjs'), ...) - if (READ_WITH_INLINE_CJS_RE.test(content)) { - violations.push({ - reason: 'readFileSync with inline .cjs path literal', - fix: 'Replace with runGsdTools() behavioral test, or add // allow-test-rule: ', - }); - } - - // Pattern B: const FOO_PATH = path.join(..., 'foo.cjs') + readFileSync(FOO_PATH, ...) - const cjsConsts = setFromMatches(content, CJS_PATH_CONST_RE); - if (cjsConsts.size > 0) { - const readConsts = setFromMatches(content, READ_WITH_CONST_RE); - const overlap = [...cjsConsts].filter(c => readConsts.has(c)); - if (overlap.length > 0) { - violations.push({ - reason: `source .cjs path constant(s) used in readFileSync: ${overlap.join(', ')}`, - fix: 'Replace with runGsdTools() behavioral test, or add // allow-test-rule: ', - }); - } - } - - // Patterns C..E: raw text matching against process output or file content. - // See CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs". - for (const { re, label } of RAW_MATCH_PATTERNS) { - if (re.test(content)) { - violations.push({ - reason: label, - fix: 'Expose typed IR from production code; assert on structured fields. Or add // allow-test-rule: ', - }); - } - } - - // Patterns F..G (#2982): var-binding readFileSync().() and - // assert.ok(.match(...)). These escape the simpler patterns above - // because the bind and the use are on different lines or wrapped. - const extras = require('./lint-no-source-grep-extras.cjs'); - const varBindFindings = extras.detectVarBindingViolations(content); - if (varBindFindings.length > 0) { - const samples = varBindFindings.slice(0, 3) - .map((f) => `${f.variable}.${f.method}()`) - .join(', '); - violations.push({ - reason: `readFileSync-bound variable used in text-match method: ${samples}${varBindFindings.length > 3 ? `, …+${varBindFindings.length - 3} more` : ''}`, - fix: 'Expose typed IR; assert on structured fields. Or // allow-test-rule: ', - }); - } - const wrappedFindings = extras.detectWrappedAssertOkMatch(content); - if (wrappedFindings.length > 0) { - violations.push({ - reason: `assert.ok(.match(...)) — escapes assert.match rule (${wrappedFindings.length} occurrence${wrappedFindings.length > 1 ? 's' : ''})`, - fix: 'Use assert.equal on a typed field, not regex match on text. Or // allow-test-rule: ', - }); - } - - if (violations.length === 0) return null; - return { file: rel, violations }; -} - -function findTestFiles(dir) { - const results = []; - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const full = path.join(dir, entry.name); - if (entry.isDirectory()) { - results.push(...findTestFiles(full)); - } else if (entry.name.endsWith('.test.cjs')) { - results.push(full); - } - } - return results; -} - -const testFiles = findTestFiles(TESTS_DIR); - -const violations = testFiles.map(check).filter(Boolean); - -if (violations.length === 0) { - console.log(`ok lint-no-source-grep: ${testFiles.length} test files checked, 0 violations`); - process.exit(0); -} - -const totalIssues = violations.reduce((n, v) => n + v.violations.length, 0); -process.stderr.write(`\nERROR lint-no-source-grep: ${totalIssues} violation(s) across ${violations.length} file(s)\n\n`); -for (const f of violations) { - process.stderr.write(` ${f.file}\n`); - for (const v of f.violations) { - process.stderr.write(` Problem : ${v.reason}\n`); - process.stderr.write(` Fix : ${v.fix}\n`); - } - process.stderr.write('\n'); -} -process.stderr.write('See CONTRIBUTING.md "Prohibited: Source-Grep Tests" and\n'); -process.stderr.write('"Prohibited: Raw Text Matching on Test Outputs" for guidance.\n'); -process.stderr.write('Structural tests that legitimately read source files: add // allow-test-rule: \n\n'); -process.exit(1); diff --git a/scripts/lint-pr-check-project-dir.cjs b/scripts/lint-pr-check-project-dir.cjs index a3fd1a237..b3126528b 100644 --- a/scripts/lint-pr-check-project-dir.cjs +++ b/scripts/lint-pr-check-project-dir.cjs @@ -10,7 +10,6 @@ const DEFAULT_RELATIVE_FILES = [ '.github/workflows/test.yml', '.github/workflows/pr-template-format.yml', '.github/workflows/changeset-required.yml', - 'scripts/lint-no-source-grep.cjs', 'scripts/lint-command-contract.cjs', 'scripts/lint-skill-deps.cjs', 'scripts/lint-descriptions.cjs', diff --git a/tests/bug-2982-lint-var-binding.test.cjs b/tests/bug-2982-lint-var-binding.test.cjs deleted file mode 100644 index ec561b166..000000000 --- a/tests/bug-2982-lint-var-binding.test.cjs +++ /dev/null @@ -1,103 +0,0 @@ -// allow-test-rule: structural-regression-guard -// Reads hook .js or bin/install.js source to assert structural invariants -// (search array order, function wiring, path constants) that cannot be -// verified by observing runtime outputs alone. Per CONTRIBUTING.md exception matrix. -'use strict'; -// Migrated (#455): detectVarBindingViolations/detectWrappedAssertOkMatch are pure -// functions returning typed violation records; all assertions use typed fields. - -process.env.GSD_TEST_MODE = '1'; - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const path = require('node:path'); - -const { detectVarBindingViolations, VIOLATION } = require(path.join(__dirname, '..', 'scripts', 'lint-no-source-grep-extras.cjs')); - -// detectVarBindingViolations is pure: takes source text, returns a list of -// violation records. Tests assert on the structured records, not on the -// detector's prose (per "Prohibited: Raw Text Matching on Test Outputs"). - -describe('Bug #2982: var-binding readFileSync.includes() detector', () => { - test('VIOLATION enum exposes the documented codes', () => { - assert.deepEqual( - Object.keys(VIOLATION).sort(), - ['VAR_FROM_READFILE_USED_IN_TEXT_MATCH', 'WRAPPED_ASSERT_OK_MATCH'].sort(), - ); - }); - - test('flags a single var bound from readFileSync then used with .includes() later', () => { - const src = [ - "const x = fs.readFileSync('foo.cjs', 'utf8');", - '// some lines later', - "if (x.includes('foo')) { /* ... */ }", - ].join('\n'); - const findings = detectVarBindingViolations(src); - assert.equal(findings.length, 1); - assert.equal(findings[0].kind, VIOLATION.VAR_FROM_READFILE_USED_IN_TEXT_MATCH); - assert.equal(findings[0].variable, 'x'); - assert.equal(findings[0].method, 'includes'); - }); -}); - -describe('Bug #2982: var-binding detector — coverage of methods (#2982)', () => { - const { detectVarBindingViolations, VIOLATION } = require(require('node:path').join(__dirname, '..', 'scripts', 'lint-no-source-grep-extras.cjs')); - - for (const method of ['includes', 'startsWith', 'endsWith', 'match', 'search']) { - test(`flags .${method}( on a readFileSync-bound variable`, () => { - const src = `const c = fs.readFileSync('x.cjs','utf8');\nc.${method}('foo');\n`; - const findings = detectVarBindingViolations(src); - assert.equal(findings.length, 1); - assert.equal(findings[0].method, method); - }); - } - - test('flags multiple violations across multiple variables', () => { - const src = [ - "const a = readFileSync('a.cjs', 'utf8');", - "const b = fs.readFileSync('b.cjs');", - "if (a.includes('x')) {}", - "if (b.startsWith('y')) {}", - "a.endsWith('z');", - ].join('\n'); - const findings = detectVarBindingViolations(src); - assert.equal(findings.length, 3); - const byVar = findings.reduce((acc, f) => { - acc[f.variable] = (acc[f.variable] || []).concat(f.method); - return acc; - }, {}); - assert.deepEqual(byVar.a.sort(), ['endsWith', 'includes']); - assert.deepEqual(byVar.b, ['startsWith']); - }); - - test('does NOT flag .includes() on a variable that was not bound from readFileSync', () => { - const src = "const arr = [1, 2, 3];\nif (arr.includes(2)) {}"; - assert.deepEqual(detectVarBindingViolations(src), []); - }); - - test('does NOT flag a fresh string literal substring check', () => { - const src = "if ('hello world'.includes('world')) {}"; - assert.deepEqual(detectVarBindingViolations(src), []); - }); -}); - -describe('Bug #2982: assert.ok(...match(...)) detector', () => { - const { detectWrappedAssertOkMatch, VIOLATION } = require(require('node:path').join(__dirname, '..', 'scripts', 'lint-no-source-grep-extras.cjs')); - - test('flags assert.ok(text.match(/.../)) which escapes assert.match', () => { - const src = "assert.ok(text.match(/Failures: \\d+/));"; - const findings = detectWrappedAssertOkMatch(src); - assert.equal(findings.length, 1); - assert.equal(findings[0].kind, VIOLATION.WRAPPED_ASSERT_OK_MATCH); - }); - - test('does NOT flag assert.match itself (covered by base lint)', () => { - const src = "assert.match(text, /foo/);"; - assert.deepEqual(detectWrappedAssertOkMatch(src), []); - }); - - test('does NOT flag .matchAll(...) — matchAll is not match, so assert.ok(.matchAll(...)) is not flagged', () => { - const src = "assert.ok([...text.matchAll(/foo/g)].length > 0);"; - assert.deepEqual(detectWrappedAssertOkMatch(src), []); - }); -});