Skip to content

Commit c946cb9

Browse files
feat(formula): a CEL syntax refusal keeps the engine's fault code and source range (#27904) (#28679)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XKy1Tn6rK9yK1erywxCx5Bk)_ Fixes #27904 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:公式引擎拒绝一条写错的 CEL 表达式时,除了原来那句英文,现在还带上引擎自己的错误类别(比如「缺一个右括号」「多了一个不认识的字符」)和出错的字符位置。 - **为什么改**:编辑器之前只能显示一句笼统的「语法错误」加原始报错,没法用用户的语言说「第 17 个字符处缺一个右括号」,也没法把光标指到出错处。 - **风险与回滚**:只新增两个可选字段,原有英文句子一字不变;回滚即还原这一个 commit。引擎读的是平台改写过的表达式时不给位置(宁缺不错)。 - **先看**:`packages/formula/src/cel-engine.ts`(`compileCel` 与 `celFaultRange` 的「只在读的是作者原文时给位置」判断)、`packages/formula/src/validate.ts`(`invalid-cel` 分支)、`packages/formula/src/expression-refusal.ts`(新字段的契约说明)。 - **两分钟验证**:`pnpm --filter @objectstack/formula exec vitest run src/cel-fault-kind-position.test.ts`,预期 15 个用例全绿。 - **需要你做的**:无,席位落地。 ## What changes - `invalid-cel` params (`ExpressionRefusalParams`) gain two **optional** members beside `role` and `detail`: - `celCode`: cel-js's own structured code for the fault, read off the error object (`ParseError#code` / `TypeError#code`), never out of its prose. Parser codes for a syntax fault (`unexpected_token`, `expected_token`, `unexpected_character`, `unterminated_string`, `invalid_exponent`, …), checker codes for a type fault (`no_such_overload`, …). - `pos`: `{ start, end }`, the range cel-js reports, as UTF-16 offsets into `source` with `end` exclusive. A missing `)` reads `start === end === source.length`. - `EvalError.pos`, declared and never filled, is now filled by `celEngine.compile` and `celEngine.evaluate` for a cel-js parse, check or evaluation fault that carries a range. - Neither member is ever guessed. Both are absent when the engine reported none (`expression_must_be_string`, a native throw such as `matches()`'s regex `SyntaxError`). `pos` is also absent when cel-js read a source the platform rewrote first (the #3306 null-guard ternary lowering, the #25267 indexed `has()` lowering, the evaluate-time temporal-equality rewrite): a range into reserialized text would point at characters the author never wrote. - The code reaches `validateExpression` through a package-internal `compileCel(source)`, which `celEngine.compile` now delegates to. It is not re-exported from the package entry, and the code is kept off `EvalError` on purpose: `EvalError` reaches logs and error bodies, where a second `code` beside an ADR-0112 `error.code` would read as one. - `expression-refusal.ts`'s contract gains one clause: a LOCATOR param is a structured reading of the fault `detail` renders, the one exception to "params are the interpolated values". The refusal's `message` is byte-identical, and every other code (`cel-too-large`, `cel-method-call`, `cel-unknown-function`, `cel-template-brace`, …) keeps its params unchanged. Adding `pos` to those four from the same read would be one line each; it is left out because no reader asks for it yet. ## Tests - New `packages/formula/src/cel-fault-kind-position.test.ts`, 15 cases: - five parse-fault kinds through `validateExpression`: unexpected end (`record.rating >=`), missing `)` (`(record.a > 1 && record.b < 2`), unexpected character mid-source (`record.a # 1`), stray `)` (`record.a > 1 )`), unterminated string (`record.a == 'abc`). Each pins `celCode`, `pos`, the text `source.slice(start, end)` points at (`#`, `)`, `'abc`, or empty at `source.length`), and the message byte for byte as our words around the engine's pass-through `detail`; - a type fault (`1 + 'a'` → `no_such_overload`, `{0, 7}`); - absence over a rewritten source, with a positive control that the source really is rewritten; - `EvalError.pos` on `compile` (parse, missing `)`, check, rewritten → absent) and on `evaluate` (parse, runtime `No such key` pinned to `missing`, rewritten → absent, native throw → absent). - **Red on base** `9da2f4b0`: 11 failed, 2 passed (the two absence controls hold on base by construction). - `expression-refusal-codes.test.ts`: the two `invalid-cel` pins now include the new members (that table pins params exactly), and the sweep judges the locator params by what they are (`invalid-cel` only, a snake_case cel-js code, a range inside the source) instead of by message containment. <details><summary>Ablation (4 legs, each red, each restored to the HEAD blob with an empty diff against HEAD)</summary> All legs through `node scripts/ablation-replace.mjs` (anchor hit 1 → 0 on disk); formula tests import `./validate` / `./cel-engine` from `src/`, so no rebuild applies. | Leg | Mutation | Result | |---|---|---| | 1 | `invalid-cel` params back to `{ role: celRole, detail }` | 8 failed (7 new + `invalid-cel` pin) | | 2 | `classifyError` never fills `pos` | 9 failed | | 3 | compile check path drops the `parsed === source` guard | 2 failed (both rewritten-source absence cases) | | 4 | evaluate drops the `readSource === source` guard | 1 failed (evaluate rewritten-source case) | </details> <details><summary>Suites on head ddf2020</summary> - `@objectstack/formula` full suite: Test Files 61 passed, Tests 1578 passed; `pnpm typecheck` (incl. `check:test-typecheck`) exit 0; `pnpm build` exit 0. - `ExpressionRefusalParams` importers outside `packages/formula`: none (`git grep`, only a CHANGELOG line). - `EvalError` importer `@objectstack/objectql` (downstream): Test Files 513 passed, Tests 9903 passed. - The ui reader that acts on `invalid-cel` (`celAuthoring.ts` key map) and every ui test naming `celAuthoring` / `invalidCel`: Test Files 51 passed, Tests 470 passed, against the rebuilt formula `dist/`. - `eslint --no-inline-config packages/formula`: exit 0. </details> ## Acceptance notes - **Changeset level.** The dispatch order asked for `minor`; the changeset is `patch`. AGENTS.md ties the level to the `Metadata-protocol` kind alone, and `check-changeset-no-major.mjs` refuses `none` beside a `minor`. Nothing a metadata author writes changes here, so `none` + `patch`. - **The ui's plain-word mapping and caret** are the reader that acts on these members (Studio/Setup CEL editors, `celAuthoring.ts`); that is ui-lane work and out of this diff. - cel-js names which token it expected (`Expected RPAREN, got EOF`) only in prose; the structured `celCode` is `expected_token` without the token. An editor that wants to say "a closing bracket" specifically has `pos` (end of source) and `detail`; a structured expected token would need cel-js upstream. ## Verification Gate list from `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on head `ddf2020b` (60 commands), each run with its exit code captured before any pipe, then reconciled with `--ran`: **60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN**, every one exit 0. Three families first answered exit 3 (prerequisite not met, nothing measured) and were re-run after their prerequisite: `check-plugin-teardown-shape.mjs --self-test` (fixture commit fetched), `check:lean-entry-closure` and `check:dual-build-cjs-loads` (dists built). All three exit 0 on the re-run. `pnpm lint` scoped to `packages/formula`: exit 0. The repo-wide farm and `test:repo` belong to CI; read the PR's check-runs. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nkzMdFu8KxmmxsudB)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent f5a400a commit c946cb9

7 files changed

Lines changed: 408 additions & 41 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/formula': patch
3+
---
4+
5+
feat(formula): a CEL syntax refusal keeps the engine's own fault code and the source range it points at, so an editor can say what is wrong and where in its own words (#27904)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`validateExpression`'s catch-all CEL refusal, `invalid-cel`, now carries two optional params beside `role` and `detail`:
11+
12+
- `celCode` — cel-js's own code for the fault, read off its error object: the parser's for a syntax fault (`unexpected_token` when an operand or the end is missing, `expected_token` when a closing `)` / `]` / `}` is missing, `unexpected_character`, `unterminated_string`, …) or the checker's for a type fault (`no_such_overload`, …).
13+
- `pos` — `{ start, end }`, the UTF-16 offsets into `source` that the engine points at (`end` exclusive). A missing `)` reads `start === end === source.length`.
14+
15+
Both are absent when the engine reported none, and `pos` is absent when the engine read a source the platform rewrote first (the null-guard ternary lowering), since a range into that text would point at characters the author never wrote. A guessed `0` is never written.
16+
17+
`EvalError.pos`, declared before and never filled, is now filled by `celEngine.compile` and `celEngine.evaluate` for a cel-js parse, check or evaluation fault that carries a range, on the same terms.
18+
19+
The refusal's `message` is unchanged, byte for byte, and so are the codes and the params of every other refusal.

‎packages/formula/src/cel-engine.ts‎

Lines changed: 104 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* third-party plugins can't ship runaway predicates.
1414
*/
1515

16-
import { Environment, EvaluationError, ParseError, serialize } from '@marcbachmann/cel-js';
16+
import { Environment, EvaluationError, ParseError, TypeError as CelTypeError, serialize } from '@marcbachmann/cel-js';
1717
import type { ASTNode } from '@marcbachmann/cel-js';
1818
import type { Expression } from '@objectstack/spec';
1919

@@ -1969,44 +1969,112 @@ function classifyCelFault(err: unknown): 'parse' | 'bounds' | 'type' | 'runtime'
19691969
*
19701970
* All four are one native regex-compilation failure, i.e. `runtime`.
19711971
*/
1972-
function classifyError(err: unknown): EvalResult<never> {
1972+
function classifyError(err: unknown, located = false): EvalResult<never> {
19731973
const message = err instanceof Error ? err.message : String(err);
19741974
const kind = classifyCelFault(err) ?? 'runtime';
1975-
return { ok: false, error: { kind, message } };
1975+
const pos = located ? celFaultRange(err) : undefined;
1976+
return { ok: false, error: pos ? { kind, message, pos } : { kind, message } };
1977+
}
1978+
1979+
/** A cel-js error object — the three classes that carry a structured `code` and `range`. */
1980+
function isCelFault(err: unknown): err is ParseError | EvaluationError | CelTypeError {
1981+
return err instanceof ParseError || err instanceof EvaluationError || err instanceof CelTypeError;
1982+
}
1983+
1984+
/**
1985+
* The source range a cel-js fault points at (#27904): `ParseError#range` /
1986+
* `TypeError#range` / `EvaluationError#range`, UTF-16 offsets into the text
1987+
* cel-js read, `end` exclusive. `undefined` when the fault carries none
1988+
* (`expression_must_be_string`, a non-cel-js throw) — ⛔ never a guessed `0`.
1989+
*
1990+
* The offsets are into the text cel-js READ. Every caller passes `located`
1991+
* only when that text is the author's source byte for byte; a source the
1992+
* #3306 / #25267 rewrites reserialized is read as different text, and a range
1993+
* into it would point the author at characters they never wrote.
1994+
*/
1995+
function celFaultRange(err: unknown): { start: number; end: number } | undefined {
1996+
if (!isCelFault(err)) return undefined;
1997+
const range = err.range;
1998+
if (!range || !Number.isInteger(range.start) || !Number.isInteger(range.end)) return undefined;
1999+
return { start: range.start, end: range.end };
2000+
}
2001+
2002+
/**
2003+
* cel-js's own structured code for a fault (#27904) — the parser's
2004+
* (`unexpected_token`, `expected_token`, `unexpected_character`,
2005+
* `unterminated_string`, …) or the checker's (`no_such_overload`, …). Read off
2006+
* the error object, never off its prose (the #6223 rule {@link classifyCelFault}
2007+
* follows); `undefined` for anything that is not a cel-js error.
2008+
*/
2009+
function celFaultCode(err: unknown): string | undefined {
2010+
if (!isCelFault(err)) return undefined;
2011+
return typeof err.code === 'string' && err.code ? err.code : undefined;
2012+
}
2013+
2014+
/**
2015+
* What {@link compileCel} answers: the {@link DialectEngine.compile} verdict,
2016+
* and beside it the engine's own fault code when it refused.
2017+
*
2018+
* Package-internal — `validateExpression` reads `celCode` into the
2019+
* `invalid-cel` refusal's params. Not on {@link EvalError}: that type reaches
2020+
* callers that put it into logs and error bodies, where a second `code` beside
2021+
* an ADR-0112 `error.code` would read as one.
2022+
*/
2023+
export interface CelCompileOutcome {
2024+
readonly result: EvalResult<unknown>;
2025+
/** cel-js's code for the fault ({@link celFaultCode}); absent on success or when it named none. */
2026+
readonly celCode?: string;
2027+
}
2028+
2029+
/**
2030+
* {@link celEngine.compile}, keeping the fault's cel-js code (#27904).
2031+
* `celEngine.compile` is this with the code dropped, so the two can never
2032+
* disagree about a verdict.
2033+
*/
2034+
export function compileCel(source: string): CelCompileOutcome {
2035+
let parsed: string | undefined;
2036+
try {
2037+
// The canonical parse env (#24536): parsing and checking neither mutate
2038+
// it nor call the stdlib, so one environment serves every compile.
2039+
const env = (canonicalParseEnv ??= buildEnv(() => new Date(0)));
2040+
// #3306 — accept the null-guard idiom `cond ? <value> : null`, which cel-js's
2041+
// ternary unifier otherwise rejects (`int`/`double`/`string` ≠ `null`). Same
2042+
// rewrite the runtime uses, so build and eval agree on what is valid.
2043+
// #25267 — and `has()` over an indexed receiver, lowered the same way.
2044+
parsed = rewriteNullableTernary(rewriteIndexedHas(source));
2045+
const compiled = env.parse(parsed);
2046+
// Surface check errors eagerly. cel-js's `check()` returns a
2047+
// `TypeCheckResult` object (`{ valid, type?, error? }`) — NOT an array —
2048+
// so the type fault (including `found no matching overload for 'PRIOR(dyn)'`
2049+
// when a condition calls an UNKNOWN function) only surfaces when we read
2050+
// `valid === false`. The previous `Array.isArray(...)` guard never matched
2051+
// an object, so unknown-function predicates type-checked clean and were
2052+
// silently accepted by `objectstack build` / `registerFlow`, then no-op'd
2053+
// the flow at runtime (#1877). Reading the documented shape closes that.
2054+
const checkResult = compiled.check?.();
2055+
if (checkResult && checkResult.valid === false) {
2056+
const fault: unknown = checkResult.error;
2057+
const message = checkResult.error?.message ?? 'expression failed type checking';
2058+
// #27904 — the range only when the checker read the author's own text.
2059+
const pos = parsed === source ? celFaultRange(fault) : undefined;
2060+
const celCode = celFaultCode(fault);
2061+
return {
2062+
result: { ok: false, error: pos ? { kind: 'type', message, pos } : { kind: 'type', message } },
2063+
...(celCode ? { celCode } : {}),
2064+
};
2065+
}
2066+
return { result: { ok: true, value: compiled.ast } };
2067+
} catch (err) {
2068+
const celCode = celFaultCode(err);
2069+
return { result: classifyError(err, parsed === source), ...(celCode ? { celCode } : {}) };
2070+
}
19762071
}
19772072

19782073
export const celEngine: DialectEngine = {
19792074
dialect: 'cel',
19802075

19812076
compile(source: string): EvalResult<unknown> {
1982-
try {
1983-
// The canonical parse env (#24536): parsing and checking neither mutate
1984-
// it nor call the stdlib, so one environment serves every compile.
1985-
const env = (canonicalParseEnv ??= buildEnv(() => new Date(0)));
1986-
// #3306 — accept the null-guard idiom `cond ? <value> : null`, which cel-js's
1987-
// ternary unifier otherwise rejects (`int`/`double`/`string` ≠ `null`). Same
1988-
// rewrite the runtime uses, so build and eval agree on what is valid.
1989-
// #25267 — and `has()` over an indexed receiver, lowered the same way.
1990-
const compiled = env.parse(rewriteNullableTernary(rewriteIndexedHas(source)));
1991-
// Surface check errors eagerly. cel-js's `check()` returns a
1992-
// `TypeCheckResult` object (`{ valid, type?, error? }`) — NOT an array —
1993-
// so the type fault (including `found no matching overload for 'PRIOR(dyn)'`
1994-
// when a condition calls an UNKNOWN function) only surfaces when we read
1995-
// `valid === false`. The previous `Array.isArray(...)` guard never matched
1996-
// an object, so unknown-function predicates type-checked clean and were
1997-
// silently accepted by `objectstack build` / `registerFlow`, then no-op'd
1998-
// the flow at runtime (#1877). Reading the documented shape closes that.
1999-
const checkResult = compiled.check?.();
2000-
if (checkResult && checkResult.valid === false) {
2001-
return {
2002-
ok: false,
2003-
error: { kind: 'type', message: checkResult.error?.message ?? 'expression failed type checking' },
2004-
};
2005-
}
2006-
return { ok: true, value: compiled.ast };
2007-
} catch (err) {
2008-
return classifyError(err);
2009-
}
2077+
return compileCel(source).result;
20102078
},
20112079

20122080
evaluate<T = unknown>(expr: Expression, ctx: EvalContext): EvalResult<T> {
@@ -2030,6 +2098,9 @@ export const celEngine: DialectEngine = {
20302098
}
20312099

20322100
const now = () => ctx.now ?? new Date();
2101+
// #27904 — the text cel-js read, so a fault's range is reported only when
2102+
// that text is the author's source (see `celFaultRange`).
2103+
let readSource: string | undefined;
20332104
try {
20342105
// Scope FIRST: `can` is answered about the acting subject by IDENTITY, and
20352106
// the subject is the canonical `EvalUser` object `buildScope` mints. The
@@ -2064,6 +2135,7 @@ export const celEngine: DialectEngine = {
20642135
// which cel-js's macro refuses, to the spec-equivalent `e.?f.hasValue()`.
20652136
// All rewrites are no-ops for sources that don't need them.
20662137
const evalSource = rewriteNullableTernary(rewriteTemporalEquality(rewriteIndexedHas(source), scope));
2138+
readSource = evalSource;
20672139
try {
20682140
const raw = runProgram(env, evalSource, scope);
20692141
return { ok: true, value: coerce(raw) as T };
@@ -2096,7 +2168,7 @@ export const celEngine: DialectEngine = {
20962168
activeBinding = outer;
20972169
}
20982170
} catch (err) {
2099-
return classifyError(err);
2171+
return classifyError(err, readSource === source);
21002172
}
21012173
},
21022174
};

0 commit comments

Comments
 (0)