diff --git a/.changeset/visibility-predicate-over-budget.md b/.changeset/visibility-predicate-over-budget.md new file mode 100644 index 0000000000..8e37e5c798 --- /dev/null +++ b/.changeset/visibility-predicate-over-budget.md @@ -0,0 +1,46 @@ +--- +'@objectstack/lint': patch +--- + +`visibility-predicate-syntax`: an over-budget predicate is a SIZE fault, not "not valid CEL" (#7217) + +view / page `visibleWhen` 的门禁把两类拒绝合成了一类。`parseCelToAst` 对"不是 CEL" +和"是 CEL、但超过平台解析预算"返回同一个 `null`（生产侧刻意如此），于是一条 +80 项合取的谓词——完全合法的 bare CEL，只是超过 `maxAstNodes` 256——被报成 +`visibility predicate is not valid CEL`，并附上方言处方（"写 `==` 不是 `===`、 +`&&` 不是 `and` …"）。标题是假的，处方在这条源码上根本不可能成功：作者（尤其是 +照着最后一句话执行的 LLM 作者）会去改一堆本来就没错的运算符，然后带着同一条超预算 +谓词回来。#7073 / PR #7209 在 ADR-0032 的共享生产者上修的是同一个缺陷，而本门禁 +按其自身 docblock 刻意不走 `validateExpression`，所以生产侧的修复到不了这里。 + +**判定不变**：拒绝的输入集合、严重级别、每条坏谓词一个 finding，全部与修复前一致。 +红绿边界仍然是规范前端接受什么——`celRefusal` 改问 `parseCelToAstWithReason`，而 +`parseCelToAst` 本身就是"它把 reason 丢掉"（同一个 env、同一套 limits），所以没有 +任何一条源码换了颜色。变的只有解释。 + +新增第三个 error 级 id **`visibility-predicate-over-budget`**（与 +`visibility-predicate-syntax` / `visibility-bare-identifier` 并列导出），理由与 +#6778 / PR #6831 在 RLS 侧把 `rls-predicate-over-budget` 从 +`rls-predicate-unparseable` 拆出来时相同：后果相同，**修法不同**，而 `--json` +消费者与抑制清单都按 id 取值。超预算谓词现在报： + +> visibility predicate is syntactically valid CEL but overruns the `maxAstNodes` +> budget (platform limit 256) (Exceeded maxAstNodes (256)) (predicate: …) … +> +> hint: There is no syntax or dialect error to correct here — this is a SIZE +> fault, not a dialect mistake, so re-spelling the predicate will not fix it. +> Make it smaller, or move the work off the predicate: (1) collapse a long +> `record.f == 'a' || record.f == 'b' || …` chain into a single +> `record.f in ['a', 'b', …]` …; (2) precompute the heavy part into a +> formula/rollup field on the object and test that one field instead. … + +越界的那条界（`maxAstNodes` / `maxDepth` / `maxListElements` / …）与平台取值来自 +前端自己的结构化 `overrun`，不是硬编码，也不是二次解析它的散文（#6223）；提示里的 +绑定根随 layer 走（runtime 用 `record`，`*.form.ts` 元数据表单用 `data`），否则处方 +本身又会是一句照做不了的话。真正的方言/语法错误保持 #6253 的 id、message 与 hint +逐字不变——两个方向都有 pin。 + +**兼容性**：这是新增 id，不是改名。抑制 `visibility-predicate-syntax` 的配置从此不再 +抑制超预算这一类——与 #6778 接受的代价相同，而且本来就是抑制错了对象（抑制的是 +"语法"，命中的是"太大"）。仓库内除 `packages/lint` 自身与 changelog 外，没有任何 +配置、文档或示例引用这些 id。 diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index ba82e16df1..3861846974 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -166,6 +166,7 @@ export { VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, VISIBILITY_PREDICATE_SYNTAX, + VISIBILITY_PREDICATE_OVER_BUDGET, } from './validate-visibility-predicates.js'; export type { VisibilityFinding, diff --git a/packages/lint/src/validate-visibility-predicates.test.ts b/packages/lint/src/validate-visibility-predicates.test.ts index 5ddeba80af..e02de72892 100644 --- a/packages/lint/src/validate-visibility-predicates.test.ts +++ b/packages/lint/src/validate-visibility-predicates.test.ts @@ -6,6 +6,7 @@ import { VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, VISIBILITY_PREDICATE_SYNTAX, + VISIBILITY_PREDICATE_OVER_BUDGET, } from './validate-visibility-predicates.js'; import { AUTHORING_RULES } from './authoring-rules.js'; @@ -713,19 +714,15 @@ describe('visibility-predicate-syntax (#6253)', () => { expect(validateVisibilityPredicates(formStack('type(record.x) == string'))).toEqual([]); }); - it('a `DEFAULT_LIMITS` overrun is reported too, in the front end\'s own words', () => { - // `parseCelToAst` also returns null for a source over the platform bounds. - // That is a bounds fault, not a syntax one, and the message says so rather - // than pretending to have found a typo — the same way ADR-0032 already - // reports it under the "invalid CEL predicate" heading. + it('a `DEFAULT_LIMITS` overrun is NOT this rule any more — it is `over-budget` (#7217)', () => { + // Was: "reported too, in the front end's own words", asserted under + // `visibility-predicate-syntax`. #7217 keeps the verdict and moves the + // wording and the id; this case is now the exclusivity pin for the split, + // and its full coverage lives in the `over-budget` block below. const overrun = `record.a${' + record.b'.repeat(400)}`; - const findings = syntaxFindings(formStack(overrun)); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('Exceeded maxAstNodes'); - // The echoed predicate is elided, so one runaway expression cannot flood - // the console with a 4KB finding. - expect(findings[0].message).not.toContain(overrun); - expect(findings[0].message).toContain('...'); + expect(syntaxFindings(formStack(overrun))).toEqual([]); + expect(validateVisibilityPredicates(formStack(overrun)).map((f) => f.rule)) + .toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]); }); it.each([ @@ -806,3 +803,146 @@ describe('visibility-predicate-syntax (#6253)', () => { expect([...entry!.commands].sort()).toEqual(['build', 'lint', 'validate']); }); }); + +// ───────────────────────────────────────────────────────────────────── +// `visibility-predicate-over-budget` — #7217. +// +// The defect this closes is an INSTRUCTION that makes an obedient author +// worse. An over-budget `visibleWhen` is flawless bare CEL; the gate refused it +// (correctly) as "not valid CEL" (false) and prescribed the dialect ("write +// `==` not `===`, `&&` not `and` …"), which is advice that cannot succeed. An +// LLM author follows the last sentence it was handed, rewrites operators that +// were never wrong, and returns with the same 80-clause predicate. +// +// The verdict is untouched: the same sources are refused before and after, at +// the same severity, one finding each. Only the class, the id and the words +// changed. Both directions are pinned deliberately — a fix that turned EVERY +// refusal into a size refusal would be green on the first block below, which is +// why the syntax block re-asserts the dialect wording it must not lose. +// ───────────────────────────────────────────────────────────────────── + +/** Only the over-budget findings. */ +function overBudgetFindings(stack: Record, opts?: { layer: 'runtime' | 'metadata' }) { + return validateVisibilityPredicates(stack, opts).filter((f) => f.rule === VISIBILITY_PREDICATE_OVER_BUDGET); +} + +/** The escalation's own shape — 80-term conjunction, `maxAstNodes` (#6833 / #7073). */ +const OVER_AST_NODES = Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && '); +/** 60-level parenthesis nest — `maxDepth`. Recursion that leaves no AST node. */ +const OVER_DEPTH = `${'('.repeat(60)}record.a${')'.repeat(60)} == 1`; +/** 200-element list literal — `maxListElements`. */ +const OVER_LIST = `record.id in [${Array.from({ length: 200 }, (_, i) => `'u${i}'`).join(',')}]`; + +describe('visibility-predicate-over-budget (#7217)', () => { + describe('the acceptance pair', () => { + it('an over-budget but valid predicate names the SIZE fault and the bound, never the dialect', () => { + const findings = validateVisibilityPredicates(formStack(OVER_AST_NODES)); + + // The whole reported set: one finding, the new id, still gating. + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('views[0].sections[0].fields[0]'); + expect(findings[0].where).toBe('view "task_form"'); + + // ⛔ The headline was FALSE: this IS valid CEL. + expect(findings[0].message).not.toContain('is not valid CEL'); + expect(findings[0].message).toContain('syntactically valid CEL'); + // The front end's own summary is quoted, and the bound is NAMED with the + // platform's value for it — which is what "shrink it to fit" needs. + expect(findings[0].message).toContain('Exceeded maxAstNodes (256)'); + expect(findings[0].message).toContain('`maxAstNodes` budget (platform limit 256)'); + // The consequence is unchanged: it still falls OPEN on screen. + expect(findings[0].message).toContain('#5149'); + + // ⛔ The defect itself: the dialect prescription must not reach this class. + expect(findings[0].hint).not.toMatch(/bare CEL/); + expect(findings[0].hint).not.toContain('`===`'); + expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake'); + expect(findings[0].hint).toContain("`record.f in ['a', 'b', …]`"); + }); + + it('the SAME predicate under budget is clean — paired so it cannot pass vacuously', () => { + // Delete the rule and the first expectation goes red; loosen the verdict + // (report everything) and the second does. + expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1); + const underBudget = Array.from({ length: 8 }, (_, i) => `record.f${i} == ${i}`).join(' && '); + expect(validateVisibilityPredicates(formStack(underBudget))).toEqual([]); + }); + }); + + it.each([ + ['maxAstNodes (80-term conjunction)', OVER_AST_NODES, 'maxAstNodes', 256], + ['maxDepth (60-level nest)', OVER_DEPTH, 'maxDepth', 32], + ['maxListElements (200-element list)', OVER_LIST, 'maxListElements', 64], + ])('%s — the bound that was actually exceeded is the one named', (_name, source, limit, value) => { + // Not one hard-coded bound: the name and its value are read off the front + // end's structured overrun, so a source that overruns a DIFFERENT axis is + // sent to shorten that axis and not `maxAstNodes` by default. + const findings = overBudgetFindings(formStack(source as string)); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain(`\`${limit}\` budget (platform limit ${value})`); + expect(findings[0].hint).toContain('SIZE fault, not a dialect mistake'); + }); + + it('a genuine dialect fault keeps the #6253 id, message and hint verbatim', () => { + // The flipped pin. The obvious way to get #7217 wrong is to turn EVERY + // refusal into a size refusal — green on every case above, and a total loss + // of the wording #6253 shipped. + const findings = validateVisibilityPredicates(formStack('country === "USA"')); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]); + expect(findings[0].message).toContain('is not valid CEL'); + expect(findings[0].hint).toContain('`===` is not a CEL operator'); + expect(findings[0].hint).not.toMatch(/SIZE fault/); + }); + + it('a dialect fault with no single-token equivalent keeps the FALLBACK dialect hint', () => { + // The arm the card names: the fallback hint fires when no NON_CEL_SPELLINGS + // row matches — which is exactly the arm an over-budget source used to land + // in. It must still fire for a source that genuinely is not CEL. + const findings = validateVisibilityPredicates(formStack('record.stage @@ "won"')); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]); + expect(findings[0].hint).toContain('Visibility predicates are bare CEL'); + expect(findings[0].hint).not.toMatch(/SIZE fault/); + }); + + it('still exactly ONE finding — the bare-identifier rule stays out of the way', () => { + // An over-budget source yields no AST, so there are no identifiers to judge. + // The exclusivity property #6253/#6128 established survives the split. + const rootless = Array.from({ length: 80 }, (_, i) => `f${i} == ${i}`).join(' && '); + expect(validateVisibilityPredicates(formStack(rootless)).map((f) => f.rule)) + .toEqual([VISIBILITY_PREDICATE_OVER_BUDGET]); + }); + + it('elides the echoed predicate — one runaway expression cannot flood the console', () => { + const findings = overBudgetFindings(formStack(OVER_AST_NODES)); + expect(findings[0].message).not.toContain(OVER_AST_NODES); + expect(findings[0].message).toContain('...'); + }); + + it('a blank predicate is still not a fault of any class', () => { + // `parseCelToAstWithReason` answers `empty` rather than `parse`; paired with + // a live case so the assertion can actually fail. + expect(overBudgetFindings(formStack(OVER_AST_NODES))).toHaveLength(1); + expect(validateVisibilityPredicates(formStack(' '))).toEqual([]); + expect(validateVisibilityPredicates(formStack(undefined))).toEqual([]); + }); + + it('speaks the LAYER\'s binding root in its remedy', () => { + // A `*.form.ts` author binds `data`, so a `record.`-flavoured example would + // be a second wrong prescription in a rule whose whole point is that the + // prescription must be followable. + const metadata = overBudgetFindings(formStack(OVER_AST_NODES), { layer: 'metadata' }); + expect(metadata).toHaveLength(1); + expect(metadata[0].hint).toContain("`data.f in ['a', 'b', …]`"); + expect(metadata[0].hint).not.toContain('`record.f in'); + }); + + it('reaches page components too — the id is not view-only', () => { + const stack = { + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: OVER_AST_NODES }] }] }], + }; + const findings = overBudgetFindings(stack); + expect(findings.map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']); + expect(findings[0].where).toBe('page "p"'); + }); +}); diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index cc1ab3549b..8920a914a9 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -53,13 +53,18 @@ * normalized tier, and none was affected by the retirement. * * One advisory rule (`warning` — nothing is broken, a mis-rooted predicate just - * never matches) plus TWO **gating** rules (`error` — the predicate can never + * never matches) plus THREE **gating** rules (`error` — the predicate can never * evaluate at all): * * - `visibility-predicate-syntax` (**error**, #6253) — a predicate the canonical - * CEL front end refuses outright (`country === "USA"` — `===` is not CEL). See - * the §Syntax block below for why this surface has to say it and who owns the - * verdict. + * CEL front end refuses as not being CEL (`country === "USA"` — `===` is not + * CEL). See the §Syntax block below for why this surface has to say it and who + * owns the verdict. + * - `visibility-predicate-over-budget` (**error**, #7217) — a predicate the same + * front end refuses for its SIZE: flawless bare CEL that overruns a + * `DEFAULT_LIMITS` parse bound (`maxAstNodes` 256, `maxDepth` 32, …). Same + * verdict, same severity, different EDIT — see §Syntax for why that is worth + * its own id. * - `visibility-bare-identifier` (**error**, #6128 / #5149 requirement 3) — a * predicate referencing a top-level identifier that no binding root can * resolve (`status == 'active'` instead of `record.status == 'active'`). See @@ -134,10 +139,34 @@ * `compile()` would silently overturn it from the syntax branch, widening an * `error`-level gate from "does not parse" to "does not type-check" on a surface * whose predicates are overwhelmingly `dyn`. The ruling says syntax; the parse - * verdict is exactly syntax. `parseCelToAst` also refuses a `DEFAULT_LIMITS` - * overrun, which is a bounds fault rather than a syntax one; it is reported here - * too, quoting the front end's own words, exactly as ADR-0032 already reports it - * under the same "invalid CEL predicate" heading. + * verdict is exactly syntax. + * + * ### The refusal is one verdict and TWO edits (#7217) + * + * The canonical front end also refuses a source that overruns a + * `DEFAULT_LIMITS` bound — valid CEL, merely too big — and `parseCelToAst` + * returns the same `null` for it as for `===`. That collapse is deliberate on + * the producer's side and right for a caller that only wants an AST; it is wrong + * for a caller whose job is to REPORT, and until #7217 this gate was the caller + * paying for it: an 80-clause conjunction was announced as "not valid CEL" and + * handed the dialect prescription ("write `==` not `===`, `&&` not `and` …"), + * which is advice that cannot succeed on a source already spelled in bare CEL. + * An author who follows the last sentence they were given — an LLM author above + * all — rewrites operators that were never wrong and returns with the same + * over-budget predicate. + * + * So the two classes are separated **in the explanation, never in the verdict**. + * The red/green boundary is still exactly what the canonical front end accepts: + * {@link celRefusal} asks `parseCelToAstWithReason`, which `parseCelToAst` is + * literally implemented as (same env, same limits, reason discarded), so no + * source changes colour. Identical inputs are refused before and after; one + * class of them is told the truth about why, under its own id + * (`visibility-predicate-over-budget`) for the reason the RLS gate split + * `rls-predicate-over-budget` off `rls-predicate-unparseable` in #6778 / + * PR #6831: the consequence is identical, the FIX is not, and `--json` consumers + * and allowlists key on the id. #7073 / PR #7209 made the same correction at + * ADR-0032's shared producer, which this gate deliberately does not go through + * (that is the paragraph above) — hence the same defect had to be fixed here too. * * ## Bare identifiers — the gap between two gates that both wave it through * @@ -204,10 +233,12 @@ * returns `null` there, so the declaredness check has no AST to reason about * and this rule gives no BARE-IDENTIFIER verdict on it. That is a division of * labour, not silence: since #6253 the same source is reported by - * `visibility-predicate-syntax` (§Syntax above), and the two are mutually - * exclusive by construction — a predicate that parses cannot be a syntax - * fault, and one that does not parse yields no identifiers to judge. A single - * broken predicate therefore produces exactly one finding, never two. + * `visibility-predicate-syntax` — or, since #7217, by + * `visibility-predicate-over-budget` when the refusal was a size fault — and + * the rules are mutually exclusive by construction: a predicate that parses + * cannot be a refusal, and one that does not parse yields no identifiers to + * judge. A single broken predicate therefore produces exactly one finding, + * never two. * - **Nested composite / repeater sub-fields** (`fields[].fields[]`, * `view.zod.ts:1477`). The traversal stops at a section's direct fields. A * sub-field of a repeater row is evaluated against a binding this rule cannot @@ -224,8 +255,13 @@ * build error — pinned by a test so it reads as a decision. */ -import { collectCelRootIdentifiers, firstUndeclaredReference, parseCelToAst } from '@objectstack/formula'; -import type { CelAstNode } from '@objectstack/formula'; +import { + collectCelRootIdentifiers, + firstUndeclaredReference, + parseCelToAst, + parseCelToAstWithReason, +} from '@objectstack/formula'; +import type { CelAstNode, CelBoundsOverrun } from '@objectstack/formula'; import { collectionEntries } from './collection-entries.js'; import { walkPageComponents } from './page-walk.js'; @@ -234,6 +270,14 @@ import { formViewSites } from './view-walk.js'; export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered'; export const VISIBILITY_BARE_IDENTIFIER = 'visibility-bare-identifier'; export const VISIBILITY_PREDICATE_SYNTAX = 'visibility-predicate-syntax'; +/** + * Valid CEL that overruns a `DEFAULT_LIMITS` parse bound (`maxAstNodes` 256, + * `maxDepth` 32, `maxListElements` 64, …) — #7217. A separate id from + * {@link VISIBILITY_PREDICATE_SYNTAX} for the reason `rls-predicate-over-budget` + * is a separate id from `rls-predicate-unparseable` (#6778 / PR #6831): the + * consequence is identical, the FIX is not, and `--json` consumers key on the id. + */ +export const VISIBILITY_PREDICATE_OVER_BUDGET = 'visibility-predicate-over-budget'; export type VisibilitySeverity = 'error' | 'warning'; @@ -253,8 +297,9 @@ export interface VisibilityOptions { export interface VisibilityFinding { /** * `warning` for the ADR-0089 D3b advisory (`visibility-root-mislayered`); - * `error` for the two rules that gate — `visibility-predicate-syntax` and - * `visibility-bare-identifier` (see module note). + * `error` for the three rules that gate — `visibility-predicate-syntax`, + * `visibility-predicate-over-budget` and `visibility-bare-identifier` (see + * module note). */ severity: VisibilitySeverity; /** Diagnostic rule id, e.g. `visibility-root-mislayered`. */ @@ -296,7 +341,8 @@ function usesRoot(source: string, root: string): boolean { return new RegExp(`(^|[^.\\w$])${root}\\.\\w`).test(source); } -// ── `visibility-predicate-syntax` (#6253) ─────────────────────────── +// ── `visibility-predicate-syntax` (#6253) / +// `visibility-predicate-over-budget` (#7217) ────────────────────── /** * `source` with every string literal blanked out — same length, so nothing else @@ -353,12 +399,26 @@ const NON_CEL_SPELLINGS: ReadonlyArray<{ /** What the canonical front end refused, and the corrective wording for it. */ interface CelSyntaxFault { + kind: 'syntax'; /** The front end's own one-line diagnostic, quoted rather than paraphrased. */ detail: string; /** The recognised non-CEL spelling, when the source contains one. */ token: { wrote: string; cel: string; example: string } | null; } +/** Valid CEL the front end refused only for being too big (#7217). */ +interface CelBoundsFault { + kind: 'bounds'; + /** WHICH bound, its platform value, and the front end's own summary line. */ + overrun: CelBoundsOverrun; +} + +/** + * The two classes of refusal this file reports, kept apart because the EDIT + * they call for is different — which is the whole of #7217. + */ +type CelRefusal = CelSyntaxFault | CelBoundsFault; + /** The predicate as it appears in a message — whitespace flattened, long sources elided. */ function quoteSource(source: string): string { const flat = source.replace(/\s+/g, ' ').trim(); @@ -372,28 +432,73 @@ function quoteSource(source: string): string { * this function neither parses with an environment of its own nor reaches for * `celEngine.compile` / `validateExpression`, which would widen the gate from * "does not parse" to "does not type-check". + * + * ## Why `parseCelToAstWithReason` is NOT that widening (#7217) + * + * The verdict is still `parseCelToAst`'s, by construction and not by + * convention: `parseCelToAst` IS `parseCelToAstWithReason` with the reason + * thrown away (`cel-engine.ts`: `const parsed = parseCelToAstWithReason(source); + * return parsed.ok ? parsed.ast : null;`). Identical env, identical limits, + * identical accept/reject set — so swapping the entrance moves no source across + * the red/green boundary, which is exactly the property the paragraph above is + * protecting and exactly what `compile()` would have broken. What the sister + * entrance adds is the one bit `null` cannot carry: WHICH refusal it was. + * + * That bit matters here because `parseCelToAst` collapses "this is not CEL" and + * "this is CEL, and too big" into one `null` (documented as deliberate on the + * producer's side, and right for a caller that only needs an AST). A reporter is + * the caller it is wrong for: until #7217 an 80-clause conjunction — flawless + * bare CEL, merely past `maxAstNodes` — was announced as "not valid CEL" and + * prescribed the dialect, i.e. told the author to change the one thing that was + * never wrong. An author who obeys the last sentence they were handed (an LLM + * author above all) rewrites the operators and comes back with the same + * over-budget predicate. Same defect family as #7073 at the producer and + * #6778 / PR #6831 on the RLS side. */ -function celSyntaxFault(source: string): CelSyntaxFault | null { - // A blank predicate is not a syntax fault. `parseCelToAst` returns `null` for - // an empty or whitespace-only source too, so without this guard the rule would - // report `visibleWhen: ' '` — which is "no predicate", exactly what the - // author meant, and what `validateExpression` itself short-circuits on. +function celRefusal(source: string): CelRefusal | null { + // A blank predicate is not a fault at all. The front end answers `empty` for a + // whitespace-only source, and without this the rule would report + // `visibleWhen: ' '` — which is "no predicate", exactly what the author + // meant, and what `validateExpression` itself short-circuits on. Kept as an + // explicit guard as well as a handled `kind`, so the intent is readable at + // both ends. if (!source.trim()) return null; - // The verdict, from the one entry that owns it (#4812). - if (parseCelToAst(source) !== null) return null; - // The MESSAGE, from the same package's parse-only classifier — `compile()` - // would answer a wider question, and re-throwing the parse ourselves would be - // the private front end #4812 removed. - const parsed = collectCelRootIdentifiers(source); - const detail = parsed.ok + // The verdict, from the one entry that owns it (#4812) — reached through the + // reason-carrying sister entrance, which answers the SAME parse question. + const parsed = parseCelToAstWithReason(source); + if (parsed.ok || parsed.kind === 'empty') return null; + // Valid CEL, over budget. The bound and its value come from the front end's + // own structured overrun, never from re-reading its prose (#6223). + if (parsed.kind === 'bounds') return { kind: 'bounds', overrun: parsed.overrun }; + // A genuine syntax fault: unchanged from #6253, deliberately. The MESSAGE + // comes from the same package's parse-only classifier — `compile()` would + // answer a wider question, and re-throwing the parse ourselves would be the + // private front end #4812 removed. + const identifiers = collectCelRootIdentifiers(source); + const detail = identifiers.ok // Unreachable while both entries parse the same source through the same env // under the same limits. Kept as a truthful fallback rather than a `!` // assertion, so a future divergence degrades to a vaguer message instead of // throwing inside a linter. ? 'the expression could not be parsed' - : parsed.error.split('\n')[0].trim(); + : identifiers.error.split('\n')[0].trim(); const scannable = withoutStringLiterals(source); - return { detail, token: NON_CEL_SPELLINGS.find((s) => s.re.test(scannable)) ?? null }; + return { kind: 'syntax', detail, token: NON_CEL_SPELLINGS.find((s) => s.re.test(scannable)) ?? null }; +} + +/** + * The exceeded bound named in prose, with the platform's value for it. + * + * `limit` is `null` only for a bounds fault `@objectstack/formula` cannot NAME + * (unreachable on cel-js 8.0.0, where `Parser#limitExceeded` always phrases it + * `Exceeded ()`). Degrade honestly there rather than guess a key, which + * would send the author to shorten the wrong axis — the same choice the RLS gate + * makes on the same field. + */ +function boundName(overrun: CelBoundsOverrun): string { + return overrun.limit && overrun.limitValue !== null + ? `the \`${overrun.limit}\` budget (platform limit ${overrun.limitValue})` + : "one of the platform's parse budgets"; } // ── `visibility-bare-identifier` (#6128) ──────────────────────────── @@ -565,22 +670,51 @@ function checkElement( // at the severity every other predicate surface already applies to a syntax // fault (ADR-0032 via `validateExpression`); this surface is the one that had // no such gate, so a `===` shipped clean and then failed OPEN in the console. - const syntaxFault = source ? celSyntaxFault(source) : null; - if (source && syntaxFault) { + // + // #7217 splits the refusal in two — same verdict, two different edits. A + // predicate that is flawless CEL and merely over a `DEFAULT_LIMITS` bound gets + // a SIZE finding under its own id; only a genuine dialect/syntax fault keeps + // the wording (and the id) #6253 shipped. + const refusal = source ? celRefusal(source) : null; + if (source && refusal?.kind === 'bounds') { + const bound = boundName(refusal.overrun); + const root = CANONICAL_ROOT_BY_LAYER[layer]; + findings.push({ + severity: 'error', + rule: VISIBILITY_PREDICATE_OVER_BUDGET, + where, + path, + message: + `visibility predicate is syntactically valid CEL but overruns ${bound} ` + + `(${refusal.overrun.summary}) (predicate: \`${quoteSource(source)}\`). The canonical front ` + + `end refuses it, so it can never evaluate, and the console falls OPEN: the element renders ` + + `unconditionally and looks exactly like one with no predicate at all (#5149).`, + hint: + `There is no syntax or dialect error to correct here — this is a SIZE fault, not a dialect ` + + `mistake, so re-spelling the predicate will not fix it. Make it smaller, or move the work ` + + `off the predicate: (1) collapse a long \`${root}.f == 'a' || ${root}.f == 'b' || …\` chain ` + + `into a single \`${root}.f in ['a', 'b', …]\`, which is far fewer AST nodes ` + + `(\`maxListElements\` is 64, so a very large set needs option 2); (2) precompute the heavy ` + + `part into a formula/rollup field on the object and test that one field instead. Logic ` + + `genuinely this large is not element visibility — compute it once on the record rather ` + + `than re-deriving it in every predicate that needs it.`, + }); + } + if (source && refusal?.kind === 'syntax') { findings.push({ severity: 'error', rule: VISIBILITY_PREDICATE_SYNTAX, where, path, message: - `visibility predicate is not valid CEL — ${syntaxFault.detail} ` + + `visibility predicate is not valid CEL — ${refusal.detail} ` + `(predicate: \`${quoteSource(source)}\`). A predicate that does not parse can never ` + `evaluate, and the console falls OPEN: the element renders unconditionally and looks ` + `exactly like one with no predicate at all (#5149).`, - hint: syntaxFault.token - ? `\`${syntaxFault.token.wrote}\` is not a CEL operator — CEL spells it ` + - `\`${syntaxFault.token.cel}\`. Replace \`${syntaxFault.token.wrote}\` with ` + - `\`${syntaxFault.token.cel}\`, e.g. \`${syntaxFault.token.example}\`.` + hint: refusal.token + ? `\`${refusal.token.wrote}\` is not a CEL operator — CEL spells it ` + + `\`${refusal.token.cel}\`. Replace \`${refusal.token.wrote}\` with ` + + `\`${refusal.token.cel}\`, e.g. \`${refusal.token.example}\`.` : `Visibility predicates are bare CEL, e.g. \`record.status == 'open'\`. Spellings from ` + `other languages do not parse: write \`==\` (not \`===\`), \`!=\` (not \`!==\` or \`<>\`), ` + `\`&&\` (not \`and\`), \`||\` (not \`or\`), \`!\` (not \`not\`).`, @@ -593,12 +727,13 @@ function checkElement( // layer, under neither a total nor a sparse record (#4953) — so there is no // reading of the metadata under which it was going to work. // - // Skipped when (2) fired: the declaredness check needs an AST, and a source - // that does not parse has none. Written as an explicit `else` rather than - // relying on `firstBareIdentifier`'s own null-AST guard, so the one-finding - // -per-broken-predicate property is visible at the call site instead of - // depending on a callee's internals. - if (source && !syntaxFault) { + // Skipped when (2) fired — in EITHER of its arms (#7217): the declaredness + // check needs an AST, and a source the front end refused has none, whether it + // was refused for its dialect or for its size. Written as an explicit `else` + // rather than relying on `firstBareIdentifier`'s own null-AST guard, so the + // one-finding-per-broken-predicate property is visible at the call site + // instead of depending on a callee's internals. + if (source && !refusal) { const bare = firstBareIdentifier(source); if (bare) { const root = CANONICAL_ROOT_BY_LAYER[layer]; @@ -639,9 +774,9 @@ function isFieldObject(entry: unknown): entry is AnyRec { * error elsewhere would have stopped the parse. See `AuthoringRuleInputTier`.) * * Returns findings (empty = clean). `visibility-root-mislayered` is advisory - * (`warning`); `visibility-predicate-syntax` (#6253) and - * `visibility-bare-identifier` (#6128) are `error` and the caller is expected to - * fail the build on them. + * (`warning`); `visibility-predicate-syntax` (#6253), + * `visibility-predicate-over-budget` (#7217) and `visibility-bare-identifier` + * (#6128) are `error` and the caller is expected to fail the build on them. * * The binding-root check is layer-directional (ADR-0089 D3): pass * `opts.layer = 'metadata'` when linting a `*.form.ts` metadata-editing form (so a