diff --git a/package.json b/package.json index cf814805..7e9a3617 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,8 @@ "doctor": "node tools/doctor.mjs", "launch": "node scripts/launch.mjs", "lint": "eslint .", - "typecheck": "tsc --noEmit --checkJs -p jsconfig.json" + "typecheck": "tsc --noEmit --checkJs -p jsconfig.json", + "check:imports": "node tools/check-imports.mjs" }, "engines": { "node": ">=22" diff --git a/test/check-imports.test.js b/test/check-imports.test.js new file mode 100644 index 00000000..7423524f --- /dev/null +++ b/test/check-imports.test.js @@ -0,0 +1,110 @@ +// Boundary checker: fixture rules, plus the real tree's current violation list. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { findSpecifiers, scan } from '../tools/check-imports.mjs'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); + +function fixture(files) { + const root = mkdtempSync(join(tmpdir(), 'sp-imp-')); + for (const [rel, body] of Object.entries(files)) { + const abs = join(root, rel); + mkdirSync(dirname(abs), { recursive: true }); + writeFileSync(abs, body); + } + return root; +} + +test('comments are not imports, and a multiline import is', () => { + const specs = findSpecifiers(` + // import fs from 'node:fs' + /* require('node:path') */ + import { + a, + b, + } from './tier1.js'; + export function fromThing() { return 1; } + `); + assert.deepEqual(specs.map((s) => s.spec), ['./tier1.js']); +}); + +test('sim and match rules on a fixture tree', () => { + const root = fixture({ + 'server/sim/clean.js': ` + // import fs from 'node:fs' + import { a } from './other.js'; + export function load() { return 1; } + export { a }; + `, + 'server/sim/bad.js': ` + import '../match/Match.js'; + import '../lobby.js'; + import '../index.js'; + import '../net.js'; + import '../../public/js/main.js'; + import fs from 'node:fs'; + import 'fs'; + export { z } from '../match/board.js'; + const p = await import('node:path'); + `, + 'server/match/ok.js': ` + import '../sim/rng.js'; + import 'node:fs'; + import '../data.js'; + `, + 'server/match/bad.js': ` + import '../../public/js/main.js'; + import 'node:http'; + import 'ws'; + import '../net.js'; + import '../index.js'; + `, + }); + try { + const keys = scan(root).violations.map((v) => `${v.file} ${v.spec} ${v.code}`).sort(); + assert.deepEqual(keys, [ + 'server/match/bad.js ../../public/js/main.js match-client', + 'server/match/bad.js ../index.js match-http', + 'server/match/bad.js ../net.js match-http', + 'server/match/bad.js node:http match-http', + 'server/match/bad.js ws match-http', + 'server/sim/bad.js ../../public/js/main.js sim-client', + 'server/sim/bad.js ../index.js sim-entry', + 'server/sim/bad.js ../lobby.js sim-lobby', + 'server/sim/bad.js ../match/Match.js sim-match', + 'server/sim/bad.js ../match/board.js sim-match', + 'server/sim/bad.js ../net.js sim-http', + 'server/sim/bad.js fs sim-node', + 'server/sim/bad.js node:fs sim-node', + 'server/sim/bad.js node:path sim-node', + ]); + assert.equal(scan(root).notes.length, 0); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('this repo: only the Node data loader crosses the sim boundary', () => { + const { violations, notes } = scan(ROOT); + assert.deepEqual(violations.map((v) => `${v.file}:${v.line}: ${v.spec} [${v.code}]`), [ + 'server/sim/nodeData.js:11: node:fs [sim-node]', + 'server/sim/nodeData.js:12: node:path [sim-node]', + 'server/sim/nodeData.js:13: node:url [sim-node]', + ]); + assert.ok(notes.some((n) => n.file === 'server/sim/nodeData.js' && n.resolved === 'server/data.js')); + assert.ok(notes.every((n) => n.resolved === 'server/data.js')); +}); + +test('the CLI lists hits and fails only with --strict', () => { + const loose = spawnSync(process.execPath, ['tools/check-imports.mjs'], { cwd: ROOT, encoding: 'utf8' }); + assert.equal(loose.status, 0); + assert.match(loose.stdout, /server\/sim\/nodeData\.js:11: node:fs \[sim-node\]/); + const strict = spawnSync(process.execPath, ['tools/check-imports.mjs', '--strict'], { cwd: ROOT, encoding: 'utf8' }); + assert.equal(strict.status, 1); + assert.match(strict.stdout, /node:fs/); +}); diff --git a/tools/check-imports.mjs b/tools/check-imports.mjs new file mode 100644 index 00000000..d8e27c51 --- /dev/null +++ b/tools/check-imports.mjs @@ -0,0 +1,310 @@ +// Import boundaries for the sim and the match layer. +// +// server/sim is served to the browser, so it must not import the match, the lobby, the HTTP entry, +// the net layer, public/, or any Node builtin. server/match must not import public/ or that net layer. +// server/data.js is allowed (the Node loader reaches it on purpose). +// +// Default exit code is 0 even when violations are listed. --strict exits 1 when any are listed. +// Usage: node tools/check-imports.mjs [--strict] [--root ] + +import { readdirSync, readFileSync, realpathSync, statSync } from 'node:fs'; +import { builtinModules } from 'node:module'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const HTTP_SPECS = new Set([ + 'http', 'https', 'http2', 'net', 'tls', 'dgram', 'ws', + 'node:http', 'node:https', 'node:http2', 'node:net', 'node:tls', 'node:dgram', +]); + +const builtins = new Set(builtinModules); + +/** Drop comments. Strings, including the import specifiers, stay. Newlines stay so line numbers match. */ +export function maskComments(src) { + let out = ''; + let i = 0; + while (i < src.length) { + const c = src[i]; + const n = src[i + 1]; + if (c === '/' && n === '/') { + i += 2; + while (i < src.length && src[i] !== '\n') i++; + continue; + } + if (c === '/' && n === '*') { + i += 2; + while (i < src.length && !(src[i] === '*' && src[i + 1] === '/')) { + out += src[i] === '\n' ? '\n' : ' '; + i++; + } + i += 2; + continue; + } + if (c === "'" || c === '"') { + const end = endOfString(src, i); + out += src.slice(i, end); + i = end; + continue; + } + if (c === '`') { + const end = endOfTemplate(src, i); + out += src.slice(i, end); + i = end; + continue; + } + out += c; + i++; + } + return out; +} + +function endOfString(src, i) { + const q = src[i]; + let j = i + 1; + while (j < src.length && src[j] !== q) { + if (src[j] === '\\') { j += 2; continue; } + if (src[j] === '\n') break; + j++; + } + return Math.min(src.length, j + 1); +} + +function endOfTemplate(src, i) { + let j = i + 1; + while (j < src.length && src[j] !== '`') { + if (src[j] === '\\') { j += 2; continue; } + if (src[j] === '$' && src[j + 1] === '{') { + j += 2; + let depth = 1; + while (j < src.length && depth > 0) { + if (src[j] === "'" || src[j] === '"') { j = endOfString(src, j); continue; } + if (src[j] === '`') { j = endOfTemplate(src, j); continue; } + if (src[j] === '{') depth++; + else if (src[j] === '}') depth--; + if (depth > 0) j++; + } + j++; + continue; + } + j++; + } + return Math.min(src.length, j + 1); +} + +function lineOf(src, index) { + let line = 1; + for (let i = 0; i < index && i < src.length; i++) if (src[i] === '\n') line++; + return line; +} + +function skipWs(src, i) { + while (i < src.length && /\s/.test(src[i])) i++; + return i; +} + +function readQuoted(src, i) { + if (src[i] !== "'" && src[i] !== '"') return null; + const q = src[i]; + let j = i + 1; + let spec = ''; + while (j < src.length && src[j] !== q) { + if (src[j] === '\\') { spec += src[j + 1] ?? ''; j += 2; continue; } + if (src[j] === '\n') return null; + spec += src[j]; + j++; + } + if (src[j] !== q) return null; + return { spec, quoteAt: i, end: j + 1 }; +} + +/** The `from 'spec'` of an import/export clause, or null when the statement is not one. */ +function readFrom(src, i) { + let depth = 0; + const start = i; + while (i < src.length && i - start < 8000) { + const c = src[i]; + if (c === "'" || c === '"') { + const str = readQuoted(src, i); + if (!str) return null; + i = str.end; + continue; + } + if (c === '{') depth++; + else if (c === '}') depth = Math.max(0, depth - 1); + else if (c === ';' && depth === 0) return null; + else if (depth === 0 && src.startsWith('from', i) && !/\w/.test(src[i - 1] ?? '') && !/\w/.test(src[i + 4] ?? '')) { + const str = readQuoted(src, skipWs(src, i + 4)); + return str; + } + i++; + } + return null; +} + +/** + * @param {string} src raw source + * @returns {{ spec: string, line: number }[]} + */ +export function findSpecifiers(src) { + const masked = maskComments(src); + /** @type {{ spec: string, line: number }[]} */ + const found = []; + const re = /\b(import|export|require)\b/g; + let m; + while ((m = re.exec(masked))) { + const kw = m[1]; + let i = skipWs(masked, m.index + kw.length); + if (kw === 'require') { + if (masked[i] !== '(') continue; + const str = readQuoted(masked, skipWs(masked, i + 1)); + if (str) found.push({ spec: str.spec, line: lineOf(masked, str.quoteAt) }); + continue; + } + if (kw === 'import' && masked[i] === '.') continue; // import.meta + if (kw === 'import' && masked[i] === '(') { + const str = readQuoted(masked, skipWs(masked, i + 1)); + if (str) found.push({ spec: str.spec, line: lineOf(masked, str.quoteAt) }); + continue; + } + if (kw === 'import' && (masked[i] === "'" || masked[i] === '"')) { + const str = readQuoted(masked, i); + if (str) found.push({ spec: str.spec, line: lineOf(masked, str.quoteAt) }); + continue; + } + if (kw === 'export') { + if (masked.startsWith('type', i) && /\s/.test(masked[i + 4] ?? '')) i = skipWs(masked, i + 4); + const c = masked[i]; + if (c !== '{' && c !== '*') continue; + } + const from = readFrom(masked, i); + if (from) found.push({ spec: from.spec, line: lineOf(masked, from.quoteAt) }); + } + return found; +} + +function isBuiltin(spec) { + if (spec.startsWith('node:')) return true; + return builtins.has(spec); +} + +function isHttp(spec, resolved) { + if (HTTP_SPECS.has(spec)) return true; + if (spec === 'ws' || spec.startsWith('ws/')) return true; + if (resolved === 'server/net.js' || resolved === 'server/index.js') return true; + return false; +} + +/** + * @param {string} spec + * @param {string} fromFile posix path relative to the root + * @param {'sim'|'match'} area + * @returns {{ code: string, resolved: string } | { note: true, resolved: string } | null} + */ +export function classify(spec, fromFile, area) { + const fromDir = path.posix.dirname(fromFile); + let resolved = spec; + if (spec.startsWith('.')) { + resolved = path.posix.normalize(path.posix.join(fromDir, spec)); + } + const escaped = resolved.startsWith('..') || path.posix.isAbsolute(spec); + if (area === 'sim') { + if (resolved === 'server/match' || resolved.startsWith('server/match/')) return { code: 'sim-match', resolved }; + if (resolved === 'server/lobby.js' || resolved.startsWith('server/lobby/')) return { code: 'sim-lobby', resolved }; + if (resolved === 'server/index.js') return { code: 'sim-entry', resolved }; + if (isHttp(spec, resolved)) return { code: 'sim-http', resolved }; + if (resolved === 'public' || resolved.startsWith('public/')) return { code: 'sim-client', resolved }; + if (escaped || isBuiltin(spec)) return { code: 'sim-node', resolved }; + if (resolved === 'server/data.js') return { note: true, resolved }; + return null; + } + if (resolved === 'public' || resolved.startsWith('public/')) return { code: 'match-client', resolved }; + if (isHttp(spec, resolved)) return { code: 'match-http', resolved }; + return null; +} + +function walkJs(dir, rel, out) { + let entries; + try { entries = readdirSync(dir); } catch { return; } + for (const name of entries) { + if (name === 'node_modules') continue; + const abs = path.join(dir, name); + const child = rel ? `${rel}/${name}` : name; + let st; + try { st = statSync(abs); } catch { continue; } + if (st.isDirectory()) walkJs(abs, child, out); + else if (/\.(?:js|mjs|cjs)$/.test(name)) out.push(child.split(path.sep).join('/')); + } +} + +/** + * @param {string} root absolute repository root + * @returns {{ violations: { file: string, line: number, spec: string, code: string, resolved: string }[], notes: { file: string, line: number, spec: string, resolved: string }[] }} + */ +export function scan(root) { + const files = []; + walkJs(path.join(root, 'server', 'sim'), 'server/sim', files); + walkJs(path.join(root, 'server', 'match'), 'server/match', files); + /** @type {{ file: string, line: number, spec: string, code: string, resolved: string }[]} */ + const violations = []; + /** @type {{ file: string, line: number, spec: string, resolved: string }[]} */ + const notes = []; + for (const file of files) { + const area = file.startsWith('server/sim/') ? 'sim' : 'match'; + let src; + try { src = readFileSync(path.join(root, file), 'utf8'); } catch { continue; } + for (const hit of findSpecifiers(src)) { + const result = classify(hit.spec, file, area); + if (!result) continue; + if (result.note) notes.push({ file, line: hit.line, spec: hit.spec, resolved: result.resolved }); + else violations.push({ file, line: hit.line, spec: hit.spec, code: result.code, resolved: result.resolved }); + } + } + const byPos = (a, b) => a.file.localeCompare(b.file) || a.line - b.line || a.spec.localeCompare(b.spec); + violations.sort(byPos); + notes.sort(byPos); + return { violations, notes }; +} + +export function formatReport({ violations, notes }) { + const lines = []; + if (violations.length === 0) lines.push('no import-boundary violations'); + else { + lines.push(`${violations.length} import-boundary violation(s):`); + for (const v of violations) lines.push(`${v.file}:${v.line}: ${v.spec} [${v.code}]`); + } + if (notes.length) { + lines.push(`${notes.length} allowed note(s):`); + for (const n of notes) lines.push(`note ${n.file}:${n.line}: ${n.spec} -> ${n.resolved}`); + } + return lines.join('\n') + '\n'; +} + +function main(argv) { + let strict = false; + let root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === '--strict') strict = true; + else if (a === '--root') { + const dir = argv[++i]; + if (!dir || dir.startsWith('--')) { + console.error('check-imports: --root needs a directory'); + return 1; + } + root = path.resolve(dir); + } else if (a === '--help' || a === '-h') { + console.log('usage: node tools/check-imports.mjs [--strict] [--root ]'); + return 0; + } else { + console.error(`check-imports: unknown argument ${a}`); + return 1; + } + } + const report = scan(root); + process.stdout.write(formatReport(report)); + if (strict && report.violations.length) return 1; + return 0; +} + +const invoked = process.argv[1] && import.meta.url === pathToFileURL(realpathSync(process.argv[1])).href; +if (invoked) process.exit(main(process.argv.slice(2)));