fix(formula): has() over an indexed or call receiver evaluates as the CEL spec defines it

.changeset/25267-cel-has-indexed-receiver.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/formula': minor
+---
+
+fix(formula): `has()` over a selection whose receiver is an index or a call result evaluates as a presence test, as the CEL spec defines it
+
+Clause-②: yes (widening)
+Metadata-protocol: added
+
+The CEL spec defines `has(e.f)` for any expression `e`. The engine used to refuse every such argument whose selection chain was not rooted in a plain name — `has(data.environments[0].storage.usedMb)`, `has(record.items[0].name)`, `has(dyn(x).y)` — with `has() invalid argument`, at evaluation and at `compile` / `validateExpression` alike. A page or view `visibleWhen`, whose publish gate checks syntax but not types, published such a predicate and then failed open in the console.
+
+- **What now works.** The call is a presence test on the last field: `true` when the receiver has the key, `false` when it does not, and a fault (`No such key`) when a step before the last one is absent, exactly like `has(record.a.b)`. `evaluate`, `compile`, `validateExpression`, `firstUndeclaredReference` and `inferCelType` all give that one answer.
+- **What widens.** Object, flow, action, sharing-rule and hook predicates that `validateExpression` used to refuse for this argument now pass. They evaluate as written.
+- **What is still refused.** An argument the spec itself refuses: a bare name (`has(status)`) or an index as the last step (`has(record.items[0])`). Test a key with `'key' in m`, or a list position with `size(list) > n`.
+- **Nothing to migrate.** A predicate already written in the `'key' in e` form keeps working and answers the same.

packages/formula/src/cel-engine.ts

@@ -404,6 +404,9 @@ let recordScopeEnv: Environment | undefined;
  *    faults `has() invalid argument`. `@objectstack/lint`'s
  *    `validate-visibility-predicates` masks `has(…)` spans at its own call site
  *    (#16118) and that mask stays load-bearing; no other consumer has one.
+ *    A select whose receiver is an index or a call (`has(a[0].b)`) is NOT in
+ *    this class: the spec admits it, and {@link rewriteIndexedHas} lowers it
+ *    before this check runs (#25267).
  *
  * ⇒ A `null` from this helper is still "nothing was reported", never "the
  * source is clean". ⛔ Do not close the remaining classes by widening the regex
@@ -423,7 +426,9 @@ export function firstUndeclaredReference(
     const env = knownFields.length === 0
       ? (recordScopeEnv ??= buildScopedEnv([]))
       : buildScopedEnv(knownFields);
-    const result = env.parse(source).check?.() as
+    // #25267 — judged in the form the engine checks, so an indexed `has()`
+    // receiver is not a first error that masks every reference behind it.
+    const result = env.parse(rewriteIndexedHas(source)).check?.() as
       | { valid: boolean; error?: { message?: string } }
       | undefined;
     if (result && result.valid === false) {
@@ -862,7 +867,8 @@ export function inferCelType(source: string, knownFields: readonly string[] = []
     const env = knownFields.length === 0
       ? (recordScopeEnv ??= buildScopedEnv([]))
       : buildScopedEnv(knownFields);
-    const result = env.parse(source).check?.() as
+    // #25267 — typed in the form the engine checks (an indexed `has()` is `bool`).
+    const result = env.parse(rewriteIndexedHas(source)).check?.() as
       | { valid?: boolean; type?: unknown }
       | undefined;
     if (!result || result.valid === false) return null;
@@ -1066,7 +1072,7 @@ export function firstTypeMismatch(
     // unifier *before* the check reaches an inner date-arith overload; rewrite it
     // first (same pass the runtime uses) so the real fault surfaces (#3306).
     const env = buildTypedEnv(fieldCelTypes, scope);
-    const result = env.parse(rewriteNullableTernary(source)).check?.() as
+    const result = env.parse(rewriteNullableTernary(rewriteIndexedHas(source))).check?.() as
       | { valid?: boolean; error?: { message?: string } }
       | undefined;
     if (!result || result.valid !== false) return null;
@@ -1463,6 +1469,111 @@ function rememberNullableRewrite(source: string, rewritten: string): void {
   nullableTernaryCache.set(source, rewritten);
 }
 
+/**
+ * #25267 — make `has()` over a selection whose receiver is not a plain name
+ * (`has(data.environments[0].storage.usedMb)`, `has(lookup(x).y)`) type-check
+ * and evaluate, as the CEL spec defines it: `has(e.f)` is a presence test on
+ * field `f` of whatever `e` evaluates to, and `e` may be any expression.
+ *
+ * cel-js 8.0.0's `has()` macro walks the selection chain back to its root and
+ * refuses the argument (`has() invalid argument`) unless that root is an
+ * identifier — so an index or a call anywhere in the chain faulted every time
+ * it was checked or run, while a page's `visibleWhen` (whose publish gate
+ * parses but does not type-check) published clean and failed open.
+ *
+ * The call is lowered to `e.?f.hasValue()`. That is the CEL spec's own
+ * definition of optional field selection (`e.?f` is `optional.of(e.f)` when
+ * `has(e.f)`, else `optional.none()`), so the two are the same test, and cel-js
+ * evaluates the receiver `e` exactly as `has()` does: an absent key on the way
+ * to the last selection still faults (`No such key`), only the last one answers
+ * `false`. `cel-has-indexed-receiver.test.ts` pins the two forms against each
+ * other over every value kind.
+ *
+ * The rewrite is:
+ *   - **narrow** — it fires ONLY on a `has()` whose argument is a `.` selection
+ *     rooted in something other than an identifier. `has(record.a.b)` is left to
+ *     cel-js, and so is every argument the spec itself refuses: a bare name
+ *     (`has(status)`) or an index as the LAST step (`has(a[0])`) still faults
+ *     `has() invalid argument`;
+ *   - **applied where cel-js type-checks** — `evaluate`, `compile` and the
+ *     checker-backed helpers ({@link firstUndeclaredReference},
+ *     {@link inferCelType}, the overload-fault probe), which are the only places
+ *     the macro's refusal can arise. The parse-only front end
+ *     ({@link parseCelToAst}) keeps the authored `has(…)` call, which parses
+ *     already, so an AST consumer that keys on a `has()` call still finds it;
+ *   - **idempotent** — the lowered form carries no `has()` call to lower again.
+ *
+ * Returns the (possibly rewritten) source; only reserializes when a rewrite
+ * actually happened. Memoized; a parse fault returns the source unchanged.
+ */
+export function rewriteIndexedHas(source: string): string {
+  if (typeof source !== 'string' || !source.trim()) return source;
+  const cached = indexedHasCache.get(source);
+  if (cached !== undefined) return cached;
+  // Cheap gate: a rewrite needs a `has(` call.
+  if (!HAS_CALL_RE.test(source)) {
+    rememberIndexedHasRewrite(source, source);
+    return source;
+  }
+  let ast: unknown;
+  try {
+    ast = (recordScopeEnv ??= buildScopedEnv([])).parse(source).ast;
+  } catch {
+    rememberIndexedHasRewrite(source, source);
+    return source;
+  }
+  let changed = false;
+  // Children first, so a `has()` nested inside a lowered receiver is lowered too.
+  const lower = (value: unknown): unknown => {
+    if (Array.isArray(value)) {
+      for (let i = 0; i < value.length; i++) value[i] = lower(value[i]);
+      return value;
+    }
+    if (!isCelNode(value)) return value;
+    if (Array.isArray(value.args)) lower(value.args);
+    const selection = indexedHasSelection(value);
+    if (!selection) return value;
+    changed = true;
+    return { op: 'rcall', args: ['hasValue', { op: '.?', args: selection.args }, []] };
+  };
+  const lowered = lower(ast);
+  const out = changed ? serialize(lowered as Parameters<typeof serialize>[0]) : source;
+  rememberIndexedHasRewrite(source, out);
+  return out;
+}
+
+/** A `has(` call, spaced or not, not the tail of a longer name or a member call. */
+const HAS_CALL_RE = /(^|[^\w$.])has\s*\(/;
+
+/**
+ * The argument of `node` when it is a `has()` call {@link rewriteIndexedHas}
+ * lowers — a `.` selection whose chain of `.` / `.?` steps ends in something
+ * other than an identifier — or `null`.
+ */
+function indexedHasSelection(node: CelNode): CelNode | null {
+  if (node.op !== 'call' || !Array.isArray(node.args) || node.args[0] !== 'has') return null;
+  const callArgs: unknown = node.args[1];
+  if (!Array.isArray(callArgs) || callArgs.length !== 1) return null;
+  const selection: unknown = callArgs[0];
+  if (!isCelNode(selection) || selection.op !== '.' || !Array.isArray(selection.args)) return null;
+  let root: unknown = selection.args[0];
+  while (isCelNode(root) && (root.op === '.' || root.op === '.?') && Array.isArray(root.args)) {
+    root = root.args[0];
+  }
+  return isCelNode(root) && root.op !== 'id' ? selection : null;
+}
+
+/** Bounded memo of source → indexed-`has()`-lowered source (#25267). */
+const indexedHasCache = new Map<string, string>();
+const INDEXED_HAS_CACHE_MAX = 500;
+function rememberIndexedHasRewrite(source: string, rewritten: string): void {
+  if (indexedHasCache.size >= INDEXED_HAS_CACHE_MAX) {
+    const first = indexedHasCache.keys().next().value;
+    if (first !== undefined) indexedHasCache.delete(first);
+  }
+  indexedHasCache.set(source, rewritten);
+}
+
 /** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
 function coerce(value: unknown): unknown {
   if (typeof value === 'bigint') {
@@ -1870,7 +1981,8 @@ export const celEngine: DialectEngine = {
       // #3306 — accept the null-guard idiom `cond ? <value> : null`, which cel-js's
       // ternary unifier otherwise rejects (`int`/`double`/`string` ≠ `null`). Same
       // rewrite the runtime uses, so build and eval agree on what is valid.
-      const compiled = env.parse(rewriteNullableTernary(source));
+      // #25267 — and `has()` over an indexed receiver, lowered the same way.
+      const compiled = env.parse(rewriteNullableTernary(rewriteIndexedHas(source)));
       // Surface check errors eagerly. cel-js's `check()` returns a
       // `TypeCheckResult` object (`{ valid, type?, error? }`) — NOT an array —
       // so the type fault (including `found no matching overload for 'PRIOR(dyn)'`
@@ -1942,8 +2054,10 @@ export const celEngine: DialectEngine = {
         // row against the values in hand. Non-date operands are untouched.
         // #3306 — then relax the null-guard idiom `cond ? <value> : null` so a
         // nullable numeric/string formula evaluates instead of faulting cel-js's
-        // ternary unifier. All rewrites are no-ops for sources that don't need them.
-        const evalSource = rewriteNullableTernary(rewriteTemporalEquality(source, scope));
+        // ternary unifier. #25267 — and lower `has()` over an indexed receiver,
+        // which cel-js's macro refuses, to the spec-equivalent `e.?f.hasValue()`.
+        // All rewrites are no-ops for sources that don't need them.
+        const evalSource = rewriteNullableTernary(rewriteTemporalEquality(rewriteIndexedHas(source), scope));
         try {
           const raw = runProgram(env, evalSource, scope);
           return { ok: true, value: coerce(raw) as T };

packages/formula/src/cel-has-indexed-receiver.test.ts

@@ -0,0 +1,119 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, expect, it } from 'vitest';
+import { celEngine, firstUndeclaredReference, inferCelType, rewriteIndexedHas } from './cel-engine';
+import { validateExpression } from './validate';
+
+/**
+ * #25267 — `has()` over a selection whose receiver is not a plain name.
+ *
+ * The CEL spec defines `has(e.f)` for any expression `e`; cel-js 8.0.0's macro
+ * refused every receiver chain not rooted in an identifier with `has() invalid
+ * argument`. Measured on `origin/main` 17f3c65a1d before the change:
+ * `has(data.environments[0].storage.usedMb)` answered `ok: false` from
+ * `evaluate`, `compile` and `validateExpression` alike, while a page's
+ * `visibleWhen` (parse-only gate) published it and the console failed open.
+ *
+ * The engine now lowers that call to the spec's equivalent `e.?f.hasValue()`
+ * ({@link rewriteIndexedHas}); these pins hold every door to one answer.
+ */
+
+const ENVIRONMENTS = { environments: [{ storage: { usedMb: 5 } }, { storage: {} }] };
+
+function evaluate(source: string, extra: Record<string, unknown>) {
+  return celEngine.evaluate({ dialect: 'cel', source }, { extra });
+}
+
+describe('has() over an indexed receiver evaluates as a presence test (#25267)', () => {
+  it('the filed reading: present answers true, absent answers false', () => {
+    expect(evaluate('has(data.environments[0].storage.usedMb)', { data: ENVIRONMENTS }))
+      .toEqual({ ok: true, value: true });
+    expect(evaluate('has(data.environments[1].storage.usedMb)', { data: ENVIRONMENTS }))
+      .toEqual({ ok: true, value: false });
+  });
+
+  it('agrees with the `in` form the filing worked around with', () => {
+    for (const at of [0, 1]) {
+      const viaHas = evaluate(`has(data.environments[${at}].storage.usedMb)`, { data: ENVIRONMENTS });
+      const viaIn = evaluate(`'usedMb' in data.environments[${at}].storage`, { data: ENVIRONMENTS });
+      expect(viaHas).toEqual(viaIn);
+    }
+  });
+
+  it('answers exactly what the identifier-rooted form answers, over every value kind', () => {
+    // `has(v.…)` is cel-js's own macro (identifier root, untouched); `has(w[0].…)`
+    // is the lowered form over the same value. Faults compare by kind and the
+    // macro's first line, since the lowered source is echoed in the excerpt.
+    const kinds: Record<string, unknown> = {
+      'map with the key': { f: { g: 1 } },
+      'map with the key null': { f: null },
+      'map with the key false': { f: false },
+      'map without the key': { h: 1 },
+      'empty map': {},
+      null: null,
+      list: [1],
+      int: 5,
+      string: 's',
+      bool: true,
+    };
+    const shape = (r: ReturnType<typeof evaluate>) =>
+      r.ok ? { ok: true, value: r.value } : { ok: false, kind: r.error.kind, first: r.error.message.split('\n')[0] };
+    for (const [kind, v] of Object.entries(kinds)) {
+      for (const path of ['f', 'f.g']) {
+        const native = shape(evaluate(`has(v.${path})`, { v, w: [v] }));
+        const lowered = shape(evaluate(`has(w[0].${path})`, { v, w: [v] }));
+        expect({ kind, path, ...lowered }).toEqual({ kind, path, ...native });
+      }
+    }
+  });
+
+  it('a receiver that is a call result is a receiver too', () => {
+    expect(evaluate("has(dyn(m).a)", { m: { a: 1 } })).toEqual({ ok: true, value: true });
+    expect(evaluate("has(dyn(m).b)", { m: { a: 1 } })).toEqual({ ok: true, value: false });
+  });
+});
+
+describe('every door gives the same verdict (#25267)', () => {
+  const lowered = 'has(record.items[0].name)';
+
+  it('compile and validateExpression accept what evaluate runs', () => {
+    expect(celEngine.compile(lowered).ok).toBe(true);
+    expect(validateExpression('predicate', { dialect: 'cel', source: lowered }).ok).toBe(true);
+    expect(celEngine.evaluate({ dialect: 'cel', source: lowered }, { record: { items: [{ name: 'x' }] } }))
+      .toEqual({ ok: true, value: true });
+  });
+
+  it('the checker-backed helpers judge the lowered form', () => {
+    expect(inferCelType(lowered)).toBe('bool');
+    // The `has()` fault no longer takes the checker's one error slot, so a bare
+    // reference behind it is reported instead of masked.
+    expect(firstUndeclaredReference(`${lowered} && status == 'x'`)).toBe('status');
+  });
+
+  it('controls: an argument the spec refuses is still refused at every door', () => {
+    for (const source of ['has(status)', 'has(record.items[0])']) {
+      const compiled = celEngine.compile(source);
+      expect(compiled.ok).toBe(false);
+      expect(compiled.ok ? '' : compiled.error.message).toContain('has() invalid argument');
+      expect(validateExpression('predicate', { dialect: 'cel', source }).ok).toBe(false);
+      const ran = celEngine.evaluate({ dialect: 'cel', source }, { record: { items: [{ name: 'x' }] } });
+      expect(ran.ok).toBe(false);
+    }
+  });
+});
+
+describe('rewriteIndexedHas (#25267)', () => {
+  it('lowers only a `has()` whose selection is rooted in something other than a name', () => {
+    expect(rewriteIndexedHas('has(a[0].b.c)')).toBe('a[0].b.?c.hasValue()');
+    expect(rewriteIndexedHas('has(a[0].b) && has(c.d)')).toBe('a[0].?b.hasValue() && has(c.d)');
+    for (const untouched of ['has(record.a.b)', 'has(status)', 'has(a[0])', "x.has('y')", 'record.has_items']) {
+      expect(rewriteIndexedHas(untouched)).toBe(untouched);
+    }
+  });
+
+  it('is idempotent, and lowers a `has()` nested in a lowered receiver', () => {
+    const once = rewriteIndexedHas('has(a[has(b[0].c) ? 0 : 1].d)');
+    expect(once).not.toContain('has(');
+    expect(rewriteIndexedHas(once)).toBe(once);
+  });
+});