Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/adr-0089-d3b-bidirectional-visibility-root.md
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 6 additions & 1 deletion packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
58 changes: 58 additions & 0 deletions packages/lint/src/validate-visibility-predicates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});
});
111 changes: 85 additions & 26 deletions packages/lint/src/validate-visibility-predicates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.<var>`),
* 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.<var>`), 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
Expand All @@ -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;
Expand Down Expand Up @@ -72,22 +90,58 @@ function predicateSource(v: unknown): string | undefined {
return undefined;
}

/** Does the predicate reference `data.<x>` 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 `<root>.<x>` as a leading binding root? */
function usesRoot(source: string, root: string): boolean {
// `<root>` 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.<var>`). 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`.
Expand All @@ -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.<var>\`). Use e.g. \`record.status == 'open'\` instead of ` +
`\`data.status == 'open'\`.`,
message: rule.message,
hint: rule.hint,
});
}
}
Expand All @@ -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 ──────────
Expand All @@ -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);
}
}
}
Expand All @@ -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);
}
}
}
Expand Down