Skip to content

Commit dcf1f60

Browse files
committed
refactor(lint): converge the triplicated collectionEntries and view binding ladder (#6662)
#6381 converged the "views[] entry to its real form/view sites" DESCENT onto one shared walker (`view-walk.ts`), and deliberately left two smaller helpers used by the very same rules at three copies each. This is that follow-up, now that #6422 has closed and `validate-translation-references.ts` is no longer held. 1. `collectionEntries` — 3 copies, now one (`collection-entries.ts`): validate-form-layout.ts / validate-translatable-sections.ts (byte-identical) validate-visibility-predicates.ts (same function, predicates open-coded) 2. The binding ladder `objectName -> object -> data.object` — 3 copies under two names, now one (`viewObjectName`, exported from `view-walk.ts`): boundObject in validate-form-layout.ts viewObjectName in validate-translatable-sections.ts viewObjectName in validate-translation-references.ts Only the BASE ladder is shared. Each rule's fallback COMPOSITION stays in its own file, because they differ on purpose and #6657 preserved that deliberately: form-layout falls back to the container, translatable-sections to the container and then to the default `list`'s binding, translation-references to the record, and visibility-predicates needs no binding at all. `lint-view-refs.ts` is untouched: its deeper ladder was judged reasoned difference rather than drift by #6381. Verdicts are unchanged, measured rather than asserted: a temporary differential (not committed) ran all four rules against their origin/main baselines over 2520 generated stacks each -- 10,080 rule runs, 11,624 findings compared with JSON.stringify so order counts -- byte-identical throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn
1 parent 5087ac6 commit dcf1f60

8 files changed

Lines changed: 497 additions & 137 deletions
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The shared stack-collection enumeration (#6662).
5+
*
6+
* Three rules had their own copy of this helper — `validate-form-layout`,
7+
* `validate-translatable-sections`, `validate-visibility-predicates` — and two
8+
* of the three were byte-identical while the third open-coded its record
9+
* predicate inline. This file pins the behaviour ONCE, where it now lives, and
10+
* then asserts the property the convergence buys: all three consumers report
11+
* the same paths for the same collection.
12+
*/
13+
14+
import { describe, expect, it } from 'vitest';
15+
16+
import { collectionEntries } from './collection-entries.js';
17+
import { validateFormLayout } from './validate-form-layout.js';
18+
import { validateTranslatableSections } from './validate-translatable-sections.js';
19+
import { validateVisibilityPredicates } from './validate-visibility-predicates.js';
20+
21+
type AnyRec = Record<string, unknown>;
22+
23+
describe('collectionEntries — the array shape', () => {
24+
it('yields each record at its index path', () => {
25+
const a = { name: 'a' };
26+
const b = { name: 'b' };
27+
expect(collectionEntries([a, b], 'views')).toEqual([
28+
{ rec: a, path: 'views[0]' },
29+
{ rec: b, path: 'views[1]' },
30+
]);
31+
});
32+
33+
it('hands back the caller’s own record, not a copy', () => {
34+
// Consumers mutate nothing, but they DO compare identity against the sites
35+
// `view-walk.ts` yields, so a defensive copy here would break that.
36+
const rec = { name: 'a' };
37+
expect(collectionEntries([rec], 'views')[0].rec).toBe(rec);
38+
});
39+
40+
it('skips non-records but keeps the index of the records it keeps', () => {
41+
// The index is the AUTHORED position, so a skipped entry must not shift the
42+
// ones after it — the path has to be one the author can look up.
43+
const rec = { name: 'real' };
44+
expect(collectionEntries([null, 'str', 42, rec], 'views')).toEqual([
45+
{ rec, path: 'views[3]' },
46+
]);
47+
});
48+
49+
it('skips a NESTED array — an array is not a record', () => {
50+
expect(collectionEntries([[{ name: 'a' }]], 'views')).toEqual([]);
51+
});
52+
});
53+
54+
describe('collectionEntries — the name-keyed map shape', () => {
55+
it('yields each record at its key path, with the key spread in as `name`', () => {
56+
// The map key IS the entry's name on this shape. Reporting `views[0]` for
57+
// it would name a position that does not exist in the author's file.
58+
expect(collectionEntries({ case_views: { object: 'crm_case' } }, 'views')).toEqual([
59+
{ rec: { name: 'case_views', object: 'crm_case' }, path: 'views.case_views' },
60+
]);
61+
});
62+
63+
it('lets an entry’s OWN `name` win over the map key', () => {
64+
// `{ name, ...def }` — key first, so the spread overwrites it.
65+
expect(collectionEntries({ keyed: { name: 'declared' } }, 'views')[0].rec).toEqual({
66+
name: 'declared',
67+
});
68+
});
69+
70+
it('skips non-record values', () => {
71+
const entries = collectionEntries({ a: null, b: 'str', c: 42, d: [], e: { ok: true } }, 'views');
72+
expect(entries).toEqual([{ rec: { name: 'e', ok: true }, path: 'views.e' }]);
73+
});
74+
});
75+
76+
describe('collectionEntries — what is not a collection', () => {
77+
it.each([
78+
['null', null],
79+
['undefined', undefined],
80+
['a string', 'views'],
81+
['a number', 42],
82+
['a boolean', true],
83+
])('returns nothing for %s', (_label, v) => {
84+
expect(collectionEntries(v, 'views')).toEqual([]);
85+
});
86+
87+
it('returns nothing for an empty collection of either shape', () => {
88+
expect(collectionEntries([], 'views')).toEqual([]);
89+
expect(collectionEntries({}, 'views')).toEqual([]);
90+
});
91+
});
92+
93+
/**
94+
* The one textual divergence between the copies this file converged (#6662).
95+
*
96+
* `validate-visibility-predicates` open-coded its record predicate inline, and
97+
* its MAP-branch guard read `v && typeof v === 'object'` with no
98+
* `!Array.isArray(v)` — where the other two copies called `isRec`, which has
99+
* that third clause. The two are the same function because the ARRAY branch
100+
* returns unconditionally, so the map guard is only ever evaluated on a value
101+
* that is already not an array. This asserts that domination directly: an array
102+
* must never be enumerated as a map, however it is decorated.
103+
*/
104+
describe('collectionEntries — an array is never enumerated as a map', () => {
105+
it('reads only index entries, never an array’s other own keys', () => {
106+
const arr: unknown[] & { extra?: AnyRec } = [{ name: 'indexed' }];
107+
arr.extra = { name: 'not_an_entry' };
108+
109+
expect(collectionEntries(arr, 'views')).toEqual([
110+
{ rec: { name: 'indexed' }, path: 'views[0]' },
111+
]);
112+
});
113+
114+
it('yields index paths for an empty-but-decorated array, not key paths', () => {
115+
const arr: unknown[] & { case_views?: AnyRec } = [];
116+
arr.case_views = { object: 'crm_case' };
117+
118+
expect(collectionEntries(arr, 'views')).toEqual([]);
119+
});
120+
});
121+
122+
/**
123+
* The property the convergence buys: ONE coercion, THREE consumers.
124+
*
125+
* Before #6662 each of these rules decided on its own what path to print for a
126+
* map-shaped collection. Break the coercion now and all three columns go red
127+
* together, which is the whole point — the failure this class of duplication
128+
* produces is the next author fixing one copy and leaving two behind (#6381's
129+
* own history: #6128 / #6248, then #6251).
130+
*/
131+
describe('one coercion, three consumers (#6662)', () => {
132+
const objects = [{ name: 'crm_case', fields: { subject: {}, status: {} } }];
133+
const translations = [{ 'zh-CN': { objects: { crm_case: { label: '个案' } } } }];
134+
135+
/** A form body that trips all three rules at once, at one site. */
136+
const body = () => ({
137+
sections: [
138+
{
139+
label: 'Basics', // → translatable-sections
140+
fields: [
141+
'ghost_field', // → form-layout (unknown field)
142+
{ field: 'subject', visibleWhen: 'status == "open"' }, // → visibility
143+
],
144+
},
145+
],
146+
});
147+
148+
const view = () => ({ object: 'crm_case', ...body() });
149+
150+
it('all three report the ARRAY path for an array-shaped `views`', () => {
151+
const stack = { objects, views: [view()], translations };
152+
153+
expect(validateVisibilityPredicates(stack).map((f) => f.path)).toEqual([
154+
'views[0].sections[0].fields[1]',
155+
]);
156+
expect(validateFormLayout(stack).map((f) => f.path)).toEqual([
157+
'views[0].sections[0].fields[0]',
158+
]);
159+
expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([
160+
'views[0].sections[0]',
161+
]);
162+
});
163+
164+
it('all three report the KEY path for a map-shaped `views`', () => {
165+
const stack = { objects, views: { case_views: view() }, translations };
166+
167+
expect(validateVisibilityPredicates(stack).map((f) => f.path)).toEqual([
168+
'views.case_views.sections[0].fields[1]',
169+
]);
170+
expect(validateFormLayout(stack).map((f) => f.path)).toEqual([
171+
'views.case_views.sections[0].fields[0]',
172+
]);
173+
expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([
174+
'views.case_views.sections[0]',
175+
]);
176+
});
177+
178+
it('all three take the map KEY as the entry’s name in the `where` line', () => {
179+
// The unnamed-but-keyed entry: nothing declares `name`, so the key is the
180+
// only thing that can locate it for the author.
181+
const stack = { objects, views: { case_views: view() }, translations };
182+
183+
expect(validateVisibilityPredicates(stack)[0].where).toContain('case_views');
184+
expect(validateFormLayout(stack)[0].where).toContain('case_views');
185+
expect(validateTranslatableSections(stack)[0].where).toContain('case_views');
186+
});
187+
});
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Shared stack-collection enumeration (issue #6662) — the one coercion from a
5+
* collection authored EITHER as an array OR as a name-keyed map into the
6+
* records it holds, each carrying the config path it actually sits at.
7+
*
8+
* This helper had grown THREE independent copies in this package, all on the
9+
* same view-walking rules that #6381 had just converged onto one descent
10+
* (`view-walk.ts`): `validate-form-layout.ts`, `validate-translatable-sections.ts`
11+
* and `validate-visibility-predicates.ts`. The duplication was already
12+
* acknowledged in-tree — `validate-form-layout.ts`'s copy said out loud "Same
13+
* helper, same reasoning as `validate-visibility-predicates.ts` and
14+
* `validate-translatable-sections.ts`" — which records the cost without paying
15+
* it. The copy COUNT is the argument for this file, exactly as it was for
16+
* `view-walk.ts` and `page-walk.ts` (#3583): with three, the next author fixes
17+
* one and the two survivors keep the old answer.
18+
*
19+
* ## Why the PATH is the point
20+
*
21+
* The sibling rules that do not report a location coerce with a local `asArray`
22+
* and throw the path away. A rule that emits findings cannot: findings are
23+
* consumed as EDIT TARGETS (`os lint --json`, Studio's finding renderer), so a
24+
* map-shaped collection must not report a synthetic array index nobody can look
25+
* up. `views[2]` is the honest path for the array shape and
26+
* `views.contact_views` for the map, and this helper is the only place that
27+
* decides which.
28+
*
29+
* ## Why the map shape injects `name`
30+
*
31+
* The map key IS the entry's name on that shape, so it is spread in as `name`
32+
* (`{ name, ...def }`, key first so an entry's own `name` still wins). That is
33+
* how an unnamed-but-keyed view still locates itself in a message — a rule that
34+
* read only `rec.name` would otherwise print an anonymous finding for an entry
35+
* the author named perfectly well.
36+
*
37+
* ## Non-records are skipped, not coerced
38+
*
39+
* On both shapes an entry that is not a record is dropped rather than repaired.
40+
* Callers therefore receive records only, which is what lets
41+
* `viewContainerSites` open with a defensive `isRec` guard it documents as
42+
* unreachable from the in-repo callers.
43+
*/
44+
45+
type AnyRec = Record<string, unknown>;
46+
47+
/** One record of a collection, with the config path it sits at. */
48+
export interface CollectionEntry {
49+
/** The record itself. On the map shape, with the map key spread in as `name`. */
50+
rec: AnyRec;
51+
/** Config path — `views[2]` for the array shape, `views.contact_views` for the map. */
52+
path: string;
53+
}
54+
55+
function isRec(v: unknown): v is AnyRec {
56+
return !!v && typeof v === 'object' && !Array.isArray(v);
57+
}
58+
59+
/**
60+
* Every record in a collection authored either as an array or as a name-keyed
61+
* map, each with its config path. `base` is the caller's path prefix for the
62+
* collection itself (e.g. `views`, `objects[0].views`).
63+
*
64+
* Order is the collection's own order — array index order, or `Object.entries`
65+
* insertion order for the map — because findings are emitted in walk order and
66+
* every consumer's pinned output order depends on it.
67+
*/
68+
export function collectionEntries(v: unknown, base: string): CollectionEntry[] {
69+
if (Array.isArray(v)) {
70+
const out: CollectionEntry[] = [];
71+
for (let i = 0; i < v.length; i++) {
72+
if (isRec(v[i])) out.push({ rec: v[i] as AnyRec, path: `${base}[${i}]` });
73+
}
74+
return out;
75+
}
76+
if (isRec(v)) {
77+
return Object.entries(v)
78+
.filter(([, def]) => isRec(def))
79+
.map(([name, def]) => ({ rec: { name, ...(def as AnyRec) }, path: `${base}.${name}` }));
80+
}
81+
return [];
82+
}

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

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
* never guesses at an arbitrary component's object binding.
3030
*/
3131

32-
import { formViewSites } from './view-walk.js';
32+
import { collectionEntries } from './collection-entries.js';
33+
import { formViewSites, viewObjectName } from './view-walk.js';
3334

3435
export const FORM_FIELD_UNKNOWN = 'form-field-unknown';
3536
export const FORM_COLSPAN_ABSOLUTE = 'absolute-colspan-discouraged';
@@ -70,30 +71,6 @@ function strName(v: unknown): string | undefined {
7071
return typeof v === 'string' && v.length > 0 ? v : undefined;
7172
}
7273

73-
/**
74-
* Every record in a collection authored either as an array or as a name-keyed
75-
* map, each with its config PATH — `views[2]` for the array shape,
76-
* `views.contact_views` for the map. Findings here are consumed as edit targets
77-
* (`os lint --json`, Studio's finding renderer), so a map-shaped collection must
78-
* not report a synthetic index nobody can look up. Same helper, same reasoning
79-
* as `validate-visibility-predicates.ts` and `validate-translatable-sections.ts`.
80-
*/
81-
function collectionEntries(v: unknown, base: string): Array<{ rec: AnyRec; path: string }> {
82-
if (Array.isArray(v)) {
83-
const out: Array<{ rec: AnyRec; path: string }> = [];
84-
for (let i = 0; i < v.length; i++) {
85-
if (isRec(v[i])) out.push({ rec: v[i] as AnyRec, path: `${base}[${i}]` });
86-
}
87-
return out;
88-
}
89-
if (isRec(v)) {
90-
return Object.entries(v)
91-
.filter(([, def]) => isRec(def))
92-
.map(([name, def]) => ({ rec: { name, ...(def as AnyRec) }, path: `${base}.${name}` }));
93-
}
94-
return [];
95-
}
96-
9774
/**
9875
* The bare-form site (the `views[]` entry itself) is NOT a phantom check, and
9976
* the distinction is worth keeping straight where this rule reads it: strict
@@ -121,32 +98,6 @@ function fieldNameOf(entry: unknown): string | null {
12198
return null;
12299
}
123100

124-
/**
125-
* The object a view — or one of its sub-containers — binds to, across the shapes
126-
* it is authored in.
127-
*
128-
* The ladder is `objectName` → `object` → `data.object`, identical to
129-
* `validate-translation-references.ts` and `validate-translatable-sections.ts`'s
130-
* `viewObjectName` (and to the CLI i18n walker's), so all of them agree on which
131-
* object a form belongs to. On the canonical container shape the binding lives
132-
* INSIDE the sub-container (`form.data.object`) while the container itself
133-
* carries `object`, which is why the caller resolves the site first and falls
134-
* back to the container — a record-level lookup alone resolves to nothing on the
135-
* shape real apps ship.
136-
*
137-
* `name` is deliberately NOT a rung. A stack-level container's `name` may be the
138-
* object name (`view.zod.ts` says so for object-scoped containers), but a form
139-
* view's `name` is its own — `contract_form`, not `contract` — and reading it
140-
* here would bind the wrong object and report every field on the form as unknown.
141-
*/
142-
function boundObject(view: AnyRec): string | undefined {
143-
return (
144-
strName(view.objectName) ??
145-
strName(view.object) ??
146-
(isRec(view.data) ? strName(view.data.object) : undefined)
147-
);
148-
}
149-
150101
/**
151102
* Validate authored form-view layout. Returns findings (empty = clean).
152103
* Advisory only — the caller must never fail the build on these alone.
@@ -169,16 +120,17 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] {
169120
// A container names itself with `name`, or binds with `object` — and an
170121
// artifact-emitted one may carry neither, so the path is the last resort.
171122
const viewName = strName(view.name) ?? strName(view.object) ?? viewPath;
172-
const containerObject = boundObject(view);
123+
const containerObject = viewObjectName(view);
173124

174125
for (const site of formViewSites(view, viewPath)) {
175126
// A sub-container declares its own binding (`form.data.object`) and
176127
// otherwise inherits the container's — the resolution order every other
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
128+
// view-walking rule in this package uses. The base rung is the shared
129+
// `viewObjectName` (#6662); this FALLBACK is deliberately NOT folded into
130+
// the shared walker, because the consumers compose it differently (see
131+
// `view-walk.ts`) and a refactor that changes a verdict is a failed
180132
// refactor.
181-
const objName = boundObject(site.view) ?? containerObject;
133+
const objName = viewObjectName(site.view) ?? containerObject;
182134
// Only reference-check when the bound object resolves; otherwise we can't.
183135
const known = objName ? objectFields.get(objName) : undefined;
184136
const where = site.surface ? `view "${viewName}" · ${site.surface}` : `view "${viewName}"`;

0 commit comments

Comments
 (0)