Skip to content

Commit 67ea334

Browse files
refactor(devx): move the four inline ESLint rule plugins into a private workspace package (#26177)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi97zYZvbkizsECxcnTpJKKpP)_ Fixes #26093 Clause-②: no Metadata-protocol: none ## What changed The four inline rule plugins in `eslint.config.mjs` (slot-lookup, query-options, verify-stand-in, comment-swallow) move into a private workspace package, `packages/eslint-plugin` (`@objectstack/eslint-plugin`, `"private": true`, no build step). The root config imports it by relative path and keeps only configuration. - **Plain ESM, not TypeScript.** `pnpm lint` loads the config with native `import`, and the repo's `engines` floor is Node 22.0; a `.ts` source would need Node 22.18 type stripping. `tsconfig.json` sets `allowJs` + `checkJs`, so `tsc` reads the rule sources too (JSDoc types; AST helpers are deliberately untyped, documented in each file). - **Rule ids, messages and plugin namespaces are unchanged.** Each rule keeps its prefix (`slot-lookup/…` etc.), so the ratchet scripts and their rule-id / message matching see the same thing. - **Configuration stays in the config.** `SLOT_LOOKUPS`, `UNCONTRACTED_SLOTS` and `VERIFY_STAND_IN_CHECKS` stay in `eslint.config.mjs` (the slot-lookup selectors read the first two, and both the message text and `check:verify-stand-in` say they live there). Those two rules now get the lists as **required** options: a config that drops them fails ESLint's schema validation instead of running against an empty set (pinned in the tests). - **The export surface of `eslint.config.mjs` is kept.** Every named export the scripts import is still exported (re-exported from the package). The only one dropped is `COMMENT_SWALLOW_CASES`, which nothing imported. It moved into the tests. - **The self-assert block is now tests.** `assertCommentSwallow()` and its 17 pinned cases became `test/comment-swallow.test.ts`, along with the "files glob must cover plain JS" assertion. - `pnpm-lock.yaml`: only the new importer (+18 lines, from `pnpm install`). No new resolutions. `pnpm-workspace.yaml` untouched (`packages/*` glob). No baseline file changed. ## Behaviour preserved: before / after Before = `origin/main` d1ea236, after = head 4d18cf1, same worktree: | reading | before | after | |---|---|---| | `pnpm lint` (exact root script) | exit 0, no output | exit 0, no output; byte-identical (diff empty apart from the worktree path in the pnpm banner) | | `pnpm check:slot-lookup` | 100 unswept sites in 25 files; self-test 5 shapes / 8 canonical | identical output (diff empty) | | `pnpm check:query-options-erasure` | 50 non-test sites in 15 files; test surface 236 in 47 | identical output (diff empty) | | `pnpm check:verify-stand-in` | 2 guarded, 18 exempt, 10 call sites, 0 asserted | identical output (diff empty) | `pnpm lint` has nothing to report on either side, so the empty diff is weak evidence by itself. The three ratchets are the strong reading: they import this config, lift its grandfathering, and count each rule's reports per file, and their self-tests drive the live rules through the config ("reports all 5 erasure shapes"). Their outputs are byte-identical before and after. ## Comments: moved, not rewritten (#25919 ruling A) Of the 635 non-blank lines removed from `eslint.config.mjs`, 585 appear verbatim in the package. Each plugin file has a short header saying the history moved from the config, so "above" / "below" in it refer to that file. The other 50 lines are accounted for: - rule code lines that gained a JSDoc type, an `export`, or read options instead of the config constants; - the `assertCommentSwallow()` function body, rewritten as vitest assertions; - two pointers that named `assertCommentSwallow` (one in the package, one left in the config) now point at the test; - the cases docblock, whose "runs where the config loads" sentence is no longer true. Config-block comments that justify configuration (scope, ignores, the lists) stay where they were. The query-options block's "dedicated plugin rule, not selectors" + known-residual paragraphs moved, since they describe the rule. Three new one-line pointers name the rule file. ## Acceptance - [x] `pnpm lint` identical before/after: 0 errors, same (empty) warning set. Table above. - [ ] `wc -l eslint.config.mjs` under about 400: **649** (was 1,271). Deviation, see below. - [x] Tests run in the lane `packages/lint` runs in. `scripts/partition-test-shards.mjs` over `turbo ls --affected` puts `@objectstack/eslint-plugin` on Test Core shard 1/8. Locally: `pnpm --filter @objectstack/eslint-plugin test` gives 4 files, 73 tests, all pass. - [x] `pnpm typecheck` covers the new package. `turbo run typecheck --filter=@objectstack/eslint-plugin` runs `tsc --noEmit` with exit 0. `--listFiles` shows all 5 `src/*.mjs`, all 5 `test/*.ts` and `vitest.config.ts`. `check:type-check-coverage` and `check:type-check-debt` are green. ### Why 649 and not about 400 The card's estimate takes 1,271 minus the "906 lines of plugins". But lines 1–120 of the old file are the lint-script header (`--stack-size` rationale, ~85 lines) and the subpath / defineX constants, and several constants in the 1–906 range are configuration that has to stay (slot lists, both baselines, `QUERY_OPTIONS_TEST_GLOBS`, `VERIFY_STAND_IN_CHECKS`, the rule ids). What remains is 225 code lines and 402 comment lines. All of those comments explain configuration, not rule history. Under ruling A ("存量不横扫") and the dispatch's "其它注释不改", getting to ~400 would mean sweeping them. That is not done here. If the seat wants the number, it is a separate comment-budget decision. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at 4d18cf1: 68 families. Each was run on that head, with its exit captured before any pipe. All 68 exit 0. `--ran` reconciliation: "68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN". Build-reading gates (`check:dts-closure`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:sourcemap-no-sources-content`) ran after a full `pnpm build` (119/119 tasks). The plugin-teardown self-test ran after `git fetch --unshallow`. The first run had two reds, both from one string: a moved comment-swallow case contains the example line ` * import { createHonoApp } from "@objectstack/hono";`. `check:cross-package-test-inputs` and `check:test-source-alias` scan test source text and read that string as an undeclared workspace import. The test now splits the specifier across a `+` (the runtime string is byte-identical; the original case list already split `/a*` + `/` the same way). Fixed in 4d18cf1. ## Acceptance notes - **Gate selection after this move (finding, not fixed here: outside the file surface).** `scripts/ci/select-gate-families.sh` runs the `slot_lookup` and `query_options_erasure` families for a `packages/*` path only when it is TS source (`is_ts_source`). Before, these rules lived in `eslint.config.mjs`, a root-config path that runs every family. Now they live in `packages/eslint-plugin/src/*.mjs`, so a later PR that edits only a rule file would skip both ratchet re-measures in CI. Their CI coverage would then be `pnpm lint` (always runs) and the package's own RuleTester suite in Test Core. This PR itself touches `eslint.config.mjs`, so every family runs on it. Suggested fix for the seat to file: add `packages/eslint-plugin/*` to those two families' trigger set. - `@objectstack/eslint-plugin` is private and has no `dist`; it ships nothing, hence `skip-changeset`. --- _Generated by [Claude Code](https://claude.ai/code/session_01CHC483oAMSwyfmTu1xajpZ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 43b4715 commit 67ea334

17 files changed

Lines changed: 1130 additions & 662 deletions

‎eslint.config.mjs‎

Lines changed: 39 additions & 661 deletions
Large diffs are not rendered by default.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"name": "@objectstack/eslint-plugin",
3+
"version": "0.1.0",
4+
"private": true,
5+
"license": "Apache-2.0",
6+
"description": "The four custom ESLint rules the root `eslint.config.mjs` registers for this repository's own code: slot-lookup, query-options, verify-stand-in and comment-swallow. Not published, no build step, plain ESM so `pnpm lint` loads it on every Node the repo supports. The root config imports the source path relatively, never this bare name.",
7+
"type": "module",
8+
"exports": {
9+
".": "./src/index.mjs"
10+
},
11+
"scripts": {
12+
"test": "vitest run",
13+
"typecheck": "tsc --noEmit"
14+
},
15+
"devDependencies": {
16+
"@types/node": "^26.6.3",
17+
"@typescript-eslint/parser": "^8.70.1",
18+
"eslint": "^10.11.0",
19+
"typescript": "^6.0.3",
20+
"vitest": "^4.1.11"
21+
},
22+
"keywords": [
23+
"objectstack",
24+
"eslint",
25+
"eslintplugin"
26+
],
27+
"author": "ObjectStack",
28+
"repository": {
29+
"type": "git",
30+
"url": "https://github.com/objectstack-ai/objectstack.git",
31+
"directory": "packages/eslint-plugin"
32+
},
33+
"homepage": "https://objectstack.ai/docs",
34+
"bugs": "https://github.com/objectstack-ai/objectstack/issues"
35+
}
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// `comment-swallow/no-code-inside-block-comment`. The root `eslint.config.mjs`
4+
// registers it over `COMMENT_SWALLOW_FILES`; `test/comment-swallow.test.ts`
5+
// carries the pinned cases that used to run where the config loads.
6+
//
7+
// The comments below moved verbatim from `eslint.config.mjs`; "above" and
8+
// "below" in them refer to that file.
9+
10+
// ---------------------------------------------------------------------------
11+
// issue #9758 — a statement swallowed by an unterminated block comment.
12+
// ---------------------------------------------------------------------------
13+
//
14+
// A block comment that is never closed is NOT a syntax error. The next comment
15+
// terminator in the file closes it — normally the docblock of the following
16+
// declaration — so the file parses, every gate stays green, and the only
17+
// symptom is a statement that quietly stopped existing. #9640 is one instance:
18+
// `export { maskComments };` in `scripts/pm/dispatch-gates.mjs` had been comment
19+
// TEXT, while that module's own header went on claiming the re-export.
20+
//
21+
// The class is invisible to review and trivial to parse: the swallowed line
22+
// looks like code (textually it IS code) and the terminator looks like it
23+
// belongs to the docblock below. ESLint already visits every file with a real
24+
// parser and hands a rule the comment nodes for free, so the checker is the
25+
// predicate below and nothing else.
26+
//
27+
// ## Why a rule here and not a `check:*` family — #9758's question, measured
28+
//
29+
// #9758 recorded the class at 1 instance in 4,595 files and asked whether that
30+
// base rate earns a checker at all. Re-derived on this tree — 4,679 files, a
31+
// real parser rather than the repo masker — the count is 0: #9640 was the
32+
// singleton and its PR repaired it.
33+
//
34+
// A zero base rate is a real argument against a new gate FAMILY: a script, a
35+
// workflow step, a required context, its own self-test and watch hints, and one
36+
// more line of reader attention in every dispatch derivation, all to hold a
37+
// class nobody trips. It is not an argument against THIS, which is one rule in
38+
// the file that already carries three inline plugins, under a `pnpm lint` that
39+
// already parses all 4,679 files. Zero new CI steps, zero new required
40+
// contexts, zero new check families, zero new dispatch leads.
41+
// `verify-stand-in/no-asserted-driver-argument` below is the precedent for a
42+
// guard that starts at a zero baseline: the state worth having is that the
43+
// count stays 0, and holding a 0 is what a rule is for.
44+
//
45+
// ## The false-positive surface, which is the whole risk — measured
46+
//
47+
// Commented-out code is legitimate and common, so a predicate that flags every
48+
// code-shaped line inside a block comment is unshippable. Two facts bound it,
49+
// both measured over this tree's 21,007 multi-line block comments:
50+
//
51+
// • all 240,529 of their interior source lines carry the `*` prose marker —
52+
// not most, all. So exempting marker lines costs no recall on real prose,
53+
// and it is what makes the predicate quiet: 540 interior lines ARE
54+
// code-shaped (`@example` blocks — `* import { createHonoApp } from …`,
55+
// `* export default app;`) and every one of them is a marker line. Without
56+
// the exemption those 540 are false positives; with it they are invisible.
57+
// • the predicate matches statement SHAPES, not statement keywords. `let`,
58+
// `class`, `type`, `import` and `export` are ordinary English words, and a
59+
// keyword test flags prose that opens with one. `let us assume…` does not
60+
// match `let <ident> [:=]`; `class hierarchies are…` does not match
61+
// `class <ident> (extends|implements|{|<)`; `import lists are…` does not
62+
// match an import form. Those three are in the pinned cases below.
63+
//
64+
// Against those two, plus the block-comment OPENER signature (comments do not
65+
// nest, so a `/*` at the head of a line inside a comment span is the structural
66+
// signature of "never closed"), a sweep over 261,536 candidate lines in 4,679
67+
// files reports 0. So the rule ships with no baseline and no ignores beyond the
68+
// build directories — there is nothing to grandfather.
69+
//
70+
// ## What it cannot see, stated rather than implied
71+
//
72+
// ESLint never sees a file that does not parse, so this rule's domain is
73+
// exactly the SILENT half of the class. When the swallowed span happens to
74+
// leave behind text that is not valid JavaScript — a glob literal such as
75+
// `packages/` + `**` + `/*.ts` carries a comment terminator and closes a
76+
// phantom span mid-token — the parser rejects the file loudly and no checker is
77+
// needed. That split is also why the predicate is allowed to be narrow: the
78+
// loud half is already covered, by the compiler.
79+
//
80+
// The other half it cannot see is a swallowed line that is neither
81+
// statement-shaped nor a comment opener — a bare call, a JSX fragment, an
82+
// object continuation. Widening to "any line inside a block comment without a
83+
// `*` marker" would catch those and measures 0 on this tree too, but it is a
84+
// STYLE claim wearing a defect's clothes: it would reject a perfectly ordinary
85+
// marker-less `/* TODO: … */` block, and this repo lints with
86+
// `--no-inline-config`, so there would be no per-site escape from it. The
87+
// escape that does exist is the house style itself — prefix the line with `*`.
88+
89+
90+
export const COMMENT_SWALLOW_MESSAGE =
91+
'This line is inside a block comment, and it is shaped like code. That is the signature of a ' +
92+
'block comment that was never closed: an unterminated opener is not a syntax error — the next ' +
93+
'comment terminator in the file closes it, usually the docblock of the following declaration — ' +
94+
'so the file parses, every gate stays green, and a statement quietly stops existing while the ' +
95+
'header above it goes on describing it (#9640: `export { maskComments };` was comment text for ' +
96+
'as long as nobody looked). Add the missing terminator to the comment above. If the line really ' +
97+
'is prose or a deliberately commented-out example, prefix it with the `*` marker every one of ' +
98+
"this tree's 240,529 block-comment lines already carries — that is the exemption, and it is the " +
99+
'house style rather than an opt-out. See issues #9640 and #9758.';
100+
101+
/**
102+
* Statement SHAPES, not statement keywords — see the false-positive section
103+
* above. Each pattern is anchored on the punctuation that makes the line code
104+
* rather than a sentence that happens to open with a reserved word.
105+
*/
106+
export const COMMENT_SWALLOW_PATTERNS = [
107+
// `export { a };` / `export * from './x';` / `export default f;`
108+
/^export\s*(?:\{|\*|default\b)/,
109+
// the four import forms, and none of `import lists are checked elsewhere,`
110+
/^import\s*(?:\{|\*|type\s|['"]|[A-Za-z_$][\w$]*\s*(?:,|from\b))/,
111+
// a binding: `const x =`, `let { a } =`, `export var n:` — never `let us …`
112+
/^(?:export\s+)?(?:const|let|var)\s+(?:[{[]|[A-Za-z_$][\w$]*\s*[:=])/,
113+
// `function f(`, `async function* g<`, `export default function h(`
114+
/^(?:export\s+)?(?:default\s+)?(?:async\s+)?function\s*\*?\s*[A-Za-z_$][\w$]*\s*[(<]/,
115+
// `class C {` / `export abstract class C extends D` — never `class hierarchies …`
116+
/^(?:export\s+)?(?:default\s+)?(?:abstract\s+)?class\s+[A-Za-z_$][\w$]*\s*(?:extends\b|implements\b|\{|<)/,
117+
// the TypeScript declaration shapes
118+
/^(?:export\s+)?(?:declare\s+)?interface\s+[A-Za-z_$][\w$]*\s*(?:extends\b|\{|<)/,
119+
/^(?:export\s+)?(?:declare\s+)?(?:const\s+)?enum\s+[A-Za-z_$][\w$]*\s*\{/,
120+
/^(?:export\s+)?type\s+[A-Za-z_$][\w$]*\s*[=<]/,
121+
// CommonJS, which `scripts/` still writes
122+
/^(?:module\.exports|exports\.[A-Za-z_$][\w$]*)\s*=/,
123+
// a block-comment OPENER. Comments do not nest, so one that begins a line
124+
// inside a comment span is the structural signature of "never closed" — and
125+
// in the #9640 shape it is the docblock of the declaration below, which is
126+
// exactly the thing that made the defect unreadable.
127+
/^\/\*/,
128+
];
129+
130+
/**
131+
* True when this source line, taken on its own, reads as code rather than
132+
* prose. The caller supplies the fact that the line lies inside a block-comment
133+
* span; this half decides nothing about spans and everything about shape.
134+
*
135+
* @param {string} sourceLine
136+
*/
137+
export function looksLikeSwallowedCode(sourceLine) {
138+
const text = sourceLine.trim();
139+
if (!text || text.startsWith('*')) return false;
140+
return COMMENT_SWALLOW_PATTERNS.some((pattern) => pattern.test(text));
141+
}
142+
143+
/**
144+
* The extension list is load-bearing and DELIBERATELY wider than the
145+
* `verify-stand-in` block's: it carries `js`/`jsx`/`mjs`/`cjs`. The only
146+
* instance this class has ever had on this tree was `scripts/pm/
147+
* dispatch-gates.mjs`, a `.mjs` file, and a block copied from the
148+
* `{ts,tsx,mts,cts}` sibling below would have covered every file except the
149+
* one kind that has actually carried the defect. `test/comment-swallow.test.ts`
150+
* pins it, so the narrowing cannot happen quietly.
151+
*/
152+
export const COMMENT_SWALLOW_FILES = ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'];
153+
154+
/** @type {{ rules: Record<string, import('eslint').Rule.RuleModule> }} */
155+
export const commentSwallowPlugin = {
156+
rules: {
157+
'no-code-inside-block-comment': {
158+
meta: {
159+
type: 'problem',
160+
docs: {
161+
description:
162+
'Ban a code-shaped line inside a block-comment span — the signature of a block ' +
163+
'comment that was never closed.',
164+
},
165+
schema: [],
166+
messages: { swallowed: COMMENT_SWALLOW_MESSAGE },
167+
},
168+
create(context) {
169+
return {
170+
Program() {
171+
const { sourceCode } = context;
172+
const lines = sourceCode.lines;
173+
for (const comment of sourceCode.getAllComments()) {
174+
if (comment.type !== 'Block') continue;
175+
const { start, end } = /** @type {import('eslint').AST.SourceLocation} */ (comment.loc);
176+
if (end.line === start.line) continue;
177+
// From the line AFTER the opener through the terminator's own
178+
// line. The opener's line is excluded because whatever precedes a
179+
// `/*` on it is live code; the terminator's line is INCLUDED
180+
// because a span can close mid-line, on a line that is otherwise
181+
// a statement — a regex or a glob literal a few lines down
182+
// carries a terminator and ends the phantom span right there.
183+
for (let line = start.line + 1; line <= end.line; line++) {
184+
const text = lines[line - 1];
185+
if (text === undefined || !looksLikeSwallowedCode(text)) continue;
186+
context.report({
187+
loc: { start: { line, column: 0 }, end: { line, column: text.length } },
188+
messageId: 'swallowed',
189+
});
190+
}
191+
}
192+
},
193+
};
194+
},
195+
},
196+
},
197+
};
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// One plugin object per rule namespace. The root `eslint.config.mjs` registers
4+
// each under its historical prefix (`slot-lookup`, `query-options`,
5+
// `verify-stand-in`, `comment-swallow`), so every rule id is unchanged.
6+
7+
export { SLOT_LOOKUP_ANY_MESSAGE, slotLookupPlugin } from './slot-lookup.mjs';
8+
export { QUERY_OPTIONS_ANY_MESSAGE, queryOptionsPlugin } from './query-options.mjs';
9+
export { VERIFY_STAND_IN_MESSAGE, verifyStandInPlugin } from './verify-stand-in.mjs';
10+
export {
11+
COMMENT_SWALLOW_FILES,
12+
COMMENT_SWALLOW_MESSAGE,
13+
COMMENT_SWALLOW_PATTERNS,
14+
commentSwallowPlugin,
15+
looksLikeSwallowedCode,
16+
} from './comment-swallow.mjs';

0 commit comments

Comments
 (0)