diff --git a/.changeset/10538-cel-macro-bound-variable.md b/.changeset/10538-cel-macro-bound-variable.md new file mode 100644 index 0000000000..cf456161ad --- /dev/null +++ b/.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. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.test.ts index d7f145b478..5e0ffa2d3c 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.test.ts +++ b/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'); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.ts index b0e946b660..dccc7553c9 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.ts +++ b/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,12 +74,58 @@ 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. @@ -84,9 +133,13 @@ function rootIdentifiers(src: string): string[] { 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;