diff --git a/.changeset/adr-0089-d3b-bidirectional-visibility-root.md b/.changeset/adr-0089-d3b-bidirectional-visibility-root.md new file mode 100644 index 0000000000..e3dc2f5c78 --- /dev/null +++ b/.changeset/adr-0089-d3b-bidirectional-visibility-root.md @@ -0,0 +1,5 @@ +--- +"@objectstack/lint": minor +--- + +ADR-0089 D3b: make the `visibility-root-mislayered` lint check bidirectional. `validateVisibilityPredicates` now accepts an optional `{ layer }` option — `'runtime'` (default, unchanged) flags a `data.`-rooted predicate on a `*.view.ts` / `*.page.ts` surface, and `'metadata'` flags a `record.`-rooted predicate on a `*.form.ts` metadata-editing form. Both directions of the ADR's binding-root rule are now covered. Adds the `VisibilityLayer` / `VisibilityOptions` exported types. Fully back-compat: existing single-argument callers keep the runtime behavior. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 33a127d517..68880ebf34 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -74,7 +74,12 @@ export { VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED, } from './validate-visibility-predicates.js'; -export type { VisibilityFinding, VisibilitySeverity } from './validate-visibility-predicates.js'; +export type { + VisibilityFinding, + VisibilitySeverity, + VisibilityLayer, + VisibilityOptions, +} from './validate-visibility-predicates.js'; export { validateCapabilityReferences, diff --git a/packages/lint/src/validate-visibility-predicates.test.ts b/packages/lint/src/validate-visibility-predicates.test.ts index b21dd54dc1..32be87fdec 100644 --- a/packages/lint/src/validate-visibility-predicates.test.ts +++ b/packages/lint/src/validate-visibility-predicates.test.ts @@ -126,4 +126,62 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { expect(validateVisibilityPredicates({})).toEqual([]); expect(validateVisibilityPredicates({ views: [], pages: [] })).toEqual([]); }); + + // ── Layer-directional binding-root check (ADR-0089 D3, both directions) ── + describe('metadata layer (opts.layer: "metadata")', () => { + it('flags a `record.`-rooted predicate in a metadata-editing form as mis-layered', () => { + const stack = { + views: [ + { name: 'page_form', sections: [{ fields: [{ field: 'template', visibleWhen: "record.type != 'list'" }] }] }, + ], + }; + const findings = validateVisibilityPredicates(stack, { layer: 'metadata' }); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toContain('record.'); + expect(findings[0].hint).toContain('data'); + }); + + it('is clean for the canonical `data.` root in a metadata-editing form', () => { + const stack = { + views: [ + { name: 'page_form', sections: [{ visibleWhen: "data.type != 'list'", fields: [{ field: 'template', visibleWhen: "data.type == 'grid'" }] }] }, + ], + }; + expect(validateVisibilityPredicates(stack, { layer: 'metadata' })).toEqual([]); + }); + + it('does NOT flag `data.` as mis-layered in the metadata layer (the runtime rule is the opposite)', () => { + const stack = { + views: [{ name: 'f', sections: [{ fields: [{ field: 'x', visibleWhen: "data.type == 'grid'" }] }] }], + }; + // Runtime (default) flags `data.`… + expect(validateVisibilityPredicates(stack).map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + // …metadata does not. + expect(validateVisibilityPredicates(stack, { layer: 'metadata' })).toEqual([]); + }); + + it('still flags a deprecated alias key in the metadata layer (alias check is layer-agnostic)', () => { + const stack = { + views: [{ name: 'f', sections: [{ visibleOn: "data.a == 1", fields: [] }] }], + }; + const rules = validateVisibilityPredicates(stack, { layer: 'metadata' }).map((f) => f.rule); + // alias present, but `data.` is correct for the metadata layer → only the alias finding. + expect(rules).toEqual([VISIBILITY_ALIAS_DEPRECATED]); + }); + + it('does not confuse an identifier ending in `record` (e.g. `my_record.x`) for a record root', () => { + const stack = { + views: [{ name: 'f', sections: [{ fields: [{ field: 'x', visibleWhen: "my_record.x == 1" }] }] }], + }; + expect(validateVisibilityPredicates(stack, { layer: 'metadata' })).toEqual([]); + }); + }); + + it('runtime is the default layer (no opts) — flags `data.`, not `record.`', () => { + const dataStack = { views: [{ name: 'f', sections: [{ fields: [{ field: 'x', visibleWhen: 'data.a == 1' }] }] }] }; + const recordStack = { views: [{ name: 'f', sections: [{ fields: [{ field: 'x', visibleWhen: 'record.a == 1' }] }] }] }; + expect(validateVisibilityPredicates(dataStack).map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(validateVisibilityPredicates(recordStack)).toEqual([]); + }); }); diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index f7ec62c784..b3bad84be8 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -17,11 +17,16 @@ * * - `visibility-alias-deprecated` — a `visibleOn` / `visibility` key in authored * source. Autofix intent: rename the key to `visibleWhen` (same value). - * - `visibility-root-mislayered` — a runtime view/page visibility predicate - * rooted at `data.` (the metadata-editing-form root, ADR-0089 §Context). Runtime - * record surfaces bind `record` + `current_user` (pages also expose `page.`), - * so a `data.`-rooted predicate here is almost always a wrong-layer paste that - * silently never matches. + * - `visibility-root-mislayered` — a visibility predicate whose binding root does + * not match its layer (ADR-0089 D3, §Context). The check is **bidirectional**: + * - **runtime** view/page surfaces (`*.view.ts` / `*.page.ts`) bind + * `record` + `current_user` (pages also expose `page.`), so a `data.`-rooted + * predicate here is a wrong-layer paste that silently never matches; and + * - **metadata-editing** forms (`*.form.ts` — the row under edit) bind `data`, so + * a `record.`-rooted predicate there is the same bug in the other direction. + * The layer is supplied by the caller (`opts.layer`, default `'runtime'`): the + * app-lint path (`os validate` / `compile`) always lints runtime surfaces, while a + * file-aware caller linting a `*.form.ts` passes `layer: 'metadata'`. * * Scope: `views` (form `sections` / legacy `groups`, and their `fields`) and * `pages` (`regions[].components[]`). Data-field `visibleWhen` is already covered @@ -33,6 +38,19 @@ export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered'; export type VisibilitySeverity = 'error' | 'warning'; +/** + * Which binding environment the linted surface belongs to (ADR-0089 §Context): + * - `runtime` — `*.view.ts` / `*.page.ts`; binds `record` + `current_user` (+ `page`). + * - `metadata` — `*.form.ts` metadata-editing forms; binds `data` (the row under edit). + */ +export type VisibilityLayer = 'runtime' | 'metadata'; + +/** Options for {@link validateVisibilityPredicates}. */ +export interface VisibilityOptions { + /** Binding layer of the surface being linted. Defaults to `'runtime'`. */ + layer?: VisibilityLayer; +} + export interface VisibilityFinding { /** Always `warning` today — both rules are advisory (see module note). */ severity: VisibilitySeverity; @@ -72,22 +90,58 @@ function predicateSource(v: unknown): string | undefined { return undefined; } -/** Does the predicate reference `data.` as a binding root? */ -function usesDataRoot(source: string): boolean { - // `data` as a leading identifier followed by a property access — the - // metadata-editing-form root. Excludes `foo.data` (a field named data). - return /(^|[^.\w$])data\.\w/.test(source); +/** Does the predicate reference `.` as a leading binding root? */ +function usesRoot(source: string, root: string): boolean { + // `` as a leading identifier followed by a property access. The leading + // `(^|[^.\w$])` guard excludes a nested access like `foo.data` (a field named + // `data`) or `my_record.x` (an identifier that merely ends in `record`). + return new RegExp(`(^|[^.\\w$])${root}\\.\\w`).test(source); } +/** + * Per-layer mis-rooted-predicate description. The `runtime` layer forbids the + * metadata-editing-form root (`data.`); the `metadata` layer forbids the runtime + * record-surface root (`record.`) — ADR-0089 D3 spells out both directions. + */ +const MISLAYER_BY_LAYER: Record< + VisibilityLayer, + { forbiddenRoot: string; message: string; hint: string } +> = { + runtime: { + forbiddenRoot: 'data', + message: + 'visibility predicate is rooted at `data.` — that is the ' + + 'metadata-editing-form root (a `*.form.ts` row under edit), not a runtime ' + + 'surface. A runtime view/page predicate that binds `data.` never matches ' + + 'and the element renders unconditionally (ADR-0089).', + hint: + 'Runtime record surfaces bind `record` + `current_user` (pages also ' + + "expose `page.`). Use e.g. `record.status == 'open'` instead of " + + "`data.status == 'open'`.", + }, + metadata: { + forbiddenRoot: 'record', + message: + 'visibility predicate is rooted at `record.` — that is the runtime ' + + 'record-surface root (a `*.view.ts` / `*.page.ts` live record), not a ' + + 'metadata-editing form. A `*.form.ts` predicate that binds `record.` never ' + + 'matches and the element renders unconditionally (ADR-0089).', + hint: + 'Metadata-editing forms bind `data` (the row under edit). Use e.g. ' + + "`data.type == 'grid'` instead of `record.type == 'grid'`.", + }, +}; + /** * Inspect one element carrying a visibility predicate. Emits the alias-deprecated * finding (when an alias key is present) and the mis-layered-root finding (when - * the effective predicate is rooted at `data.`). + * the effective predicate's binding root does not match `layer`). */ function checkElement( el: AnyRec, where: string, path: string, + layer: VisibilityLayer, findings: VisibilityFinding[], ): void { // (1) deprecated alias key present → steer to `visibleWhen`. @@ -107,24 +161,19 @@ function checkElement( } } - // (2) mis-layered binding root — check the effective predicate (canonical wins). + // (2) mis-layered binding root — check the effective predicate (canonical wins) + // against the root expected for this layer. const raw = el[CANONICAL] ?? el.visibleOn ?? el.visibility; const source = predicateSource(raw); - if (source && usesDataRoot(source)) { + const rule = MISLAYER_BY_LAYER[layer]; + if (source && usesRoot(source, rule.forbiddenRoot)) { findings.push({ severity: 'warning', rule: VISIBILITY_ROOT_MISLAYERED, where, path, - message: - `visibility predicate is rooted at \`data.\` — that is the ` + - `metadata-editing-form root (a \`*.form.ts\` row under edit), not a runtime ` + - `surface. A runtime view/page predicate that binds \`data.\` never matches ` + - `and the element renders unconditionally (ADR-0089).`, - hint: - `Runtime record surfaces bind \`record\` + \`current_user\` (pages also ` + - `expose \`page.\`). Use e.g. \`record.status == 'open'\` instead of ` + - `\`data.status == 'open'\`.`, + message: rule.message, + hint: rule.hint, }); } } @@ -141,8 +190,18 @@ function isFieldObject(entry: unknown): entry is AnyRec { * `visibleOn` / `visibility` aliases before the schema folds them into * `visibleWhen`. Returns findings (empty = clean); all advisory (`warning`) — * the caller must never fail the build on these alone. + * + * The binding-root check is layer-directional (ADR-0089 D3): pass + * `opts.layer = 'metadata'` when linting a `*.form.ts` metadata-editing form (so a + * `record.`-rooted predicate is flagged), or leave it at the `'runtime'` default for + * `*.view.ts` / `*.page.ts` surfaces (so a `data.`-rooted predicate is flagged). The + * alias-deprecated check is layer-agnostic. */ -export function validateVisibilityPredicates(stack: AnyRec): VisibilityFinding[] { +export function validateVisibilityPredicates( + stack: AnyRec, + opts: VisibilityOptions = {}, +): VisibilityFinding[] { + const layer: VisibilityLayer = opts.layer ?? 'runtime'; const findings: VisibilityFinding[] = []; // ── Views: form sections / legacy groups, and their fields ────────── @@ -161,13 +220,13 @@ export function validateVisibilityPredicates(stack: AnyRec): VisibilityFinding[] const sec = sections[s]; if (!sec || typeof sec !== 'object') continue; const secPath = `views[${i}].${bucket}[${s}]`; - checkElement(sec as AnyRec, where, secPath, findings); + checkElement(sec as AnyRec, where, secPath, layer, findings); const secFields = Array.isArray((sec as AnyRec).fields) ? ((sec as AnyRec).fields as unknown[]) : []; for (let f = 0; f < secFields.length; f++) { const entry = secFields[f]; if (isFieldObject(entry)) { - checkElement(entry, where, `${secPath}.fields[${f}]`, findings); + checkElement(entry, where, `${secPath}.fields[${f}]`, layer, findings); } } } @@ -190,7 +249,7 @@ export function validateVisibilityPredicates(stack: AnyRec): VisibilityFinding[] for (let c = 0; c < components.length; c++) { const comp = components[c]; if (comp && typeof comp === 'object') { - checkElement(comp as AnyRec, where, `pages[${i}].regions[${r}].components[${c}]`, findings); + checkElement(comp as AnyRec, where, `pages[${i}].regions[${r}].components[${c}]`, layer, findings); } } }