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
67 changes: 67 additions & 0 deletions .changeset/visibility-alias-deprecated-retired.md
Original file line numberDiff line numberDiff line change
@@ -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.

<!-- adr-0087: not-required (already-registered view-visibleOn-to-visibleWhen, page-component-visibility-to-visibleWhen) The authored alias surface is already covered by those two D2 conversions, which are unchanged by this PR — they keep accepting `visibleOn` / `visibility`, keep folding them to `visibleWhen`, and keep their protocol-16 retirement window. This change removes only a lint rule id from a TS export surface; no authored or stored metadata shape changes, so there is nothing new for the ledger to carry. -->
81 changes: 70 additions & 11 deletions packages/lint/src/authoring-rule-input-tier.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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[]', {
Expand All@@ -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);
Expand All@@ -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: [{
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/authoring-rules.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand Down
1 change: 0 additions & 1 deletion packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
Loading
Loading