From 4d0df92290e8fe941d40a5b5001e2613c2a6b4ce Mon Sep 17 00:00:00 2001 From: implementer Date: Fri, 28 Aug 2026 08:30:34 +0000 Subject: [PATCH] feat: add dependency-boundary rule and architecture import test (E00-S01-T04) Adds a declarative dependency-boundary rule (dependency-boundaries.json) classifying workspace packages into apps/packages/extensions groups and forbidding packages/* (core) from importing extensions/* (concrete extensions); core depends only on extension contracts, never concrete extensions. Adds tests/architecture-import.test.mjs (node:test, zero new dependencies, lockfile untouched) that loads the rule, walks every workspace package's source and resolves each import/export/require specifier (bare workspace names, relative paths, dynamic import(), require(), export-from) to a group, failing on any forbidden core -> extension edge. Comment-aware scanning avoids false positives; line numbers in violation reports map to the original source. Synthetic negative cases prove detection (bare name, relative path, export-from, dynamic import, require), while the current compliant graph passes with zero violations. Verified: 10/10 tests pass; injecting a real core -> extension import makes the integration test fail with a per-file/line violation report; pnpm 11.23.0 install --frozen-lockfile passes unchanged (CI frozen-install job stays green). Closes #157 --- dependency-boundaries.json | 16 + tests/architecture-import.test.mjs | 488 +++++++++++++++++++++++++++++ 2 files changed, 504 insertions(+) create mode 100644 dependency-boundaries.json create mode 100644 tests/architecture-import.test.mjs diff --git a/dependency-boundaries.json b/dependency-boundaries.json new file mode 100644 index 0000000..a45341b --- /dev/null +++ b/dependency-boundaries.json @@ -0,0 +1,16 @@ +{ + "description": "Dependency-boundary rule for the EPPP workspace. Enforced by tests/architecture-import.test.mjs. Groups classify workspace packages by directory; boundaries declare which from-group -> to-group import edges are forbidden.", + "groups": { + "apps": { "path": "apps/" }, + "packages": { "path": "packages/" }, + "extensions": { "path": "extensions/" } + }, + "boundaries": [ + { + "from": "packages", + "to": "extensions", + "allow": false, + "reason": "Core packages must not import concrete extensions. Core depends only on extension contracts (abstract APIs); concrete extensions are wired in by the app layer." + } + ] +} diff --git a/tests/architecture-import.test.mjs b/tests/architecture-import.test.mjs new file mode 100644 index 0000000..56cff6e --- /dev/null +++ b/tests/architecture-import.test.mjs @@ -0,0 +1,488 @@ +/** + * Architecture import test — enforces the dependency-boundary rule + * declared in dependency-boundaries.json (E00-S01-T04). + * + * The rule: no core package (packages/*) may import a concrete extension + * (extensions/*). Core depends only on extension contracts (abstract APIs); + * concrete extensions are wired in by the app layer. + * + * The test loads the rule file, walks every workspace package's source and + * resolves each import/export/require specifier (bare workspace names, + * relative/absolute paths, dynamic import(), require(), export-from) to a + * workspace group, then fails on any forbidden edge. + * + * Run: `node --test tests/architecture-import.test.mjs` + * (node:test — built into Node >= 18; no dependencies, lockfile untouched.) + */ + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { readdir } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const RULE_FILE = path.join(REPO_ROOT, 'dependency-boundaries.json'); + +/** Extensions scanned as package source. */ +const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs']); + +/** Directories never scanned as source. */ +const IGNORED_DIRS = new Set(['node_modules', 'dist', 'coverage', '.git']); + +// --------------------------------------------------------------------------- +// Rule loading +// --------------------------------------------------------------------------- + +function loadRule(file = RULE_FILE) { + const raw = JSON.parse(readFileSync(file, 'utf8')); + assert.ok(raw && typeof raw === 'object', 'rule file must be a JSON object'); + assert.ok(raw.groups && typeof raw.groups === 'object', 'rule must declare groups'); + for (const [name, group] of Object.entries(raw.groups)) { + assert.ok(typeof group?.path === 'string', `group "${name}" must have a string path`); + } + assert.ok(Array.isArray(raw.boundaries), 'rule must declare boundaries'); + for (const b of raw.boundaries) { + assert.ok(b && typeof b === 'object', 'each boundary must be an object'); + assert.ok(typeof b.from === 'string', 'boundary must have a string "from"'); + assert.ok(typeof b.to === 'string', 'boundary must have a string "to"'); + assert.ok(typeof b.allow === 'boolean', `boundary ${b.from} -> ${b.to} must have a boolean "allow"`); + assert.ok(raw.groups[b.from], `boundary "from" group "${b.from}" is not declared in groups`); + assert.ok(raw.groups[b.to], `boundary "to" group "${b.to}" is not declared in groups`); + } + return raw; +} + +// --------------------------------------------------------------------------- +// Comment stripping (string/comment aware, preserves line count) +// --------------------------------------------------------------------------- + +/** + * Replaces comments with whitespace while preserving line structure, so + * specifier extraction never fires on commented-out imports and reported + * line numbers still match the original source. + */ +function stripComments(source) { + let out = ''; + let i = 0; + const n = source.length; + let state = 'code'; // code | line | block | sq | dq | tpl + const tplStack = []; // template states to resume after `${...}` closes + let braceDepth = 0; + + while (i < n) { + const c = source[i]; + const next = source[i + 1]; + + if (state === 'code') { + if (tplStack.length > 0) { + if (c === '{') { + braceDepth += 1; + out += c; + i += 1; + continue; + } + if (c === '}') { + braceDepth -= 1; + out += c; + i += 1; + if (braceDepth === 0) state = tplStack.pop(); + continue; + } + } + if (c === '/' && next === '/') { + out += ' '; + i += 2; + state = 'line'; + continue; + } + if (c === '/' && next === '*') { + out += ' '; + i += 2; + state = 'block'; + continue; + } + if (c === "'") { + out += c; + i += 1; + state = 'sq'; + continue; + } + if (c === '"') { + out += c; + i += 1; + state = 'dq'; + continue; + } + if (c === '`') { + out += c; + i += 1; + state = 'tpl'; + continue; + } + out += c; + i += 1; + continue; + } + + if (state === 'line') { + if (c === '\n') { + out += c; + i += 1; + state = 'code'; + } else { + out += ' '; + i += 1; + } + continue; + } + + if (state === 'block') { + if (c === '*' && next === '/') { + out += ' '; + i += 2; + state = 'code'; + } else { + out += c === '\n' ? c : ' '; + i += 1; + } + continue; + } + + if (state === 'sq' || state === 'dq') { + const quote = state === 'sq' ? "'" : '"'; + out += c; + if (c === '\\' && next !== undefined) { + out += next; + i += 2; + } else { + if (c === quote) state = 'code'; + i += 1; + } + continue; + } + + // template literal + out += c; + if (c === '\\' && next !== undefined) { + out += next; + i += 2; + continue; + } + if (c === '`') { + state = 'code'; + i += 1; + continue; + } + if (c === '$' && next === '{') { + out += next; + i += 2; + tplStack.push('tpl'); + braceDepth = 1; + state = 'code'; + continue; + } + i += 1; + } + return out; +} + +// --------------------------------------------------------------------------- +// Specifier extraction +// --------------------------------------------------------------------------- + +const STATIC_IMPORT = /\b(?:import|export)\s+(?:type\s+)?(?:[\w*{},\s]*?\s+from\s*)?(['"])([^'"]+)\1/g; +const REQUIRE_CALL = /\brequire\s*\(\s*(['"])([^'"]+)\1\s*\)/g; +const DYNAMIC_IMPORT = /\bimport\s*\(\s*(['"])([^'"]+)\1\s*\)/g; + +/** + * Returns [{ specifier, index }] for every module specifier in the + * (comment-stripped) source. `index` is an offset into the stripped source, + * which preserves line structure, so it maps back to the original lines. + */ +function extractSpecifiers(stripped) { + const found = []; + for (const re of [STATIC_IMPORT, REQUIRE_CALL, DYNAMIC_IMPORT]) { + re.lastIndex = 0; + let m; + while ((m = re.exec(stripped)) !== null) { + found.push({ specifier: m[2], index: m.index }); + } + } + found.sort((a, b) => a.index - b.index); + return found; +} + +// --------------------------------------------------------------------------- +// Workspace package discovery +// --------------------------------------------------------------------------- + +async function enumeratePackages(rule) { + const packages = []; + for (const [group, { path: groupPath }] of Object.entries(rule.groups)) { + const absGroup = path.join(REPO_ROOT, groupPath); + let entries; + try { + entries = await readdir(absGroup, { withFileTypes: true }); + } catch { + continue; // a declared group with no directory yet is not a package source + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const dir = path.join(absGroup, entry.name); + const manifestPath = path.join(dir, 'package.json'); + let manifest; + try { + manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); + } catch { + continue; // no package.json -> not a workspace package + } + assert.ok( + typeof manifest.name === 'string' && manifest.name.length > 0, + `${path.relative(REPO_ROOT, manifestPath)} must declare a non-empty "name"`, + ); + packages.push({ + name: manifest.name, + group, + dir, + manifestPath, + }); + } + } + return packages; +} + +/** Walks a package directory and returns [{ absPath, content }] for source files. */ +async function collectSourceFiles(packageDir) { + const files = []; + async function walk(dir) { + let entries; + try { + entries = await readdir(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + if (entry.isDirectory()) { + if (!IGNORED_DIRS.has(entry.name)) await walk(path.join(dir, entry.name)); + continue; + } + if (SOURCE_EXTENSIONS.has(path.extname(entry.name))) { + const absPath = path.join(dir, entry.name); + files.push({ absPath, content: readFileSync(absPath, 'utf8') }); + } + } + } + await walk(packageDir); + return files; +} + +// --------------------------------------------------------------------------- +// Target resolution + violation finding +// --------------------------------------------------------------------------- + +/** + * Resolves a module specifier to a workspace package, or null when it is not + * a workspace-internal reference (external dependency, node builtin, or a + * path that stays outside every workspace package). + */ +function resolveTarget(specifier, importingFile, packages) { + // Bare name or scoped subpath of a workspace package: @personal-blog/core, + // @personal-blog/example-extension/subpath, ... + const byName = packages.find( + (p) => specifier === p.name || specifier.startsWith(`${p.name}/`), + ); + if (byName) return byName; + + // Relative or absolute (file:) path that lands inside a workspace package. + if (specifier.startsWith('.') || specifier.startsWith('/') || specifier.startsWith('file:')) { + const base = specifier.startsWith('file:') ? specifier.slice('file:'.length) : specifier; + const resolved = path.resolve(path.dirname(importingFile), base); + const matches = packages + .filter((p) => resolved === p.dir || resolved.startsWith(`${p.dir}${path.sep}`)) + .sort((a, b) => b.dir.length - a.dir.length); + if (matches.length > 0) return matches[0]; + } + + return null; +} + +function lineOf(stripped, index) { + return stripped.slice(0, index).split('\n').length; +} + +/** + * Returns violations for the given source files under the rule. + * `packages` is the enumerated workspace package list; each file's importing + * package is the workspace package whose directory is its nearest ancestor. + */ +function findViolations(rule, packages, files) { + const violations = []; + for (const file of files) { + const fromPkg = packages + .filter((p) => file.absPath === p.dir || file.absPath.startsWith(`${p.dir}${path.sep}`)) + .sort((a, b) => b.dir.length - a.dir.length)[0]; + if (!fromPkg) continue; + + const stripped = stripComments(file.content); + for (const { specifier, index } of extractSpecifiers(stripped)) { + const target = resolveTarget(specifier, file.absPath, packages); + if (!target) continue; + const ruleEntry = rule.boundaries.find( + (b) => b.from === fromPkg.group && b.to === target.group, + ); + if (ruleEntry && ruleEntry.allow === false) { + violations.push({ + file: path.relative(REPO_ROOT, file.absPath), + line: lineOf(stripped, index), + specifier, + from: `${fromPkg.name} [${fromPkg.group}]`, + to: `${target.name} [${target.group}]`, + rule: `${ruleEntry.from} -> ${ruleEntry.to}`, + }); + } + } + } + return violations; +} + +// --------------------------------------------------------------------------- +// Test fixtures +// --------------------------------------------------------------------------- + +const V = '/virtual/workspace'; + +function virtualPackages() { + return [ + { name: '@personal-blog/server', group: 'apps', dir: `${V}/apps/server` }, + { name: '@personal-blog/core', group: 'packages', dir: `${V}/packages/core` }, + { name: '@personal-blog/example-extension', group: 'extensions', dir: `${V}/extensions/example` }, + ]; +} + +function virtualFile(relPath, content) { + return { absPath: `${V}/${relPath}`, content }; +} + +const rule = { + groups: { + apps: { path: 'apps/' }, + packages: { path: 'packages/' }, + extensions: { path: 'extensions/' }, + }, + boundaries: [ + { from: 'packages', to: 'extensions', allow: false, reason: 'core must not import concrete extensions' }, + ], +}; + +const CORE_FILE = 'packages/core/src/index.ts'; +const EXT_FILE = 'extensions/example/src/index.ts'; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test('dependency-boundaries.json is present and declares the core -> extensions boundary', () => { + const loaded = loadRule(); + const forbidden = loaded.boundaries.filter((b) => !b.allow); + assert.ok( + forbidden.some((b) => b.from === 'packages' && b.to === 'extensions'), + 'rule must forbid packages/* -> extensions/* imports', + ); +}); + +test('comment stripping ignores commented-out imports', () => { + const src = [ + '// import { x } from "@personal-blog/example-extension";', + '/* import { y } from \'@personal-blog/example-extension\'; */', + "import { ok } from './internal.js';", + '', + ].join('\n'); + const stripped = stripComments(src); + const specifiers = extractSpecifiers(stripped); + assert.deepEqual(specifiers.map((s) => s.specifier), ['./internal.js']); + assert.equal(lineOf(stripped, specifiers[0].index), 3); +}); + +test('stripping preserves line numbers across block comments', () => { + const src = 'const a = 1;\n/* multi\nline */\nimport { b } from "./x.js";\n'; + const stripped = stripComments(src); + const [spec] = extractSpecifiers(stripped); + assert.equal(spec.specifier, './x.js'); + assert.equal(lineOf(stripped, spec.index), 4); +}); + +test('core -> extension by bare package name is a violation', () => { + const violations = findViolations(rule, virtualPackages(), [ + virtualFile(CORE_FILE, "import { register } from '@personal-blog/example-extension';\n"), + ]); + assert.equal(violations.length, 1); + assert.equal(violations[0].rule, 'packages -> extensions'); + assert.equal(violations[0].to, '@personal-blog/example-extension [extensions]'); + assert.equal(violations[0].line, 1); +}); + +test('core -> extension by relative path is a violation', () => { + const violations = findViolations(rule, virtualPackages(), [ + virtualFile(CORE_FILE, "import { x } from '../../../extensions/example/src/index.js';\n"), + ]); + assert.equal(violations.length, 1); + assert.equal(violations[0].specifier, '../../../extensions/example/src/index.js'); +}); + +test('core -> extension via export-from, dynamic import and require are violations', () => { + const content = [ + "export { register } from '@personal-blog/example-extension';", + "const m = await import('@personal-blog/example-extension');", + "const r = require('@personal-blog/example-extension');", + '', + ].join('\n'); + const violations = findViolations(rule, virtualPackages(), [virtualFile(CORE_FILE, content)]); + assert.equal(violations.length, 3); +}); + +test('allowed directions produce no violations', () => { + const files = [ + virtualFile(EXT_FILE, "import { lifecycle } from '@personal-blog/core';\n"), + virtualFile('apps/server/src/index.ts', "import { core } from '@personal-blog/core';\n"), + virtualFile('extensions/example/src/index.ts', "import { x } from './internal.js';\n"), + ]; + assert.deepEqual(findViolations(rule, virtualPackages(), files), []); +}); + +test('external and node builtin imports are not violations', () => { + const files = [ + virtualFile(CORE_FILE, "import fastify from 'fastify';\nimport fs from 'node:fs';\nimport { join } from 'node:path';\n"), + ]; + assert.deepEqual(findViolations(rule, virtualPackages(), files), []); +}); + +test('core package importing another core package is not a violation', () => { + const packages = [ + ...virtualPackages(), + { name: '@personal-blog/domain', group: 'packages', dir: `${V}/packages/domain` }, + ]; + const files = [virtualFile('packages/domain/src/index.ts', "import { core } from '@personal-blog/core';\n")]; + assert.deepEqual(findViolations(rule, packages, files), []); +}); + +test('the architecture import test passes on the current compliant workspace graph', async () => { + const loaded = loadRule(); + const packages = await enumeratePackages(loaded); + const names = new Set(packages.map((p) => p.name)); + for (const expected of ['@personal-blog/server', '@personal-blog/core', '@personal-blog/example-extension']) { + assert.ok(names.has(expected), `expected workspace package ${expected} to be discovered`); + } + + const files = []; + for (const pkg of packages) { + files.push(...(await collectSourceFiles(pkg.dir))); + } + const violations = findViolations(loaded, packages, files); + + const report = violations + .map((v) => ` ${v.file}:${v.line} imports ${v.specifier} (${v.to}) — violates ${v.rule}`) + .join('\n'); + assert.deepEqual(violations, [], `no core package may import a concrete extension:\n${report}`); +});