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
57 changes: 57 additions & 0 deletions .changeset/predicate-bare-word-rhs-reconciled.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
---
"@objectstack/lint": patch
---

fix(lint): one prescription for a bare unquoted word on the right of `==` (#7696)

For a single token — a bare word on the right of `==` / `!=` in a
metadata-editing form's visibility predicate, e.g. `data.type == active` —
three publish-time rules fired and prescribed **opposite** fixes:

| rule | severity | read it as | prescribed |
|---|---|---|---|
| `visibility-bare-identifier` | `error` | a dropped binding root | `<root>.active` |
| `predicate-path-unrooted` | `error` (when the word is also a schema key) | a dropped binding root | `data.active` |
| `predicate-rhs-path-shaped` | `warning` | a literal missing its quotes | `'active'` |

The `error`s were the ones that blocked the write, and they asked for a spelling
**this same gate refuses**: `data.type == data.active` is a path on the RIGHT,
which is `predicate-rhs-path-shaped`'s `error` arm. An author who obeyed the
loud finding landed on a louder one, and "fixing" that by making the path
resolve reached `predicate-path-unresolved` — a three-corner walk over metadata
that renders correctly today (objectui#4049).

**The bare-word right-hand position now produces exactly one finding.** The two
root-prescribing rules stand down for identifiers that occur *only* as a bare
right operand of `==` / `!=` on a schema-bound metadata form, and
`predicate-rhs-path-shaped`'s advisory carries both readings: it names the
quoted spelling (`== 'active'`) and the field spelling (move the path to the
LEFT, `data.active == 'yes'`), and says in as many words that adding the root in
place is not a third option. ⛔ Which reading the author meant is still not
decided — that is the thing no linter can know, and inventing an answer is what
the contradicting messages were doing.

**What this changes for you.** A `view` write whose only defect is a bare word
on the right of `==` on a metadata-editing form is no longer refused; it comes
back as an advisory on the 2xx response, at the severity #7659 already argued
for a spelling that renders correctly. Nothing else moves:

- a bare word on the LEFT (`status == active`) is still an `error`;
- a word that also occurs outside a right-hand slot is still an `error`;
- a dotted chain on the right (`data.a == data.b`) is still an `error`;
- a runtime `*.view.ts` / `*.page.ts` predicate is untouched — that surface goes
to real CEL, where a path on the right is legal and a bare word there really
is a dropped root, so `record.status == active` keeps its refusal and its
`record.active` hint;
- the #7659 severity split (`error` on a dotted chain, `warning` on a bare word)
is unchanged.

**One coverage increase, in the safe direction.** A form whose `schemaId`
resolves to no schema this package can see used to be skipped wholesale, taking
the right-hand check with it even though that check needs no schema oracle. Such
sites are now walked with no scope: the two path-resolution rules stay silent
(no oracle, no verdict) and the right-hand position is judged. Without it, the
stand-down above would have been a silence on that shape rather than a
reconciliation. Measured at 0 new findings over the shipped
`METADATA_FORM_REGISTRY` corpus and over `examples/app-showcase` /
`examples/app-crm`.
170 changes: 170 additions & 0 deletions packages/lint/src/predicate-rhs-position.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* **The right-hand-of-`==` POSITION, shared by every rule that judges it**
* (#7696).
*
* On a metadata-editing form the console's evaluator
* (`packages/app-shell/src/views/metadata-admin/predicate.ts`, objectui#4049)
* resolves the LEFT side of `==` / `!=` through `resolveValue` and hands the
* RIGHT side to `parseLiteral`. So the right-hand slot is a LITERAL slot: a
* token sitting there is never resolved as a reference, whatever it is spelled
* like. That single fact is read by three rules across two files, and #7696 was
* filed because they read it differently and prescribed opposite fixes for one
* token:
*
* - `visibility-bare-identifier` (`validate-visibility-predicates.ts`, #6128)
* read a bare `active` as a dropped binding root and said, at `error`, write
* `<root>.active`.
* - `predicate-path-unrooted` (`validate-predicate-path-refs.ts`, #7010) said
* the same thing at `error` whenever the word also happens to be a schema
* key.
* - `predicate-rhs-path-shaped` (`validate-predicate-path-refs.ts`, #7659) read
* it as a literal missing its quotes and said, at `warning`, write
* `'active'`.
*
* The first two prescribe a spelling this gate itself REFUSES: `data.type ==
* data.active` is a dotted chain on the right, which is `predicate-rhs-path-
* shaped`'s `error` arm. An author who complies with the loud finding lands on
* a louder one. That is the walk #7696 is about, and it is why the rooted
* prescription is not "a different opinion" here — on this surface it is not a
* spelling at all.
*
* {@link bareRhsOnlyIdentifiers} is the mechanism that stands the two
* root-prescribing rules down for exactly this position, leaving
* `predicate-rhs-path-shaped` as the single voice. It answers a narrow
* question: which identifiers occur ONLY as the bare right operand of a
* `==` / `!=`?
*
* ## Why "only", and why the set is deliberately small
*
* `status == active` must still be REFUSED — `status` is a genuine dropped root
* on the left, and the fact that `active` is a literal says nothing about it.
* So the suppression is per-identifier and per-occurrence: a name that appears
* anywhere other than a bare right operand (`active == data.x`,
* `data.a == active && active`) keeps its finding. The two callers add this set
* to the names they treat as resolvable, which is the same conservative
* direction `namespaceRoots` already takes in the sibling file — every name it
* adds can only remove a finding, never create one.
*
* ## Where it deliberately stands nothing down
*
* Suppression is only safe where the replacement finding actually fires, so the
* walk refuses to suppress anywhere `predicate-rhs-path-shaped` does not reach:
*
* - **Inside a comprehension-macro body.** `equalitySites` in
* `validate-predicate-path-refs.ts` skips macro bodies (the interim
* evaluator supports no macros at all, so a comparison in there is not a
* statement about this subset), so an `==` in there produces no replacement.
* This walk descends into the body with suppression turned OFF, which files
* every identifier it finds there as "occurs elsewhere".
* - **A dotted chain on the right.** `data.a == data.b` is already the `error`
* arm and needs no reconciliation; only a BARE `id` node is collected.
* - **A surface the metadata-admin evaluator does not render.** That is the
* caller's condition, not this walk's — {@link schemaIdOf} is the test, and
* a runtime `*.view.ts` predicate (real CEL, where a path on the right is
* perfectly legal) never reaches it.
*/

type AnyRec = Record<string, unknown>;
type AstNode = { op?: string; args?: unknown };

function isNode(v: unknown): v is AstNode {
return !!v && typeof v === 'object' && typeof (v as AstNode).op === 'string';
}

/** The comparison operators whose right side the metadata-admin evaluator parses as a literal. */
export const EQUALITY_OPS = new Set(['==', '!=']);

/**
* CEL comprehension macros: the receiver-call forms that BIND their first
* argument as a loop variable.
*/
export const COMPREHENSION_MACROS = new Set(['all', 'exists', 'exists_one', 'map', 'filter']);

/** The bare identifier name a node spells, or `null` when it is anything else. */
function bareId(node: unknown): string | null {
if (!isNode(node)) return null;
return node.op === 'id' && typeof node.args === 'string' ? node.args : null;
}

/**
* Identifiers that occur ONLY as the bare right operand of a `==` / `!=`.
*
* `suppressible` is threaded rather than checked at the top because the answer
* changes with DEPTH: inside a comprehension-macro body no equality produces a
* `predicate-rhs-path-shaped` finding, so an identifier found there must count
* as an ordinary occurrence even though it sits in a right-hand slot.
*/
export function bareRhsOnlyIdentifiers(ast: unknown): Set<string> {
const rhs = new Set<string>();
const elsewhere = new Set<string>();

const walk = (node: unknown, suppressible: boolean): void => {
if (Array.isArray(node)) {
for (const child of node) walk(child, suppressible);
return;
}
if (!isNode(node)) return;
const args = node.args;

// A comprehension binds a loop variable and its body is out of the
// replacement rule's reach — descend with suppression off. The RECEIVER is
// an ordinary sub-expression and keeps the caller's mode.
if (
node.op === 'rcall' && Array.isArray(args) && typeof args[0] === 'string'
&& COMPREHENSION_MACROS.has(args[0])
) {
walk(args[1], suppressible);
walk(args[2], false);
return;
}

if (
typeof node.op === 'string' && EQUALITY_OPS.has(node.op)
&& Array.isArray(args) && args.length === 2
) {
const right = suppressible ? bareId(args[1]) : null;
walk(args[0], suppressible);
if (right !== null) {
rhs.add(right);
return; // the right operand IS the suppressible occurrence — not "elsewhere"
}
walk(args[1], suppressible);
return;
}

const name = bareId(node);
if (name !== null) {
elsewhere.add(name);
return;
}
walk(args, suppressible);
};

walk(ast, true);
for (const name of elsewhere) rhs.delete(name);
return rhs;
}

function isRec(v: unknown): v is AnyRec {
return !!v && typeof v === 'object' && !Array.isArray(v);
}

/**
* The `schemaId` a form view resolves its row shape from, or `undefined` when
* the view is not schema-bound. Read off `ViewDataSourceSchema`'s `schema`
* member (`view.zod.ts:151-161`) — the shape `defineForm` writes.
*
* Shared rather than duplicated: it is the test for "the metadata-admin
* evaluator renders this", which is the precondition BOTH the rule that
* reports the right-hand position and the rules that stand down for it must
* agree on. Two copies of it is exactly how one side starts suppressing on a
* surface the other side has gone quiet on.
*/
export function schemaIdOf(view: AnyRec): string | undefined {
const data = view.data;
if (!isRec(data)) return undefined;
if (data.provider !== 'schema') return undefined;
return typeof data.schemaId === 'string' ? data.schemaId : undefined;
}
56 changes: 50 additions & 6 deletions packages/lint/src/runtime-gate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -341,17 +341,60 @@ describe('the views[] visibility-predicate family at the runtime publish gate (#
// literal string "active" today — very likely what the author meant — so
// this rule does not refuse the write over metadata that renders correctly.
//
// The write IS refused, by `visibility-bare-identifier` from the sibling
// file, which reads `active` as a dropped binding root. Both findings are
// true about the token and they prescribe DIFFERENT fixes (`data.active` vs
// `'active'`), so this pins the pair rather than asserting a clean `errors`
// list that would go stale the moment either side moved.
// #7696: it is now the ONLY thing the author hears about that token. This
// pin used to record the opposite — `visibility-bare-identifier` refusing
// the same write and prescribing `<root>.active` — and the pair it pinned
// was a contradiction, not a division of labour: the rooted spelling it
// asked for is a path on the RIGHT, which this same gate refuses at `error`
// (the test below measures it). So the root-prescribing rules stand down
// for this position and the advisory carries both readings.
const result = gateView(schemaBoundForm('data.type == active'));
const f = result.advisories.find((a) => a.rule === 'predicate-rhs-path-shaped');
expect(f, 'the subset boundary must still reach the author').toBeDefined();
expect(f!.severity).toBe('warning');
expect(result.errors.map((e) => e.rule)).not.toContain('predicate-rhs-path-shaped');
expect(result.errors.map((e) => e.rule)).toContain('visibility-bare-identifier');
expect(
result.errors.map((e) => e.rule),
'one token, one prescription — the write is no longer blocked by a rule asking for a '
+ 'spelling this gate refuses (#7696)',
).toEqual([]);
// The merged message must state BOTH readings and must not prescribe the
// in-place rooted spelling, which is the thing the next test refuses.
expect(f!.hint).toMatch(/'active'/);
expect(f!.hint).toMatch(/data\.active == /);
expect(f!.hint).toMatch(/Do NOT simply add the root in place/);
});

it('does not prescribe a fix it would itself refuse (#7696)', () => {
// The accept bar the old pair failed. `visibility-bare-identifier` told the
// author to root the word in place; both rootings of that exact predicate
// are a dotted chain on the RIGHT, and the gate refuses them at `error`.
for (const root of ['data', 'record']) {
const { errors } = gateView(schemaBoundForm(`data.type == ${root}.active`));
expect(
errors.map((e) => e.rule),
`\`== ${root}.active\` is the spelling the old error asked for`,
).toContain('predicate-rhs-path-shaped');
}
});

it('still REFUSES the same bare word on the LEFT, and on a runtime surface', () => {
// The scanner proves it can see before any silence is believed. The
// stand-down is per-IDENTIFIER and per-SURFACE, so three neighbours of the
// reconciled case keep their refusal unchanged.
expect(
gateView(schemaBoundForm('status == active')).errors.map((e) => e.rule),
'a bare word on the LEFT is a genuine dropped root, whatever sits on the right',
).toContain('visibility-bare-identifier');
expect(
gateView(schemaBoundForm('data.name == active && active')).errors.map((e) => e.rule),
'the word also occurs outside a right-hand slot — not a literal position',
).toContain('visibility-bare-identifier');
expect(
gateView(runtimeView('record.status == active')).errors.map((e) => e.rule),
'a runtime surface goes to real CEL, where a path on the right is legal and a bare word '
+ 'there really is a dropped root — nothing about it was reconciled',
).toContain('visibility-bare-identifier');
});

it('reports a MISLAYERED root through the advisory channel, not a refusal', () => {
Expand DownExpand Up@@ -418,6 +461,7 @@ describe('the views[] visibility-predicate family at the runtime publish gate (#
schemaBoundForm("data.tpye == 'text'"), // unresolvable path
schemaBoundForm("type == 'text'"), // unrooted schema key
schemaBoundForm("data.type == 'text'"), // resolvable path
schemaBoundForm('data.type == active'), // bare word on the RIGHT (#7696)
];

const fingerprints = (fs: readonly AuthoringFinding[]) =>
Expand Down
74 changes: 74 additions & 0 deletions packages/lint/src/validate-predicate-path-refs.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -377,6 +377,80 @@ describe('validatePredicatePathRefs — path-shaped right-hand side (#7659)', ()
});
});

describe('validatePredicatePathRefs — one position, one prescription (#7696)', () => {
const rhs = (source: string) =>
run(form([{ label: 'S', fields: [{ field: 'name', visibleWhen: source }] }]));

it('does not ALSO call a right-hand bare word a dropped root', () => {
// `type` is a declared key of DemoSchema, so before #7696 this drew
// `predicate-path-unrooted` at `error` ("write `data.type`") on top of the
// right-hand `warning` ("write `'type'`") — two findings, opposite fixes,
// and the blocking one asking for a spelling this same rule refuses.
const findings = rhs('data.name == type');
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
expect(findings[0].severity).toBe('warning');
});

it('proves that silence is the stand-down, not a walk that stopped seeing keys', () => {
// Same schema key, LEFT of the comparison: the unrooted limb must still
// fire, or the assertion above is measuring a dead limb.
expect(rhs("type == 'formula'").map((f) => f.rule)).toEqual([PREDICATE_PATH_UNROOTED]);
// And in a right-hand slot it stays reported whenever it ALSO occurs
// somewhere that is not one.
expect(rhs('data.name == type && type').map((f) => f.rule).sort())
.toEqual([PREDICATE_PATH_UNROOTED, PREDICATE_RHS_PATH_SHAPED]);
});

it('stands down for `!=` as well, and not for a dotted chain', () => {
expect(rhs('data.name != type').map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
// A dotted chain is the `error` arm and was never part of the contradiction.
expect(rhs('data.name == data.type')[0].severity).toBe('error');
});

it('never suppresses inside a comprehension-macro body, where nothing replaces it', () => {
// `equalitySites` skips macro bodies, so an `==` in there produces no
// right-hand finding. Suppressing the unrooted verdict there would be a
// silence rather than a reconciliation.
expect(rhs('data.tags.all(t, t == type)').map((f) => f.rule)).toEqual([PREDICATE_PATH_UNROOTED]);
});

it('names both readings and refuses to prescribe the in-place rooting', () => {
const f = rhs('data.name == type')[0];
expect(f.hint).toMatch(/quote it: `== 'type'`/);
expect(f.hint).toMatch(/data\.type == 'yes'/);
expect(f.hint).toMatch(/Do NOT simply add the root in place/);
// The bar: BOTH spellings the message recommends must be clean…
expect(rhs("data.type == 'yes'")).toEqual([]);
expect(rhs("data.name == 'type'")).toEqual([]);
// …and the spelling it warns AGAINST must be reported, or the warning is
// noise. This is the exact edit the old `error` used to demand.
expect(rhs('data.name == data.type').map((x) => x.severity)).toEqual(['error']);
});

it('the LEFT-side spelling it recommends is judged on its own merits, not circular', () => {
// If the word names no field at all, moving it left is answered by #7214 —
// "`active` is not a key" — which is a NEW statement about a different
// mistake, not the old ring-around. The right-hand rule has nothing further
// to say once the token is out of the literal slot.
expect(rhs("data.active == 'yes'").map((x) => x.rule)).toEqual([PREDICATE_PATH_UNRESOLVED]);
});

it('judges the right-hand position on a form whose schemaId resolves to nothing', () => {
// The two resolution limbs need an oracle and correctly go quiet; the
// right-hand limb never did. Walking past the whole site made the
// stand-down above a silence on this shape, so the site is now walked with
// no scope instead of skipped.
const unknown = (source: string) =>
run(form([{ label: 'S', fields: [{ field: 'name', visibleWhen: source }] }], 'no_such_type'));
expect(unknown('data.a == data.b').map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
expect(unknown('data.a == active').map((f) => f.severity)).toEqual(['warning']);
// The resolution limbs stay silent there — no oracle, no verdict.
expect(unknown("data.tpye == 'x'")).toEqual([]);
// Control: the same predicate against a schema that DOES resolve reports.
expect(rhs("data.tpye == 'x'").map((f) => f.rule)).toEqual([PREDICATE_PATH_UNRESOLVED]);
});
});

describe('registry wiring', () => {
it('is registered in AUTHORING_RULES as a gating rule on all three commands', () => {
const entry = AUTHORING_RULES.find((r) => r.name === 'validatePredicatePathRefs');
Expand Down
Loading
Loading