diff --git a/.changeset/formula-canonical-parse-entry.md b/.changeset/formula-canonical-parse-entry.md new file mode 100644 index 0000000000..302305dc67 --- /dev/null +++ b/.changeset/formula-canonical-parse-entry.md @@ -0,0 +1,18 @@ +--- +'@objectstack/formula': minor +--- + +新增规范 parse-to-AST 入口 `parseCelToAst(source)`,并 re-export AST 节点类型 `CelAstNode`(#4812)。 + +`parseCelToAst` 与 `compile` / `evaluate` / `collectCelRootIdentifiers` 共用同一条前端链路 +——#3306 的 `rewriteNullableTernary` 重写、`DEFAULT_LIMITS` 边界、以及注册了 stdlib 的 +`unlistedVariablesAreDyn: true` 环境 —— 因此全仓对「什么能解析」只有一个答案。此前消费方 +若自建 `new Environment(...)`,拿到的是一份**不带 limits** 的答案:它会解析、并进而推理 +`compile()` 直接拒绝的表达式。 + +`parseCelToAst` 只做 parse,不做 check(后者是 `compile()` 的职责):解析成功但类型检查失败的 +表达式(大量 `dyn` 操作数的谓词即是)仍然会拿到 AST。解析失败返回 `null` 而不抛错。 + +`CelAstNode` 的 re-export 补上了一个既有缺口:`lowerCelAst` 一直接收 cel-js 的 `ASTNode`, +而该类型从未导出,消费方只能越过本包直接依赖 `@marcbachmann/cel-js` —— 这正是第二个解析入口 +的成因。 diff --git a/.changeset/lint-null-guards-canonical-parse.md b/.changeset/lint-null-guards-canonical-parse.md new file mode 100644 index 0000000000..a72733e395 --- /dev/null +++ b/.changeset/lint-null-guards-canonical-parse.md @@ -0,0 +1,17 @@ +--- +'@objectstack/lint': patch +--- + +null-guard 闸门改走 `@objectstack/formula` 的规范解析入口,并移除对 `@marcbachmann/cel-js` +的直接依赖(#4812)。 + +`validate-null-guards.ts` 此前自建了一个**不带 limits** 的 cel-js `Environment`,于是它会解析、 +并进而判定平台自身拒绝的谓词 —— 超过 `maxAstNodes` (256) / `maxDepth` (32) / +`maxListElements` (64) 的表达式在 lint 侧照常出 finding,在 `compile()` 侧却是 +`Exceeded max…`。两个解析入口,两个答案,而这个闸门握着更宽松的那个。 + +改走 `parseCelToAst` 后两者合一。超界表达式不再由本闸门二次判定,而是交还给同一批调用点上 +本就在跑的 `validateExpression` —— 它以 blocking error 报告边界错误,措辞面向自纠;作者修好 +边界问题后,null-guard 判定自然回来。规则判定本身没有变化:#3306 的三元重写对本 pass 是 +verdict-neutral(重写仅在三元的某一支恰为 `null` 字面量时触发,而该支本就证明不出任何 +guard),已加测试钉住。 diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index 2e595cafeb..0544838e28 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -14,6 +14,7 @@ */ import { Environment, serialize } from '@marcbachmann/cel-js'; +import type { ASTNode } from '@marcbachmann/cel-js'; import type { Expression } from '@objectstack/spec'; import { buildScope, registerNumericCoercions, registerStdLib } from './stdlib'; @@ -177,6 +178,78 @@ export function collectCelRootIdentifiers( } } +/** + * A parsed CEL AST node, re-exported so a consumer can name the type this + * package already hands it without importing `@marcbachmann/cel-js` itself. + * + * The alias is not cosmetic. {@link lowerCelAst} has always *taken* a cel-js + * `ASTNode` while the type stayed unexported, so every caller that wanted to + * hold an AST had to reach past this package to the parser — which is precisely + * how a second, differently-configured parse entry gets built (#4812). Prefixed + * `Cel` to match the package's other CEL-domain public names + * (`CelFilterCompileResult`, `collectCelRootIdentifiers`, `isPushdownableCel`); + * bare `ASTNode` would be ambiguous in a package that also owns the cron and + * template dialects. + */ +export type CelAstNode = ASTNode; + +/** + * The canonical parse env. Identical in configuration to the one + * {@link celEngine.compile} builds per call — same `unlistedVariablesAreDyn`, + * same `enableOptionalTypes`, same {@link DEFAULT_LIMITS}, same stdlib — and + * built once because `parse` neither mutates the environment nor depends on the + * `now()` it was given (the same reasoning `recordScopeEnv` already relies on). + * The parity suite pins the equivalence against a freshly-built env, so this + * memo cannot silently drift away from `compile`. + */ +let canonicalParseEnv: Environment | undefined; + +/** + * Parse a CEL source to its AST through the **canonical** front end — the one + * answer in this repo to "what parses" (#4812). + * + * Every other entry point in this package (`compile`, `evaluate`, + * {@link collectCelRootIdentifiers}) reaches the parser through the same three + * things, and so does this one: + * + * 1. {@link rewriteNullableTernary} — the #3306 `cond ? value : null` rewrite, + * so the AST a consumer analyses is the AST the runtime will execute, not + * the shape the author happened to type; + * 2. {@link DEFAULT_LIMITS} — the platform's bounds. A source over + * `maxAstNodes` / `maxDepth` / `maxListElements` does **not** parse here, + * because it does not parse anywhere else on the platform either; + * 3. the registered stdlib and `unlistedVariablesAreDyn: true` env. + * + * A consumer that built its own `new Environment(...)` instead got a different + * answer to (2) in particular — it would happily parse, and then reason about, + * a predicate `compile()` rejects outright. That is not a hypothetical: it is + * what `@objectstack/lint`'s null-guard pass did until #4812. + * + * Returns `null` — never throws — when the source is empty or does not parse, + * so a caller whose job is *not* to adjudicate syntax can skip it in one line + * and leave the verdict to the gate that owns it (`validateExpression`, which + * reports both the syntax fault and the bounds fault with a message written for + * self-correction). + * + * This is `parse` only, deliberately **not** `parse + check`: `compile()` is the + * entry that also type-checks. A caller that wants the AST of an expression + * which parses but does not type-check (a great many predicates over `dyn` + * operands) must not be denied it, and a caller that wants the type verdict + * should ask `compile()` for it. The parity suite pins both halves of that + * asymmetry so neither side drifts. + */ +export function parseCelToAst(source: string): CelAstNode | null { + if (typeof source !== 'string' || !source.trim()) return null; + try { + // A wall-clock-free `now()` — the stdlib is registered for parse-time shape + // only and is never called on this path. + canonicalParseEnv ??= buildEnv(() => new Date(0)); + return canonicalParseEnv.parse(rewriteNullableTernary(source)).ast; + } catch { + return null; + } +} + /** * The result type cel-js's type-checker infers for a `value`/`predicate` * expression — its raw CEL type name (`'int'`, `'double'`, `'string'`, `'bool'`, diff --git a/packages/formula/src/index.ts b/packages/formula/src/index.ts index f88eebc92a..58becc8bd0 100644 --- a/packages/formula/src/index.ts +++ b/packages/formula/src/index.ts @@ -15,6 +15,12 @@ export { celEngine, DEFAULT_LIMITS } from './cel-engine'; // (approval `expression` approvers): lint and the runtime pre-check share this // one helper so what they accept can never drift. export { collectCelRootIdentifiers } from './cel-engine'; +// #4812 — the canonical parse-to-AST entry. Any consumer that needs the AST of +// an authored CEL source takes it from here, so "what parses" has exactly ONE +// answer across build, lint and runtime. Building a private `new Environment()` +// instead silently opts out of the platform's rewrite AND its bounds. +export { parseCelToAst } from './cel-engine'; +export type { CelAstNode } from './cel-engine'; export { cronEngine } from './cron-engine'; export { templateEngine, TEMPLATE_FORMATTERS, formatValue } from './template-engine'; export { registerStdLib, buildScope } from './stdlib'; diff --git a/packages/formula/src/parse-cel-to-ast.test.ts b/packages/formula/src/parse-cel-to-ast.test.ts new file mode 100644 index 0000000000..7173d089dc --- /dev/null +++ b/packages/formula/src/parse-cel-to-ast.test.ts @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4812 — `parseCelToAst` is the canonical parse-to-AST entry: the ONE answer in +// this repo to "what parses". These tests pin it against `celEngine.compile`, +// which is the entry the runtime actually uses, in both directions — what both +// accept, what both reject, and the one asymmetry that is deliberate. +// +// They also pin the two things a consumer silently opts out of by building its +// own `new Environment(...)` instead: the #3306 nullable-ternary rewrite, and +// `DEFAULT_LIMITS`. `@objectstack/lint`'s null-guard pass did exactly that until +// #4812 — and it was the *bounds* it diverged on, so a bare-env comparison is +// asserted here rather than described. + +import { Environment } from '@marcbachmann/cel-js'; +import { describe, expect, it } from 'vitest'; + +import { celEngine, parseCelToAst, DEFAULT_LIMITS } from './cel-engine'; + +/** + * A bare cel-js environment — byte-for-byte the one `validate-null-guards.ts` + * built for itself before #4812, and the shape any consumer naturally reaches + * for. Its only difference from the canonical env is what it does NOT carry: + * no `limits`, no stdlib, no rewrite. + */ +const bareEnv = new Environment({ unlistedVariablesAreDyn: true, enableOptionalTypes: true }); +const bareParses = (source: string): boolean => { + try { + bareEnv.parse(source); + return true; + } catch { + return false; + } +}; + +/** Sources the platform accepts — drawn from the shapes this package's own suites use. */ +const ACCEPTED = [ + 'record.amount > 1000', + 'record.end_date < record.start_date', + 'record.start_date != null && record.end_date != null && record.end_date < record.start_date', + 'has(record.start_date) && record.start_date < record.end_date', + '"manager" in os.user.positions', + 'record.rating >= 4', + 'record.end_date <= daysFromNow(60)', + 'daysBetween(record.start_date, record.end_date) + 1', + 'record.budget == null || record.budget > 100', + '!isBlank(record.owner) && record.owner != record.creator', + 'record.status == "open" ? record.amount * 0.1 : null', + 'true ? 5 : null', + 'size(record.items) > 0', + '[1, 2, 3].all(x, x > 0)', + 'record.?owner.orValue("none") == "none"', +]; + +/** Sources the platform rejects at PARSE — syntax faults, in both entries. */ +const SYNTAX_REJECTED = [ + 'record.budget >', + 'record.a $$ 1', + 'record.a ?? 3', // cel-js 8 has no `??` + '((record.a)', +]; + +/** + * The comparable content of a cel-js AST: `op` plus `args`, recursively. + * + * A raw deep-equal cannot be used here. `compile()` runs cel-js's `check()`, + * which decorates every node IN PLACE with an entire evaluation plan — measured + * on `record.amount > 1000`, the root gains `left`, `right`, `candidates`, + * `handle` (a bound function), `rightStaticType` and `checkedType`, and + * `candidates.registry` points back at the Environment, so the decorated tree is + * circular. `parseCelToAst` is parse-only and carries none of it. + * + * `op` + `args` is also exactly the surface every AST consumer in this repo + * walks (`lowerCelAst`, `collectCelRootIdentifiers`, lint's null-guard pass), so + * equality on this projection is the claim that matters: the two entries hand a + * consumer the same tree. + */ +function shapeOf(value: unknown): unknown { + if (Array.isArray(value)) return value.map(shapeOf); + if (value && typeof value === 'object' && typeof (value as { op?: unknown }).op === 'string') { + return { op: (value as { op: string }).op, args: shapeOf((value as { args: unknown }).args) }; + } + // Leaves: identifier / member / function-name strings and literal values + // (including the BigInts cel-js produces for `int`). + return value; +} + +/** Sources that parse but do NOT type-check — the deliberate asymmetry. */ +const PARSES_BUT_FAILS_CHECK = [ + '1 + "x"', + 'NOPE(record.a) > 1', + 'record.a.NOPE()', +]; + +/** Over the platform's bounds — the divergence a bare env does not see. */ +const OVER_BOUNDS: Record = { + maxAstNodes: Array.from({ length: 300 }, (_, i) => `record.f${i}`).join(' + '), + maxDepth: '('.repeat(60) + 'record.a' + ')'.repeat(60), + maxListElements: `[${Array.from({ length: 200 }, (_, i) => i).join(',')}].size() > 0`, +}; + +describe('parseCelToAst — parity with celEngine.compile (#4812)', () => { + it.each(ACCEPTED)('returns the SAME ast compile() returns: %s', (source) => { + const compiled = celEngine.compile(source); + expect(compiled.ok).toBe(true); + const ast = parseCelToAst(source); + expect(ast).not.toBeNull(); + // `compile()` builds a FRESH env per call; `parseCelToAst` memoizes one. + // Deep equality here is what pins that memo as equivalent — if the two ever + // drift (a rewrite applied on one side, a limit on the other), this fails. + expect(shapeOf(ast)).toEqual( + shapeOf(compiled.ok ? compiled.value : undefined), + ); + }); + + it.each(SYNTAX_REJECTED)('rejects exactly what compile() rejects at parse: %s', (source) => { + // The parity claim is the accept/reject verdict itself. `compile`'s error + // *classification* is asserted separately below — cel-js does not phrase + // every syntax fault the same way, and `classifyError` reads the phrasing. + expect(parseCelToAst(source)).toBeNull(); + expect(celEngine.compile(source).ok).toBe(false); + }); + + it('yields a plain parse tree, while compile() yields a type-ANNOTATED one', () => { + // The concrete difference between "parse" and "parse + check", pinned so the + // two entries are not mistaken for interchangeable. A consumer walking + // `.op`/`.args` sees the same tree from either; only `compile()` has run the + // type checker over it. + const source = 'record.amount > 1000'; + const compiled = celEngine.compile(source); + expect(compiled.ok).toBe(true); + expect(compiled.ok && (compiled.value as { checkedType?: unknown }).checkedType).toBeDefined(); + expect((parseCelToAst(source) as unknown as { checkedType?: unknown }).checkedType).toBeUndefined(); + }); + + it('classifies the common syntax fault as `parse`', () => { + const compiled = celEngine.compile('record.budget >'); + expect(compiled.ok).toBe(false); + if (!compiled.ok) expect(compiled.error.kind).toBe('parse'); + // NOT asserted for `((record.a)`: cel-js phrases an unbalanced delimiter as + // `Expected RPAREN, got EOF`, which `classifyError`'s + // /parse|unexpected|syntax/i does not match, so a genuine syntax fault is + // reported to the author as `runtime`. Pre-existing, out of scope for #4812, + // filed separately — asserting it here would enshrine it. + }); + + it.each(PARSES_BUT_FAILS_CHECK)( + 'still yields an AST for a source that parses but fails check(): %s', + (source) => { + // The asymmetry is deliberate and load-bearing: `parseCelToAst` is parse + // ONLY, `compile()` is parse + check. A consumer analysing an AST (the + // null-guard pass, the pushdown compiler) must not be denied one just + // because cel-js cannot type an expression over `dyn` operands — and a + // caller who wants the type verdict asks `compile()`. Asserted so that + // nobody "tightens" this entry into a second compile(). + expect(parseCelToAst(source)).not.toBeNull(); + const compiled = celEngine.compile(source); + expect(compiled.ok).toBe(false); + if (!compiled.ok) expect(compiled.error.kind).toBe('type'); + }, + ); + + it('never throws, and answers null for an empty source', () => { + expect(parseCelToAst('')).toBeNull(); + expect(parseCelToAst(' ')).toBeNull(); + expect(parseCelToAst(undefined as unknown as string)).toBeNull(); + expect(parseCelToAst(null as unknown as string)).toBeNull(); + }); +}); + +describe('parseCelToAst — carries the #3306 nullable-ternary rewrite', () => { + // `true ? 5 : null` is the specimen: it PARSES in any env, but cel-js's + // ternary unifier rejects it at check ("Ternary branches must have the same + // type, got 'int' and 'null'"), so without the rewrite the blessed + // `guard ? value : null` shape does not compile. The rewrite wraps the + // non-null branch in `dyn(...)`, which is what makes it legal — and it is the + // AST the runtime executes. An entry that skipped the rewrite would hand a + // consumer a DIFFERENT tree from the one the platform runs. + it('wraps the non-null branch in dyn(...), matching what the runtime executes', () => { + const ast = parseCelToAst('true ? 5 : null') as unknown as { + op: string; + args: [unknown, { op: string; args: [string, unknown[]] }, unknown]; + }; + expect(ast).not.toBeNull(); + expect(ast.op).toBe('?:'); + expect(ast.args[1].op).toBe('call'); + expect(ast.args[1].args[0]).toBe('dyn'); + }); + + it('agrees with compile() on the rewritten shape, which only compile() could evaluate', () => { + const source = 'record.status == "open" ? record.amount * 0.1 : null'; + const compiled = celEngine.compile(source); + expect(compiled.ok).toBe(true); + expect(shapeOf(parseCelToAst(source))).toEqual( + shapeOf(compiled.ok ? compiled.value : undefined), + ); + }); + + it('cannot change WHETHER a source parses — only the AST it yields', () => { + // Stated as a test because it is the fact that makes #4812's originally + // suspected hole ("formula rewrites something bare cel-js cannot parse, so + // lint silently skips it") impossible by construction: the rewrite parses + // the source FIRST and returns it unchanged on failure. Every source below + // therefore gets the same accept/reject verdict from both entries. + for (const source of [...ACCEPTED, ...PARSES_BUT_FAILS_CHECK]) { + expect(bareParses(source)).toBe(true); + expect(parseCelToAst(source)).not.toBeNull(); + } + for (const source of SYNTAX_REJECTED) { + expect(bareParses(source)).toBe(false); + expect(parseCelToAst(source)).toBeNull(); + } + }); +}); + +describe('parseCelToAst — carries the platform bounds (the real #4812 divergence)', () => { + it.each(Object.entries(OVER_BOUNDS))( + 'refuses a source over %s, which a bare env happily parses', + (_limit, source) => { + // This is the measured divergence, in the direction it actually runs: a + // consumer with its own limitless env parses — and then reasons about — a + // predicate the platform rejects outright. Both halves asserted, so the + // test states the divergence rather than merely benefiting from its fix. + expect(bareParses(source)).toBe(true); + expect(parseCelToAst(source)).toBeNull(); + + const compiled = celEngine.compile(source); + expect(compiled.ok).toBe(false); + if (!compiled.ok) { + expect(compiled.error.kind).toBe('bounds'); + expect(compiled.error.message).toMatch(/Exceeded max/i); + } + }, + ); + + it('pins the bounds the entry enforces to DEFAULT_LIMITS', () => { + // If DEFAULT_LIMITS moves, the fixtures above must move with it; this + // assertion is the tripwire that says so out loud. + expect(DEFAULT_LIMITS.maxAstNodes).toBe(256); + expect(DEFAULT_LIMITS.maxDepth).toBe(32); + expect(DEFAULT_LIMITS.maxListElements).toBe(64); + }); +}); diff --git a/packages/lint/package.json b/packages/lint/package.json index fc0be6b722..bde78e6382 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -26,7 +26,6 @@ "check:doc-formula-expressions": "node scripts/check-doc-formula-expressions.mjs --self-test && node scripts/check-doc-formula-expressions.mjs" }, "dependencies": { - "@marcbachmann/cel-js": "^8.0.0", "@objectstack/formula": "workspace:*", "@objectstack/sdui-parser": "workspace:*", "@objectstack/spec": "workspace:*", diff --git a/packages/lint/src/validate-null-guards.test.ts b/packages/lint/src/validate-null-guards.test.ts index 3315f0e665..a575108b62 100644 --- a/packages/lint/src/validate-null-guards.test.ts +++ b/packages/lint/src/validate-null-guards.test.ts @@ -6,6 +6,8 @@ import { describe, it, expect } from 'vitest'; +import { celEngine, parseCelToAst } from '@objectstack/formula'; + import { findUnguardedNullableOperands, nullGuardMessage, @@ -114,6 +116,98 @@ describe('findUnguardedNullableOperands — what stays legal', () => { }); }); +describe('findUnguardedNullableOperands — one answer to "what parses" (#4812)', () => { + // Until #4812 this module built its own bare cel-js `Environment`, with no + // `limits`. It therefore parsed — and adjudicated — predicates the platform + // rejects outright, holding the MORE permissive of two answers to "what can + // be parsed". Routing through `@objectstack/formula`'s `parseCelToAst` makes + // the two answers one. + // + // Note the direction, because #4812's issue body guessed the other one: the + // hole was never "formula accepts something lint cannot parse, so lint + // silently skips it". That is impossible by construction — `rewriteNullableTernary` + // parses first and returns the source unchanged on failure, so it can never + // make an unparseable source parseable (pinned in + // `packages/formula/src/parse-cel-to-ast.test.ts`). The real divergence ran + // the opposite way, and it was the bounds. + + /** + * `record.budget > 100 && record.f0 + record.f1 + … + record.f299 > 0`. + * + * Two properties, and BOTH are load-bearing: + * - it is over `DEFAULT_LIMITS.maxAstNodes` (256) — 300 member accesses alone + * are 600 nodes — so the canonical entry will not parse it; + * - `record.budget > 100` is an ordering operator over an UNGUARDED nullable + * declared field, so a parse that DOES succeed yields exactly one finding. + * + * The second property is what makes the assertion below mean anything. A + * first draft of this fixture guarded budget (`record.budget != null && …`) + * and returned `[]` under both parses — passing because nothing was produced + * rather than because the boundary moved. Reverse-verification caught it. + */ + const overBounds = + 'record.budget > 100 && ' + + Array.from({ length: 300 }, (_, i) => `record.f${i}`).join(' + ') + + ' > 0'; + + it('defers an over-bounds predicate to the gate that owns that verdict', () => { + // The canonical entry cannot parse it — because the platform cannot either. + expect(parseCelToAst(overBounds)).toBeNull(); + expect(find(overBounds)).toEqual([]); + }); + + it('would have judged that very predicate under the old limitless parse', () => { + // The other half of the red/green line, asserted rather than asserted-about: + // shrink the same shape to something within bounds and the unguarded + // `record.budget > 100` IS reported. So the `[]` above is the bounds + // boundary moving, not an empty walk over a tree with nothing in it. + const sameShapeWithinBounds = + 'record.budget > 100 && ' + + Array.from({ length: 10 }, (_, i) => `record.f${i}`).join(' + ') + + ' > 0'; + expect(parseCelToAst(sameShapeWithinBounds)).not.toBeNull(); + expect(find(sameShapeWithinBounds).map((f) => f.operand)).toEqual(['record.budget']); + }); + + it('loses no coverage doing so — the bounds gate speaks, and loudly', () => { + // The silence above is only correct because `validateExpression` runs at the + // SAME call sites (`validate-expressions.ts` calls `check()` alongside + // `checkNullGuards()`) and rejects this source with a blocking error. A + // second, quieter finding about null guards on a predicate that can never be + // published would be noise; once the author fixes the bounds fault the + // null-guard verdict comes back. + const compiled = celEngine.compile(overBounds); + expect(compiled.ok).toBe(false); + if (!compiled.ok) { + expect(compiled.error.kind).toBe('bounds'); + expect(compiled.error.message).toMatch(/Exceeded maxAstNodes/i); + } + }); + + it('still judges the same predicate once it is within bounds', () => { + // Same shape, small enough to parse: the verdict is unchanged, so the only + // thing #4812 moved is where the boundary sits — not what the rule decides. + const withinBounds = 'record.f0 + record.f1 > 0 && record.budget > 100'; + expect(parseCelToAst(withinBounds)).not.toBeNull(); + expect(find(withinBounds).map((f) => f.operand)).toEqual(['record.budget']); + }); + + it('is verdict-neutral for the #3306 rewrite, which is why nothing else moved', () => { + // The canonical entry hands this pass a rewritten AST (`cond ? dyn(x) : null`). + // That cannot change a verdict here: the rewrite fires ONLY when exactly one + // ternary branch is the `null` literal, and `truthGuards`/`falseGuards` + // already prove nothing from a `null` branch — so the guard sets are + // identical before and after, and `dyn(...)` is transparent to the operand + // walk. Pinned on both outcomes rather than argued. + expect(find('record.budget != null ? record.budget - 1 : null')).toEqual([]); + // `record.owner` is not a declared nullable field here, so the condition + // contributes nothing and the dyn-wrapped branch is the only thing judged. + expect(find('record.owner == "acme" ? record.budget - 1 : null').map((f) => f.operand)).toEqual( + ['record.budget'], + ); + }); +}); + describe('nullGuardMessage', () => { it('names the rule, the operand and the `!= null` fix', () => { const [finding] = find('has(record.end_date) && record.end_date < 5'); diff --git a/packages/lint/src/validate-null-guards.ts b/packages/lint/src/validate-null-guards.ts index 952e69ad1f..554964a013 100644 --- a/packages/lint/src/validate-null-guards.ts +++ b/packages/lint/src/validate-null-guards.ts @@ -119,8 +119,8 @@ * field-existence / bare-reference verdicts, which it never did before. */ -import { Environment } from '@marcbachmann/cel-js'; -import type { ASTNode } from '@marcbachmann/cel-js'; +import { parseCelToAst } from '@objectstack/formula'; +import type { CelAstNode } from '@objectstack/formula'; /** * The corrective sentence, lifted **verbatim** from `unevaluableRuleError` in @@ -160,19 +160,9 @@ export interface NullGuardFinding { hasOnlyGuard: boolean; } -// A check-only parse environment: no evaluation, no stdlib needed, every -// identifier stays `dyn` so any authored predicate parses. Built once. -let parseEnv: Environment | undefined; -function getParseEnv(): Environment { - if (!parseEnv) { - parseEnv = new Environment({ unlistedVariablesAreDyn: true, enableOptionalTypes: true }); - } - return parseEnv; -} - type AnyNode = { op?: string; args?: unknown }; -function isNode(v: unknown): v is AnyNode & ASTNode { +function isNode(v: unknown): v is AnyNode & CelAstNode { return !!v && typeof v === 'object' && typeof (v as AnyNode).op === 'string'; } @@ -332,8 +322,26 @@ function collectHasOperands(node: unknown, roots: readonly string[], out: Set(); collectHasOperands(ast, roots, hasOperands); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9972cc22c4..cd5780d047 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1003,9 +1003,6 @@ importers: packages/lint: dependencies: - '@marcbachmann/cel-js': - specifier: ^8.0.0 - version: 8.0.0 '@objectstack/formula': specifier: workspace:* version: link:../formula