diff --git a/.changeset/visibility-alias-deprecated-retired.md b/.changeset/visibility-alias-deprecated-retired.md new file mode 100644 index 0000000000..c70b3bc138 --- /dev/null +++ b/.changeset/visibility-alias-deprecated-retired.md @@ -0,0 +1,67 @@ +--- +"@objectstack/lint": minor +--- + +refactor(lint)!: retire `visibility-alias-deprecated` — the rule could not fire on any real CLI input (#6318, ADR-0049) + +`@objectstack/lint` shipped a fourth conditional-visibility rule whose only job +was to report the deprecated predicate **key** (`visibleOn` on a view form +section/field, `visibility` on a page component) and steer the author to +`visibleWhen`. It never reported on anything a command actually loads. + +**Why it could not fire.** The rule is registered `input: 'normalized'`, so what +`os validate` / `os build` / `os lint` hand it is the output of +`normalizeStackInput`. The two ADR-0087 D2 conversions that fold the alias — +`view-visibleOn-to-visibleWhen` and `page-component-visibility-to-visibleWhen` — +run **inside** `normalizeStackInput`, one layer above. The key is therefore +already renamed by the time the rule sees the stack. Re-measured per site: + +| alias site | rule fed the raw authored object | rule fed the `normalized` tier | +|---|---|---| +| `views[].form.sections[]` | 1 finding | **0** | +| `views[].formViews.edit.sections[]` | 1 finding | **0** | +| `pages[].regions[].components[]` | 1 finding | **0** | + +The one shape it did still fire on is a view **container** carrying top-level +`sections` — the shape its own unit tests used, and the shape strict +`ViewSchema` refuses outright (`Unrecognized key(s) on this view container: +\`sections\``). A green unit test over a fixture production can never send. + +**No working app loses a signal.** Authors were never hearing this rule, and +they do hear the conversion: the same D2 entry emits a `warnConversionNotice` +from `defineStack` that names the site, the conversion id and the retirement +window — wording the lint rule never had. + +``` +defineStack: views[0].form.sections[0].visibleWhen: 'visibleOn' -> 'visibleWhen' + (converted at load; conversion 'view-visibleOn-to-visibleWhen', retires in protocol 16). + Update the source to the canonical shape — the conversion stops running then. +``` + +**Authored metadata is unaffected.** `visibleOn` / `visibility` remain accepted +exactly as before, still fold to `visibleWhen`, and still retire with protocol +16. Nothing an app author writes has to change. + +**Consumer migration — one removed export.** The rule id constant leaves the +published barrel: + +- `VISIBILITY_ALIAS_DEPRECATED` (`'visibility-alias-deprecated'`) is removed from + `@objectstack/lint`. Delete the import; no finding carries that `rule` value + any more, so a `suppressWarnings: ['visibility-alias-deprecated']` entry or a + filter comparing against it is now dead code and can go with it. + +The other three rules in the same module are **unchanged** — they judge the +predicate's *value*, which crosses the fold into `visibleWhen` intact, and each +still reports normally on the `normalized` tier: +`visibility-root-mislayered`, `visibility-bare-identifier`, +`visibility-predicate-syntax`. `checkElement` also keeps reading the predicate +through the deprecated keys (canonical-first, so an alias can never override +`visibleWhen`), which is what lets those three still judge an alias-spelled +predicate handed to the exported function directly. + +Retired rather than re-anchored: making the rule read a genuine pre-normalize +value would have changed `runAuthoringRules`' external input contract, which is +a `packages/lint` public-API decision for the maintainer rather than a rule +file's to take. + + diff --git a/packages/lint/src/authoring-rule-input-tier.test.ts b/packages/lint/src/authoring-rule-input-tier.test.ts index 36d6b3b885..73f527d1b7 100644 --- a/packages/lint/src/authoring-rule-input-tier.test.ts +++ b/packages/lint/src/authoring-rule-input-tier.test.ts @@ -212,20 +212,42 @@ describe('premise 2 (FALSE): "the parse strips `userFilters`/`quickFilters` on a describe('premise 3 (FALSE): "the `visibleOn` alias survives until the parse"', () => { // The fold is an ADR-0087 D2 conversion inside `normalizeStackInput` — one // layer BEFORE this tier — not a parse-time `.transform()`. So the alias is - // gone from the tier's own input on every door, `os lint` included. #6318. - const aliasSites: Array<[string, AnyRec]> = [ + // gone from the tier's own input on every door, `os lint` included. + // + // #6318 acted on that measurement: the alias-KEY rule this premise was + // written to justify (`visibility-alias-deprecated`) has been RETIRED, so the + // assertions below no longer count its findings. They pin the mechanism that + // outlives it, which is what makes the retirement safe to keep: + // + // * the KEY does not cross the fold — nothing downstream can judge it; + // * the VALUE does cross it intact — which is why the three surviving rules + // work on this tier and were not swept in; + // * the author is not silent — the D2 conversion notice fires in + // `defineStack`, and that notice IS the recorded guard for this surface. + // + // Read `toHaveLength(0)` on the raw leg as "the alias key is nobody's verdict + // any more"; the fold measurement itself now lives in the VALUE assertions, + // which are non-empty and can actually fail. + /** + * The three spec-valid alias sites, each carrying `predicate` under its + * DEPRECATED key. Parameterised on the predicate so the same three shapes can + * be measured twice: once with a clean value (nothing to find but the retired + * key) and once with a bare-identifier value (a VALUE defect the surviving + * gate must still reach through the fold). + */ + const aliasSitesWith = (predicate: string): Array<[string, AnyRec]> => [ ['views[].form.sections[]', { manifest, views: [{ name: 'tier_form', - form: { type: 'simple', sections: [{ label: 'S', visibleOn: 'record.a == 1', fields: [{ field: 'name' }] }] }, + form: { type: 'simple', sections: [{ label: 'S', visibleOn: predicate, fields: [{ field: 'name' }] }] }, }], }], ['views[].formViews.edit.sections[]', { manifest, views: [{ name: 'tier_form2', - formViews: { edit: { type: 'simple', sections: [{ label: 'S', visibleOn: 'record.a == 1', fields: [{ field: 'name' }] }] } }, + formViews: { edit: { type: 'simple', sections: [{ label: 'S', visibleOn: predicate, fields: [{ field: 'name' }] }] } }, }], }], ['pages[].regions[].components[]', { @@ -235,19 +257,55 @@ describe('premise 3 (FALSE): "the `visibleOn` alias survives until the parse"', label: 'P', type: 'home', object: 'tier_task', - regions: [{ name: 'main', components: [{ type: 'element:text', visibility: "page.selectedId != ''" }] }], + regions: [{ name: 'main', components: [{ type: 'element:text', visibility: predicate }] }], }], }], ]; - it.each(aliasSites)('%s: the alias is folded BEFORE the tier, so the rule reports 0', (_site, stack) => { - // Fed the raw authored object (what the rule's own unit tests do) it reports. - expect(validateVisibilityPredicates(structuredClone(stack))).toHaveLength(1); - // Fed the `normalized` tier (what all three commands do) it does not. + /** Clean, canonically-rooted predicate: the only thing wrong is the key spelling. */ + const aliasSites = aliasSitesWith('record.a == 1'); + /** Same three sites, predicate rooted nowhere (#5149 Repro 1) — a VALUE defect. */ + const aliasSitesBadValue = aliasSitesWith('approved'); + + it.each(aliasSites)('%s: no rule judges the alias KEY any longer (#6318 retirement)', (_site, stack) => { + // Both doors report nothing, and for TWO DIFFERENT reasons that must not be + // conflated. Raw: the rule that would have judged the key is retired. + // Normalized: the key is not even there — the D2 fold renamed it one layer + // up. The `normalized` leg is a VACUOUS green after the retirement (it is + // empty because no rule exists, not because of the fold), so it is labelled + // as such and carries no weight on its own; the leg below is the one that + // measures the fold. + expect(validateVisibilityPredicates(structuredClone(stack))).toHaveLength(0); expect(validateVisibilityPredicates(normalizeStackInput(structuredClone(stack)) as AnyRec)).toEqual([]); }); + it.each(aliasSitesBadValue)( + '%s: the KEY does not cross the fold but the VALUE does — measured on a NON-EMPTY finding set', + (_site, stack) => { + // The replacement for the vacuous green above, and the assertion that + // actually measures the fold. Same three sites, but the predicate is now + // a bare identifier — a VALUE defect. If the fold dropped the predicate + // instead of renaming its key, or if the surviving gate stopped reaching + // it, this set would be EMPTY and the assertion would fail. It cannot + // pass by producing nothing, which is exactly what the leg above can do. + const normalized = normalizeStackInput(structuredClone(stack)) as AnyRec; + // The KEY is gone from the tier's own input … + expect(JSON.stringify(normalized)).not.toContain('visibleOn'); + expect(JSON.stringify(normalized)).not.toContain('"visibility"'); + // … and the VALUE arrived under the canonical key, where the surviving + // gate reads it. Exactly one finding, named. + expect(validateVisibilityPredicates(normalized).map((f) => f.rule)).toEqual([ + 'visibility-bare-identifier', + ]); + }, + ); + it('the author is NOT left silent — the D2 conversion notice names the site and its retirement', () => { + // This notice is the RECORDED GUARD for the alias surface: it is why #6318 + // could retire the lint rule instead of re-anchoring it, and why no working + // app lost a signal. If this test ever goes red, the retirement's premise is + // gone and the surface is genuinely unguarded — re-open #6318, do not delete + // this assertion. const { error, warnings } = quietly(() => defineStack(structuredClone(aliasSites[0][1]) as never)); expect(error).toBeUndefined(); expect(warnings).toHaveLength(1); @@ -257,8 +315,9 @@ describe('premise 3 (FALSE): "the `visibleOn` alias survives until the parse"', }); it('the predicate-VALUE rules in the same file are unaffected — do not connect them', () => { - // The value moves into `visibleWhen` intact, so these two still report on the - // tier. #6318 is about the alias-KEY rule only. + // The value moves into `visibleWhen` intact, so these still report on the + // tier. #6318 was about the alias-KEY rule only, and this is the pin that + // says so from the far side of the retirement. const bare = { manifest, views: [{ diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 4ad71ac1f0..f37a044373 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -236,7 +236,11 @@ export type AuthoringRuleTier = 'gating' | 'advisory'; * ADR-0087 D2 conversion (`view-visibleOn-to-visibleWhen`, * `page-component-visibility-to-visibleWhen`) folds it into `visibleWhen` * INSIDE `normalizeStackInput` — one layer before the tier, not during the - * parse. See #6318. + * parse. #6318 acted on that: the alias-KEY rule this premise justified + * (`visibility-alias-deprecated`) was retired, since the D2 conversion + * notice already covers its whole evidence surface with better wording and + * a stated retirement window. The alias-KEY half of premise 3 is therefore + * no longer merely false — there is nothing left reading it. * * ## What it does buy, and why the tier stays * @@ -746,17 +750,29 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateSeedStateMachine(stack), }, - // ADR-0089 D3b — deprecated visibility aliases and a mis-layered binding root, - // plus (#6128) the bare-identifier gate. This entry used to read "pre-parse: - // the schema folds `visibleOn`/`visibility` into `visibleWhen` during parse, - // so the alias the author wrote is gone from `result.data`". Measured false - // at #6073: the ADR-0087 D2 conversions do that fold INSIDE + // ADR-0089 D3b — a mis-layered binding root, plus (#6128) the bare-identifier + // gate and (#6253) the syntax gate. This entry used to read "pre-parse: the + // schema folds `visibleOn`/`visibility` into `visibleWhen` during parse, so + // the alias the author wrote is gone from `result.data`". Measured false at + // #6073: the ADR-0087 D2 conversions do that fold INSIDE // `normalizeStackInput`, one layer BEFORE this tier, so on every spec-valid - // alias site `visibility-alias-deprecated` reports zero here too — see #6318, - // which carries the per-site table and the retire-or-rewire question. The two - // predicate-VALUE rules (`visibility-bare-identifier`, - // `visibility-root-mislayered`) are unaffected: the value moves into - // `visibleWhen` intact and both still report on this tier. + // alias site the alias-KEY rule reported zero here too. + // + // #6318 closed that: `visibility-alias-deprecated` was RETIRED rather than + // re-anchored. Re-anchoring would have had to move this entry's input to a + // pre-`normalizeStackInput` value that `runAuthoringRules` does not accept — + // a change to this package's external input contract, and the maintainer's + // call, not a rule file's. Retirement is ADR-0049 (declared ≠ enforced) and + // costs no author a signal: the same D2 conversion already shouts through + // `warnConversionNotice` in `defineStack`, naming the site, the conversion and + // the protocol-16 retirement window — better wording than the rule ever had. + // + // Every rule left in the family judges the predicate's VALUE, and the value + // moves into `visibleWhen` intact, so all three report normally on this tier. + // The tier therefore stays `normalized` on its SURVIVING justification (a + // finding still reaches the author when an unrelated schema error would stop + // the parse — see `AuthoringRuleInputTier`), never on the retired + // "pre-parse evidence" one. // // `gating` since #6128: `visibility-bare-identifier` emits `error`. The two // ADR-0089 rules stay advisory findings within it — the tier is a property of diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 19e8da742f..a223a698da 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -137,7 +137,6 @@ export type { FormLayoutFinding, FormLayoutSeverity } from './validate-form-layo export { validateVisibilityPredicates, - VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, VISIBILITY_PREDICATE_SYNTAX, diff --git a/packages/lint/src/validate-visibility-predicates.test.ts b/packages/lint/src/validate-visibility-predicates.test.ts index 915ad86d53..5ddeba80af 100644 --- a/packages/lint/src/validate-visibility-predicates.test.ts +++ b/packages/lint/src/validate-visibility-predicates.test.ts @@ -3,7 +3,6 @@ import { describe, it, expect } from 'vitest'; import { validateVisibilityPredicates, - VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, VISIBILITY_PREDICATE_SYNTAX, @@ -37,40 +36,88 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { expect(validateVisibilityPredicates(stack)).toEqual([]); }); - it('flags a deprecated `visibleOn` alias on a form section (→ visibleWhen)', () => { - const stack = { - views: [ - { name: 'task_form', sections: [{ label: 'S', visibleOn: "record.a == 1", fields: [] }] }, - ], - }; - const findings = validateVisibilityPredicates(stack); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(VISIBILITY_ALIAS_DEPRECATED); - expect(findings[0].severity).toBe('warning'); - expect(findings[0].path).toBe('views[0].sections[0].visibleOn'); - }); + // ── #6318: the alias-KEY rule is RETIRED; the alias-VALUE read is not ── + // + // Three tests here used to assert that a `visibleOn` / `visibility` KEY + // produced a `visibility-alias-deprecated` finding, one per carrier. That + // rule never reached a real input: through all three CLI commands the stack + // arrives at the `normalized` tier, and the ADR-0087 D2 conversions rename the + // key INSIDE `normalizeStackInput`, one layer above. Measured at retirement, + // per site: raw object 1 finding, `normalized` tier 0. The only shape that + // still fired is the one those three fixtures used — a view CONTAINER with + // top-level `sections` — which strict `ViewSchema` refuses outright + // ("Unrecognized key(s) on this view container: `sections`"). Green unit test, + // impossible fixture: #4984 / #6251. + // + // What SURVIVES those three assertions, and is what the replacements below + // pin instead: the traversal reaches all three carriers, and `checkElement` + // still reads the predicate VALUE through the deprecated key, so a value + // defect written under an alias is judged rather than skipped. Each asserts + // the EXACT finding set — non-empty and named, so it cannot pass by producing + // nothing, and restoring the retired rule would fail it on set inequality. + // + // The surface itself is not unguarded: the same D2 conversion emits a + // `warnConversionNotice` from `defineStack` naming the site, the conversion + // and the protocol-16 retirement window. `authoring-rule-input-tier.test.ts` + // (premise 3) pins that notice as the recorded guard. + describe('the deprecated alias keys (#6318 — key rule retired, value read kept)', () => { + it('a `visibleOn` KEY with a clean value is clean — no rule judges the spelling', () => { + const stack = { + views: [ + { name: 'task_form', sections: [{ label: 'S', visibleOn: "record.a == 1", fields: [] }] }, + ], + }; + // Deliberately labelled: this green is VACUOUS with respect to the fold — + // it is empty because the rule is gone, not because anything normalized. + // It earns its place only as the retirement's direct statement, and the + // three assertions below are what carry the real evidence. + expect(validateVisibilityPredicates(stack)).toEqual([]); + }); - it('flags a deprecated `visibleOn` alias on a form field', () => { - const stack = { - views: [ - { name: 'task_form', sections: [{ fields: [{ field: 'notes', visibleOn: "record.a == 1" }] }] }, - ], - }; - const findings = validateVisibilityPredicates(stack); - expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ALIAS_DEPRECATED]); - expect(findings[0].path).toBe('views[0].sections[0].fields[0].visibleOn'); - }); + it('a form SECTION predicate is still read through `visibleOn` (value verdict survives)', () => { + const stack = { + views: [ + { name: 'task_form', sections: [{ label: 'S', visibleOn: "data.a == 1", fields: [] }] }, + ], + }; + const findings = validateVisibilityPredicates(stack); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].path).toBe('views[0].sections[0]'); + }); - it('flags a deprecated `visibility` alias on a page component', () => { - const stack = { - pages: [ - { name: 'p', regions: [{ components: [{ type: 'element:text', visibility: "page.x != ''" }] }] }, - ], - }; - const findings = validateVisibilityPredicates(stack); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(VISIBILITY_ALIAS_DEPRECATED); - expect(findings[0].path).toBe('pages[0].regions[0].components[0].visibility'); + it('a form FIELD predicate is still read through `visibleOn`', () => { + const stack = { + views: [ + { name: 'task_form', sections: [{ fields: [{ field: 'notes', visibleOn: "data.a == 1" }] }] }, + ], + }; + const findings = validateVisibilityPredicates(stack); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].path).toBe('views[0].sections[0].fields[0]'); + }); + + it('a PAGE COMPONENT predicate is still read through the page-side `visibility` key', () => { + const stack = { + pages: [ + { name: 'p', regions: [{ components: [{ type: 'element:text', visibility: "data.x != ''" }] }] }, + ], + }; + const findings = validateVisibilityPredicates(stack); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].path).toBe('pages[0].regions[0].components[0]'); + }); + + it('the canonical key still wins over an alias on the same element (ADR-0089 ordering)', () => { + // Why the surviving alias limbs can only add coverage and never change a + // verdict: `visibleWhen` is read first, so a stale alias beside it is + // ignored rather than allowed to overrule the canonical spelling. + const stack = { + views: [ + { name: 'f', sections: [{ visibleWhen: "record.ok == 1", visibleOn: "data.stale == 1", fields: [] }] }, + ], + }; + expect(validateVisibilityPredicates(stack)).toEqual([]); + }); }); it('flags a `data.`-rooted predicate in a runtime view as mis-layered', () => { @@ -84,14 +131,16 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { expect(findings[0].severity).toBe('warning'); }); - it('reports BOTH alias + mis-layer when a `visibleOn` predicate is `data.`-rooted', () => { + it('a `data.`-rooted predicate under `visibleOn` reports the mis-layer ALONE (#6318)', () => { + // Was: "reports BOTH alias + mis-layer". The alias half is retired; the + // surviving half — the mis-layer verdict is reached THROUGH the deprecated + // key — is what this now pins, as an exact one-element set. const stack = { views: [ { name: 'task_form', sections: [{ visibleOn: "data.status == 'x'", fields: [] }] }, ], }; - const rules = validateVisibilityPredicates(stack).map((f) => f.rule).sort(); - expect(rules).toEqual([VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED].sort()); + expect(validateVisibilityPredicates(stack).map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); }); it('does not confuse a field literally named `data` (e.g. `record.data`) for a data root', () => { @@ -117,12 +166,20 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { }); it('walks legacy `groups` (alias of sections) too', () => { + // The `groups` bucket has no other test in this file, so the half-fact this + // used to carry — the traversal descends `groups` at all — had to survive + // #6318's retirement of the alias-KEY rule that used to demonstrate it. It + // is re-demonstrated with a rule that still exists, and with the alias key + // kept on the fixture so the group walk and the alias-value read are pinned + // together exactly as before. const stack = { views: [ - { name: 'f', groups: [{ visibleOn: "record.a == 1", fields: [] }] }, + { name: 'f', groups: [{ visibleOn: "data.a == 1", fields: [{ field: 'x', visibleWhen: 'approved' }] }] }, ], }; - expect(validateVisibilityPredicates(stack).map((f) => f.rule)).toEqual([VISIBILITY_ALIAS_DEPRECATED]); + const findings = validateVisibilityPredicates(stack); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER]); + expect(findings.map((f) => f.path)).toEqual(['views[0].groups[0]', 'views[0].groups[0].fields[0]']); }); it('is clean on an empty / model-less stack', () => { @@ -164,13 +221,21 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { expect(validateVisibilityPredicates(stack, { layer: 'metadata' })).toEqual([]); }); - it('still flags a deprecated alias key in the metadata layer (alias check is layer-agnostic)', () => { + it('reads the value through a deprecated alias key on the metadata layer too (#6318)', () => { + // Was: "still flags a deprecated alias key in the metadata layer (alias + // check is layer-agnostic)". The alias-KEY rule is retired, so the + // layer-agnostic claim about it is gone. What survives — and is the half + // that ever mattered — is that the alias-VALUE read is layer-agnostic: + // the same key feeds the layer-directional root verdict in BOTH + // directions. Pinned bidirectionally on one fixture, exact sets both ways. const stack = { - views: [{ name: 'f', sections: [{ visibleOn: "data.a == 1", fields: [] }] }], + views: [{ name: 'f', sections: [{ visibleOn: "record.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]); + // metadata layer forbids the runtime `record.` root → reported through the alias key… + expect(validateVisibilityPredicates(stack, { layer: 'metadata' }).map((f) => f.rule)) + .toEqual([VISIBILITY_ROOT_MISLAYERED]); + // …and the same alias-spelled predicate is correct on the runtime layer. + expect(validateVisibilityPredicates(stack)).toEqual([]); }); it('does not confuse an identifier ending in `record` (e.g. `my_record.x`) for a record root', () => { @@ -236,10 +301,13 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { expect(bareFindings(stack).map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']); }); - it('reads the value through the deprecated `visibleOn` alias too (alias + bare, both reported)', () => { + it('reads the value through the deprecated `visibleOn` alias too (bare ALONE since #6318)', () => { + // Was an "alias + bare, both reported" pair. The alias half is retired; + // the half this test existed for — the gate reaches a bare identifier + // written under the deprecated key — is unchanged and is now the whole + // expected set. const stack = { views: [{ name: 'f', sections: [{ visibleOn: "status == 'x'", fields: [] }] }] }; - const rules = validateVisibilityPredicates(stack).map((f) => f.rule).sort(); - expect(rules).toEqual([VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_BARE_IDENTIFIER].sort()); + expect(validateVisibilityPredicates(stack).map((f) => f.rule)).toEqual([VISIBILITY_BARE_IDENTIFIER]); }); it('reads the value through the deprecated page-side `visibility` alias too', () => { @@ -689,12 +757,14 @@ describe('visibility-predicate-syntax (#6253)', () => { expect(findings[0].where).toBe('page "p"'); }); - it('reads the value through the deprecated `visibleOn` alias (alias + syntax, both reported)', () => { - // Two independent defects on one element, so unlike the syntax/bare-ref - // pair these DO both report. + it('reads the value through the deprecated `visibleOn` alias (syntax ALONE since #6318)', () => { + // Was an "alias + syntax, both reported" pair, whose point was that two + // INDEPENDENT defects on one element both report. The alias half is + // retired, so what is left is the syntax verdict reached through the + // deprecated key — and the independence claim it was making now belongs + // to the syntax/bare-ref exclusivity pin, which states it directly. const stack = { views: [{ name: 'f', sections: [{ visibleOn: 'status === "x"', fields: [] }] }] }; - expect(validateVisibilityPredicates(stack).map((f) => f.rule).sort()) - .toEqual([VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_PREDICATE_SYNTAX].sort()); + expect(validateVisibilityPredicates(stack).map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]); }); it('reads the value through the deprecated page-side `visibility` alias', () => { diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index 6bc6c48246..97f6aec92d 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -7,33 +7,55 @@ * canonical key **`visibleWhen`** across data fields, view form sections/fields, * and page components. The deprecated spellings — `visibleOn` (view form) and * `visibility` (page component) — stay accepted and are folded into `visibleWhen` - * at the schema boundary (a zod `.transform()`). - * - * **The fold does NOT happen during `parse()` (measured, #6073).** This header - * used to say it did, and that running on the pre-parse (normalized) stack was - * therefore enough to see what the author wrote. It is not: the ADR-0087 D2 - * conversions `view-visibleOn-to-visibleWhen` and - * `page-component-visibility-to-visibleWhen` rename the key INSIDE - * `normalizeStackInput` — whose output *is* the normalized tier. On all three - * spec-valid alias sites (`views[].form.*`, `views[].formViews.*`, - * `pages[].regions[].components[]`) `visibility-alias-deprecated` therefore - * reports ZERO through every CLI door, `os lint` on a raw config included. The - * only shape on which it still fires is `views[].sections[]` — the shape the - * unit tests below use, and the one strict `ViewSchema` refuses. The author is - * not left silent (the D2 conversion emits its own, better-worded notice naming - * the protocol-16 retirement), so nothing is broken for a user today; #6318 - * carries the per-site table and the retire-or-rewire question. - * - * The other two rules in this file judge the predicate's **value**, which the - * fold carries into `visibleWhen` intact — both still report normally on the - * normalized tier, and neither is affected by the above. - * - * One advisory rule pair (both `warning` — nothing is broken, the alias still - * works and a mis-rooted predicate just never matches) plus TWO **gating** rules - * (`error` — the predicate can never evaluate at all): - * - * - `visibility-alias-deprecated` — a `visibleOn` / `visibility` key in authored - * source. Autofix intent: rename the key to `visibleWhen` (same value). + * before this file ever sees them. + * + * ## Why there is no alias-KEY rule here any more (#6318, retired) + * + * This file used to carry a fourth rule, `visibility-alias-deprecated`, whose + * whole job was to report the alias KEY. It was **unreachable on every real + * input shape** and is gone. The measurement, re-run at retirement time: + * + * | alias site | rule fed the RAW authored object | rule fed the `normalized` tier | + * |---|---|---| + * | `views[].form.sections[]` | 1 finding | **0** | + * | `views[].formViews.edit.sections[]` | 1 finding | **0** | + * | `pages[].regions[].components[]` | 1 finding | **0** | + * + * The `normalized` tier is what all three commands hand this rule family + * (`authoring-rules.ts`, `input: 'normalized'`), and the ADR-0087 D2 conversions + * `view-visibleOn-to-visibleWhen` / `page-component-visibility-to-visibleWhen` + * rename the key INSIDE `normalizeStackInput` — whose output *is* that tier. So + * the key was always already gone. The one shape the rule did still fire on, + * `views[].sections[]` (a view CONTAINER with top-level sections), is the shape + * its own unit tests used and the one strict `ViewSchema` refuses outright: + * "Unrecognized key(s) on this view container: `sections`". A green unit test + * over a fixture production can never send — the #4984 / #6251 signature. + * + * **The surface is still guarded, and better.** The same D2 conversion shouts + * through `warnConversionNotice` in `defineStack`, with wording this rule never + * had — it names the site, the conversion, and the retirement window: + * + * ``` + * defineStack: views[0].form.sections[0].visibleWhen: 'visibleOn' -> 'visibleWhen' + * (converted at load; conversion 'view-visibleOn-to-visibleWhen', retires in protocol 16). + * Update the source to the canonical shape — the conversion stops running then. + * ``` + * + * So no working app lost a signal when the rule went: authors were never hearing + * this rule, and they do hear the conversion notice. Retired under ADR-0049 + * (declared ≠ enforced) rather than re-anchored, because re-anchoring would have + * had to change `runAuthoringRules`' external input contract — a `packages/lint` + * public-API question that is the maintainer's to settle, not this file's. + * `authoring-rule-input-tier.test.ts` (premise 3) holds the regression pins. + * + * The three rules that remain judge the predicate's **value**, which the fold + * carries into `visibleWhen` intact — all three report normally on the + * normalized tier, and none was affected by the retirement. + * + * One advisory rule (`warning` — nothing is broken, a mis-rooted predicate just + * never matches) plus TWO **gating** rules (`error` — the predicate can never + * evaluate at all): + * * - `visibility-predicate-syntax` (**error**, #6253) — a predicate the canonical * CEL front end refuses outright (`country === "USA"` — `===` is not CEL). See * the §Syntax block below for why this surface has to say it and who owns the @@ -205,7 +227,6 @@ import type { CelAstNode } from '@objectstack/formula'; import { walkPageComponents } from './page-walk.js'; -export const VISIBILITY_ALIAS_DEPRECATED = 'visibility-alias-deprecated'; export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered'; export const VISIBILITY_BARE_IDENTIFIER = 'visibility-bare-identifier'; export const VISIBILITY_PREDICATE_SYNTAX = 'visibility-predicate-syntax'; @@ -227,12 +248,12 @@ export interface VisibilityOptions { export interface VisibilityFinding { /** - * `warning` for the two ADR-0089 D3b advisories; `error` for the two rules - * that gate — `visibility-predicate-syntax` and `visibility-bare-identifier` - * (see module note). + * `warning` for the ADR-0089 D3b advisory (`visibility-root-mislayered`); + * `error` for the two rules that gate — `visibility-predicate-syntax` and + * `visibility-bare-identifier` (see module note). */ severity: VisibilitySeverity; - /** Diagnostic rule id, e.g. `visibility-alias-deprecated`. */ + /** Diagnostic rule id, e.g. `visibility-root-mislayered`. */ rule: string; /** Human-readable location, e.g. `view "contact_form"`. */ where: string; @@ -246,9 +267,13 @@ export interface VisibilityFinding { type AnyRec = Record; -/** The canonical key and its two deprecated aliases (ADR-0089). */ +/** + * The canonical predicate key (ADR-0089). The two deprecated spellings + * (`visibleOn` / `visibility`) are read inline in `checkElement`, canonical-first + * — the `ALIASES` list that used to sit here existed only to iterate the retired + * alias-KEY rule, and went with it (#6318). + */ const CANONICAL = 'visibleWhen'; -const ALIASES = ['visibleOn', 'visibility'] as const; /** * Every record in a collection authored either as an array or as a name-keyed @@ -516,9 +541,13 @@ const MISLAYER_BY_LAYER: Record< }; /** - * 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's binding root does not match `layer`). + * Inspect one element carrying a visibility predicate. Emits the mis-layered-root + * finding (when the effective predicate's binding root does not match `layer`), + * the syntax finding, and the bare-identifier finding. + * + * There is no alias-KEY step here: `visibility-alias-deprecated` was retired + * under #6318 (see the module note for the per-site measurement and for the D2 + * conversion notice that guards the surface instead). */ function checkElement( el: AnyRec, @@ -527,25 +556,19 @@ function checkElement( layer: VisibilityLayer, findings: VisibilityFinding[], ): void { - // (1) deprecated alias key present → steer to `visibleWhen`. - for (const alias of ALIASES) { - if (el[alias] !== undefined) { - findings.push({ - severity: 'warning', - rule: VISIBILITY_ALIAS_DEPRECATED, - where, - path: `${path}.${alias}`, - message: - `\`${alias}\` is the deprecated spelling of the conditional-visibility ` + - `predicate (ADR-0089). It still works — it is normalized to \`visibleWhen\` ` + - `at parse — but the canonical key is \`visibleWhen\`.`, - hint: `Rename the key \`${alias}\` → \`visibleWhen\` (same CEL value).`, - }); - } - } - - // (2) mis-layered binding root — check the effective predicate (canonical wins) + // (1) mis-layered binding root — check the effective predicate (canonical wins) // against the root expected for this layer. + // + // The two alias limbs STAY, and are not the thing #6318 retired. That issue is + // about the alias KEY as a verdict; this is the predicate VALUE, and the value + // is what all three surviving rules judge. Through the three CLI commands the + // limbs are already unreachable (the D2 fold renamed the key one layer up), but + // `validateVisibilityPredicates` is a PUBLISHED export that a caller — this + // package's own unit tests included — may hand a raw authored object, and on + // that door an alias-spelled predicate must still be judged rather than skipped + // silently. Ordering is canonical-first per ADR-0089, so an alias can never + // override a `visibleWhen` that is present: the limbs can only ever add + // coverage, never change a verdict. const raw = el[CANONICAL] ?? el.visibleOn ?? el.visibility; const source = predicateSource(raw); const rule = MISLAYER_BY_LAYER[layer]; @@ -560,7 +583,7 @@ function checkElement( }); } - // (3) #6253 — the canonical CEL front end refuses the source outright. GATES, + // (2) #6253 — the canonical CEL front end refuses the source outright. GATES, // at the severity every other predicate surface already applies to a syntax // fault (ADR-0032 via `validateExpression`); this surface is the one that had // no such gate, so a `===` shipped clean and then failed OPEN in the console. @@ -586,13 +609,13 @@ function checkElement( }); } - // (4) #6128 — a reference no binding root can resolve. Unlike (2) this one + // (3) #6128 — a reference no binding root can resolve. Unlike (1) this one // GATES: a mis-rooted predicate is at least a statement about a namespace // someone binds somewhere, while a bare identifier resolves nowhere, on no // layer, under neither a total nor a sparse record (#4953) — so there is no // reading of the metadata under which it was going to work. // - // Skipped when (3) fired: the declaredness check needs an AST, and a source + // Skipped when (2) fired: the declaredness check needs an AST, and a source // that does not parse has none. Written as an explicit `else` rather than // relying on `firstBareIdentifier`'s own null-AST guard, so the one-finding // -per-broken-predicate property is visible at the call site instead of @@ -680,12 +703,17 @@ function formViewSites( } /** - * Validate conditional-visibility keys across authored views and pages. + * Validate conditional-visibility predicates across authored views and pages. + * + * Every rule here judges the predicate's **VALUE**, which survives both the + * ADR-0087 D2 alias fold and the Zod parse intact — so the registered + * `normalized` tier sees exactly what a `parsed` one would. (It is registered + * `normalized` for the tier's surviving reason, not the retired one: a + * normalized-tier finding still reaches the author when an unrelated schema + * error elsewhere would have stopped the parse. See `AuthoringRuleInputTier`.) * - * Runs on the **pre-parse** (normalized) stack so it can see the deprecated - * `visibleOn` / `visibility` aliases before the schema folds them into - * `visibleWhen`. Returns findings (empty = clean). The two ADR-0089 D3b rules - * are advisory (`warning`); `visibility-predicate-syntax` (#6253) and + * Returns findings (empty = clean). `visibility-root-mislayered` is advisory + * (`warning`); `visibility-predicate-syntax` (#6253) and * `visibility-bare-identifier` (#6128) are `error` and the caller is expected to * fail the build on them. * @@ -693,7 +721,7 @@ function formViewSites( * `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. + * syntax and bare-identifier checks are layer-agnostic. */ export function validateVisibilityPredicates( stack: AnyRec,