diff --git a/.changeset/22428-field-existence-every-spelling.md b/.changeset/22428-field-existence-every-spelling.md new file mode 100644 index 0000000000..c5b4405469 --- /dev/null +++ b/.changeset/22428-field-existence-every-spelling.md @@ -0,0 +1,29 @@ +--- +"@objectstack/formula": minor +--- + +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) + +Clause-②: yes (narrowing) + + + +**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. + +`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. + +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: + +- `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`. +- 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. +- 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. + +**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. + +**Unchanged.** + +- A computed key (`record[record.kind]`, `record[someVar]`) names no member before evaluation, so it is not judged. +- A method call on a member (`record.name.startsWith('A')`) still judges the member (`name`). +- 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. +- 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. +- 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. diff --git a/packages/formula/src/relationship-traversal.test.ts b/packages/formula/src/relationship-traversal.test.ts index ead573f36a..2477980db5 100644 --- a/packages/formula/src/relationship-traversal.test.ts +++ b/packages/formula/src/relationship-traversal.test.ts @@ -2,9 +2,12 @@ import { describe, expect, it } from 'vitest'; +import { parseCelToAst } from './cel-engine'; import { analyzeRelationshipTraversals, findTraversalConflicts, + readRootMembers, + traversalsOf, } from './relationship-traversal'; import { validateExpression } from './validate'; @@ -108,6 +111,48 @@ describe('analyzeRelationshipTraversals — which hops an expression names', () }); }); +describe('readRootMembers — the one member reader both analyses fold', () => { + const readsOf = (source: string, roots: readonly string[]) => { + const ast = parseCelToAst(source); + expect(ast, source).not.toBeNull(); + return readRootMembers(ast!, roots); + }; + + it('reads every member spelling of several roots in one walk, in source order', () => { + expect(readsOf( + "record['a'] == 1 && previous.?b.orValue(0) == 2 && has(record.c) && record[?'d'].orValue(0) == 3", + ['record', 'previous'], + )).toEqual([ + { root: 'record', field: 'a', deeper: false }, + { root: 'previous', field: 'b', deeper: false }, + { root: 'record', field: 'c', deeper: false }, + { root: 'record', field: 'd', deeper: false }, + ]); + }); + + it('carries the next segment and whether the read goes deeper', () => { + expect(readsOf("record.a['b'] == 1 && record.c.?d.e == 2", ['record'])).toEqual([ + { root: 'record', field: 'a', leaf: 'b', deeper: false }, + { root: 'record', field: 'c', leaf: 'd', deeper: true }, + ]); + }); + + it('reads no member for a computed key, a method on the root, or a root name in member position', () => { + expect(readsOf('record[k] == 1 && record.size() > 0 && vars.record.x == 1', ['record'])).toEqual([]); + }); + + it('a method call on a member uses the member as a value — no leaf', () => { + expect(readsOf("record.name.startsWith('A')", ['record'])).toEqual([ + { root: 'record', field: 'name', deeper: false }, + ]); + }); + + it('analyzeRelationshipTraversals is exactly the fold of its root\'s reads', () => { + const source = "record.crm_account.type == 'x' && record.crm_account == 'acc_1' && record.owner.team.lead == 'y'"; + expect(analyzeRelationshipTraversals(source)).toEqual(traversalsOf(readsOf(source, ['record']))); + }); +}); + describe('findTraversalConflicts — what the authoring layer refuses', () => { it('refuses a FK that is both traversed and compared bare', () => { const a = analyzeRelationshipTraversals( diff --git a/packages/formula/src/relationship-traversal.ts b/packages/formula/src/relationship-traversal.ts index 002192dc93..ff9ce0ce42 100644 --- a/packages/formula/src/relationship-traversal.ts +++ b/packages/formula/src/relationship-traversal.ts @@ -45,7 +45,7 @@ * is reported as a conflict rather than resolved by a precedence rule. */ -import { parseCelToAst } from './cel-engine'; +import { parseCelToAst, type CelAstNode } from './cel-engine'; /** The default scope root a record-scoped predicate traverses from. */ export const DEFAULT_TRAVERSAL_ROOT = 'record'; @@ -75,13 +75,6 @@ export interface RelationshipTraversalAnalysis { readonly multiHopFields: ReadonlySet; } -/** `{ op: 'id', args: '' }` — a bare identifier node for `root`. */ -function isRootId(node: unknown, root: string): boolean { - if (!node || typeof node !== 'object') return false; - const { op, args } = node as { op?: unknown; args?: unknown }; - return op === 'id' && args === root; -} - /** * A member access on `node`, whatever spelling it was written in, or `null`. * @@ -132,6 +125,98 @@ function asMember(node: unknown): { receiver: unknown; name: string } | null { return null; } +/** + * One read of a member of a named scope root, as {@link readRootMembers} + * reports it — whatever spelling the author wrote it in. + */ +export interface RootMemberRead { + /** The scope root the read starts from — `record`, `previous`, … */ + readonly root: string; + /** The member read on the root: `crm_account` in `record['crm_account'].type`. */ + readonly field: string; + /** + * The member read in turn on `root.`, when the expression reads one: + * `type` in `record.crm_account.?type`. Absent when `root.` is used + * as a value — compared, handed to a function, or the receiver of a METHOD + * call (`record.name.startsWith('A')`), which reads the value, not a member. + */ + readonly leaf?: string; + /** The read continues past `leaf` (`record.a.b.c`): more than one hop. */ + readonly deeper: boolean; +} + +/** + * Every read of a member of one of `roots` in a parsed CEL expression, in every + * member spelling {@link asMember} recognises — `.`, `.?`, `['…']` and + * `[?'…']` — in source order, one entry per occurrence. + * + * ⭐ The ONE member reader in this package. Two questions are answered from it + * and nothing else: which hops an expression takes through a root + * ({@link analyzeRelationshipTraversals}), and whether each member it names + * exists (`validateExpression`'s field-existence pass). One reader means the + * two can never disagree about which spellings name a member — a spelling one + * of them missed is exactly how a typo used to pass the existence check while + * the traversal check saw it. + * + * What is NOT a read, by construction: + * - an index whose key is not a string literal (`record[someVar]`) — which + * member it names is not knowable before evaluation, so there is nothing to + * judge, and a check built on this reader gives no verdict for it; + * - a method call on the root itself (`record.size()`), which reads the root, + * not a member of it; + * - a root name in member position (`vars.record.x`) — only a bare + * identifier is a root. + * + * Takes the AST rather than the source so a caller asking both questions of + * one expression parses it once. Parse it with {@link parseCelToAst}, the + * platform's one answer to "what parses". + */ +export function readRootMembers(ast: CelAstNode, roots: readonly string[]): RootMemberRead[] { + const rootSet = new Set(roots); + const reads: RootMemberRead[] = []; + + // `{ op: 'id', args: '' }` — a bare identifier node naming one of `roots`. + const rootNameOf = (node: unknown): string | undefined => { + if (!node || typeof node !== 'object') return undefined; + const { op, args } = node as { op?: unknown; args?: unknown }; + return op === 'id' && typeof args === 'string' && rootSet.has(args) ? args : undefined; + }; + + // `via` is the member access this node is the RECEIVER of, handed down only + // along a receiver edge: its name is the next segment, and `deeper` says + // whether that access is itself read through in turn. + const walk = (node: unknown, via?: { name: string; deeper: boolean }): void => { + if (Array.isArray(node)) { + for (const child of node) walk(child); + return; + } + if (!node || typeof node !== 'object') return; + + const member = asMember(node); + if (member) { + const root = rootNameOf(member.receiver); + if (root !== undefined) { + reads.push({ + root, + field: member.name, + ...(via ? { leaf: via.name } : {}), + deeper: via?.deeper ?? false, + }); + return; + } + // The receiver is read through this access. The access's other operand + // is its member name or a string-literal key, neither of which can hold + // a read. + walk(member.receiver, { name: member.name, deeper: via !== undefined }); + return; + } + + for (const value of Object.values(node as Record)) walk(value); + }; + walk(ast); + return reads; +} + /** * Analyse one authored CEL source for the hops it takes through `root`. * @@ -152,61 +237,30 @@ export function analyzeRelationshipTraversals( ): RelationshipTraversalAnalysis | null { const ast = parseCelToAst(source); if (ast == null) return null; + return traversalsOf(readRootMembers(ast, [root])); +} +/** + * Fold one root's {@link readRootMembers} into the traversal analysis: a read + * with a `leaf` is a hop through `field` (and more than one when it goes + * `deeper`), a read without one uses `field` as a value. + */ +export function traversalsOf(reads: readonly RootMemberRead[]): RelationshipTraversalAnalysis { const traversals = new Map>(); const bareFields = new Set(); const multiHopFields = new Set(); - - // Member-access nodes reached AS THE RECEIVER of another member access are - // traversals, not values. Collect those first so the value pass can exclude - // them by identity rather than by re-deriving the shape. - const traversalReceivers = new Set(); - - const walk = (node: unknown): void => { - if (Array.isArray(node)) { - for (const child of node) walk(child); - return; - } - if (!node || typeof node !== 'object') return; - - const outer = asMember(node); - if (outer) { - const inner = asMember(outer.receiver); - if (inner && isRootId(inner.receiver, root)) { - // root.. — one hop through `inner.name`. - traversalReceivers.add(outer.receiver); - let fields = traversals.get(inner.name); - if (!fields) traversals.set(inner.name, (fields = new Set())); - fields.add(outer.name); - } else if (inner) { - // Deeper than one hop: root.a.b.c reaches here as (root.a.b).c, whose - // own receiver is itself a traversal. Attribute it to the FIRST field - // so the refusal can name what the author wrote. - const base = asMember(inner.receiver); - if (base && isRootId(base.receiver, root)) multiHopFields.add(base.name); - } + for (const read of reads) { + if (read.leaf === undefined) { + bareFields.add(read.field); + continue; } - - for (const value of Object.values(node as Record)) walk(value); - }; - walk(ast); - - // Second pass: every `root.` node that was NOT consumed as a traversal - // receiver is a value use. - const walkValues = (node: unknown): void => { - if (Array.isArray(node)) { - for (const child of node) walkValues(child); - return; - } - if (!node || typeof node !== 'object') return; - const member = asMember(node); - if (member && isRootId(member.receiver, root) && !traversalReceivers.has(node)) { - bareFields.add(member.name); - } - for (const value of Object.values(node as Record)) walkValues(value); - }; - walkValues(ast); - + let fields = traversals.get(read.field); + if (!fields) traversals.set(read.field, (fields = new Set())); + fields.add(read.leaf); + // Deeper than one hop: `root.a.b.c`. Attributed to the FIRST field so the + // refusal can name what the author wrote. + if (read.deeper) multiHopFields.add(read.field); + } return { traversals, bareFields, multiHopFields }; } diff --git a/packages/formula/src/validate-attached-on-read.test.ts b/packages/formula/src/validate-attached-on-read.test.ts index cabc9c019e..a300355ae4 100644 --- a/packages/formula/src/validate-attached-on-read.test.ts +++ b/packages/formula/src/validate-attached-on-read.test.ts @@ -119,8 +119,40 @@ describe('ExprSchemaHint.attachedOnRead — record..', () => { expect(refusalsOf('record.submitter_id.nmae == "x"', DECLARED)).toEqual([]); }); - it('leaves a method call, an index read and a third segment unjudged — missed catches, never false refusals', () => { - expect(refusalsOf("record.viewer['can_actt'] == true", DECLARED)).toEqual([]); + it('judges the leaf in every member spelling, as it judges the dot', () => { + const refused = [{ + code: 'unknown-field', + params: { + field: 'viewer.can_actt', + objectName: 'sys_approval_request', + suggestion: 'viewer.can_act', + block: 'viewer', + leaves: ['can_act', 'can_override', 'is_submitter'], + }, + }]; + for (const source of [ + "record.viewer['can_actt'] == true", + "record.viewer.?can_actt.orValue(false) == true", + "record.viewer[?'can_actt'].orValue(false) == true", + "record['viewer'].can_actt == true", + "previous.?viewer.can_actt.orValue(false) == true", + 'has(record.viewer.can_actt)', + ]) { + expect(refusalsOf(source, DECLARED), source).toEqual(refused); + } + // Control: the declared leaf in the same spellings. + for (const source of [ + "record.viewer['can_act'] == true", + "record.viewer.?can_act.orValue(false) == true", + "record['viewer'][?'can_act'].orValue(false) == true", + 'has(record.viewer.can_act)', + ]) { + expect(refusalsOf(source, DECLARED), source).toEqual([]); + } + }); + + it('leaves a computed key, a method call and a third segment unjudged — missed catches, never false refusals', () => { + expect(refusalsOf('record.viewer[record.status] == true', DECLARED)).toEqual([]); expect(refusalsOf("record.viewer.split(',') == []", DECLARED)).toEqual([]); expect(refusalsOf('record.viewer.can_act.foo == true', DECLARED)).toEqual([]); }); diff --git a/packages/formula/src/validate-field-existence-spellings.test.ts b/packages/formula/src/validate-field-existence-spellings.test.ts new file mode 100644 index 0000000000..fb2ced270a --- /dev/null +++ b/packages/formula/src/validate-field-existence-spellings.test.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Field existence judges a member of `record` / `previous` in EVERY spelling + * that names one — not only the dot spelling. + * + * `record.f`, `record.?f`, `record['f']` and `record[?'f']` read the same + * column, and `has(…)` around any of them reads it too. A check that saw only + * the dot let a typo written any other way reach the evaluator with no + * authoring verdict. The members now come from `readRootMembers`, the one AST + * member reader the relationship-traversal analysis is folded from, so every + * spelling gets the same verdict as the dot. + * + * Each verdict is asserted on its `code` and `params` (the named subject), the + * machine-readable half of the refusal. + */ + +import { describe, it, expect } from 'vitest'; + +import { validateExpression, type ExprSchemaHint } from './validate'; + +const SCHEMA: ExprSchemaHint = { + objectName: 'fx_probe', + fields: ['name', 'status', 'amount'], + scope: 'record', +}; + +/** Every spelling that names member `f` of `root`, as an author writes it in a predicate. */ +const spellings = (root: string, f: string): ReadonlyArray => [ + ['dot', `${root}.${f} == 'a'`], + ['bracket', `${root}['${f}'] == 'a'`], + ['double-quoted bracket', `${root}["${f}"] == 'a'`], + ['optional selection', `${root}.?${f}.orValue('') == 'a'`], + ['optional index', `${root}[?'${f}'].orValue('') == 'a'`], + ['has() presence test', `has(${root}.${f})`], +]; + +function refusalsOf(source: string, schema: ExprSchemaHint = SCHEMA) { + return validateExpression('predicate', source, schema).errors.map((e) => ({ code: e.code, params: e.params })); +} + +describe('field existence — every member spelling on record and previous', () => { + for (const root of ['record', 'previous']) { + for (const [spelling, source] of spellings(root, 'zz_typo')) { + it(`refuses an undeclared field written as ${root} ${spelling}: ${source}`, () => { + expect(refusalsOf(source)).toEqual([ + { code: 'unknown-field', params: { field: 'zz_typo', objectName: 'fx_probe' } }, + ]); + }); + } + + for (const [spelling, source] of spellings(root, 'status')) { + it(`CONTROL — accepts a declared field written as ${root} ${spelling}: ${source}`, () => { + const r = validateExpression('predicate', source, SCHEMA); + expect(r.errors).toEqual([]); + expect(r.ok).toBe(true); + }); + } + } + + it('keeps the did-you-mean in a non-dot spelling', () => { + expect(refusalsOf("record['stauts'] == 'a'")).toEqual([ + { code: 'unknown-field', params: { field: 'stauts', objectName: 'fx_probe', suggestion: 'status' } }, + ]); + }); + + it('reports one name once, across spellings and across both roots', () => { + expect(refusalsOf("record.zz == 1 && record['zz'] == 2 && previous.?zz.orValue(0) == 3")).toEqual([ + { code: 'unknown-field', params: { field: 'zz', objectName: 'fx_probe' } }, + ]); + }); + + it('reports distinct names in source order, whatever their spellings', () => { + expect(refusalsOf("record['zz_b'] == 1 && record.zz_a == 2").map((r) => r.params)).toEqual([ + { field: 'zz_b', objectName: 'fx_probe' }, + { field: 'zz_a', objectName: 'fx_probe' }, + ]); + }); + + it('judges a member read inside a comprehension body', () => { + expect(refusalsOf("[1, 2].exists(x, record['zz_typo'] == x)")).toEqual([ + { code: 'unknown-field', params: { field: 'zz_typo', objectName: 'fx_probe' } }, + ]); + }); + + it('gives a computed key no verdict — it names no member before evaluation', () => { + // The key is another field's VALUE; only the read that names a member + // (`record.name`, declared) is judged. + const r = validateExpression('predicate', "record[record.name] == 'a'", SCHEMA); + expect(r.errors).toEqual([]); + expect(r.ok).toBe(true); + }); + + it('reads members, not text: a string literal that spells `record.` is not a read', () => { + expect(refusalsOf("record.name == 'record.zz_typo'")).toEqual([]); + }); + + it('judges nothing without a field list, in any spelling (control)', () => { + for (const [, source] of spellings('record', 'zz_typo')) { + expect(refusalsOf(source, { scope: 'record' }), source).toEqual([]); + } + }); +}); diff --git a/packages/formula/src/validate.ts b/packages/formula/src/validate.ts index f7e2467542..3fc4e3033d 100644 --- a/packages/formula/src/validate.ts +++ b/packages/formula/src/validate.ts @@ -22,11 +22,13 @@ import { firstUndeclaredReference, firstTypeMismatch, inferCelType, + parseCelToAst, parseCelToAstWithReason, + type CelAstNode, type FieldCelType, } from './cel-engine'; import { templateEngine } from './template-engine'; -import { analyzeRelationshipTraversals, findTraversalConflicts } from './relationship-traversal'; +import { findTraversalConflicts, readRootMembers, traversalsOf, type RootMemberRead } from './relationship-traversal'; import { REFERENCE_VALUE_TYPES } from '@objectstack/spec/data'; // #13594 — the one reader of cel-js's `found no matching overload for '…'` // template. Both this module (which asks whether the name is ADVERTISED, to word @@ -63,16 +65,22 @@ export type ExprInput = string | { dialect?: string; source?: string } | null | export interface ExprSchemaHint { /** Object the expression is authored against (for error text). */ objectName?: string; - /** Known top-level field names, so `record.` can be checked. */ + /** + * Known top-level field names, so each member read on `record` / `previous` + * can be checked — in every spelling that names a member: `record.f`, + * `record.?f`, `record['f']`, `record[?'f']`, and the same inside `has(…)`. + * A computed key (`record[someVar]`) names no member before evaluation and + * gets no verdict. + */ fields?: readonly string[]; /** * The object's read attachments (`ObjectSchema.attachedOnRead`) — block name * → the leaf keys that block declares. A block is computed per caller and * attached to each served row, never stored, so it is not a field; this map * lets the field-existence pass judge the SECOND segment of - * `record..` (and `previous.…`): a leaf the block does not - * declare is refused under the same `unknown-field` code, and the refusal - * names the leaves the block does declare. + * `record..` (and `previous.…`, in every member spelling): a + * leaf the block does not declare is refused under the same `unknown-field` + * code, and the refusal names the leaves the block does declare. * * It only ADDS the second-segment judgement. Whether `record.` * resolves at all is still {@link fields}' question, so a caller lists each @@ -323,16 +331,8 @@ function typeSoundnessIssue( /** A bare `{x}` that is NOT part of a `{{x}}` mustache hole. */ const SINGLE_BRACE_RE = /(?:^|[^{])\{\s*([A-Za-z_$][\w.$]*)\s*\}(?!\})/; -/** `record.` / `previous.` head references for field-existence. */ -const RECORD_REF_RE = /\b(?:record|previous)\.([A-Za-z_$][\w$]*)/g; -/** - * The member read right after a {@link RECORD_REF_RE} head — `.can_act` in - * `record.viewer.can_act` — matched STICKY at the head's end, so the head scan - * itself is untouched. A member followed by `(` is a method call, not a member - * read, and is not captured; the trailing `(?![\w$])` stops a backtrack from - * capturing a prefix of the name instead. - */ -const SECOND_SEGMENT_RE = /\.([A-Za-z_$][\w$]*)(?![\w$]|\s*\()/y; +/** The roots whose members the field-existence pass judges against `schema.fields`. */ +const FIELD_EXISTENCE_ROOTS = ['record', 'previous'] as const; /** The dialect a field role expects (Decision 2). */ export function expectedDialect(role: FieldRole): 'cel' | 'template' { @@ -645,22 +645,44 @@ function receiverCallHint(celMessage: string, source: string): { text: string; n }; } -function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined, errors: ExprValidationError[]): void { +/** + * Every member `source` reads on `record` / `previous`, judged against + * `schema.fields` — one `unknown-field` refusal per undeclared name. + * + * The members come from `readRootMembers`, the AST member reader the + * relationship-traversal analysis is built on, so every spelling that names a + * member is judged once and the same way: `record.f`, `record.?f`, + * `record['f']`, `record[?'f']`, and any of them inside `has(…)`. A reader that + * saw only the dot spelling let a typo written any other way reach the + * evaluator with no authoring verdict, to fault there on every row instead. + * A computed key (`record[someVar]`) names no member before evaluation, so it + * gets no verdict here. + * + * `ast` is null only for a source the canonical front end will not parse; the + * caller reaches this after `celEngine.compile` accepted the source, which owns + * that verdict, so there is nothing to judge. + */ +function checkFieldExistence( + ast: () => CelAstNode | null, + source: string, + schema: ExprSchemaHint | undefined, + errors: ExprValidationError[], +): void { if (!schema?.fields || schema.fields.length === 0) return; + const tree = ast(); + if (tree == null) return; const known = new Set(schema.fields); const seen = new Set(); const blocks = schema.attachedOnRead; const seenLeaves = new Set(); - let m: RegExpExecArray | null; - RECORD_REF_RE.lastIndex = 0; - while ((m = RECORD_REF_RE.exec(source)) !== null) { - const field = m[1]; + for (const read of readRootMembers(tree, FIELD_EXISTENCE_ROOTS)) { + const field = read.field; if (known.has(field)) { // [#22211 ruling A] A head that names a declared read attachment has a // closed second segment: judge it against the block's declared leaves. // Own-key test, so an inherited name (`constructor`) is never a block. if (blocks && Object.prototype.hasOwnProperty.call(blocks, field)) { - checkAttachedLeaf(source, schema.objectName, field, blocks[field], RECORD_REF_RE.lastIndex, seenLeaves, errors); + checkAttachedLeaf(source, schema.objectName, field, blocks[field], read, seenLeaves, errors); } continue; } @@ -691,25 +713,27 @@ function checkFieldExistence(source: string, schema: ExprSchemaHint | undefined, * leaves the block declares — the remedy — carried as `block` + `leaves`, * present together exactly when the message carries that clause. * - * Unjudged, deliberately (each a missed catch, never a false refusal): index - * access (`record.viewer['can_act']`), a method call on the block, and any - * segment past the second — a leaf is a scalar, so a third segment is not this - * declaration's question. + * The leaf is judged in every spelling that names a member, exactly like the + * block itself: `record.viewer.can_act`, `record.viewer.?can_act`, + * `record.viewer['can_act']`, `record['viewer'][?'can_act']`, inside `has(…)` + * or not. Unjudged, deliberately (each a missed catch, never a false refusal): + * a computed key (`record.viewer[k]`), a method call on the block, which reads + * the block's value rather than a member of it, and any segment past the + * second — a leaf is a scalar, so a third segment is not this declaration's + * question. */ function checkAttachedLeaf( source: string, objectName: string | undefined, block: string, leaves: unknown, - headEnd: number, + read: RootMemberRead, seenLeaves: Set, errors: ExprValidationError[], ): void { if (!Array.isArray(leaves)) return; - SECOND_SEGMENT_RE.lastIndex = headEnd; - const next = SECOND_SEGMENT_RE.exec(source); - if (!next) return; - const leaf = next[1]; + const leaf = read.leaf; + if (leaf === undefined) return; if (leaves.includes(leaf)) return; const field = `${block}.${leaf}`; if (seenLeaves.has(field)) return; @@ -955,6 +979,11 @@ export function validateExpression( return { ok: false, errors, warnings }; } const compiled = celEngine.compile(source); + // One parse through the canonical front end, shared by the two passes that + // read members — field existence and the relationship-traversal arm — and + // taken only when one of them runs. + let parsedAst: CelAstNode | null | undefined; + const ast = (): CelAstNode | null => (parsedAst === undefined ? (parsedAst = parseCelToAst(source)) : parsedAst); if (!compiled.ok) { // #7073 — a bounds refusal gets the SIZE prescription, never the dialect // trailer: the source is already bare CEL, so "write bare CEL" is advice @@ -1003,7 +1032,7 @@ export function validateExpression( )); } } else { - checkFieldExistence(source, schema, errors); + checkFieldExistence(ast, source, schema, errors); checkRoleCatalog(source, schema, errors); if (schema?.scope === 'record') { // In a `record`-scoped site a bare top-level identifier is a silent bug — @@ -1088,7 +1117,8 @@ export function validateExpression( // Metadata authored through Studio, written straight to `sys_metadata` or // produced by an agent never reaches this check. if (schema?.fieldTypes && schema.traversalHydration === true && role === 'predicate') { - const analysis = analyzeRelationshipTraversals(source); + const tree = ast(); + const analysis = tree ? traversalsOf(readRootMembers(tree, [DEFAULT_TRAVERSAL_ROOT])) : null; if (analysis) { const conflicts = findTraversalConflicts( analysis, diff --git a/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts b/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts new file mode 100644 index 0000000000..d24f263e97 --- /dev/null +++ b/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * An undeclared field is refused at BOTH doors in every member spelling, at + * every record-scoped slot an object carries — not only when it is written + * with a dot. + * + * ## The state this closes + * + * `@objectstack/formula`'s field-existence pass read `record.` / + * `previous.` with a dot-only regex. `record['zz_typo']`, + * `previous['zz_typo']`, `record.?zz_typo` and `record[?'zz_typo']` named the + * same column and reached the evaluator with no authoring verdict, so + * `os build` and the object save door both published them clean. The pass now + * reads members through the formula package's AST member reader — the one the + * relationship-traversal analysis is folded from — so there is no lint-side + * arm here: the slot walk is unchanged, and the build and the door give the + * shared validator's verdict exactly as they give it for the dot. + * + * ## What is pinned + * + * Three slots (a select option's `visibleWhen`, a field's `requiredWhen`, a + * validation rule's `condition`) × six spellings, at the build's rule table and + * at the runtime gate the object write path runs. The control: every spelling + * of a DECLARED field publishes clean at both doors. + * + * Each refusal asserts the named subject (`unknown field \`zz_typo\``), the + * location the author edits, and the severity — not the prose around them. + */ +import { describe, expect, it } from 'vitest'; +import { EXPRESSION_INVALID, runAuthoringRules } from './authoring-rules.js'; +import { runRuntimeAuthoringRules } from './runtime-gate.js'; + +type Slot = 'option visibleWhen' | 'requiredWhen' | 'validation condition'; + +/** The probe object with `expression` on one record-scoped slot. */ +const fxObject = (slot: Slot, expression: string) => ({ + name: 'fx_probe', + label: 'Probe', + // Keeps `security-owd-unset` quiet, so a refusal is the expression rule's. + sharingModel: 'private', + fields: { + name: { + type: 'text', + label: 'Name', + ...(slot === 'requiredWhen' ? { requiredWhen: expression } : {}), + }, + status: { type: 'text', label: 'Status' }, + tier: { + type: 'select', + label: 'Tier', + options: [ + { label: 'Standard', value: 'standard' }, + { label: 'Gold', value: 'gold', ...(slot === 'option visibleWhen' ? { visibleWhen: expression } : {}) }, + ], + }, + }, + ...(slot === 'validation condition' + ? { validations: [{ name: 'probe_rule', type: 'script', condition: expression, message: 'Probe.', severity: 'error' }] } + : {}), +}); + +const WHERE: Readonly> = { + 'option visibleWhen': "object 'fx_probe' · field 'tier' option 'gold' visibleWhen", + requiredWhen: "object 'fx_probe' · field 'name' requiredWhen", + 'validation condition': "object 'fx_probe' · validation 'probe_rule'", +}; + +const SLOTS = Object.keys(WHERE) as Slot[]; + +/** Every member spelling that names `f`; the dot row is the one that was already refused. */ +const spellings = (f: string): readonly string[] => [ + `record.${f} == 'a'`, + `record['${f}'] == 'a'`, + `previous['${f}'] == 'a'`, + `record.?${f}.orValue('') == 'a'`, + `record[?'${f}'].orValue('') == 'a'`, + `has(record.${f})`, +]; + +const expressionFindings = (fs: readonly T[]): T[] => + fs.filter((f) => f.rule === EXPRESSION_INVALID); + +const atBuild = (item: unknown) => { + const stack = { objects: [item] }; + return expressionFindings(runAuthoringRules('build', { normalized: stack, parsed: stack })); +}; + +const atDoor = (item: unknown) => runRuntimeAuthoringRules({ type: 'object', item, context: { objects: [] } }); + +const dump = (r: unknown) => JSON.stringify(r, null, 2); + +describe('an undeclared field is refused in every member spelling, at both doors', () => { + for (const slot of SLOTS) { + for (const expression of spellings('zz_typo')) { + it(`⭐ LIT — ${slot}: \`${expression}\` is refused by \`os build\` and by the object door`, () => { + const build = atBuild(fxObject(slot, expression)); + expect(build, dump(build)).toHaveLength(1); + expect(build[0]).toMatchObject({ severity: 'error', where: WHERE[slot] }); + expect(build[0]!.message).toContain('unknown field `zz_typo`'); + + const door = atDoor(fxObject(slot, expression)); + expect(door.rulesRun).toContain('validateStackExpressions'); + const errs = expressionFindings(door.errors); + expect(errs, dump(door)).toHaveLength(1); + expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE[slot], path: WHERE[slot] }); + expect(errs[0]!.message).toContain('unknown field `zz_typo`'); + }); + } + + for (const expression of spellings('status')) { + it(`⭐ CONTROL — ${slot}: \`${expression}\` on a declared field publishes clean at both doors`, () => { + expect(atBuild(fxObject(slot, expression))).toEqual([]); + + const door = atDoor(fxObject(slot, expression)); + expect(door.rulesRun).toContain('validateStackExpressions'); + expect(expressionFindings(door.errors), dump(door)).toEqual([]); + expect(expressionFindings(door.advisories), dump(door)).toEqual([]); + }); + } + } +});