refactor(lint): converge the triplicated collectionEntries and view binding ladder (#6662)

packages/lint/src/collection-entries.test.ts

@@ -0,0 +1,187 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The shared stack-collection enumeration (#6662).
+ *
+ * Three rules had their own copy of this helper — `validate-form-layout`,
+ * `validate-translatable-sections`, `validate-visibility-predicates` — and two
+ * of the three were byte-identical while the third open-coded its record
+ * predicate inline. This file pins the behaviour ONCE, where it now lives, and
+ * then asserts the property the convergence buys: all three consumers report
+ * the same paths for the same collection.
+ */
+
+import { describe, expect, it } from 'vitest';
+
+import { collectionEntries } from './collection-entries.js';
+import { validateFormLayout } from './validate-form-layout.js';
+import { validateTranslatableSections } from './validate-translatable-sections.js';
+import { validateVisibilityPredicates } from './validate-visibility-predicates.js';
+
+type AnyRec = Record<string, unknown>;
+
+describe('collectionEntries — the array shape', () => {
+  it('yields each record at its index path', () => {
+    const a = { name: 'a' };
+    const b = { name: 'b' };
+    expect(collectionEntries([a, b], 'views')).toEqual([
+      { rec: a, path: 'views[0]' },
+      { rec: b, path: 'views[1]' },
+    ]);
+  });
+
+  it('hands back the caller’s own record, not a copy', () => {
+    // Consumers mutate nothing, but they DO compare identity against the sites
+    // `view-walk.ts` yields, so a defensive copy here would break that.
+    const rec = { name: 'a' };
+    expect(collectionEntries([rec], 'views')[0].rec).toBe(rec);
+  });
+
+  it('skips non-records but keeps the index of the records it keeps', () => {
+    // The index is the AUTHORED position, so a skipped entry must not shift the
+    // ones after it — the path has to be one the author can look up.
+    const rec = { name: 'real' };
+    expect(collectionEntries([null, 'str', 42, rec], 'views')).toEqual([
+      { rec, path: 'views[3]' },
+    ]);
+  });
+
+  it('skips a NESTED array — an array is not a record', () => {
+    expect(collectionEntries([[{ name: 'a' }]], 'views')).toEqual([]);
+  });
+});
+
+describe('collectionEntries — the name-keyed map shape', () => {
+  it('yields each record at its key path, with the key spread in as `name`', () => {
+    // The map key IS the entry's name on this shape. Reporting `views[0]` for
+    // it would name a position that does not exist in the author's file.
+    expect(collectionEntries({ case_views: { object: 'crm_case' } }, 'views')).toEqual([
+      { rec: { name: 'case_views', object: 'crm_case' }, path: 'views.case_views' },
+    ]);
+  });
+
+  it('lets an entry’s OWN `name` win over the map key', () => {
+    // `{ name, ...def }` — key first, so the spread overwrites it.
+    expect(collectionEntries({ keyed: { name: 'declared' } }, 'views')[0].rec).toEqual({
+      name: 'declared',
+    });
+  });
+
+  it('skips non-record values', () => {
+    const entries = collectionEntries({ a: null, b: 'str', c: 42, d: [], e: { ok: true } }, 'views');
+    expect(entries).toEqual([{ rec: { name: 'e', ok: true }, path: 'views.e' }]);
+  });
+});
+
+describe('collectionEntries — what is not a collection', () => {
+  it.each([
+    ['null', null],
+    ['undefined', undefined],
+    ['a string', 'views'],
+    ['a number', 42],
+    ['a boolean', true],
+  ])('returns nothing for %s', (_label, v) => {
+    expect(collectionEntries(v, 'views')).toEqual([]);
+  });
+
+  it('returns nothing for an empty collection of either shape', () => {
+    expect(collectionEntries([], 'views')).toEqual([]);
+    expect(collectionEntries({}, 'views')).toEqual([]);
+  });
+});
+
+/**
+ * The one textual divergence between the copies this file converged (#6662).
+ *
+ * `validate-visibility-predicates` open-coded its record predicate inline, and
+ * its MAP-branch guard read `v && typeof v === 'object'` with no
+ * `!Array.isArray(v)` — where the other two copies called `isRec`, which has
+ * that third clause. The two are the same function because the ARRAY branch
+ * returns unconditionally, so the map guard is only ever evaluated on a value
+ * that is already not an array. This asserts that domination directly: an array
+ * must never be enumerated as a map, however it is decorated.
+ */
+describe('collectionEntries — an array is never enumerated as a map', () => {
+  it('reads only index entries, never an array’s other own keys', () => {
+    const arr: unknown[] & { extra?: AnyRec } = [{ name: 'indexed' }];
+    arr.extra = { name: 'not_an_entry' };
+
+    expect(collectionEntries(arr, 'views')).toEqual([
+      { rec: { name: 'indexed' }, path: 'views[0]' },
+    ]);
+  });
+
+  it('yields index paths for an empty-but-decorated array, not key paths', () => {
+    const arr: unknown[] & { case_views?: AnyRec } = [];
+    arr.case_views = { object: 'crm_case' };
+
+    expect(collectionEntries(arr, 'views')).toEqual([]);
+  });
+});
+
+/**
+ * The property the convergence buys: ONE coercion, THREE consumers.
+ *
+ * Before #6662 each of these rules decided on its own what path to print for a
+ * map-shaped collection. Break the coercion now and all three columns go red
+ * together, which is the whole point — the failure this class of duplication
+ * produces is the next author fixing one copy and leaving two behind (#6381's
+ * own history: #6128 / #6248, then #6251).
+ */
+describe('one coercion, three consumers (#6662)', () => {
+  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', //                        → translatable-sections
+        fields: [
+          'ghost_field', //                        → form-layout (unknown field)
+          { field: 'subject', visibleWhen: 'status == "open"' }, // → visibility
+        ],
+      },
+    ],
+  });
+
+  const view = () => ({ object: 'crm_case', ...body() });
+
+  it('all three report the ARRAY path for an array-shaped `views`', () => {
+    const stack = { objects, views: [view()], translations };
+
+    expect(validateVisibilityPredicates(stack).map((f) => f.path)).toEqual([
+      'views[0].sections[0].fields[1]',
+    ]);
+    expect(validateFormLayout(stack).map((f) => f.path)).toEqual([
+      'views[0].sections[0].fields[0]',
+    ]);
+    expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([
+      'views[0].sections[0]',
+    ]);
+  });
+
+  it('all three report the KEY path for a map-shaped `views`', () => {
+    const stack = { objects, views: { case_views: view() }, translations };
+
+    expect(validateVisibilityPredicates(stack).map((f) => f.path)).toEqual([
+      'views.case_views.sections[0].fields[1]',
+    ]);
+    expect(validateFormLayout(stack).map((f) => f.path)).toEqual([
+      'views.case_views.sections[0].fields[0]',
+    ]);
+    expect(validateTranslatableSections(stack).map((f) => f.path)).toEqual([
+      'views.case_views.sections[0]',
+    ]);
+  });
+
+  it('all three take the map KEY as the entry’s name in the `where` line', () => {
+    // The unnamed-but-keyed entry: nothing declares `name`, so the key is the
+    // only thing that can locate it for the author.
+    const stack = { objects, views: { case_views: view() }, translations };
+
+    expect(validateVisibilityPredicates(stack)[0].where).toContain('case_views');
+    expect(validateFormLayout(stack)[0].where).toContain('case_views');
+    expect(validateTranslatableSections(stack)[0].where).toContain('case_views');
+  });
+});

packages/lint/src/collection-entries.ts

@@ -0,0 +1,82 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Shared stack-collection enumeration (issue #6662) — the one coercion from a
+ * collection authored EITHER as an array OR as a name-keyed map into the
+ * records it holds, each carrying the config path it actually sits at.
+ *
+ * This helper had grown THREE independent copies in this package, all on the
+ * same view-walking rules that #6381 had just converged onto one descent
+ * (`view-walk.ts`): `validate-form-layout.ts`, `validate-translatable-sections.ts`
+ * and `validate-visibility-predicates.ts`. The duplication was already
+ * acknowledged in-tree — `validate-form-layout.ts`'s copy said out loud "Same
+ * helper, same reasoning as `validate-visibility-predicates.ts` and
+ * `validate-translatable-sections.ts`" — which records the cost without paying
+ * it. The copy COUNT is the argument for this file, exactly as it was for
+ * `view-walk.ts` and `page-walk.ts` (#3583): with three, the next author fixes
+ * one and the two survivors keep the old answer.
+ *
+ * ## Why the PATH is the point
+ *
+ * The sibling rules that do not report a location coerce with a local `asArray`
+ * and throw the path away. A rule that emits findings cannot: findings are
+ * consumed as EDIT TARGETS (`os lint --json`, Studio's finding renderer), so a
+ * map-shaped collection must not report a synthetic array index nobody can look
+ * up. `views[2]` is the honest path for the array shape and
+ * `views.contact_views` for the map, and this helper is the only place that
+ * decides which.
+ *
+ * ## Why the map shape injects `name`
+ *
+ * The map key IS the entry's name on that shape, so it is spread in as `name`
+ * (`{ name, ...def }`, key first so an entry's own `name` still wins). That is
+ * how an unnamed-but-keyed view still locates itself in a message — a rule that
+ * read only `rec.name` would otherwise print an anonymous finding for an entry
+ * the author named perfectly well.
+ *
+ * ## Non-records are skipped, not coerced
+ *
+ * On both shapes an entry that is not a record is dropped rather than repaired.
+ * Callers therefore receive records only, which is what lets
+ * `viewContainerSites` open with a defensive `isRec` guard it documents as
+ * unreachable from the in-repo callers.
+ */
+
+type AnyRec = Record<string, unknown>;
+
+/** One record of a collection, with the config path it sits at. */
+export interface CollectionEntry {
+  /** The record itself. On the map shape, with the map key spread in as `name`. */
+  rec: AnyRec;
+  /** Config path — `views[2]` for the array shape, `views.contact_views` for the map. */
+  path: string;
+}
+
+function isRec(v: unknown): v is AnyRec {
+  return !!v && typeof v === 'object' && !Array.isArray(v);
+}
+
+/**
+ * Every record in a collection authored either as an array or as a name-keyed
+ * map, each with its config path. `base` is the caller's path prefix for the
+ * collection itself (e.g. `views`, `objects[0].views`).
+ *
+ * Order is the collection's own order — array index order, or `Object.entries`
+ * insertion order for the map — because findings are emitted in walk order and
+ * every consumer's pinned output order depends on it.
+ */
+export function collectionEntries(v: unknown, base: string): CollectionEntry[] {
+  if (Array.isArray(v)) {
+    const out: CollectionEntry[] = [];
+    for (let i = 0; i < v.length; i++) {
+      if (isRec(v[i])) out.push({ rec: v[i] as AnyRec, path: `${base}[${i}]` });
+    }
+    return out;
+  }
+  if (isRec(v)) {
+    return Object.entries(v)
+      .filter(([, def]) => isRec(def))
+      .map(([name, def]) => ({ rec: { name, ...(def as AnyRec) }, path: `${base}.${name}` }));
+  }
+  return [];
+}

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

@@ -29,7 +29,8 @@
  * never guesses at an arbitrary component's object binding.
  */
 
-import { formViewSites } from './view-walk.js';
+import { collectionEntries } from './collection-entries.js';
+import { formViewSites, viewObjectName } from './view-walk.js';
 
 export const FORM_FIELD_UNKNOWN = 'form-field-unknown';
 export const FORM_COLSPAN_ABSOLUTE = 'absolute-colspan-discouraged';
@@ -70,30 +71,6 @@ function strName(v: unknown): string | undefined {
   return typeof v === 'string' && v.length > 0 ? v : undefined;
 }
 
-/**
- * Every record in a collection authored either as an array or as a name-keyed
- * map, each with its config PATH — `views[2]` for the array shape,
- * `views.contact_views` for the map. Findings here are consumed as edit targets
- * (`os lint --json`, Studio's finding renderer), so a map-shaped collection must
- * not report a synthetic index nobody can look up. Same helper, same reasoning
- * as `validate-visibility-predicates.ts` and `validate-translatable-sections.ts`.
- */
-function collectionEntries(v: unknown, base: string): Array<{ rec: AnyRec; path: string }> {
-  if (Array.isArray(v)) {
-    const out: Array<{ rec: AnyRec; path: string }> = [];
-    for (let i = 0; i < v.length; i++) {
-      if (isRec(v[i])) out.push({ rec: v[i] as AnyRec, path: `${base}[${i}]` });
-    }
-    return out;
-  }
-  if (isRec(v)) {
-    return Object.entries(v)
-      .filter(([, def]) => isRec(def))
-      .map(([name, def]) => ({ rec: { name, ...(def as AnyRec) }, path: `${base}.${name}` }));
-  }
-  return [];
-}
-
 /**
  * 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
@@ -121,32 +98,6 @@ function fieldNameOf(entry: unknown): string | null {
   return null;
 }
 
-/**
- * The object a view — or one of its sub-containers — binds to, across the shapes
- * it is authored in.
- *
- * The ladder is `objectName` → `object` → `data.object`, identical to
- * `validate-translation-references.ts` and `validate-translatable-sections.ts`'s
- * `viewObjectName` (and to the CLI i18n walker's), so all of them agree on which
- * object a form belongs to. On the canonical container shape the binding lives
- * INSIDE the sub-container (`form.data.object`) while the container itself
- * carries `object`, which is why the caller resolves the site first and falls
- * back to the container — a record-level lookup alone resolves to nothing on the
- * shape real apps ship.
- *
- * `name` is deliberately NOT a rung. A stack-level container's `name` may be the
- * object name (`view.zod.ts` says so for object-scoped containers), but a form
- * view's `name` is its own — `contract_form`, not `contract` — and reading it
- * here would bind the wrong object and report every field on the form as unknown.
- */
-function boundObject(view: AnyRec): string | undefined {
-  return (
-    strName(view.objectName) ??
-    strName(view.object) ??
-    (isRec(view.data) ? strName(view.data.object) : undefined)
-  );
-}
-
 /**
  * Validate authored form-view layout. Returns findings (empty = clean).
  * Advisory only — the caller must never fail the build on these alone.
@@ -169,16 +120,17 @@ export function validateFormLayout(stack: AnyRec): FormLayoutFinding[] {
     // A container names itself with `name`, or binds with `object` — and an
     // artifact-emitted one may carry neither, so the path is the last resort.
     const viewName = strName(view.name) ?? strName(view.object) ?? viewPath;
-    const containerObject = boundObject(view);
+    const containerObject = viewObjectName(view);
 
     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. 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
+      // view-walking rule in this package uses. The base rung is the shared
+      // `viewObjectName` (#6662); this FALLBACK is deliberately NOT folded into
+      // the shared walker, because the consumers compose it differently (see
+      // `view-walk.ts`) and a refactor that changes a verdict is a failed
       // refactor.
-      const objName = boundObject(site.view) ?? containerObject;
+      const objName = viewObjectName(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}"`;