Skip to content

Commit 94379f4

Browse files
fix(formula)!: the unknown-field check judges every member spelling of record / previous, not only the dot (#22428) (#22494)
Fixes #22428 Clause-②: yes (narrowing) ## What changed `validateExpression`'s field-existence pass (`checkFieldExistence` in `packages/formula/src/validate.ts`) found the members an expression reads on `record` / `previous` with a dot-only regex. It now reads them through the AST member reader that the relationship-traversal analysis uses. Every spelling that names a member gets the dot spelling's verdict, at every slot the pass judges: - `record.FIELD`, `record['FIELD']`, `record["FIELD"]`, `record.?FIELD`, `record[?'FIELD']`, the same on `previous`, and any of them inside `has(...)`; - the attached-on-read leaf check from #22425 (`record.BLOCK.LEAF` against `ObjectSchema.attachedOnRead`) rides the same reads, so `record.viewer['can_actt']`, `record.viewer.?can_actt` and `record['viewer'].can_actt` are judged like `record.viewer.can_actt`. There is no lint arm. `@objectstack/lint`'s slot walk is unchanged, so `os build` and the object save door give the shared validator's verdict, as they already did for the dot spelling. ### One reader, one parse `relationship-traversal.ts` now has one lower-level reader, `readRootMembers(ast, roots)`. It walks a parsed AST once and reports each read of a member of the named roots in source order, with the member, the next segment when there is one, and whether the read goes deeper. Both questions fold over it: - `analyzeRelationshipTraversals(source, root)` is `parseCelToAst` plus `traversalsOf(readRootMembers(ast, [root]))`. Its signature and answers are unchanged (equivalence below). - `checkFieldExistence` reads `readRootMembers(ast, ['record', 'previous'])`. - `validateExpression` parses once, lazily, and hands the same AST to the field-existence pass and to the traversal-conflict arm. That arm used to parse a second time through `analyzeRelationshipTraversals`. `readRootMembers` and `traversalsOf` are module exports only. `src/index.ts` does not change, and `dist/index.d.ts` has zero mentions of either. Measured on the as-is reuse question (dispatch mechanism assumption 2): `analyzeRelationshipTraversals` could answer existence as written (first segments = `bareFields` plus the keys of `traversals`; leaves = `traversals.get(block)`), but only one root per call, so two parses per expression, and its sets drop source order. That is why the reader was extracted rather than reused as it was. ### What gets no verdict (dispatch mechanism assumption 5) - A computed key (`record[record.kind]`, `record[someVar]`) names no member before evaluation. The reader reports no read for it, so there is no finding. This is pinned (`validate-field-existence-spellings.test.ts`, "gives a computed key no verdict"; `validate-attached-on-read.test.ts`, `record.viewer[record.status]`). - A method call on a member (`record.name.startsWith('A')`) still judges the member. - Three shapes the regex misread as a member of the root are no longer refused, because none of them names one: text inside a string literal (`record.name == 'record.typo'`), a root name after another root (`vars.record.x`), and a method call on the root itself (`record.size()`). Measured: the regex reported `typo`, `x` and `size` as unknown fields, and the AST reader reports nothing for each. This is a small widening inside the narrowing, and the changeset states it. ## Premise, measured at both doors before the change (`origin/main` `8b713fad78`, which includes `9af0005d55`) Probe: an object with fields `name` and `tier`, the expression on one slot. "build" is `validateStackExpressions`. "door" is `runRuntimeAuthoringRules({ type: 'object' })` (the gate `saveMetaItem` runs). Counts are `expression-invalid` errors. | spelling (`zz_typo` undeclared) | option `visibleWhen` build / door | `requiredWhen` build / door | validation `condition` build / door | |---|---|---|---| | `record.zz_typo == 'a'` (control) | 1 / 1 | 1 / 1 | 1 / 1 | | `record['zz_typo'] == 'a'` | **0 / 0** | **0 / 0** | **0 / 0** | | `previous['zz_typo'] == 'a'` | **0 / 0** | **0 / 0** | **0 / 0** | | `record.?zz_typo.orValue('') == 'a'` | **0 / 0** | **0 / 0** | **0 / 0** | | `record[?'zz_typo'].orValue('') == 'a'` | **0 / 0** | **0 / 0** | **0 / 0** | | `has(record.zz_typo)` | 1 / 1 | 1 / 1 | 1 / 1 | After the change, every row reads `1 / 1` in all three slots. Every spelling of the declared `name` reads `0 / 0` before and after. Two premise details came out differently from the card: - `has(record.x)` was already refused, because the regex saw the dot inside `has(...)`. It is pinned so it stays refused. - A bare `record.?zz_typo == 'a'` is refused at compile (`invalid-cel`: cel-js finds no `==` overload for an optional of `dyn` against a string). The optional spelling that passed was `record.?zz_typo.orValue(...)`, and that is the one the pins use. ## Pins - `packages/formula/src/validate-field-existence-spellings.test.ts` (new, 31 tests): - 6 spellings × 2 roots refused, with `code: 'unknown-field'` and params; - the same 12 on a declared field, accepted; - did-you-mean in a bracket spelling; - one finding per name across spellings and roots, in source order; - a comprehension body; - a computed key gets no verdict; - a string literal is not a read; - no field list means no verdict. - `packages/formula/src/validate-attached-on-read.test.ts`: the pin that held "index read unjudged" is replaced. The leaf is now judged in 6 non-dot spellings, with the declared leaf as control in 4. A computed key, a method call and a third segment stay unjudged. - `packages/formula/src/relationship-traversal.test.ts`: 5 tests on `readRootMembers`: - multi-root source order; - leaf and deeper; - the three non-reads; - a method receiver; - `analyzeRelationshipTraversals` equals the fold of its reads. - `packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts` (new, 36 tests): 3 slots (option `visibleWhen`, `requiredWhen`, validation `condition`) × 6 spellings. Each is refused at the build's rule table (`runAuthoringRules('build')`) and at the object door (`runRuntimeAuthoringRules`), and each assertion checks the named subject, the location and the severity. A declared field in every spelling publishes clean at both doors. ## Reverse verification (committed fix first, `HEAD` `57efbdd5f6`) The mutation went through `scripts/ablation-replace.mjs`. One anchor (`for (const read of readRootMembers(tree, FIELD_EXISTENCE_ROOTS)) {`, 1 hit, then 0) was replaced with the old regex reader: the dot-only head regex plus the sticky dot-only second-segment regex. Blob `3fc4e3033d47` became `383b80a635d8`. `@objectstack/formula` was rebuilt, and `scripts/ablation-dist-preflight.mjs` showed the marker in `dist/index.js` and `dist/index.mjs`. The lint pins resolve formula through `dist/`. | suite | mutant (regex reader) | restored | |---|---|---| | formula: `validate-field-existence-spellings` + `validate-attached-on-read` | **13 failed** / 28 passed | 41 passed | | lint: `runtime-gate.object-field-existence-spellings` | **12 failed** / 24 passed | 36 passed | The direction was as predicted: - Red: every bracket, `previous[...]`, `.?` and `[?]` row (3 slots × 4 in lint), the non-dot leaf spellings, did-you-mean in a bracket, source order, the comprehension case and the string-literal case. - Green: the dot and `has()` rows, and every control. Restore was proven by `ablation-replace` (blob equal to `HEAD`, `git diff HEAD` empty) and by `ablation-dist-preflight --absent` (marker absent from all 6 built files, tree clean). Disclosed: the first mutant run failed its DTS build on an unused constant (`TS6133 FIELD_EXISTENCE_ROOTS`). The JS bundles were emitted and carried the marker, and the red counts were the same. The run was repeated with the constant referenced, so the mutant build exited 0. The numbers above come from that clean run. ## Equivalence of the extracted reader The `BASE` version of `relationship-traversal.ts` was run against this branch's version over every string literal that mentions `record`/`previous` member access in the repository's `packages/**` and `examples/**` TypeScript: 1,174 files and 2,264 expressions, each analysed for both roots, 4,528 analyses in total. 2,664 analyses parsed and 1,322 were non-empty. The two versions differed **0** times, comparing `traversals`, `bareFields` and `multiHopFields` with their insertion order. ## Population census (dispatch mechanism assumption 4) - **Text census**, over all tracked files (`git grep`): CEL in metadata that spells a `record`/`previous` member with a bracket, `.?` or `[?]` exists only in test fixtures. Each one names a declared field, has no field list in its hint, or goes through a path that does not run this check. That is an ObjectQL engine fixture registered straight into the registry, where the dot form of the same key also passes. No example, app template, platform object, plugin object or `skills/**` file uses such a spelling. - **Differential run** of `validateStackExpressions`: - Stacks covered: `examples/app-todo`, `examples/app-crm`, the two `examples/app-multi-package` sub-stacks, the objects, actions, flows, views and pages of `examples/app-showcase`, the 51 objects `@objectstack/platform-objects` exports, and `plugin-approvals`' `sys_approval_request`. - Base `8b713fad78`, before and after: identical. Every count is 0 except `sys_approval_request`, which had 8 at base because it did not yet declare its `viewer` block. - After merging `origin/main` (which brought #22387's `attachedOnRead: { viewer }`), at `501184a648`: the regex reader and the AST reader give identical output, 0 everywhere. The approvals object's eight action predicates read declared `viewer` leaves. - No producer needed an edit. ## Local verification All readings below are at `8d07b21aac` unless noted. That head is this branch with `origin/main` `35ef501e13` merged in. The last commit touches only the changeset file. - **Dispatch gates**, derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no paths): 62 commands. **62 run, all exit 0.** Each exit code was captured before any pipe. Reconciled with `--ran`: "62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED". Three families answered `PREREQUISITE NOT MET` on the first pass at `57efbdd5f6` (`check:docs-transcript-drift`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`). After `pnpm --filter '@objectstack/objectql...' --filter '@objectstack/lint...' build`, the final pass measured all three, exit 0. Families the tool places outside the runnable union (CI jobs, type-check lanes, the artifact-roster block) are CI's. - **Typecheck**: `pnpm --filter @objectstack/formula run typecheck` and `pnpm --filter @objectstack/lint run typecheck` exit 0. `tsc -p tsconfig.test.json --listFiles` includes all four touched or new test files. - **Tests**, at `501184a648`. Its `packages/` tree is byte-identical to `8d07b21aac`. - `pnpm --filter @objectstack/formula exec vitest run`: 46 files, 1315 passed. - `pnpm --filter @objectstack/lint exec vitest run`: 131 files, 5979 passed. - Both were also green before the merge, at `57efbdd5f6`. - **eslint**, narrowed to the 6 changed `.ts` files with `--no-inline-config --format json`: 6 files, 0 errors, 0 warnings. The population is from eslint's own config: `--print-config` resolves a config for each file, and none is ignored. Invariance for untouched files: `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot move the verdict on a file it does not touch. The full `pnpm lint` run is CI's. - **CJS**: `require('./packages/formula/dist/index.js')` loads, and `validateExpression` and `analyzeRelationshipTraversals` are functions. ## Acceptance notes - The pending changeset of #22425 (`.changeset/22386-validator-attached-on-read-leaf.md`) says index access on a read attachment stays unjudged. This PR makes that false for unreleased text. This PR's changeset says so ("an earlier entry in this release ... describes the check before this change"), following the convention `.changeset/22402-option-gate-fails-closed.md` uses. The other changeset is not edited. - A comprehension variable that shadows a root (`list.exists(record, record.x == 1)`) is read as the root. The regex did the same, and so does `analyzeRelationshipTraversals`, so no verdict moves. It is noted, not filed: no producer in the repository writes it. - `packages/objectql/src/validation/rule-validator.ts` has its own source reading of which columns a lock predicate reads. Its comment near line 1313 treats `record['x']` as reading every field, which is conservative. It was not touched or measured here. - The suite-duration ratchet that landed on `main` (`scripts/check-test-suite-ceilings.mjs`) has no `ceilings` table yet in `scripts/test-shard-timings.json`, so it is NOT MEASURED for `@objectstack/formula` / `@objectstack/lint`. The new lint file ran 36 tests in about 12 s locally with 2 workers on a shared box. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bw3y2DWhT9RPnrmDsNqEVG)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 535c019 commit 94379f4

7 files changed

Lines changed: 507 additions & 92 deletions
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
fix(formula)!: the unknown-field check judges every spelling of a `record` / `previous` member, not only the dot, so `os build` and the object save door refuse `record['typo']`, `previous['typo']`, `record.?typo` and `record[?'typo']` (#22428)
6+
7+
Clause-②: yes (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) A refusal at `os build` and at the object save door of an expression that names an undeclared field in a non-dot member spelling: no authorable key, spelling, export or stored shape moves, and no stored row is read, rewritten or converted. A stored object whose expression is refused keeps loading until it is next saved, and the repair is the author's own (declare the field or fix the typo), which no ledger entry can derive. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this verdict and this diff adds none (not registered / already-registered); and no exported TypeScript declaration changes (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING**: an accept-set change, in both directions, of the `os build`, `os validate` and `os lint` verdicts and of the object save door, shipped as `minor` under the launch-window convention for accept-set narrowings. It narrows: an expression that names an undeclared field of its object in a non-dot member spelling (`record['FIELD']`, `record["FIELD"]`, `record.?FIELD`, `record[?'FIELD']`, or any of those on `previous`), or an undeclared leaf of a declared read attachment in such a spelling, is now refused where it was accepted. It widens: three shapes the dot-only regex misread as a member of the root, and refused as unknown fields, are now accepted, because none of them names a member. They are text inside a string literal (`record.name == 'record.typo'`), a root name after another root (`vars.record.x`), and a method call on the root itself (`record.size()`). No refusal code is added.
12+
13+
`validateExpression`'s field-existence pass judges each member an expression reads on `record` or `previous` against the object's fields, and refuses an undeclared one with the `unknown-field` code. It found those members with a regex over the dot spelling, so it judged `record.typo` and `has(record.typo)` and nothing else. `record['typo']`, `record["typo"]`, `previous['typo']`, `record.?typo` and `record[?'typo']` name the same column, and they reached the evaluator with no verdict at every slot the pass judges, among them a select option's `visibleWhen`, a field's `requiredWhen` and a validation rule's condition. There the expression faults on a key the record never carries, or reads a value that never exists.
14+
15+
The pass now reads members through the same AST member reader the relationship-traversal analysis is built on, so every spelling gets the dot spelling's verdict, did-you-mean included:
16+
17+
- `os build`, `os validate` and `os lint` refuse an expression that names an undeclared field of its object as `record['FIELD']`, `record["FIELD"]`, `record.?FIELD`, `record[?'FIELD']` or any of those on `previous`, at `error`, with the message they already give for `record.FIELD`.
18+
- The object save door runs the same pass, so an object write in publish mode that carries such an expression is now refused with an `expression-invalid` issue located at the slot, the build's own finding.
19+
- A declared read attachment's leaf (`ObjectSchema.attachedOnRead`) is judged in the same spellings: `record.viewer['can_actt']` and `record.viewer.?can_actt` are refused like `record.viewer.can_actt`. An earlier entry in this release says index access on a read attachment stays unjudged; that describes the check before this change.
20+
21+
**Remedy.** Declare the field on the object, or fix the typo to the field the message suggests. The spelling itself is not the defect: `record['status']`, `record.?status` and `record[?'status']` on a declared `status` are accepted, as before.
22+
23+
**Unchanged.**
24+
25+
- A computed key (`record[record.kind]`, `record[someVar]`) names no member before evaluation, so it is not judged.
26+
- A method call on a member (`record.name.startsWith('A')`) still judges the member (`name`).
27+
- Stored rows are not migrated or refused on read; an object stored before this change keeps loading until it is next saved, and that save is judged. Drafts are not gated.
28+
- Measured before crossing: the stacks this repository ships give the same expression findings before and after the change. That covers `examples/app-todo`, `examples/app-crm`, both `examples/app-multi-package` sub-stacks, the objects, actions, flows, views and pages of `examples/app-showcase`, the 51 objects `@objectstack/platform-objects` exports, and `plugin-approvals`' `sys_approval_request`, whose eight action predicates read the leaves of its declared `viewer` read attachment and pass. No in-repo producer writes a non-dot spelling of an undeclared field.
29+
- No public export or signature moves; only the doc comments of `ExprSchemaHint.fields` and `ExprSchemaHint.attachedOnRead` change. `analyzeRelationshipTraversals` answers exactly what it answered before: measured identical, set order included, on 4,528 analyses of the 2,264 `record` / `previous` expressions in this repository's TypeScript sources.

‎packages/formula/src/relationship-traversal.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
import { describe, expect, it } from 'vitest';
44

5+
import { parseCelToAst } from './cel-engine';
56
import {
67
analyzeRelationshipTraversals,
78
findTraversalConflicts,
9+
readRootMembers,
10+
traversalsOf,
811
} from './relationship-traversal';
912
import { validateExpression } from './validate';
1013

@@ -108,6 +111,48 @@ describe('analyzeRelationshipTraversals — which hops an expression names', ()
108111
});
109112
});
110113

114+
describe('readRootMembers — the one member reader both analyses fold', () => {
115+
const readsOf = (source: string, roots: readonly string[]) => {
116+
const ast = parseCelToAst(source);
117+
expect(ast, source).not.toBeNull();
118+
return readRootMembers(ast!, roots);
119+
};
120+
121+
it('reads every member spelling of several roots in one walk, in source order', () => {
122+
expect(readsOf(
123+
"record['a'] == 1 && previous.?b.orValue(0) == 2 && has(record.c) && record[?'d'].orValue(0) == 3",
124+
['record', 'previous'],
125+
)).toEqual([
126+
{ root: 'record', field: 'a', deeper: false },
127+
{ root: 'previous', field: 'b', deeper: false },
128+
{ root: 'record', field: 'c', deeper: false },
129+
{ root: 'record', field: 'd', deeper: false },
130+
]);
131+
});
132+
133+
it('carries the next segment and whether the read goes deeper', () => {
134+
expect(readsOf("record.a['b'] == 1 && record.c.?d.e == 2", ['record'])).toEqual([
135+
{ root: 'record', field: 'a', leaf: 'b', deeper: false },
136+
{ root: 'record', field: 'c', leaf: 'd', deeper: true },
137+
]);
138+
});
139+
140+
it('reads no member for a computed key, a method on the root, or a root name in member position', () => {
141+
expect(readsOf('record[k] == 1 && record.size() > 0 && vars.record.x == 1', ['record'])).toEqual([]);
142+
});
143+
144+
it('a method call on a member uses the member as a value — no leaf', () => {
145+
expect(readsOf("record.name.startsWith('A')", ['record'])).toEqual([
146+
{ root: 'record', field: 'name', deeper: false },
147+
]);
148+
});
149+
150+
it('analyzeRelationshipTraversals is exactly the fold of its root\'s reads', () => {
151+
const source = "record.crm_account.type == 'x' && record.crm_account == 'acc_1' && record.owner.team.lead == 'y'";
152+
expect(analyzeRelationshipTraversals(source)).toEqual(traversalsOf(readsOf(source, ['record'])));
153+
});
154+
});
155+
111156
describe('findTraversalConflicts — what the authoring layer refuses', () => {
112157
it('refuses a FK that is both traversed and compared bare', () => {
113158
const a = analyzeRelationshipTraversals(

‎packages/formula/src/relationship-traversal.ts‎

Lines changed: 112 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
* is reported as a conflict rather than resolved by a precedence rule.
4646
*/
4747

48-
import { parseCelToAst } from './cel-engine';
48+
import { parseCelToAst, type CelAstNode } from './cel-engine';
4949

5050
/** The default scope root a record-scoped predicate traverses from. */
5151
export const DEFAULT_TRAVERSAL_ROOT = 'record';
@@ -75,13 +75,6 @@ export interface RelationshipTraversalAnalysis {
7575
readonly multiHopFields: ReadonlySet<string>;
7676
}
7777

78-
/** `{ op: 'id', args: '<root>' }` — a bare identifier node for `root`. */
79-
function isRootId(node: unknown, root: string): boolean {
80-
if (!node || typeof node !== 'object') return false;
81-
const { op, args } = node as { op?: unknown; args?: unknown };
82-
return op === 'id' && args === root;
83-
}
84-
8578
/**
8679
* A member access on `node`, whatever spelling it was written in, or `null`.
8780
*
@@ -132,6 +125,98 @@ function asMember(node: unknown): { receiver: unknown; name: string } | null {
132125
return null;
133126
}
134127

128+
/**
129+
* One read of a member of a named scope root, as {@link readRootMembers}
130+
* reports it — whatever spelling the author wrote it in.
131+
*/
132+
export interface RootMemberRead {
133+
/** The scope root the read starts from — `record`, `previous`, … */
134+
readonly root: string;
135+
/** The member read on the root: `crm_account` in `record['crm_account'].type`. */
136+
readonly field: string;
137+
/**
138+
* The member read in turn on `root.<field>`, when the expression reads one:
139+
* `type` in `record.crm_account.?type`. Absent when `root.<field>` is used
140+
* as a value — compared, handed to a function, or the receiver of a METHOD
141+
* call (`record.name.startsWith('A')`), which reads the value, not a member.
142+
*/
143+
readonly leaf?: string;
144+
/** The read continues past `leaf` (`record.a.b.c`): more than one hop. */
145+
readonly deeper: boolean;
146+
}
147+
148+
/**
149+
* Every read of a member of one of `roots` in a parsed CEL expression, in every
150+
* member spelling {@link asMember} recognises — `.`, `.?`, `['…']` and
151+
* `[?'…']` — in source order, one entry per occurrence.
152+
*
153+
* ⭐ The ONE member reader in this package. Two questions are answered from it
154+
* and nothing else: which hops an expression takes through a root
155+
* ({@link analyzeRelationshipTraversals}), and whether each member it names
156+
* exists (`validateExpression`'s field-existence pass). One reader means the
157+
* two can never disagree about which spellings name a member — a spelling one
158+
* of them missed is exactly how a typo used to pass the existence check while
159+
* the traversal check saw it.
160+
*
161+
* What is NOT a read, by construction:
162+
* - an index whose key is not a string literal (`record[someVar]`) — which
163+
* member it names is not knowable before evaluation, so there is nothing to
164+
* judge, and a check built on this reader gives no verdict for it;
165+
* - a method call on the root itself (`record.size()`), which reads the root,
166+
* not a member of it;
167+
* - a root name in member position (`vars.record.x`) — only a bare
168+
* identifier is a root.
169+
*
170+
* Takes the AST rather than the source so a caller asking both questions of
171+
* one expression parses it once. Parse it with {@link parseCelToAst}, the
172+
* platform's one answer to "what parses".
173+
*/
174+
export function readRootMembers(ast: CelAstNode, roots: readonly string[]): RootMemberRead[] {
175+
const rootSet = new Set(roots);
176+
const reads: RootMemberRead[] = [];
177+
178+
// `{ op: 'id', args: '<root>' }` — a bare identifier node naming one of `roots`.
179+
const rootNameOf = (node: unknown): string | undefined => {
180+
if (!node || typeof node !== 'object') return undefined;
181+
const { op, args } = node as { op?: unknown; args?: unknown };
182+
return op === 'id' && typeof args === 'string' && rootSet.has(args) ? args : undefined;
183+
};
184+
185+
// `via` is the member access this node is the RECEIVER of, handed down only
186+
// along a receiver edge: its name is the next segment, and `deeper` says
187+
// whether that access is itself read through in turn.
188+
const walk = (node: unknown, via?: { name: string; deeper: boolean }): void => {
189+
if (Array.isArray(node)) {
190+
for (const child of node) walk(child);
191+
return;
192+
}
193+
if (!node || typeof node !== 'object') return;
194+
195+
const member = asMember(node);
196+
if (member) {
197+
const root = rootNameOf(member.receiver);
198+
if (root !== undefined) {
199+
reads.push({
200+
root,
201+
field: member.name,
202+
...(via ? { leaf: via.name } : {}),
203+
deeper: via?.deeper ?? false,
204+
});
205+
return;
206+
}
207+
// The receiver is read through this access. The access's other operand
208+
// is its member name or a string-literal key, neither of which can hold
209+
// a read.
210+
walk(member.receiver, { name: member.name, deeper: via !== undefined });
211+
return;
212+
}
213+
214+
for (const value of Object.values(node as Record<string, unknown>)) walk(value);
215+
};
216+
walk(ast);
217+
return reads;
218+
}
219+
135220
/**
136221
* Analyse one authored CEL source for the hops it takes through `root`.
137222
*
@@ -152,61 +237,30 @@ export function analyzeRelationshipTraversals(
152237
): RelationshipTraversalAnalysis | null {
153238
const ast = parseCelToAst(source);
154239
if (ast == null) return null;
240+
return traversalsOf(readRootMembers(ast, [root]));
241+
}
155242

243+
/**
244+
* Fold one root's {@link readRootMembers} into the traversal analysis: a read
245+
* with a `leaf` is a hop through `field` (and more than one when it goes
246+
* `deeper`), a read without one uses `field` as a value.
247+
*/
248+
export function traversalsOf(reads: readonly RootMemberRead[]): RelationshipTraversalAnalysis {
156249
const traversals = new Map<string, Set<string>>();
157250
const bareFields = new Set<string>();
158251
const multiHopFields = new Set<string>();
159-
160-
// Member-access nodes reached AS THE RECEIVER of another member access are
161-
// traversals, not values. Collect those first so the value pass can exclude
162-
// them by identity rather than by re-deriving the shape.
163-
const traversalReceivers = new Set<unknown>();
164-
165-
const walk = (node: unknown): void => {
166-
if (Array.isArray(node)) {
167-
for (const child of node) walk(child);
168-
return;
169-
}
170-
if (!node || typeof node !== 'object') return;
171-
172-
const outer = asMember(node);
173-
if (outer) {
174-
const inner = asMember(outer.receiver);
175-
if (inner && isRootId(inner.receiver, root)) {
176-
// root.<inner.name>.<outer.name> — one hop through `inner.name`.
177-
traversalReceivers.add(outer.receiver);
178-
let fields = traversals.get(inner.name);
179-
if (!fields) traversals.set(inner.name, (fields = new Set()));
180-
fields.add(outer.name);
181-
} else if (inner) {
182-
// Deeper than one hop: root.a.b.c reaches here as (root.a.b).c, whose
183-
// own receiver is itself a traversal. Attribute it to the FIRST field
184-
// so the refusal can name what the author wrote.
185-
const base = asMember(inner.receiver);
186-
if (base && isRootId(base.receiver, root)) multiHopFields.add(base.name);
187-
}
252+
for (const read of reads) {
253+
if (read.leaf === undefined) {
254+
bareFields.add(read.field);
255+
continue;
188256
}
189-
190-
for (const value of Object.values(node as Record<string, unknown>)) walk(value);
191-
};
192-
walk(ast);
193-
194-
// Second pass: every `root.<field>` node that was NOT consumed as a traversal
195-
// receiver is a value use.
196-
const walkValues = (node: unknown): void => {
197-
if (Array.isArray(node)) {
198-
for (const child of node) walkValues(child);
199-
return;
200-
}
201-
if (!node || typeof node !== 'object') return;
202-
const member = asMember(node);
203-
if (member && isRootId(member.receiver, root) && !traversalReceivers.has(node)) {
204-
bareFields.add(member.name);
205-
}
206-
for (const value of Object.values(node as Record<string, unknown>)) walkValues(value);
207-
};
208-
walkValues(ast);
209-
257+
let fields = traversals.get(read.field);
258+
if (!fields) traversals.set(read.field, (fields = new Set()));
259+
fields.add(read.leaf);
260+
// Deeper than one hop: `root.a.b.c`. Attributed to the FIRST field so the
261+
// refusal can name what the author wrote.
262+
if (read.deeper) multiHopFields.add(read.field);
263+
}
210264
return { traversals, bareFields, multiHopFields };
211265
}
212266

‎packages/formula/src/validate-attached-on-read.test.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,40 @@ describe('ExprSchemaHint.attachedOnRead — record.<block>.<leaf>', () => {
119119
expect(refusalsOf('record.submitter_id.nmae == "x"', DECLARED)).toEqual([]);
120120
});
121121

122-
it('leaves a method call, an index read and a third segment unjudged — missed catches, never false refusals', () => {
123-
expect(refusalsOf("record.viewer['can_actt'] == true", DECLARED)).toEqual([]);
122+
it('judges the leaf in every member spelling, as it judges the dot', () => {
123+
const refused = [{
124+
code: 'unknown-field',
125+
params: {
126+
field: 'viewer.can_actt',
127+
objectName: 'sys_approval_request',
128+
suggestion: 'viewer.can_act',
129+
block: 'viewer',
130+
leaves: ['can_act', 'can_override', 'is_submitter'],
131+
},
132+
}];
133+
for (const source of [
134+
"record.viewer['can_actt'] == true",
135+
"record.viewer.?can_actt.orValue(false) == true",
136+
"record.viewer[?'can_actt'].orValue(false) == true",
137+
"record['viewer'].can_actt == true",
138+
"previous.?viewer.can_actt.orValue(false) == true",
139+
'has(record.viewer.can_actt)',
140+
]) {
141+
expect(refusalsOf(source, DECLARED), source).toEqual(refused);
142+
}
143+
// Control: the declared leaf in the same spellings.
144+
for (const source of [
145+
"record.viewer['can_act'] == true",
146+
"record.viewer.?can_act.orValue(false) == true",
147+
"record['viewer'][?'can_act'].orValue(false) == true",
148+
'has(record.viewer.can_act)',
149+
]) {
150+
expect(refusalsOf(source, DECLARED), source).toEqual([]);
151+
}
152+
});
153+
154+
it('leaves a computed key, a method call and a third segment unjudged — missed catches, never false refusals', () => {
155+
expect(refusalsOf('record.viewer[record.status] == true', DECLARED)).toEqual([]);
124156
expect(refusalsOf("record.viewer.split(',') == []", DECLARED)).toEqual([]);
125157
expect(refusalsOf('record.viewer.can_act.foo == true', DECLARED)).toEqual([]);
126158
});

0 commit comments

Comments
 (0)