From 8ce4663e118eb2cb00e644e665225820124acdc6 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Sun, 2 Aug 2026 15:07:26 +0100 Subject: [PATCH] fix(co-occurrence): exclude import declarations from clone detection at the tokenizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two modules using the same shared libs are forced into identical, linter-sorted import blocks — the module system working, not copy-paste debt — and a big enough preamble clears jscpd's 50-token floor on its own, false-blocking any commit touching sibling files once the gate is wired into hooks (post-#305 the repo scan showed two such blocks). Imports are now removed from the token stream itself via jscpd --ignore-pattern (four bounded, comma-free regexes: named/multi-line, default/namespace single-line, side-effect, re-export barrels; dynamic import(...) deliberately matches none). The semantic becomes "clones are measured over non-import code": nothing can ride a preamble over the token floor, and nothing real can hide behind one. A post-hoc fragment classifier was tried first and abandoned after six opus-confirmed reviewer holes across seven gate rounds (ratio leaks, dynamic import absorption, quote-terminated statements, import attributes, ...) — each patch surfaced the next hole because classifying jscpd's token-boundary fragments after the fact is unfixable by construction. The adversarial cases live on as end-to-end gate-contract tests: preamble-only pair passes, a real body behind identical imports still blocks, a duplicated dynamic-import block still blocks. Live scan 62 -> 60; the dropped sync-agents fragment's residual interface dup is sub-floor on its own merits (it only crossed 50 tokens by riding the imports). Co-Authored-By: Claude Fable 5 --- .../__tests__/clone-detector.test.mts | 49 +++++++++++++++++++ gate-engine/co-occurrence/clone-detector.mts | 32 ++++++++++++ 2 files changed, 81 insertions(+) diff --git a/gate-engine/co-occurrence/__tests__/clone-detector.test.mts b/gate-engine/co-occurrence/__tests__/clone-detector.test.mts index 05d1c5e..e5e3043 100644 --- a/gate-engine/co-occurrence/__tests__/clone-detector.test.mts +++ b/gate-engine/co-occurrence/__tests__/clone-detector.test.mts @@ -171,6 +171,55 @@ describe.skipIf(!HAS_JSCPD)('clone-detector --gate exit-code contract', () => { rmSync(mjs, { recursive: true, force: true }); }); + it('exit 0 — an identical import preamble alone is not a clone (module-system forced)', () => { + // Big enough to clear jscpd's 50-token floor on its own — the exact shape that false-blocked + // sibling CLI commands sharing the same libs. Imports are ignore-pattern'd out at the + // tokenizer, so no fragment can even contain them. + const imports = Array.from( + { length: 14 }, + (_, i) => `import { helperNumber${i}, otherThing${i} } from './lib/module-${i}.ts';`, + ).join('\n'); + const dir = mkdtempSync(join(tmpdir(), 'clone-imports-')); + writeFileSync(join(dir, 'a.ts'), `${imports}\nconst runA = () => helperNumber0(1);\n`); + writeFileSync( + join(dir, 'b.ts'), + `${imports}\nfunction makeB() {\n return otherThing3(2, 3);\n}\n`, + ); + expect(run(['scan', '--gate', '--paths', dir], {}).status).toBe(0); + rmSync(dir, { recursive: true, force: true }); + }); + + it('exit 1 — a real duplicated body behind an identical import preamble still blocks', () => { + // The whole reason the ignore-pattern approach won over a post-hoc fragment classifier: six + // reviewer-verified classifier holes each let some duplicated statement hide behind imports. + // With imports gone from the token stream, the shared body must trip the floor on its own. + const imports = Array.from( + { length: 6 }, + (_, i) => `import { helperNumber${i} } from './lib/module-${i}.ts';`, + ).join('\n'); + const dir = mkdtempSync(join(tmpdir(), 'clone-imports-body-')); + writeFileSync(join(dir, 'a.ts'), `${imports}\n${SHARED}\nexport const A_ONLY = 1;\n`); + writeFileSync(join(dir, 'b.ts'), `${imports}\n${SHARED}\nexport const B_ONLY = 2;\n`); + expect(run(['scan', '--gate', '--paths', dir], {}).status).toBe(1); + rmSync(dir, { recursive: true, force: true }); + }); + + it('exit 1 — a duplicated dynamic-import block is runtime code and still blocks', () => { + // Dynamic import(...) deliberately matches none of the ignore patterns. + const block = `export function loadWidgetPlugin(registry) { + return import('./widget-plugin').then((mod) => { + registry.register(mod.widgetName, mod.createWidget); + registry.enable(mod.widgetName, { eager: true, retries: 3 }); + return mod.initialize(registry, { verbose: false, timeoutMs: 5000 }); + }); +}`; + const dir = mkdtempSync(join(tmpdir(), 'clone-dynimport-')); + writeFileSync(join(dir, 'a.ts'), `${block}\nexport const A_ONLY = 1;\n`); + writeFileSync(join(dir, 'b.ts'), `${block}\nexport const B_ONLY = 2;\n`); + expect(run(['scan', '--gate', '--paths', dir], {}).status).toBe(1); + rmSync(dir, { recursive: true, force: true }); + }); + it('exit 0 — no cross-file clone (clean)', () => { const clean = mkdtempSync(join(tmpdir(), 'clone-clean-')); writeFileSync(join(clean, 'solo.ts'), `${SHARED}\nexport const SOLO = 1;\n`); diff --git a/gate-engine/co-occurrence/clone-detector.mts b/gate-engine/co-occurrence/clone-detector.mts index 3399a52..2f45f1d 100755 --- a/gate-engine/co-occurrence/clone-detector.mts +++ b/gate-engine/co-occurrence/clone-detector.mts @@ -151,6 +151,8 @@ export function detectClones({ out, '--ignore', DEFAULTS.ignore.join(','), + '--ignore-pattern', + IMPORT_IGNORE_PATTERNS.join(','), '--silent', ], { cwd: repoRoot, stdio: ['ignore', 'ignore', 'pipe'] }, @@ -208,6 +210,36 @@ export function relPath(f: string): string { return path.startsWith(`${root}/`) ? path.slice(root.length + 1) : path; } +/** + * Import/re-export declarations are excluded from clone detection AT THE TOKENIZER, via jscpd's + * `--ignore-pattern` (each entry becomes an ignore token rule in the format grammar; matched + * text never enters any fragment). Two modules using the same shared libs are FORCED into + * identical, linter-sorted import preambles — the module system working, not copy-paste debt — + * and a big enough preamble clears the min-tokens floor on its own, false-blocking sibling + * files. Removing imports up front gives the clean semantic "clones are measured over + * non-import code": nothing can ride a preamble over the token floor, and nothing real can hide + * behind one. (A post-hoc fragment classifier was tried first; six reviewer-verified holes — + * ratio leaks, dynamic import(...), quote-terminated statements, import attributes — showed + * that design to be unfixable. See the gate-contract tests.) + * + * Each pattern is bounded so it can never span arbitrary code: single-line forms forbid `\n`, + * the multi-line named form crosses lines only inside `{...}` and requires the `} from '...'` + * closer. Dynamic `import(...)` is runtime code and deliberately matches none of them. jscpd's + * CLI comma-splits this option, so patterns must stay comma-free. Unmatched exotic shapes + * (import attributes, default+named multi-line) fall through to normal tokenization — the safe + * direction: worst case a clone REPORTS and the allowlist is the remedy, never the reverse. + */ +export const IMPORT_IGNORE_PATTERNS = [ + // Named / namespace-in-braces, incl. multi-line specifier lists: import [type] { ... } from 'x'; + String.raw`import\s*(type\s+)?\{[^}]*\}\s*from\s*['"][^'"]+['"];?`, + // Default / namespace, single-line only: import foo from 'x'; import * as ns from 'x'; + String.raw`import\s+(type\s+)?[\w$*][^\n]*?from\s*['"][^'"]+['"];?`, + // Side-effect: import 'x'; + String.raw`import\s*['"][^'"]+['"];?`, + // Re-export barrels: export { a } from 'x'; / export * from 'x'; + String.raw`export\s*(\{[^}]*\}|\*)\s*from\s*['"][^'"]+['"];?`, +]; + /** Stable key: hash the fragment with whitespace collapsed so reformatting * doesn't change the key, but real code changes do. */ export function hashFragment(fragment: string): string {