From fad24dc6f25cec02beed370f8a3736524180e3de Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 03:17:12 +0000 Subject: [PATCH] docs(scripts): point the fixture justifications at the AST walk that replaced the regex (objectui#6996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six test suites under `scripts/__tests__/` justified how they spell their fixtures by describing `scripts-type-check.test.ts` as a TEXT matcher over `scripts/` that "cannot tell a string literal from an import". objectui#4902 replaced that regex with `workspaceImportSpecifiers()`, an AST walk over `ImportDeclaration`/`ExportDeclaration`/`ImportEqualsDeclaration` nodes, and pinned the string-literal case directly — so every one of those eight statements described a matcher that no longer exists. A wide sweep (not just the two sites found by exact wording) is what settled the shape: with six copies of the description, correcting each one in place just leaves six things that can drift again. So the authoritative account stays in `workspaceImportSpecifiers()`'s own docstring, that docstring now says siblings should point at it rather than restate it, and the six sites became pointers. The interpolation constants (`I18N_PKG` in both i18n suites) are KEPT, with corrected justifications. Measurement, not assumption: no live text-level reader of `scripts/` matches an import edge from text. `check-phantom- dependencies`, `check-unreferenced-sources`, `check-package-self-import` and `check-shell-escape-residue` all exclude `scripts/`; `check-pre-install- import-graph` and the `check-vi-mock-*` pair read text but mask comments and literals via `js-comment-mask.mjs`; the live `from '...'` regexes in `e2e-type-check.test.ts` and `vitest-setup-type-check.test.ts` read the e2e and vitest-setup programs, not `scripts/`. A probe that inlined the specifier at all 41 fixture sites and dropped both constants left every one of those readers green, `scripts-type-check.test.ts` included. The constants therefore survive on their own merit — one spelling across the fixtures — not as a workaround, and the fixtures are template literals either way, so nothing is paying an interpolation tax. No gate behaviour changes: comments only. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013uAaxiwgYDybsTNV9xwa1M --- .../check-doc-component-types.test.ts | 12 +++++----- .../check-i18n-call-site-keys.test.ts | 22 +++++++++++-------- .../__tests__/check-i18n-dead-keys.test.ts | 17 +++++++++----- scripts/__tests__/check-i18n-en-drift.test.ts | 5 +++-- .../check-package-self-import.test.ts | 22 ++++++++++++------- .../check-spec-symbol-derivation.test.ts | 14 +++++++----- scripts/__tests__/scripts-type-check.test.ts | 6 +++++ 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/scripts/__tests__/check-doc-component-types.test.ts b/scripts/__tests__/check-doc-component-types.test.ts index acb7868fe1..f21b0fc390 100644 --- a/scripts/__tests__/check-doc-component-types.test.ts +++ b/scripts/__tests__/check-doc-component-types.test.ts @@ -87,12 +87,14 @@ describe('the registered-key universe is derived from the registration calls', ( 'packages/demo/src/index.tsx', [ // No import of a workspace package here, not even as fixture TEXT. - // `scripts-type-check.test.ts` greps this project's files for the - // `from 'object-ui/…'` shape to pin the claim that - // `pnpm type-check:scripts` needs no build, and that grep cannot tell a - // string literal — or a comment quoting one — from a real import. The - // fixture does not need the import line: the derivation reads the + // The fixture does not need the import line: the derivation reads the // register CALL, not what the file imports. + // + // (This used to say the line was omitted because + // `scripts-type-check.test.ts` matched the `from '@object-ui/...'` + // shape in the file's TEXT. It reads import edges from the AST now — + // see `workspaceImportSpecifiers()` there — so the omission is a + // choice about this fixture, not a constraint a sibling gate imposes.) "ComponentRegistry.register('object-grid', Renderer, {", " namespace: 'plugin-grid',", " label: 'Object Grid',", diff --git a/scripts/__tests__/check-i18n-call-site-keys.test.ts b/scripts/__tests__/check-i18n-call-site-keys.test.ts index 4fb35852c6..bffab5680b 100644 --- a/scripts/__tests__/check-i18n-call-site-keys.test.ts +++ b/scripts/__tests__/check-i18n-call-site-keys.test.ts @@ -90,8 +90,8 @@ const tempRoots: string[] = []; * would pull a 3.2k-line package source into `tsconfig.scripts.json`'s program, * and that project's placement in `ci.yml` rests on the premise that it reads * nothing outside `scripts/` — pinned by `scripts-type-check.test.ts`, whose - * regex only looks for workspace-package specifiers and would not have caught a - * relative one. Computing the path keeps the premise true instead of stepping + * AST walk reports only workspace-package specifiers and would not have + * caught a relative one. Computing the path keeps the premise true instead of stepping * around the pin that guards it. */ const realEn: unknown = ( @@ -99,13 +99,17 @@ const realEn: unknown = ( ).default; /** - * Module specifiers that appear inside the FIXTURE SOURCES below — text to be - * analysed, not imports of this file. They are interpolated rather than written - * out because `scripts-type-check.test.ts` greps this directory for import - * statements naming a workspace package, to pin that the scripts project needs - * no workspace build, and its regex cannot tell a string literal from an import - * statement. (Nor a code comment from either — which is why this paragraph does - * not spell the pattern out.) + * The workspace specifier that appears inside the FIXTURE SOURCES below — text + * to be analysed, not an import of this file. Held in a constant so every + * fixture spells it one way and a package rename stays one edit. + * + * It is NOT interpolated to keep the specifier away from a text-level scan, + * which is what this comment used to say. That reason expired: + * `workspaceImportSpecifiers()` in `scripts-type-check.test.ts` reads import + * edges from the AST, so a specifier sitting in a string or a template + * literal's static text is not an edge to it — that function's docstring is the + * authoritative account. `check-i18n-dead-keys.test.ts` holds the same constant + * for the same reason. */ const I18N_PKG = '@object-ui/i18n'; diff --git a/scripts/__tests__/check-i18n-dead-keys.test.ts b/scripts/__tests__/check-i18n-dead-keys.test.ts index 49ff579ed5..1fd07bf6dd 100644 --- a/scripts/__tests__/check-i18n-dead-keys.test.ts +++ b/scripts/__tests__/check-i18n-dead-keys.test.ts @@ -29,12 +29,17 @@ import { propertyChainProbe, sweep, textFootprint } from '../check-i18n-dead-key const tempRoots: string[] = []; /** - * Interpolated into the fixture SOURCES below rather than written out — - * `scripts-type-check.test.ts` greps this directory for import statements - * naming a workspace package, to pin that `scripts/` needs no workspace - * build, and its regex cannot tell a string literal (or a template literal's - * static text) from a real import statement. Same reason - * `check-i18n-call-site-keys.test.ts` does this for the same specifier. + * The workspace specifier the fixture SOURCES below import, in a constant so + * this suite and `check-i18n-call-site-keys.test.ts` spell it one way. + * + * It is NOT held here to keep the specifier away from a text-level scan, which + * is what this comment used to say. That reason expired: + * `workspaceImportSpecifiers()` in `scripts-type-check.test.ts` reads import + * edges from the AST, so a specifier sitting in a string or a template + * literal's static text is not an edge to it — that function's docstring is the + * authoritative account, and a `describe` block beside it pins the + * string-literal case directly. Writing these fixtures out plainly would be + * green; the constant is kept for one spelling, not for concealment. */ const I18N_PKG = '@object-ui/i18n'; diff --git a/scripts/__tests__/check-i18n-en-drift.test.ts b/scripts/__tests__/check-i18n-en-drift.test.ts index 2b6ca320d3..726f4fcb40 100644 --- a/scripts/__tests__/check-i18n-en-drift.test.ts +++ b/scripts/__tests__/check-i18n-en-drift.test.ts @@ -59,8 +59,9 @@ const tempRoots: string[] = []; * A COMPUTED dynamic import on purpose. A static specifier would pull a * 3.2k-line package source into `tsconfig.scripts.json`'s program, and that * project's position in `ci.yml` rests on the premise that it reads nothing - * outside `scripts/` — pinned by `scripts-type-check.test.ts`, whose regex looks - * for workspace-package specifiers and would not have caught a relative one. + * outside `scripts/` — pinned by `scripts-type-check.test.ts`, whose AST walk + * reports only workspace-package specifiers and would not have caught a + * relative one. * `check-i18n-call-site-keys.test.ts` reads the same pack the same way, for the * same reason. */ diff --git a/scripts/__tests__/check-package-self-import.test.ts b/scripts/__tests__/check-package-self-import.test.ts index ff1f7dbfe0..88514c728f 100644 --- a/scripts/__tests__/check-package-self-import.test.ts +++ b/scripts/__tests__/check-package-self-import.test.ts @@ -188,9 +188,12 @@ describe('a package name that is not a module edge is not a finding', () => { // rewrite of this gate breaks on a documented import example that quotes // its own package name exactly as it would on a test constant. // - // (Spelled without the quoted specifier on purpose. `scripts-type-check.test.ts` - // greps every file in the scripts tsconfig program for that literal shape and - // cannot tell a comment from an import — see objectui#4902.) + // (Spelled as bare package names rather than whole import phrases. That was + // once required — `scripts-type-check.test.ts` matched the import shape in + // the file's TEXT — and it is not any more: objectui#4902 moved that gate to + // an AST walk, `workspaceImportSpecifiers()`, whose docstring is the + // authoritative account. The spelling is kept because the map wants + // specifiers, not import statements.) const specimens: Record = { 'packages/i18n/src/__tests__/perm-home-namespace-3546.test.tsx': '@object-ui/i18n', 'packages/core/src/utils/freeze-schema.ts': '@object-ui/core', @@ -454,11 +457,14 @@ describe('objectui itself', () => { ); expect(body).toContain("from './index'"); // The quoted SPECIFIER alone, without the import keyword in front of it: a - // quoted package name is a module edge in any of the five forms, and this - // is also the stronger assertion. Spelling the whole import phrase here - // would trip `scripts-type-check.test.ts`, whose regex cannot tell an - // assertion string from a real import — which is precisely the reason the - // gate under test parses rather than greps. + // quoted package name is a module edge in any of the five forms, and that + // is the reason this assertion is spelled the way it is: it is the stronger + // one. It once had a second reason — spelling the whole import phrase here + // would trip `scripts-type-check.test.ts` — and that one expired when + // objectui#4902 moved the sibling gate to the AST walk + // `workspaceImportSpecifiers()`, which does not read assertion strings as + // imports. The gate under test parses rather than greps for the same + // reason, and that half still stands. expect(body).not.toContain("'@object-ui/fields'"); }); diff --git a/scripts/__tests__/check-spec-symbol-derivation.test.ts b/scripts/__tests__/check-spec-symbol-derivation.test.ts index 88d6a16c34..1b6fcac8b7 100644 --- a/scripts/__tests__/check-spec-symbol-derivation.test.ts +++ b/scripts/__tests__/check-spec-symbol-derivation.test.ts @@ -187,11 +187,15 @@ export type ViewNavigationConfig = z.infer; // `SpecAuthoredInput` is recognised by NAME (it is the repo's own helper for // binding a local type to a spec schema's authoring input), so the module it // comes from does not change the verdict. Spelled as the relative import a - // file inside the react package would really use, and deliberately not as a - // bare workspace specifier: `scripts-type-check.test.ts` pins that no file in - // the scripts tsconfig program imports an `@object-ui` package, and it looks - // for that import TEXT — so a fixture string carrying one trips it and would - // move a CI step below the workspace build for no real dependency. + // file inside the react package would really use, which is what such a file + // would carry anyway. + // + // (Not, as this used to say, because a bare workspace specifier in a fixture + // string would trip `scripts-type-check.test.ts`. That gate still pins that + // no file in the scripts tsconfig program imports an `@object-ui` package, + // but objectui#4902 moved it to the AST walk `workspaceImportSpecifiers()`, + // which reads import edges rather than import TEXT — a fixture string is not + // one. See that function's docstring.) withFixture( { 'authored.ts': ` diff --git a/scripts/__tests__/scripts-type-check.test.ts b/scripts/__tests__/scripts-type-check.test.ts index 09f18ae249..3785729493 100644 --- a/scripts/__tests__/scripts-type-check.test.ts +++ b/scripts/__tests__/scripts-type-check.test.ts @@ -94,6 +94,12 @@ function parsedConsoleNodeProject(): ts.ParsedCommandLine { * kinds, only that shape. What it missed entirely was * `import core = require('…')` (`ImportEqualsDeclaration`), which has no * `from` token at all; this walk covers that form deliberately instead. + * + * Sibling suites in this directory explain their own fixture spellings by + * pointing HERE rather than restating what this matcher does. Keep it that way: + * objectui#6996 swept the directory and found six of them describing this + * matcher in their own words, every one of them still describing the TEXT match + * this walk replaced. One account can be corrected; six drift apart. */ function workspaceImportSpecifiers(fileName: string, sourceText: string): string[] { const sourceFile = ts.createSourceFile(fileName, sourceText, ts.ScriptTarget.Latest, false);