refactor(formula,lint): parseCelToAst 成为唯一的 CEL 解析入口 (#4812)

.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` —— 这正是第二个解析入口
+的成因。

.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),已加测试钉住。

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'`,

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';

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<string, string> = {
+  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);
+  });
+});

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:*",