feat: add dependency-boundary rule and architecture import test (E00-S01-T04)
CI / Frozen lockfile install (pull_request) Successful in 38s
CI / Frozen lockfile install (pull_request) Successful in 38s
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
This commit is contained in:
implementer
@@ -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."
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -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}`);
|
||||
});
|
||||
Reference in New Issue
Block a user