Fail the gate on a function over 50 lines or complexity 20 #189

Merged
lilleman merged 7 commits from item-21 into main 2026-10-31 09:01:30 +01:00
Showing only changes of commit 6f689e517b - Show all commits
+49 -31
View File
@@ -1,19 +1,17 @@
// Fails the gate on a function over ESLint's default limits, measured as ESLint measures them: // The limits and the counting are ESLint's defaults: `max-lines-per-function` (blank lines and
// `max-lines-per-function` (50 lines, blanks and comments included, IIFEs skipped) and `complexity` // comments count, an IIFE is not held to it) and `complexity`. Not ESLint itself: parsing TypeScript
// (20). acorn parses what Node's own type stripping leaves, which is the code Node runs. Not ESLint // takes typescript-eslint, which needs the compiler API TypeScript 7 no longer ships. So acorn parses
// itself: parsing TypeScript takes typescript-eslint, which needs the compiler API TypeScript 7 no // what Node's own type stripping leaves, which is the code Node runs.
// longer ships. acorn has no dependencies.
import assert from "node:assert/strict"; import assert from "node:assert/strict";
import { globSync, readFileSync } from "node:fs"; import { globSync, readFileSync } from "node:fs";
import { stripTypeScriptTypes } from "node:module"; import { stripTypeScriptTypes } from "node:module";
import { test } from "node:test"; import { test } from "node:test";
import { parse } from "acorn"; import { parse } from "acorn";
const MAX_LINES = 50; const LIMITS = { lines: 50, complexity: 20 };
const MAX_COMPLEXITY = 20;
// Each function over a limit when the gate began, at its size then. It may shrink but never grow, // Each function over a limit, at its measurement. A ceiling follows its function down, and a metric
// and a metric leaves its entry once it fits. // leaves the entry once it fits, so a function that shrank can't grow back.
const grandfathered: Record<string, { lines?: number; complexity?: number }> = { const grandfathered: Record<string, { lines?: number; complexity?: number }> = {
"src/http/app.ts createApp": { lines: 318, complexity: 28 }, "src/http/app.ts createApp": { lines: 318, complexity: 28 },
"src/http/app.ts createApp > handleRequest": { lines: 168, complexity: 43 }, "src/http/app.ts createApp > handleRequest": { lines: 168, complexity: 43 },
@@ -67,8 +65,8 @@ function nameOf(fn: AstNode, parent: AstNode | undefined): string {
} }
// A function is named by the path of names from the outermost one, e.g. `createApp > handleRequest`, // A function is named by the path of names from the outermost one, e.g. `createApp > handleRequest`,
// so an entry above survives edits that move it. // so an entry above doesn't depend on line numbers.
export function measure(source: string): Measured[] { function measure(source: string): Measured[] {
const found: Measured[] = []; const found: Measured[] = [];
const visit = (node: AstNode, parent: AstNode | undefined, enclosing: Measured | undefined): void => { const visit = (node: AstNode, parent: AstNode | undefined, enclosing: Measured | undefined): void => {
let current = enclosing; let current = enclosing;
@@ -92,48 +90,68 @@ export function measure(source: string): Measured[] {
} }
const root = new URL("..", import.meta.url); const root = new URL("..", import.meta.url);
// The drop-in mounts hold an operator's code, not this repo's; tests read top to bottom as a script. // The drop-in mounts hold an operator's code, not this repo's. Tests are left out: each is a flat
// script, and a `describe` block wraps many of them.
const files = globSync("**/*.ts", { const files = globSync("**/*.ts", {
cwd: root, cwd: root,
exclude: ["**/node_modules/**", "plugins/**", "config/**", "locales/**", "**/*.d.ts", "**/*.test.ts", "**/*.spec.ts"], exclude: ["**/node_modules/**", "plugins/**", "config/**", "locales/**", "**/*.d.ts", "**/*.test.ts", "**/*.spec.ts"],
}); });
const measured = new Map<string, Measured>( // A list, not a map: two unnamed siblings share a key, and each must be checked.
files.flatMap((file) => measure(readFileSync(new URL(file, root), "utf8")).map((m) => [`${file} ${m.name}`, m] as const)), const measured = files.flatMap((file) =>
measure(readFileSync(new URL(file, root), "utf8")).map((m) => ({ ...m, key: `${file} ${m.name}` })),
); );
const METRICS = ["lines", "complexity"] as const;
test("measure counts lines and complexity as ESLint does", () => { test("measure counts lines and complexity as ESLint does", () => {
const source = [ const source = [
"function outer(a = 1): void {", "function outer(a = 1): void {",
" if (a && b?.c) {}", " if (a && b?.c) {}",
" for (const x of y) {}",
" for (const k in y) {}",
" for (;;) break;",
" while (a) a--;",
" do {} while (a);",
" try {} catch {}",
" switch (a) { case 1: break; default: }",
" a ||= 2; a &&= 3; a ??= 4;",
" b?.();",
" const inner = () => (a ? 1 : 2);", " const inner = () => (a ? 1 : 2);",
" const o = { prop() {}, key: function () {} };",
" (function () { if (a) {} })();", " (function () { if (a) {} })();",
"}", "}",
"class K { method() {} }",
].join("\n"); ].join("\n");
assert.deepEqual(measure(source), [ assert.deepEqual(measure(source), [
{ name: "outer", lines: 5, complexity: 5 }, { name: "outer", lines: 15, complexity: 16 },
{ name: "outer > inner", lines: 1, complexity: 2 }, { name: "outer > inner", lines: 1, complexity: 2 },
{ name: "outer > prop", lines: 1, complexity: 1 },
{ name: "outer > key", lines: 1, complexity: 1 },
{ name: "outer > anonymous", lines: 0, complexity: 2 }, { name: "outer > anonymous", lines: 0, complexity: 2 },
{ name: "method", lines: 1, complexity: 1 },
]); ]);
}); });
test(`every function fits ${MAX_LINES} lines and complexity ${MAX_COMPLEXITY}, or its grandfathered ceiling`, () => { test(`every function fits ${LIMITS.lines} lines and complexity ${LIMITS.complexity}, or its grandfathered ceiling`, () => {
assert.ok(files.length > 50, "scans the repo's TypeScript"); assert.ok(files.length > 50, "found too few files: is the glob broken?");
const over = [...measured].flatMap(([key, m]) => { const over = measured.flatMap((m) =>
const ceiling = grandfathered[key] ?? {}; METRICS.filter((metric) => m[metric] > (grandfathered[m.key]?.[metric] ?? LIMITS[metric])).map(
const lines = m.lines > (ceiling.lines ?? MAX_LINES) ? [`${key}: ${m.lines} lines`] : []; (metric) => `${m.key}: ${metric} ${m[metric]}`,
const complexity = m.complexity > (ceiling.complexity ?? MAX_COMPLEXITY) ? [`${key}: complexity ${m.complexity}`] : []; ),
return [...lines, ...complexity]; );
});
assert.deepEqual(over, [], "split the function; never raise a ceiling"); assert.deepEqual(over, [], "split the function; never raise a ceiling");
}); });
test("every grandfathered metric is still over its limit", () => { test("every grandfathered ceiling equals its function's measurement, which is over the limit", () => {
const fits = Object.entries(grandfathered).flatMap(([key, ceiling]) => { const stale = Object.entries(grandfathered).flatMap(([key, ceiling]) => {
const m = measured.get(key); const found = measured.filter((m) => m.key === key);
if (!m) return [`${key}: gone`]; if (found.length === 0) return [`${key}: no such function; rename the entry if it moved, else delete it`];
const lines = ceiling.lines !== undefined && m.lines <= MAX_LINES ? [`${key}: lines fit`] : []; return METRICS.flatMap((metric) => {
const complexity = ceiling.complexity !== undefined && m.complexity <= MAX_COMPLEXITY ? [`${key}: complexity fits`] : []; const allowed = ceiling[metric];
return [...lines, ...complexity]; if (allowed === undefined) return [];
const now = Math.max(...found.map((m) => m[metric]));
if (now <= LIMITS[metric]) return [`${key}: ${metric} ${now} fits; delete it`];
return now < allowed ? [`${key}: ${metric} ${now}; lower the ceiling to it`] : [];
}); });
assert.deepEqual(fits, [], "delete what fits from the grandfathered list"); });
assert.deepEqual(stale, []);
}); });