Skip to content

Commit f123670

Browse files
refactor(lint): converge the three view-container ladder traversals onto one shared walker (#6381) (#6657)
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`. Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn Co-authored-by: Claude <noreply@anthropic.com>
1 parent 04476e7 commit f123670

5 files changed

Lines changed: 440 additions & 169 deletions

File tree

packages/lint/src/validate-form-layout.ts

Lines changed: 26 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
*
2323
* Scope: every form view reachable from a `views[]` entry — the entry itself
2424
* when it IS a bare form view, plus the container's default `form` and each
25-
* `formViews.<key>` (see {@link formViewSites} for why reading only the first
26-
* shape left both rules reporting clean on real app metadata, #6251). Forms
27-
* embedded inside page component trees are a follow-up — the walker
28-
* deliberately stays shallow so it never guesses at an arbitrary component's
29-
* object binding.
25+
* `formViews.<key>`, through the shared `view-walk.ts` ladder (#6381; see
26+
* {@link formViewSites} for why reading only the first shape left both rules
27+
* reporting clean on real app metadata, #6251). Forms embedded inside page
28+
* component trees are a follow-up — the walk deliberately stays shallow so it
29+
* never guesses at an arbitrary component's object binding.
3030
*/
3131

32+
import { formViewSites } from './view-walk.js';
33+
3234
export const FORM_FIELD_UNKNOWN = 'form-field-unknown';
3335
export const FORM_COLSPAN_ABSOLUTE = 'absolute-colspan-discouraged';
3436

@@ -93,76 +95,21 @@ function collectionEntries(v: unknown, base: string): Array<{ rec: AnyRec; path:
9395
}
9496

9597
/**
96-
* Every FORM VIEW reachable from one `views[]` entry, with the path each sits at.
97-
*
98-
* **Copied from `validate-visibility-predicates.ts`'s `formViewSites` (#6248)**
99-
* rather than re-derived: that file fixed this exact traversal hole on the
100-
* sibling rule one PR earlier, and a second hand-rolled ladder is how two rules
101-
* on one surface start disagreeing about which forms exist. The only thing added
102-
* here is the object binding each site inherits (below) — this rule resolves a
103-
* field reference, the visibility rules do not.
104-
*
105-
* Two shapes, and reading only the first is how BOTH rules in this file were
106-
* dead on real app metadata until #6251 measured it. `os build` on
107-
* `examples/app-showcase` emits its form sections at
108-
* `views[0].formViews.edit.sections[…]`; the traversal read `views[0].sections`,
109-
* found nothing, and reported clean on a stack that DOES carry form sections:
110-
*
111-
* - **View CONTAINER** (the runtime app shape). `ViewSchema` declares exactly
112-
* `name` / `label` / `object` / `list` / `form` / `listViews` / `formViews`
113-
* (`view.zod.ts:1890-1903` — the strict error map spells the container's own
114-
* keys out in prose). Form sections therefore live one level down, under
115-
* `form` and each `formViews.<key>`.
116-
* - **A bare FORM VIEW** (`FormViewSchema`, `view.zod.ts:1623-1624`), whose
117-
* `sections` / `groups` sit at the top.
118-
*
119-
* `list` / `listViews.<key>` are `ObjectListViewSchema`
120-
* (`view.zod.ts:1838` — `ListViewSchema` minus `userFilters`) and carry no
121-
* `sections` at all, so they are deliberately NOT walked. This is the one point
122-
* where the other in-repo ladder, `validate-translatable-sections.ts`'s
123-
* `collectViewSites`, is wider: it also visits `listViews.*.sections`. Measured
124-
* against the schema, that rung can only ever read `undefined` — it costs
125-
* nothing there and buys nothing here, so the narrower #6248 ladder is the one
126-
* copied. Both agree on every rung that can hold a section.
127-
*
128-
* `objects[].views` is deliberately absent for the reason #6248 states:
129-
* `object.zod.ts:1833` tombstones the key by name ("`views` is not an
130-
* ObjectSchema field"), so a branch keyed on it could only fire for stacks the
131-
* schema already rejects — the phantom check #4984 / #5017 removed elsewhere.
98+
* The bare-form site (the `views[]` entry itself) is NOT a phantom check, and
99+
* the distinction is worth keeping straight where this rule reads it: strict
100+
* `ViewSchema` refuses a `views[]` entry carrying root `sections` — measured,
101+
* `unrecognized_keys` naming `sections` — so on a parsed `defineStack` config
102+
* only the container rungs can fire. But this rule is registered
103+
* `input: 'parsed'`, and `os lint` never parses: `runAuthoringRules` hands
104+
* `parsed` rules the NORMALIZED stack, where a raw (non-`defineStack`) config's
105+
* root `sections` is still present and still the author's mistake to hear about.
132106
*
133-
* The bare-form site (the entry itself) is NOT such a phantom, and the
134-
* distinction is worth keeping straight: strict `ViewSchema` refuses a `views[]`
135-
* entry carrying root `sections` — measured, `unrecognized_keys` naming
136-
* `sections` — so on a parsed `defineStack` config only the container rungs can
137-
* fire. But this rule is registered `input: 'parsed'`, and `os lint` never
138-
* parses: `runAuthoringRules` hands `parsed` rules the NORMALIZED stack, where a
139-
* raw (non-`defineStack`) config's root `sections` is still present and still
140-
* the author's mistake to hear about.
107+
* The ladder itself — which rungs exist, which are filtered, and the schema
108+
* proof behind each — lives once in `view-walk.ts` (#6381). It used to be a
109+
* verbatim copy of `validate-visibility-predicates.ts`'s walker (#6248 → #6251);
110+
* a third independent copy in `validate-translatable-sections.ts` made three,
111+
* and three copies is how the next author fixes one and leaves two behind.
141112
*/
142-
function formViewSites(
143-
view: AnyRec,
144-
basePath: string,
145-
): Array<{ form: AnyRec; path: string; surface: string }> {
146-
// `surface` names the sub-container in the human-readable `where`. It earns
147-
// its place on exactly the shape this traversal was extended for: a runtime
148-
// container carries neither `name` nor `object` in the emitted artifact, so
149-
// without it every finding under one view reads `view "views[0]"` and the
150-
// author cannot tell the `edit` form from the `create` one.
151-
const sites = [{ form: view, path: basePath, surface: '' }];
152-
const dflt = view.form;
153-
if (isRec(dflt)) {
154-
sites.push({ form: dflt, path: `${basePath}.form`, surface: 'form' });
155-
}
156-
const named = view.formViews;
157-
if (isRec(named)) {
158-
for (const [key, sub] of Object.entries(named)) {
159-
if (isRec(sub)) {
160-
sites.push({ form: sub, path: `${basePath}.formViews.${key}`, surface: `formViews.${key}` });
161-
}
162-
}
163-
}
164-
return sites;
165-
}
166113

167114
/** A section field entry is either a bare field name or `{ field, colSpan, … }`. */
168115
function fieldNameOf(entry: unknown): string | null {
@@ -227,8 +174,11 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] {
227174
for (const site of formViewSites(view, viewPath)) {
228175
// A sub-container declares its own binding (`form.data.object`) and
229176
// otherwise inherits the container's — the resolution order every other
230-
// view-walking rule in this package uses.
231-
const objName = boundObject(site.form) ?? containerObject;
177+
// view-walking rule in this package uses. Deliberately NOT folded into
178+
// the shared walker: the three consumers compose this ladder differently
179+
// (see `view-walk.ts`), and a refactor that changes a verdict is a failed
180+
// refactor.
181+
const objName = boundObject(site.view) ?? containerObject;
232182
// Only reference-check when the bound object resolves; otherwise we can't.
233183
const known = objName ? objectFields.get(objName) : undefined;
234184
const where = site.surface ? `view "${viewName}" · ${site.surface}` : `view "${viewName}"`;
@@ -239,7 +189,7 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] {
239189
// the canonical spelling is silent on the legacy one, which is exactly the
240190
// half-coverage this issue is about.
241191
for (const bucket of ['sections', 'groups'] as const) {
242-
const sections = Array.isArray(site.form[bucket]) ? (site.form[bucket] as unknown[]) : [];
192+
const sections = Array.isArray(site.view[bucket]) ? (site.view[bucket] as unknown[]) : [];
243193

244194
for (let s = 0; s < sections.length; s++) {
245195
const sec = sections[s];

packages/lint/src/validate-translatable-sections.ts

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@
7272
*
7373
* - a view container's `sections`, its DEFAULT `form.sections` (#5415 — the
7474
* anchor that is neither a `formViews.*` entry nor the record's own), and
75-
* every `listViews.*` / `formViews.*` sub-container's `sections`;
75+
* every `listViews.*` / `formViews.*` sub-container's `sections` — reached
76+
* through the shared `view-walk.ts` ladder (#6381; never a private copy,
77+
* for the reason that file's header states — three copies of this descent
78+
* had each been fixed separately, twice for the same missing rung);
7679
* - the same three on views embedded in an object (`objects[].views`,
7780
* `objects[].listViews`);
7881
* - `record:details` sections nested anywhere in a page's component tree,
@@ -86,6 +89,7 @@
8689
*/
8790

8891
import { walkPageComponents } from './page-walk.js';
92+
import { viewContainerSites } from './view-walk.js';
8993

9094
export const TRANSLATION_SECTION_NAME_MISSING = 'translation-section-name-missing';
9195

@@ -181,49 +185,41 @@ function joinWhere(...parts: string[]): string {
181185
/**
182186
* Register every `sections` array ONE view container declares.
183187
*
184-
* The binding ladder mirrors `validate-translation-references.ts`'s
185-
* `collectViewRecord` exactly: a sub-container resolves its own object first
186-
* and falls back to the record's, then to the default list's — because on the
187-
* canonical shape the binding lives INSIDE the container (`list.data.object`),
188-
* not at the record root.
188+
* The DESCENT is the shared one (`view-walk.ts`, #6381) — the entry itself, the
189+
* container's default `form` (#5415: the anchor that is neither a `formViews.*`
190+
* entry nor the record's own), and every `listViews.*` / `formViews.*`
191+
* sub-container. This rule takes the FULL ladder, `listViews.*` included: that
192+
* rung is how it reaches an object's own `listViews` container, which the module
193+
* docblock above declares as part of its section face.
194+
*
195+
* The BINDING ladder stays here, because it is this rule's own: it mirrors
196+
* `validate-translation-references.ts`'s `collectViewRecord` — a sub-container
197+
* resolves its own object first and falls back to the record's, then to the
198+
* default list's, because on the canonical shape the binding lives INSIDE the
199+
* container (`list.data.object`), not at the record root. The sibling rules
200+
* compose their fallbacks differently and folding them together would change
201+
* verdicts.
202+
*
203+
* One equivalence worth writing down, since it is what let the two branches
204+
* collapse into one: the entry's OWN site used to resolve `recordObject ??
205+
* listBinding` while sub-containers resolved `viewObjectName(sub) ??
206+
* recordObject ?? listBinding`. For the entry, `viewObjectName(view)` IS
207+
* `recordObject`, so the sub-container formula returns exactly the same answer
208+
* on it — the uniform expression below is the old two-branch behaviour, not a
209+
* widening of it.
189210
*/
190211
function collectViewSites(view: AnyRec, basePath: string, label: string, sites: SectionSite[]): void {
191212
const recordObject = viewObjectName(view);
192213
const listBinding = isRec(view.list) ? viewObjectName(view.list) ?? recordObject : undefined;
193-
const bindingOf = (container: AnyRec): string | undefined =>
194-
viewObjectName(container) ?? recordObject;
195214

196-
sites.push({
197-
path: `${basePath}.sections`,
198-
surface: label,
199-
objectName: recordObject ?? listBinding,
200-
sections: view.sections,
201-
});
202-
203-
// The container's DEFAULT form — the one `defineView({ form: … })` declares
204-
// and `ObjectForm` renders when no named form view is asked for (#5415).
205-
if (isRec(view.form)) {
215+
for (const site of viewContainerSites(view, basePath)) {
206216
sites.push({
207-
path: `${basePath}.form.sections`,
208-
surface: joinWhere(label, 'form'),
209-
objectName: bindingOf(view.form) ?? listBinding,
210-
sections: view.form.sections,
217+
path: `${site.path}.sections`,
218+
surface: joinWhere(label, site.surface),
219+
objectName: viewObjectName(site.view) ?? recordObject ?? listBinding,
220+
sections: site.view.sections,
211221
});
212222
}
213-
214-
for (const key of ['listViews', 'formViews'] as const) {
215-
const container = view[key];
216-
if (!isRec(container)) continue;
217-
for (const [subKey, sub] of Object.entries(container)) {
218-
if (!isRec(sub)) continue;
219-
sites.push({
220-
path: `${basePath}.${key}.${subKey}.sections`,
221-
surface: joinWhere(label, `${key}.${subKey}`),
222-
objectName: bindingOf(sub) ?? listBinding,
223-
sections: sub.sections,
224-
});
225-
}
226-
}
227223
}
228224

229225
/** Every object name some translation bundle carries a node for. */

packages/lint/src/validate-visibility-predicates.ts

Lines changed: 8 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,12 @@
7777
*
7878
* Scope: `views` — every form view reachable from a `views[]` entry (the entry
7979
* itself when it IS a form view, plus the container's `form` and each
80-
* `formViews.<key>`; see {@link formViewSites} for why reading only the first
81-
* shape left this rule reporting clean on real metadata) — and `pages`, through
82-
* the shared `walkPageComponents` traversal. Data-field `visibleWhen` is already
83-
* covered by `validate-expressions` and is not re-checked here.
80+
* `formViews.<key>`), through the shared `view-walk.ts` ladder (#6381; see
81+
* {@link formViewSites} for why reading only the first shape left this rule
82+
* reporting clean on real metadata, and why `listViews.<key>` is filtered out
83+
* rather than absent) — and `pages`, through the shared `walkPageComponents`
84+
* traversal. Data-field `visibleWhen` is already covered by
85+
* `validate-expressions` and is not re-checked here.
8486
*
8587
* The predicate family is read off the schema, not guessed: `visibleWhen` is the
8688
* canonical key on all three carriers (`FormFieldBaseSchema` `view.zod.ts:1416`,
@@ -226,6 +228,7 @@ import { collectCelRootIdentifiers, firstUndeclaredReference, parseCelToAst } fr
226228
import type { CelAstNode } from '@objectstack/formula';
227229

228230
import { walkPageComponents } from './page-walk.js';
231+
import { formViewSites } from './view-walk.js';
229232

230233
export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered';
231234
export const VISIBILITY_BARE_IDENTIFIER = 'visibility-bare-identifier';
@@ -650,58 +653,6 @@ function isFieldObject(entry: unknown): entry is AnyRec {
650653
return !!entry && typeof entry === 'object' && !Array.isArray(entry);
651654
}
652655

653-
/**
654-
* Every FORM VIEW reachable from one `views[]` entry, with the path each sits at.
655-
*
656-
* Two shapes, and reading only the first is how this rule was dead on real
657-
* metadata until #6128 measured it. `os build` on `examples/app-showcase` emits
658-
* its one form predicate at
659-
* `views[0].formViews.edit.sections[0].fields[6].visibleWhen` — the traversal
660-
* read `views[0].sections`, found nothing, and reported clean on a stack that
661-
* DOES carry a view-form predicate:
662-
*
663-
* - **View CONTAINER** (the runtime app shape). `ViewSchema` declares exactly
664-
* `name` / `label` / `object` / `list` / `form` / `listViews` / `formViews`
665-
* (`view.zod.ts:1890-1903` — the strict error map spells the container's own
666-
* keys out in prose). Form sections therefore live one level down, under
667-
* `form` and each `formViews.<key>`; `list` / `listViews.<key>` are
668-
* `ObjectListViewSchema` and carry no `sections`, so they are not walked.
669-
* - **A bare FORM VIEW** (`FormViewSchema`, `view.zod.ts:1623-1624`), whose
670-
* `sections` / `groups` sit at the top. This is the `defineForm` shape the
671-
* `*.form.ts` metadata-editing forms use, i.e. the `layer: 'metadata'` caller.
672-
*
673-
* `objects[].views` is deliberately absent: `object.zod.ts:1833` tombstones the
674-
* key ("`views` is not an ObjectSchema field"), so a branch keyed on it could
675-
* only ever fire for stacks the schema already rejects by name — the phantom
676-
* check #4984 / #5017 removed from two neighbouring rules. Object-level
677-
* `listViews` (`object.zod.ts:1616`) is a list view, so it carries none of this
678-
* either.
679-
*/
680-
function formViewSites(
681-
view: AnyRec,
682-
basePath: string,
683-
): Array<{ form: AnyRec; path: string; surface: string }> {
684-
// `surface` names the sub-container in the human-readable `where`. It earns
685-
// its place on exactly the shape this traversal was extended for: a runtime
686-
// container carries neither `name` nor `object` in the emitted artifact, so
687-
// without it every finding under one view reads `view "views[0]"` and the
688-
// author cannot tell the `edit` form from the `tabbed` one.
689-
const sites = [{ form: view, path: basePath, surface: '' }];
690-
const dflt = view.form;
691-
if (dflt && typeof dflt === 'object' && !Array.isArray(dflt)) {
692-
sites.push({ form: dflt as AnyRec, path: `${basePath}.form`, surface: 'form' });
693-
}
694-
const named = view.formViews;
695-
if (named && typeof named === 'object' && !Array.isArray(named)) {
696-
for (const [key, sub] of Object.entries(named as AnyRec)) {
697-
if (sub && typeof sub === 'object' && !Array.isArray(sub)) {
698-
sites.push({ form: sub as AnyRec, path: `${basePath}.formViews.${key}`, surface: `formViews.${key}` });
699-
}
700-
}
701-
}
702-
return sites;
703-
}
704-
705656
/**
706657
* Validate conditional-visibility predicates across authored views and pages.
707658
*
@@ -743,7 +694,7 @@ export function validateVisibilityPredicates(
743694
// `sections` (canonical) and `groups` (legacy alias → sections) both hold
744695
// FormSection objects with an optional visibility predicate + `fields`.
745696
for (const bucket of ['sections', 'groups'] as const) {
746-
const sections = Array.isArray(site.form[bucket]) ? (site.form[bucket] as unknown[]) : [];
697+
const sections = Array.isArray(site.view[bucket]) ? (site.view[bucket] as unknown[]) : [];
747698
for (let s = 0; s < sections.length; s++) {
748699
const sec = sections[s];
749700
if (!sec || typeof sec !== 'object') continue;

0 commit comments

Comments
 (0)