From 3e028ce37128eccfb4aefad56a87e4ca12f9bd0a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 08:45:13 +0000 Subject: [PATCH] refactor(lint): converge the three view-container ladder traversals onto one shared walker (#6381) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The descent from a `views[]` entry down to the records that can actually carry `sections` had three independent implementations in `packages/lint`: - `formViewSites` in validate-visibility-predicates.ts (#6248) - `collectViewSites` in validate-translatable-sections.ts - `formViewSites` in validate-form-layout.ts (#6251, a verbatim copy of the first) The same rung was measured MISSING twice in two consecutive issues (#6128 → #6248, then #6251), each time fixed in one copy only. Copying the fixed walker was the cheapest move each time; the copy count is the argument for one source. `page-walk.ts` (#3583) is the in-package model and is followed here. `view-walk.ts` yields the UNION of the three ladders as typed sites — `self` / `form` / `listView` / `formView` — and each consumer filters: - `viewContainerSites()` — the full ladder. Consumed by validate-translatable-sections, which needs the `listViews.*` rung to reach an object's own `listViews` container (declared as part of its section face in its own module docblock, and pinned by its tests). - `formViewSites()` — the same ladder minus `listViews.*`, a FILTER and not a second ladder. Consumed by validate-visibility-predicates and validate-form-layout, whose tests pin that they do NOT walk list views. The union, not the intersection, is deliberate. The narrowest common shape would have deleted the `listViews.*` rung, and a schema proof was NOT sufficient licence to do so: `ObjectListViewSchema` (view.zod.ts:1864-1865, over `ListViewSchema` at :1067) declares no `sections` — the only declaration of `sections`/`groups` in the file is `FormViewSchema`'s at :1649-1650 — so on a schema-VALID stack that rung reads `undefined`, but `os lint` runs authoring rules over the NORMALIZED stack, and the third rule documents that surface. The `form` rung (#5415, the default anchor that is neither a `formViews.*` entry nor the record's own) is likewise kept and now carries its reasoning in one place. Binding resolution is deliberately NOT folded in: the three consumers compose their fallbacks differently and a refactor that changes a verdict is a failed refactor. One equivalence WAS proven and collapsed, in validate-translatable-sections: the entry's own site resolved `recordObject ?? listBinding` while sub-containers resolved `viewObjectName(sub) ?? recordObject ?? listBinding`; for the entry `viewObjectName(view)` IS `recordObject`, so the sub-container formula returns the same answer and the two branches are now one expression. Refactor-grade evidence: a differential harness (temporary, not committed) ran both the converged rules and their `origin/main` baselines over 222 generated stacks covering every rung, both collection shapes, object-embedded views, unnamed containers, junk rungs and every binding shape — 888 rule runs, 3295 findings compared with JSON.stringify so ORDER counts. Byte-identical throughout. Emission order is preserved by walking `self → form → listViews.* → formViews.*`, which the `listView` filter reduces to the two form rules' exact previous order. Reverse verification, direction predicted before each run: - drop the `formViews.*` rung → predicted all three rules red: 22 failures across all three suites AND all three differential baselines, from one edit. - drop the `form` rung (#5415) → predicted all three red: 15 failures, same spread. - drop the `listViews` rung → predicted ASYMMETRIC: only translatable-sections red. Measured 6 failures; validate-visibility-predicates.test.ts and validate-form-layout.test.ts both passed, and only the translatable differential diverged. That is the filter proving it is a filter, and the rung proving it is live for exactly one consumer. No changeset: no verdict, message, path or ordering changes, and `view-walk.ts` is internal — not exported from index.ts, matching `flow-walk.ts`. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn --- packages/lint/src/validate-form-layout.ts | 102 +++------ .../src/validate-translatable-sections.ts | 68 +++--- .../src/validate-visibility-predicates.ts | 65 +----- packages/lint/src/view-walk.test.ts | 178 ++++++++++++++++ packages/lint/src/view-walk.ts | 196 ++++++++++++++++++ 5 files changed, 440 insertions(+), 169 deletions(-) create mode 100644 packages/lint/src/view-walk.test.ts create mode 100644 packages/lint/src/view-walk.ts diff --git a/packages/lint/src/validate-form-layout.ts b/packages/lint/src/validate-form-layout.ts index c8ce57880a..b889cb0f3b 100644 --- a/packages/lint/src/validate-form-layout.ts +++ b/packages/lint/src/validate-form-layout.ts @@ -22,13 +22,15 @@ * * Scope: every form view reachable from a `views[]` entry — the entry itself * when it IS a bare form view, plus the container's default `form` and each - * `formViews.` (see {@link formViewSites} for why reading only the first - * shape left both rules reporting clean on real app metadata, #6251). Forms - * embedded inside page component trees are a follow-up — the walker - * deliberately stays shallow so it never guesses at an arbitrary component's - * object binding. + * `formViews.`, through the shared `view-walk.ts` ladder (#6381; see + * {@link formViewSites} for why reading only the first shape left both rules + * reporting clean on real app metadata, #6251). Forms embedded inside page + * component trees are a follow-up — the walk deliberately stays shallow so it + * never guesses at an arbitrary component's object binding. */ +import { formViewSites } from './view-walk.js'; + export const FORM_FIELD_UNKNOWN = 'form-field-unknown'; export const FORM_COLSPAN_ABSOLUTE = 'absolute-colspan-discouraged'; @@ -93,76 +95,21 @@ function collectionEntries(v: unknown, base: string): Array<{ rec: AnyRec; path: } /** - * Every FORM VIEW reachable from one `views[]` entry, with the path each sits at. - * - * **Copied from `validate-visibility-predicates.ts`'s `formViewSites` (#6248)** - * rather than re-derived: that file fixed this exact traversal hole on the - * sibling rule one PR earlier, and a second hand-rolled ladder is how two rules - * on one surface start disagreeing about which forms exist. The only thing added - * here is the object binding each site inherits (below) — this rule resolves a - * field reference, the visibility rules do not. - * - * Two shapes, and reading only the first is how BOTH rules in this file were - * dead on real app metadata until #6251 measured it. `os build` on - * `examples/app-showcase` emits its form sections at - * `views[0].formViews.edit.sections[…]`; the traversal read `views[0].sections`, - * found nothing, and reported clean on a stack that DOES carry form sections: - * - * - **View CONTAINER** (the runtime app shape). `ViewSchema` declares exactly - * `name` / `label` / `object` / `list` / `form` / `listViews` / `formViews` - * (`view.zod.ts:1890-1903` — the strict error map spells the container's own - * keys out in prose). Form sections therefore live one level down, under - * `form` and each `formViews.`. - * - **A bare FORM VIEW** (`FormViewSchema`, `view.zod.ts:1623-1624`), whose - * `sections` / `groups` sit at the top. - * - * `list` / `listViews.` are `ObjectListViewSchema` - * (`view.zod.ts:1838` — `ListViewSchema` minus `userFilters`) and carry no - * `sections` at all, so they are deliberately NOT walked. This is the one point - * where the other in-repo ladder, `validate-translatable-sections.ts`'s - * `collectViewSites`, is wider: it also visits `listViews.*.sections`. Measured - * against the schema, that rung can only ever read `undefined` — it costs - * nothing there and buys nothing here, so the narrower #6248 ladder is the one - * copied. Both agree on every rung that can hold a section. - * - * `objects[].views` is deliberately absent for the reason #6248 states: - * `object.zod.ts:1833` tombstones the key by name ("`views` is not an - * ObjectSchema field"), so a branch keyed on it could only fire for stacks the - * schema already rejects — the phantom check #4984 / #5017 removed elsewhere. + * The bare-form site (the `views[]` entry itself) is NOT a phantom check, and + * the distinction is worth keeping straight where this rule reads it: strict + * `ViewSchema` refuses a `views[]` entry carrying root `sections` — measured, + * `unrecognized_keys` naming `sections` — so on a parsed `defineStack` config + * only the container rungs can fire. But this rule is registered + * `input: 'parsed'`, and `os lint` never parses: `runAuthoringRules` hands + * `parsed` rules the NORMALIZED stack, where a raw (non-`defineStack`) config's + * root `sections` is still present and still the author's mistake to hear about. * - * The bare-form site (the entry itself) is NOT such a phantom, and the - * distinction is worth keeping straight: strict `ViewSchema` refuses a `views[]` - * entry carrying root `sections` — measured, `unrecognized_keys` naming - * `sections` — so on a parsed `defineStack` config only the container rungs can - * fire. But this rule is registered `input: 'parsed'`, and `os lint` never - * parses: `runAuthoringRules` hands `parsed` rules the NORMALIZED stack, where a - * raw (non-`defineStack`) config's root `sections` is still present and still - * the author's mistake to hear about. + * The ladder itself — which rungs exist, which are filtered, and the schema + * proof behind each — lives once in `view-walk.ts` (#6381). It used to be a + * verbatim copy of `validate-visibility-predicates.ts`'s walker (#6248 → #6251); + * a third independent copy in `validate-translatable-sections.ts` made three, + * and three copies is how the next author fixes one and leaves two behind. */ -function formViewSites( - view: AnyRec, - basePath: string, -): Array<{ form: AnyRec; path: string; surface: string }> { - // `surface` names the sub-container in the human-readable `where`. It earns - // its place on exactly the shape this traversal was extended for: a runtime - // container carries neither `name` nor `object` in the emitted artifact, so - // without it every finding under one view reads `view "views[0]"` and the - // author cannot tell the `edit` form from the `create` one. - const sites = [{ form: view, path: basePath, surface: '' }]; - const dflt = view.form; - if (isRec(dflt)) { - sites.push({ form: dflt, path: `${basePath}.form`, surface: 'form' }); - } - const named = view.formViews; - if (isRec(named)) { - for (const [key, sub] of Object.entries(named)) { - if (isRec(sub)) { - sites.push({ form: sub, path: `${basePath}.formViews.${key}`, surface: `formViews.${key}` }); - } - } - } - return sites; -} /** A section field entry is either a bare field name or `{ field, colSpan, … }`. */ function fieldNameOf(entry: unknown): string | null { @@ -227,8 +174,11 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] { for (const site of formViewSites(view, viewPath)) { // A sub-container declares its own binding (`form.data.object`) and // otherwise inherits the container's — the resolution order every other - // view-walking rule in this package uses. - const objName = boundObject(site.form) ?? containerObject; + // view-walking rule in this package uses. Deliberately NOT folded into + // the shared walker: the three consumers compose this ladder differently + // (see `view-walk.ts`), and a refactor that changes a verdict is a failed + // refactor. + const objName = boundObject(site.view) ?? containerObject; // Only reference-check when the bound object resolves; otherwise we can't. const known = objName ? objectFields.get(objName) : undefined; const where = site.surface ? `view "${viewName}" · ${site.surface}` : `view "${viewName}"`; @@ -239,7 +189,7 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] { // the canonical spelling is silent on the legacy one, which is exactly the // half-coverage this issue is about. for (const bucket of ['sections', 'groups'] as const) { - const sections = Array.isArray(site.form[bucket]) ? (site.form[bucket] as unknown[]) : []; + const sections = Array.isArray(site.view[bucket]) ? (site.view[bucket] as unknown[]) : []; for (let s = 0; s < sections.length; s++) { const sec = sections[s]; diff --git a/packages/lint/src/validate-translatable-sections.ts b/packages/lint/src/validate-translatable-sections.ts index 30d8691db2..a57a19403f 100644 --- a/packages/lint/src/validate-translatable-sections.ts +++ b/packages/lint/src/validate-translatable-sections.ts @@ -72,7 +72,10 @@ * * - a view container's `sections`, its DEFAULT `form.sections` (#5415 — the * anchor that is neither a `formViews.*` entry nor the record's own), and - * every `listViews.*` / `formViews.*` sub-container's `sections`; + * every `listViews.*` / `formViews.*` sub-container's `sections` — reached + * through the shared `view-walk.ts` ladder (#6381; never a private copy, + * for the reason that file's header states — three copies of this descent + * had each been fixed separately, twice for the same missing rung); * - the same three on views embedded in an object (`objects[].views`, * `objects[].listViews`); * - `record:details` sections nested anywhere in a page's component tree, @@ -86,6 +89,7 @@ */ import { walkPageComponents } from './page-walk.js'; +import { viewContainerSites } from './view-walk.js'; export const TRANSLATION_SECTION_NAME_MISSING = 'translation-section-name-missing'; @@ -181,49 +185,41 @@ function joinWhere(...parts: string[]): string { /** * Register every `sections` array ONE view container declares. * - * The binding ladder mirrors `validate-translation-references.ts`'s - * `collectViewRecord` exactly: a sub-container resolves its own object first - * and falls back to the record's, then to the default list's — because on the - * canonical shape the binding lives INSIDE the container (`list.data.object`), - * not at the record root. + * The DESCENT is the shared one (`view-walk.ts`, #6381) — the entry itself, the + * container's default `form` (#5415: the anchor that is neither a `formViews.*` + * entry nor the record's own), and every `listViews.*` / `formViews.*` + * sub-container. This rule takes the FULL ladder, `listViews.*` included: that + * rung is how it reaches an object's own `listViews` container, which the module + * docblock above declares as part of its section face. + * + * The BINDING ladder stays here, because it is this rule's own: it mirrors + * `validate-translation-references.ts`'s `collectViewRecord` — a sub-container + * resolves its own object first and falls back to the record's, then to the + * default list's, because on the canonical shape the binding lives INSIDE the + * container (`list.data.object`), not at the record root. The sibling rules + * compose their fallbacks differently and folding them together would change + * verdicts. + * + * One equivalence worth writing down, since it is what let the two branches + * collapse into one: the entry's OWN site used to resolve `recordObject ?? + * listBinding` while sub-containers resolved `viewObjectName(sub) ?? + * recordObject ?? listBinding`. For the entry, `viewObjectName(view)` IS + * `recordObject`, so the sub-container formula returns exactly the same answer + * on it — the uniform expression below is the old two-branch behaviour, not a + * widening of it. */ function collectViewSites(view: AnyRec, basePath: string, label: string, sites: SectionSite[]): void { const recordObject = viewObjectName(view); const listBinding = isRec(view.list) ? viewObjectName(view.list) ?? recordObject : undefined; - const bindingOf = (container: AnyRec): string | undefined => - viewObjectName(container) ?? recordObject; - sites.push({ - path: `${basePath}.sections`, - surface: label, - objectName: recordObject ?? listBinding, - sections: view.sections, - }); - - // The container's DEFAULT form — the one `defineView({ form: … })` declares - // and `ObjectForm` renders when no named form view is asked for (#5415). - if (isRec(view.form)) { + for (const site of viewContainerSites(view, basePath)) { sites.push({ - path: `${basePath}.form.sections`, - surface: joinWhere(label, 'form'), - objectName: bindingOf(view.form) ?? listBinding, - sections: view.form.sections, + path: `${site.path}.sections`, + surface: joinWhere(label, site.surface), + objectName: viewObjectName(site.view) ?? recordObject ?? listBinding, + sections: site.view.sections, }); } - - for (const key of ['listViews', 'formViews'] as const) { - const container = view[key]; - if (!isRec(container)) continue; - for (const [subKey, sub] of Object.entries(container)) { - if (!isRec(sub)) continue; - sites.push({ - path: `${basePath}.${key}.${subKey}.sections`, - surface: joinWhere(label, `${key}.${subKey}`), - objectName: bindingOf(sub) ?? listBinding, - sections: sub.sections, - }); - } - } } /** Every object name some translation bundle carries a node for. */ diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index 97f6aec92d..bcdc0fbf61 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -77,10 +77,12 @@ * * Scope: `views` — every form view reachable from a `views[]` entry (the entry * itself when it IS a form view, plus the container's `form` and each - * `formViews.`; see {@link formViewSites} for why reading only the first - * shape left this rule reporting clean on real metadata) — and `pages`, through - * the shared `walkPageComponents` traversal. Data-field `visibleWhen` is already - * covered by `validate-expressions` and is not re-checked here. + * `formViews.`), through the shared `view-walk.ts` ladder (#6381; see + * {@link formViewSites} for why reading only the first shape left this rule + * reporting clean on real metadata, and why `listViews.` is filtered out + * rather than absent) — and `pages`, through the shared `walkPageComponents` + * traversal. Data-field `visibleWhen` is already covered by + * `validate-expressions` and is not re-checked here. * * The predicate family is read off the schema, not guessed: `visibleWhen` is the * canonical key on all three carriers (`FormFieldBaseSchema` `view.zod.ts:1416`, @@ -226,6 +228,7 @@ import { collectCelRootIdentifiers, firstUndeclaredReference, parseCelToAst } fr import type { CelAstNode } from '@objectstack/formula'; import { walkPageComponents } from './page-walk.js'; +import { formViewSites } from './view-walk.js'; export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered'; export const VISIBILITY_BARE_IDENTIFIER = 'visibility-bare-identifier'; @@ -650,58 +653,6 @@ function isFieldObject(entry: unknown): entry is AnyRec { return !!entry && typeof entry === 'object' && !Array.isArray(entry); } -/** - * Every FORM VIEW reachable from one `views[]` entry, with the path each sits at. - * - * Two shapes, and reading only the first is how this rule was dead on real - * metadata until #6128 measured it. `os build` on `examples/app-showcase` emits - * its one form predicate at - * `views[0].formViews.edit.sections[0].fields[6].visibleWhen` — the traversal - * read `views[0].sections`, found nothing, and reported clean on a stack that - * DOES carry a view-form predicate: - * - * - **View CONTAINER** (the runtime app shape). `ViewSchema` declares exactly - * `name` / `label` / `object` / `list` / `form` / `listViews` / `formViews` - * (`view.zod.ts:1890-1903` — the strict error map spells the container's own - * keys out in prose). Form sections therefore live one level down, under - * `form` and each `formViews.`; `list` / `listViews.` are - * `ObjectListViewSchema` and carry no `sections`, so they are not walked. - * - **A bare FORM VIEW** (`FormViewSchema`, `view.zod.ts:1623-1624`), whose - * `sections` / `groups` sit at the top. This is the `defineForm` shape the - * `*.form.ts` metadata-editing forms use, i.e. the `layer: 'metadata'` caller. - * - * `objects[].views` is deliberately absent: `object.zod.ts:1833` tombstones the - * key ("`views` is not an ObjectSchema field"), so a branch keyed on it could - * only ever fire for stacks the schema already rejects by name — the phantom - * check #4984 / #5017 removed from two neighbouring rules. Object-level - * `listViews` (`object.zod.ts:1616`) is a list view, so it carries none of this - * either. - */ -function formViewSites( - view: AnyRec, - basePath: string, -): Array<{ form: AnyRec; path: string; surface: string }> { - // `surface` names the sub-container in the human-readable `where`. It earns - // its place on exactly the shape this traversal was extended for: a runtime - // container carries neither `name` nor `object` in the emitted artifact, so - // without it every finding under one view reads `view "views[0]"` and the - // author cannot tell the `edit` form from the `tabbed` one. - const sites = [{ form: view, path: basePath, surface: '' }]; - const dflt = view.form; - if (dflt && typeof dflt === 'object' && !Array.isArray(dflt)) { - sites.push({ form: dflt as AnyRec, path: `${basePath}.form`, surface: 'form' }); - } - const named = view.formViews; - if (named && typeof named === 'object' && !Array.isArray(named)) { - for (const [key, sub] of Object.entries(named as AnyRec)) { - if (sub && typeof sub === 'object' && !Array.isArray(sub)) { - sites.push({ form: sub as AnyRec, path: `${basePath}.formViews.${key}`, surface: `formViews.${key}` }); - } - } - } - return sites; -} - /** * Validate conditional-visibility predicates across authored views and pages. * @@ -743,7 +694,7 @@ export function validateVisibilityPredicates( // `sections` (canonical) and `groups` (legacy alias → sections) both hold // FormSection objects with an optional visibility predicate + `fields`. for (const bucket of ['sections', 'groups'] as const) { - const sections = Array.isArray(site.form[bucket]) ? (site.form[bucket] as unknown[]) : []; + const sections = Array.isArray(site.view[bucket]) ? (site.view[bucket] as unknown[]) : []; for (let s = 0; s < sections.length; s++) { const sec = sections[s]; if (!sec || typeof sec !== 'object') continue; diff --git a/packages/lint/src/view-walk.test.ts b/packages/lint/src/view-walk.test.ts new file mode 100644 index 0000000000..39d8c12f11 --- /dev/null +++ b/packages/lint/src/view-walk.test.ts @@ -0,0 +1,178 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; + +import { viewContainerSites, formViewSites } from './view-walk.js'; +import { validateVisibilityPredicates } from './validate-visibility-predicates.js'; +import { validateFormLayout } from './validate-form-layout.js'; +import { validateTranslatableSections } from './validate-translatable-sections.js'; + +type AnyRec = Record; + +describe('viewContainerSites — the ladder (#6381)', () => { + it('yields every rung, in ladder order, with its path / surface / kind', () => { + const view: AnyRec = { + name: 'case_views', + sections: [], + list: { type: 'grid' }, + form: { sections: [] }, + listViews: { all: { type: 'grid' }, mine: { type: 'grid' } }, + formViews: { edit: { sections: [] }, create: { sections: [] } }, + }; + + // Order is contract, not accident: findings are emitted in walk order, so + // `self → form → listViews.* → formViews.*` is what both consumers' + // pinned output orders depend on. + expect(viewContainerSites(view, 'views[0]')).toEqual([ + { view, path: 'views[0]', surface: '', kind: 'self' }, + { view: view.form, path: 'views[0].form', surface: 'form', kind: 'form' }, + { view: (view.listViews as AnyRec).all, path: 'views[0].listViews.all', surface: 'listViews.all', kind: 'listView' }, + { view: (view.listViews as AnyRec).mine, path: 'views[0].listViews.mine', surface: 'listViews.mine', kind: 'listView' }, + { view: (view.formViews as AnyRec).edit, path: 'views[0].formViews.edit', surface: 'formViews.edit', kind: 'formView' }, + { view: (view.formViews as AnyRec).create, path: 'views[0].formViews.create', surface: 'formViews.create', kind: 'formView' }, + ]); + }); + + it('always yields the entry itself — the bare FormViewSchema shape', () => { + const view: AnyRec = { sections: [{ label: 'Basics' }] }; + expect(viewContainerSites(view, 'views[2]')).toEqual([ + { view, path: 'views[2]', surface: '', kind: 'self' }, + ]); + }); + + it('descends `list` NOT at all — only `listViews.`', () => { + // `list` is `ObjectListViewSchema` exactly as `listViews.` is, but no + // rule ever asked for it and adding it would be a widening, not a merge. + const sites = viewContainerSites({ list: { sections: [] } }, 'views[0]'); + expect(sites.map((s) => s.path)).toEqual(['views[0]']); + }); + + it('skips a rung that is not a record (string / array / number / null)', () => { + const sites = viewContainerSites( + { form: 'nope', formViews: ['nope'], listViews: 42, list: null }, + 'views[0]', + ); + expect(sites.map((s) => s.kind)).toEqual(['self']); + }); + + it('skips a non-record sub-entry but keeps its record siblings', () => { + const ok = { sections: [] }; + const sites = viewContainerSites({ formViews: { bad: null, ok } }, 'views[0]'); + expect(sites.map((s) => s.path)).toEqual(['views[0]', 'views[0].formViews.ok']); + expect(sites[1].view).toBe(ok); + }); + + it('returns nothing for a non-record entry', () => { + expect(viewContainerSites(null as unknown as AnyRec, 'views[0]')).toEqual([]); + }); +}); + +describe('formViewSites — the form-carrying subset', () => { + const view: AnyRec = { + form: { sections: [] }, + listViews: { all: { sections: [] } }, + formViews: { edit: { sections: [] } }, + }; + + it('is the ladder minus `listViews.*`, order preserved', () => { + expect(formViewSites(view, 'views[0]').map((s) => [s.kind, s.path])).toEqual([ + ['self', 'views[0]'], + ['form', 'views[0].form'], + ['formView', 'views[0].formViews.edit'], + ]); + }); + + it('is a FILTER over the one ladder, not a second ladder', () => { + // The single-source property, asserted structurally: every site the subset + // yields is the very object the full ladder yielded (identity, not a copy). + const all = viewContainerSites(view, 'views[0]'); + for (const site of formViewSites(view, 'views[0]')) { + expect(all.some((s) => s.path === site.path && s.view === site.view)).toBe(true); + } + }); +}); + +/** + * The property this convergence buys: ONE ladder, THREE consumers. + * + * Each rung below is fed to all three rules at once. Break a rung in + * `view-walk.ts` and every column of the table that depends on it goes red + * together — which is the whole point. Before #6381 the same rung had to be + * fixed three times, and twice it was fixed in only one place (#6128 / #6248, + * then #6251). + * + * The `listViews.` row is deliberately asymmetric, and that asymmetry is + * the design: `ObjectListViewSchema` declares no `sections` + * (`view.zod.ts:1864-1865` over `ListViewSchema` at `:1067`; the only + * `sections` / `groups` declaration in the file is `FormViewSchema`'s at + * `:1649-1650`), so the two form rules filter the rung out while + * `validate-translatable-sections` keeps it — its module docblock declares + * `objects[].listViews` as part of its section face, and `os lint` runs over a + * NORMALIZED stack where an off-spec `listViews.x.sections` is still present. + */ +describe('one ladder, three consumers (#6381)', () => { + const objects = [{ name: 'crm_case', fields: { subject: {}, status: {} } }]; + const translations = [{ 'zh-CN': { objects: { crm_case: { label: '个案' } } } }]; + + /** A form body that trips all three rules at once, at one site. */ + const body = () => ({ + sections: [ + { + label: 'Basics', // nameless + labelled → translatable-sections + fields: [ + 'ghost_field', // → form-layout (unknown field) + { field: 'subject', visibleWhen: 'status == "open"' }, // → visibility (bare id) + ], + }, + ], + }); + + const rungs: Array<{ rung: string; view: AnyRec; sitePath: string }> = [ + { rung: 'the entry itself (bare form view)', view: { object: 'crm_case', ...body() }, sitePath: 'views[0]' }, + { rung: 'the container default `form` (#5415)', view: { object: 'crm_case', form: body() }, sitePath: 'views[0].form' }, + { rung: '`formViews.`', view: { object: 'crm_case', formViews: { edit: body() } }, sitePath: 'views[0].formViews.edit' }, + ]; + + for (const { rung, view, sitePath } of rungs) { + const stack = { objects, views: [{ name: 'case_views', ...view }], translations }; + + it(`all three rules reach ${rung}`, () => { + const visibility = validateVisibilityPredicates(stack); + expect(visibility.map((f) => f.path)).toEqual([`${sitePath}.sections[0].fields[1]`]); + + const layout = validateFormLayout(stack); + expect(layout.map((f) => f.path)).toEqual([`${sitePath}.sections[0].fields[0]`]); + + const translatable = validateTranslatableSections(stack); + expect(translatable.map((f) => f.path)).toEqual([`${sitePath}.sections[0]`]); + }); + } + + it('`listViews.` is reached by translatable-sections and by neither form rule', () => { + const stack = { + objects, + views: [{ name: 'case_views', object: 'crm_case', listViews: { all: body() } }], + translations, + }; + + // Kept: this rung is how the rule reaches an object's own `listViews`. + expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([ + 'views[0].listViews.all.sections[0]', + ]); + + // Filtered: a list view declares no `sections`, so descending it can only + // ever read `undefined` on a schema-valid stack. + expect(validateVisibilityPredicates(stack)).toEqual([]); + expect(validateFormLayout(stack)).toEqual([]); + }); + + it("reaches an object's own `listViews` container through the same rung", () => { + const stack = { + objects: [{ ...objects[0], listViews: { compact: body() } }], + translations, + }; + expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([ + 'objects[0].listViews.compact.sections[0]', + ]); + }); +}); diff --git a/packages/lint/src/view-walk.ts b/packages/lint/src/view-walk.ts new file mode 100644 index 0000000000..16fb51f0fd --- /dev/null +++ b/packages/lint/src/view-walk.ts @@ -0,0 +1,196 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Shared view-container traversal (issue #6381) — the one descent from a + * `views[]` entry down to the records that can actually carry `sections`. + * + * This walk had grown THREE independent implementations in this package, and + * the same rung was measured missing twice in two consecutive issues. Both + * times the traversal read only the container's own keys and therefore reported + * clean on real app metadata: `os build` on `examples/app-showcase` emits its + * form sections at `views[0].formViews.edit.sections[…]`, and a walker that + * reads `views[0].sections` finds nothing there. + * + * - `validate-visibility-predicates.ts` — hole found by #6128, fixed by #6248. + * - `validate-form-layout.ts` — same hole, same rule family, found by #6251, + * fixed by copying #6248's walker verbatim one PR later. + * - `validate-translatable-sections.ts` — a third, independently written + * ladder (`collectViewSites`) that happened to agree on every rung that can + * hold a section. + * + * Copying the fixed walker was the cheapest move each time. The copy COUNT is + * the argument for this file: with three, the next author fixes one of three, + * and the two survivors keep the old verdict. `page-walk.ts` says the same + * thing about page components (#3583) and is the model followed here. + * + * ## The ladder, rung by rung, and what the schema says about each + * + * A `views[]` entry is authored in one of two shapes, and the rungs below cover + * both: + * + * - **View CONTAINER** (the runtime app shape). `ViewSchema` + * (`packages/spec/src/ui/view.zod.ts:1885`) is a `strictObject` whose own + * keys are `name` / `label` / `object` / `list` / `form` / `listViews` / + * `formViews` (`view.zod.ts:1926-1929`). Sections therefore live one level + * down. + * - **A bare FORM VIEW** (`FormViewSchema`, `view.zod.ts:1599`), whose + * `sections` (`:1649`) and `groups` (`:1650`, the legacy alias) sit at the + * top. This is the `defineForm` shape the `*.form.ts` metadata-editing forms + * use. + * + * | rung | `kind` | why it is walked | + * | --- | --- | --- | + * | the entry itself | `self` | the bare `FormViewSchema` shape | + * | `form` | `form` | the container's DEFAULT form (#5415) | + * | `listViews.` | `listView` | `objects[].listViews` / container list views — see below | + * | `formViews.` | `formView` | the named form views #6128 / #6251 both missed | + * + * ### `form` — the default anchor, established deliberately (#5415) + * + * The container's `form` is the one `defineView({ form: … })` declares and + * `ObjectForm` renders when no named form view is asked for. It is neither a + * `formViews.*` entry nor the record's own `sections`, so a walker that + * enumerates only those two misses it — which is exactly what #5415 measured + * against `examples/app-showcase`'s `contact.view.ts`: four correctly + * translated section headings reported as stale keys. All three ladders walk + * this rung today and it must stay walked. + * + * ### `listViews.` — kept, and why it is not deleted as dead + * + * A list view carries no sections. `ObjectListViewSchema` + * (`view.zod.ts:1864-1865`) is `ListViewSchema.omit({ userFilters })` re-extended + * with a narrowed `userFilters`, and `ListViewSchema` (`view.zod.ts:1067`) is a + * `strictObject` that declares no `sections` and no `groups` — the only + * declaration of either key in the file is on `FormViewSchema` (`:1649-1650`). + * So on a schema-VALID stack `listViews..sections` can only read + * `undefined`, and `list` is the same schema for the same reason. + * + * That proof is why the two form-layout/visibility rules filter this rung out + * (see {@link formViewSites}) rather than paying for a descent that cannot + * produce a finding. It is NOT a licence to drop the rung from the walker: + * + * - `validate-translatable-sections.ts` declares `objects[].listViews` as part + * of its section face in its own module docblock, and reaches it by handing + * this walker a synthesised `{ object, listViews }` container. Removing the + * rung would silently narrow a documented surface. + * - `os lint` runs authoring rules over the NORMALIZED stack, not a parsed one + * (`runAuthoringRules`), so a raw, non-`defineStack` config's off-spec + * `listViews.x.sections` is still present in memory when the rule runs. + * + * Both facts point the same way: yield the rung, tag it, and let each consumer + * decide. The union is the safe shape here — an intersection would have deleted + * a step someone established on purpose, which is the "next person edits only + * one copy" failure this file exists to prevent, inverted. + * + * ### `objects[].views` — deliberately NOT a rung + * + * `packages/spec/src/data/object.zod.ts:1873` tombstones the key by name + * ("`views` is not an ObjectSchema field"), so a branch keyed on it could only + * ever fire for stacks the schema already rejects by name — the phantom check + * #4984 / #5017 removed from two neighbouring rules. + * + * ## What this walker does NOT decide + * + * **Which bucket to read.** A site is handed back as a RECORD, not as its + * `sections` array: `validate-visibility-predicates` and `validate-form-layout` + * read both `sections` and `groups` (the legacy alias, `view.zod.ts:1650`), + * while `validate-translatable-sections` reads `sections` only. Yielding the + * record keeps that a consumer's choice instead of freezing one rule's answer + * into the shared walk. + * + * **Which object a site binds to.** The three consumers compose their binding + * ladders differently on purpose — `validate-form-layout` falls back to the + * container, `validate-translatable-sections` falls back to the container and + * then to the default `list`'s binding, and `validate-visibility-predicates` + * needs no binding at all. Folding those into one walk would change verdicts, + * which a refactor may not do. + */ + +type AnyRec = Record; + +/** Which rung of the container ladder a site sits on. */ +export type ViewSiteKind = 'self' | 'form' | 'listView' | 'formView'; + +/** One record on the ladder, with everything a rule needs to locate it. */ +export interface ViewSite { + /** The record itself — the `views[]` entry, or one of its sub-containers. */ + view: AnyRec; + /** + * Config path of that record — `views[0]`, `views[0].form`, + * `views[0].formViews.edit`. Consumers append the bucket they read + * (`.sections`, `.groups[2].fields[1]`, …); findings are consumed as edit + * targets (`os lint --json`, Studio's finding renderer), so the path has to + * be one an author can actually look up. + */ + path: string; + /** + * The sub-container segment for a human-readable `where`, WITHOUT the view's + * own name — `''` for the entry itself, else `form` / `formViews.edit` / + * `listViews.all`. It earns its place on exactly the shape this traversal was + * extended for: a runtime container carries neither `name` nor `object` in + * the emitted artifact, so without it every finding under one view reads + * `view "views[0]"` and the author cannot tell the `edit` form from the + * `create` one. + */ + surface: string; + /** Which rung this is — see {@link ViewSiteKind}. */ + kind: ViewSiteKind; +} + +function isRec(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +/** + * EVERY site one `views[]` entry can carry sections on, in ladder order: + * the entry itself, its default `form`, its `listViews.*`, its `formViews.*`. + * + * `basePath` is the caller's path prefix for the entry (e.g. `views[3]`). + * + * Order is part of the contract, not an accident: findings are emitted in walk + * order, so a consumer's output order is this array's order. + */ +export function viewContainerSites(view: AnyRec, basePath: string): ViewSite[] { + // Defensive, and unreachable from the three in-repo callers — all of them + // arrive through a `collectionEntries` helper that yields records only. Same + // guard `page-walk.ts` opens with. + if (!isRec(view)) return []; + + const sites: ViewSite[] = [{ view, path: basePath, surface: '', kind: 'self' }]; + + if (isRec(view.form)) { + sites.push({ view: view.form, path: `${basePath}.form`, surface: 'form', kind: 'form' }); + } + + for (const key of ['listViews', 'formViews'] as const) { + const container = view[key]; + if (!isRec(container)) continue; + const kind: ViewSiteKind = key === 'listViews' ? 'listView' : 'formView'; + for (const [subKey, sub] of Object.entries(container)) { + if (!isRec(sub)) continue; + sites.push({ + view: sub, + path: `${basePath}.${key}.${subKey}`, + surface: `${key}.${subKey}`, + kind, + }); + } + } + + return sites; +} + +/** + * The FORM-carrying subset of {@link viewContainerSites}: the entry itself, the + * default `form`, and each `formViews.`. + * + * `list` / `listViews.` are `ObjectListViewSchema` and declare no + * `sections` (proof in the module docblock), so a rule that judges form + * sections gains nothing by descending them. This is the shape + * `validate-visibility-predicates` and `validate-form-layout` consume; it is a + * FILTER over the one ladder rather than a second ladder, so a rung that breaks + * here breaks for every consumer at once. + */ +export function formViewSites(view: AnyRec, basePath: string): ViewSite[] { + return viewContainerSites(view, basePath).filter((site) => site.kind !== 'listView'); +}