fix(app-shell): a CEL comprehension macro's iteration variable is bound inside its macro, not flagged as out of scope (objectui#10538)

.changeset/10538-cel-macro-bound-variable.md

@@ -0,0 +1,7 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(app-shell): the flow designer's "not in scope" warning no longer flags a CEL comprehension macro's own iteration variable (objectui#10538)
+
+`findUnknownRefs` read every root identifier in a CEL source as a flow variable, so the `r` in `rows.map(r, r.subject)` drew an amber "not in scope" warning. That included the assignment-value example in `@objectstack/spec`, `joinNonEmpty(rows.map(r, r.subject), "\n")`. The scanner now treats the first argument of `map`, `filter`, `exists`, `all` and `exists_one` as a variable bound inside that macro's own argument list. The same name used outside that list (after the closing parenthesis, or as another macro's receiver) is still checked as a root, and an unknown receiver is still flagged with its suggestion. The warning stays advisory; nothing about what blocks Save changes.

packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.test.ts

@@ -70,6 +70,49 @@ describe('findUnknownRefs (predicate)', () => {
   });
 });
 
+describe('findUnknownRefs (CEL comprehension macros bind their iteration variable) (objectui#10538)', () => {
+  const known = new Set(['rows']);
+  const cel = (source: string) => findUnknownRefs({ dialect: 'cel', source }, 'value', known);
+
+  it("accepts the spec's canonical assignment-value example", () => {
+    // The `AssignmentValue` JSDoc in @objectstack/spec: "joinNonEmpty(rows.map(r, r.subject), "\n")".
+    expect(cel('joinNonEmpty(rows.map(r, r.subject), "\\n")')).toEqual([]);
+  });
+
+  it('binds the iteration variable of each of the five macros inside that macro', () => {
+    expect(cel('rows.map(r, r.subject)')).toEqual([]);
+    expect(cel('rows.map(r, r.active, r.subject)')).toEqual([]);
+    expect(cel('rows.filter(x, x.done).size()')).toEqual([]);
+    expect(cel('rows.exists(e, e.a > 1)')).toEqual([]);
+    expect(cel('rows.all(a, a.ok)')).toEqual([]);
+    expect(cel('rows.exists_one(o, o.id == 1)')).toEqual([]);
+  });
+
+  it('binds both variables of a nested macro', () => {
+    expect(cel('rows.map(r, r.items.filter(i, i.ok))')).toEqual([]);
+  });
+
+  it('still flags the same name used after the macro closes', () => {
+    expect(cel('rows.map(r, r.subject) + r')).toEqual([{ token: 'r', suggestion: undefined }]);
+  });
+
+  it('still flags an inner variable used outside its own macro', () => {
+    expect(cel('rows.map(r, r.items.filter(i, i.ok) + i)')).toEqual([{ token: 'i', suggestion: undefined }]);
+  });
+
+  it("still flags a bound variable used as a different macro's receiver", () => {
+    expect(cel('rows.exists(r, r.ok) && r.items.all(i, i.ok)')).toEqual([{ token: 'r', suggestion: undefined }]);
+  });
+
+  it('still flags an unknown receiver root, with its suggestion', () => {
+    expect(cel('rowz.map(r, r.subject)')).toEqual([{ token: 'rowz', suggestion: 'rows' }]);
+  });
+
+  it('binds nothing for a member call that is not one of the five macros', () => {
+    expect(cel('rows.join(sep, other)').map((u) => u.token)).toEqual(['sep', 'other']);
+  });
+});
+
 describe('findUnknownRefs (template)', () => {
   const known = roots('approval_path', 'record');
 

packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.ts

@@ -13,6 +13,9 @@
  *     accepted.
  *   • Function / macro calls (`daysFromNow(90)`, `has(...)`, `size(...)`) are
  *     skipped — an identifier immediately followed by `(` is never a reference.
+ *   • A CEL comprehension macro's iteration variable (`r` in
+ *     `rows.map(r, r.subject)`) is bound inside that macro's argument list only;
+ *     the same name anywhere else is a root like any other (objectui#10538).
  *   • String-literal contents are stripped before scanning.
  *   • Runtime globals the engine injects (`env`, `$error`, `data`, …) and CEL
  *     keywords/literals are allow-listed.
@@ -71,22 +74,72 @@ function editDistance(a: string, b: string, max = 2): number {
   return prev[b.length];
 }
 
-/** Extract reference-position root identifiers (skipping members and calls). */
+/**
+ * CEL's comprehension macros. Each is receiver-style (`rows.map(r, r.subject)`,
+ * `rows.map(r, r.ok, r.subject)`), and its first argument DECLARES the iteration
+ * variable instead of reading one.
+ */
+const COMPREHENSION_MACROS = new Set(['map', 'filter', 'exists', 'all', 'exists_one']);
+
+/** One macro-bound iteration variable and the argument-list span it is bound in. */
+interface MacroBinding {
+  name: string;
+  /** Index of the macro call's `(`. */
+  open: number;
+  /** Index of the matching `)`, or the source length while it is still unclosed. */
+  close: number;
+}
+
+/**
+ * The iteration variables the comprehension macros in `src` bind, each with the
+ * span of its own macro's argument list. Scope is lexical: the variable is bound
+ * between that `(` and its matching `)` and nowhere else, so the receiver and
+ * anything after the closing `)` still read it as a root. `src` must already
+ * have its string literals stripped, so a `)` inside a string cannot close a span.
+ */
+function macroBindings(src: string): MacroBinding[] {
+  const out: MacroBinding[] = [];
+  // `.macro(v,` — a member call whose first argument is a bare identifier.
+  const re = /\.\s*([A-Za-z_]\w*)\s*\(\s*([A-Za-z_]\w*)\s*,/g;
+  let m: RegExpExecArray | null;
+  while ((m = re.exec(src))) {
+    if (!COMPREHENSION_MACROS.has(m[1])) continue;
+    const open = src.indexOf('(', m.index);
+    let close = src.length;
+    let depth = 0;
+    for (let i = open; i < src.length; i++) {
+      if (src[i] === '(') depth++;
+      else if (src[i] === ')' && --depth === 0) {
+        close = i;
+        break;
+      }
+    }
+    out.push({ name: m[2], open, close });
+  }
+  return out;
+}
+
+/** Extract reference-position root identifiers (skipping members, calls and macro-bound variables). */
 function rootIdentifiers(src: string): string[] {
   // Strip string literals so their contents aren't scanned as references.
   const noStrings = src
     .replace(/'(?:[^'\\]|\\.)*'/g, ' ')
     .replace(/"(?:[^"\\]|\\.)*"/g, ' ');
+  const bindings = macroBindings(noStrings);
   const out: string[] = [];
   // Lookbehind keeps this to ROOTS: an identifier not preceded by `.` (member),
   // a word char, or `$`. Zero-width, so adjacent tokens are never swallowed.
   const re = /(?<![.\w$])([A-Za-z_$][\w$]*)/g;
   let m: RegExpExecArray | null;
   while ((m = re.exec(noStrings))) {
     const name = m[1];
+    const at = m.index;
     // A trailing `(` (after optional spaces) marks a function / macro call.
-    if (/^\s*\(/.test(noStrings.slice(m.index + name.length))) continue;
+    if (/^\s*\(/.test(noStrings.slice(at + name.length))) continue;
     if (CEL_RESERVED.has(name)) continue;
+    // Inside its own macro's argument list (the declaration included), the
+    // iteration variable is bound, not a reference to a flow variable.
+    if (bindings.some((b) => b.name === name && at > b.open && at < b.close)) continue;
     out.push(name);
   }
   return out;