From 200126cd8ab3d7b593d02367cf09a0904db70104 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 00:33:14 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(lint):=20view/page=20=E5=8F=AF?= =?UTF-8?q?=E8=A7=81=E6=80=A7=E8=B0=93=E8=AF=8D=E7=9A=84=20CEL=20=E8=AF=AD?= =?UTF-8?q?=E6=B3=95=E6=9E=84=E5=BB=BA=E6=9C=9F=E9=97=B8=E9=97=A8(#6253)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 新增 error 级规则 `visibility-predicate-syntax`:view/page 的可见性谓词 (`visibleWhen` 及两个已弃用别名 `visibleOn` / `visibility`)如果规范 CEL 前端解析不了,构建期直接拒收,不再零诊断放行到运行时 fail-open。 按维护者 2026-08-07 对 #6253 的裁定:判 blocking error,与 ADR-0032 下 其它谓词面同级,不设 warning 档、不写本面豁免。 判定仍取 `parseCelToAst`(规范前端),本规则不自建 Environment、不手写 tokenizer;新增的是对既有判定的上报与自纠措辞。明确不走 `compile()` / `validateExpression`——那是 parse + 类型检查,实测会拒掉 `type == 'grid'`, 从语法分支推翻本文件已钉测试的既有盲点,并把闸门从「解析不了」扩张成 「类型检查不过」。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --- .../lint-visibility-predicate-syntax-gate.md | 56 ++++ packages/lint/src/index.ts | 1 + .../validate-visibility-predicates.test.ts | 248 +++++++++++++++++- .../src/validate-visibility-predicates.ts | 220 ++++++++++++++-- 4 files changed, 501 insertions(+), 24 deletions(-) create mode 100644 .changeset/lint-visibility-predicate-syntax-gate.md diff --git a/.changeset/lint-visibility-predicate-syntax-gate.md b/.changeset/lint-visibility-predicate-syntax-gate.md new file mode 100644 index 0000000000..a24a93c4ac --- /dev/null +++ b/.changeset/lint-visibility-predicate-syntax-gate.md @@ -0,0 +1,56 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): view/page 可见性谓词的 CEL 语法构建期闸门 —— `country === "USA"` 不再零诊断(#6253) + +新增 **error 级** 规则 `visibility-predicate-syntax`:view/page 的可见性谓词 +(`visibleWhen` 及其两个已弃用别名 `visibleOn` / `visibility`)如果规范 CEL 前端根本 +解析不了,`os validate` / `os build` / `os lint` 一律拒收。`===` 这类写法从此发不出去。 + +按维护者 2026-08-07 对 #6253 的裁定落地:**判 blocking error**,与其它谓词面 +(validation rule / flow / action,ADR-0032)同级;不设 warning 档,也不为本面写豁免—— +warning 在 CI 里通常不拦,那只是「多绕几步的静默」。 + +**为什么这一面此前无人判**:`validate-expressions.ts`(ADR-0032)对它遍历到的每条谓词都跑 +`validateExpression`,语法错报 blocking error——但它的遍历面是 objects / flows / actions / +sharingRules / hooks,**从不走 `views` 与 `pages`**。走这一面的三条规则(ADR-0089 D3b 两条 +advisory,加 #6128 的裸标识符闸)都明确不判语法,理由是「不发明第二个语法判定」。那条政策 +在它自己的调用点上成立(`validateExpression` 就在同一批调用点上跑),**在 view/page 面上不成立: +那里没有第二个判定,沉默就是没人报**。后果与 #5149 同型:谓词求值失败 → `evalFieldPredicate` +返回 fallback → 可见性 fallback 是 `true` → 元素无条件渲染,与「没写谓词」在屏幕上一模一样。 +`packages/spec/src/ui/view.test.ts` 的 fixture 就写着 `'country === "USA"'`,正说明这是作者 +(尤其 AI)会写出来的形状。 + +**判定仍然不是本包给的**——旧政策要保护的正是这一点,它完整保留:判定取 `parseCelToAst` +(规范前端,带 #3306 改写与 `DEFAULT_LIMITS`,#4812),本规则不自建 `Environment`、不手写 +tokenizer。#6253 加的是**对既有判定的上报**,外加原始报错缺的自纠措辞:cel-js 只说 +`Unexpected character: =` 并画一个 caret,既没点名作者写的运算符,也没给出 CEL 的写法。 + +**明确不走 `validateExpression` / `celEngine.compile`**,尽管那才是 ADR-0032 的入口: +`compile()` 是 parse **+ 类型检查**,差别不是理论上的——实测它会以 +`no such overload: type == string` 拒掉 `type == 'grid'`,而那正是本文件**已钉测试的既有盲点** +(字段名与 CEL 类型名相同时不判,因为改读 overload 消息会误杀合法的 `type(record.x) == string`)。 +从语法分支绕过去会把那条决定悄悄推翻,并把一条 error 级闸门从「解析不了」扩张成「类型检查不过」—— +而这一面的谓词绝大多数是 `dyn`。裁定说的是语法,parse 判定恰好就是语法。 + +**消息自纠**:实测过的非 CEL 拼法各自点名并给出 CEL 写法——`===`→`==`、`!==`→`!=`、 +`<>`→`!=`、`and`→`&&`、`or`→`||`、`not`→`!`、单个 `=`→`==`。扫描前先把字符串字面量抹平, +所以 `record.msg == 'a === b' and record.n > 1` 归咎于 `and` 而不是字面量里的 `===`; +`record.msg == 'a === b'` 本身能解析,压根不报。`??` 与 SQL 的 `IN (…)` 故意不进表:两者 +都会解析失败、都照报(带前端原话),但都没有「换一个 token」就能修好的等价写法,给半个修法 +只会让作者多跑一趟。 + +**边界**(均已钉测试):空/纯空白谓词不是语法错(`parseCelToAst` 对空源也返回 `null`, +没有这道 guard 会把「没写谓词」报成坏 CEL);`DEFAULT_LIMITS` 超限属于**边界**错而非语法错, +照报但引用前端原话、不假装找到了 typo,与 ADR-0032 把两者一并归入「invalid CEL predicate」 +的既有做法一致,且超长谓词在消息里省略,单条 runaway 表达式刷不满控制台;一条坏谓词**只出一个 +finding**——源码解析不出 AST 就没有标识符可判,裸标识符闸自动让位,该互斥性由「断言整个上报集合」 +钉住而不是靠调用方内部实现。 + +注册表无需改动:`validateVisibilityPredicates` 的 tier 在 #6128 已是 `gating`、commands 已是 +build/lint/validate,本规则的 `error` 直接沿用(已加测试复核该前提仍然成立)。 + +仓内清扫:除 `packages/spec/src/ui/view.test.ts` 那几条**纯 schema 测试样本**(它们只跑 +`FormFieldSchema.parse`,不经 lint,属于本单援引的证据而非待修点)外,全仓 examples / apps / +packages 的 view/page 可见性谓词均能通过规范前端解析,无需修改任何示例内容。 diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 27c3f6cabc..19e8da742f 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -140,6 +140,7 @@ export { VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, + VISIBILITY_PREDICATE_SYNTAX, } 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 6ee70282c5..2ec36dcc62 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_ALIAS_DEPRECATED, VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, + VISIBILITY_PREDICATE_SYNTAX, } from './validate-visibility-predicates'; import { AUTHORING_RULES } from './authoring-rules.js'; @@ -459,13 +460,16 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { expect(bareFindings(formStack("record.data == 1"))).toEqual([]); }); - it('a predicate the canonical front end will not parse is left to the syntax verdict', () => { - // `===` is not CEL. `parseCelToAst` returns null and this rule stays - // silent rather than inventing a second syntax verdict — the same policy - // `validate-null-guards.ts` states. (Documented gap: nothing validates - // view/page predicate SYNTAX today, so this one is currently un-reported.) - expect(validateVisibilityPredicates(formStack('country === "USA"'))).toEqual([]); - expect(validateVisibilityPredicates(formStack('status =='))).toEqual([]); + it('a predicate that does not parse yields no BARE-IDENTIFIER verdict (the syntax rule owns it)', () => { + // This case used to assert whole-rule SILENCE on an unparseable source, + // on the policy that a second syntax verdict must not be invented. #6253 + // ruled that policy wrong for this surface specifically — nothing else + // judges view/page syntax — so the source is now reported, by + // `visibility-predicate-syntax`. What survives from the old assertion is + // the division of labour: the declaredness check needs an AST and gets + // none, so `country` is NOT also reported as a bare identifier. + expect(bareFindings(formStack('country === "USA"'))).toEqual([]); + expect(bareFindings(formStack('status =='))).toEqual([]); }); it('an absent / empty predicate is not a finding', () => { @@ -502,3 +506,233 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { }); }); }); + +// ───────────────────────────────────────────────────────────────────── +// `visibility-predicate-syntax` — #6253 (maintainer ruling 2026-08-07: +// blocking error, same severity as every other predicate surface; no warning +// tier and no exception for this surface). +// +// The surface this closes: `validate-expressions.ts` (ADR-0032) runs +// `validateExpression` over every predicate it walks, but it walks objects / +// flows / actions / sharingRules / hooks and never `views` / `pages`. The rules +// that DO walk views/pages all declined the syntax verdict so as not to invent a +// second one — correct wherever `validateExpression` runs alongside, and wrong +// here, where nothing did. Net effect before this rule: `country === "USA"` +// built clean and then failed OPEN at runtime (#5149). +// ───────────────────────────────────────────────────────────────────── + +/** Only the syntax findings, for assertions that ignore the other three rules. */ +function syntaxFindings(stack: Record, opts?: { layer: 'runtime' | 'metadata' }) { + return validateVisibilityPredicates(stack, opts).filter((f) => f.rule === VISIBILITY_PREDICATE_SYNTAX); +} + +describe('visibility-predicate-syntax (#6253)', () => { + describe('the acceptance pair', () => { + it('`===` is an ERROR whose message names the token and shows the CEL spelling', () => { + // `country === "USA"` is the fixture-proven shape: `packages/spec/src/ui/ + // view.test.ts` writes it at :1126 / :1240 / :1245 / :1291 / :1373, which + // is what the issue cites as evidence that authors reach for it. + const findings = validateVisibilityPredicates(formStack('country === "USA"')); + + // The WHOLE reported set, not just "a syntax finding is present" — so the + // bare-identifier rule staying out of the way is pinned here too. + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]); + 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"'); + + // Self-correcting, per the ruling: name the offending token, show the CEL + // spelling. The raw parser message does neither — cel-js says + // `Unexpected character: =` and points a caret, which tells an author + // nothing about `==`. + expect(findings[0].hint).toContain('`===`'); + expect(findings[0].hint).toContain('`==`'); + // The front end's own diagnostic is quoted rather than paraphrased, and + // the predicate is echoed so the finding is self-contained. + expect(findings[0].message).toContain('Unexpected character: ='); + expect(findings[0].message).toContain('country === "USA"'); + // The consequence is stated, because on screen it is invisible. + expect(findings[0].message).toContain('#5149'); + }); + + it('the CEL spelling of the SAME predicate is clean — paired so it cannot pass vacuously', () => { + // A lone "no syntax finding is reported" assertion is green whenever the + // feature is absent, so it can never detect a regression. Pairing it with + // the positive case in one test fixes that: delete the rule and the FIRST + // expectation goes red. + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); + expect(validateVisibilityPredicates(formStack("record.country == 'USA'"))).toEqual([]); + }); + }); + + describe('every non-CEL spelling in the table names its own token', () => { + // Each row is measured against the canonical front end — `parseCelToAst` + // really does refuse all of these — so none of them is a guessed hint. + it.each([ + ['country === "USA"', '===', '=='], + ['country !== "USA"', '!==', '!='], + ["record.country <> 'USA'", '<>', '!='], + ["record.a == 1 and record.b == 2", 'and', '&&'], + ["record.a == 1 or record.b == 2", 'or', '||'], + ['not record.archived', 'not', '!'], + ["record.status = 'open'", '=', '=='], + ])('%s → names `%s`, prescribes `%s`', (predicate, wrote, cel) => { + const findings = syntaxFindings(formStack(predicate)); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].hint).toContain(`\`${wrote}\``); + expect(findings[0].hint).toContain(`\`${cel}\``); + }); + + it('a fault with no single-token equivalent still reports, with the parser\'s own words', () => { + // `status ==` is a truncated expression: nothing to swap, so the hint + // falls back to the general shape instead of inventing a token. + const findings = syntaxFindings(formStack('status ==')); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('Unexpected token: EOF'); + expect(findings[0].hint).toContain("record.status == 'open'"); + }); + + it('blames the operator that actually broke it, not one quoted inside a string', () => { + // The predicate fails on `and`; the `===` sits inside a string literal and + // is none of the reason. Blaming it would send the author to edit a + // perfectly good literal. (String literals are blanked before the scan.) + const findings = syntaxFindings(formStack("record.msg == 'a === b' and record.n > 1")); + expect(findings).toHaveLength(1); + expect(findings[0].hint).toContain('`and`'); + expect(findings[0].hint).not.toContain('`===`'); + }); + + it('a non-CEL operator INSIDE a string literal is not a fault at all', () => { + // Same string, no `and` — this parses, so there is no finding to word. + expect(validateVisibilityPredicates(formStack("record.msg == 'a === b'"))).toEqual([]); + }); + }); + + describe('the boundaries', () => { + it('an absent / blank predicate is NOT a syntax fault', () => { + // `parseCelToAst` returns null for an empty source too, so without an + // explicit guard this rule would report "no predicate" as broken CEL. + // Paired with a live case so the assertion can actually fail. + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); + expect(validateVisibilityPredicates(formStack(undefined))).toEqual([]); + expect(validateVisibilityPredicates(formStack(' '))).toEqual([]); + expect(validateVisibilityPredicates(formStack(''))).toEqual([]); + }); + + it('exactly ONE finding per broken predicate — the syntax rule, not also the bare-ref rule', () => { + // `country` is rootless as well as mis-spelled, but a source with no AST + // yields no identifiers to judge. Asserting the whole set (not just "a + // syntax finding exists") is what pins the exclusivity. + expect(validateVisibilityPredicates(formStack('country === "USA"')).map((f) => f.rule)) + .toEqual([VISIBILITY_PREDICATE_SYNTAX]); + // …and the converse: a source that PARSES is judged by the bare-ref rule + // and never by this one. + expect(validateVisibilityPredicates(formStack("status == 'active'")).map((f) => f.rule)) + .toEqual([VISIBILITY_BARE_IDENTIFIER]); + }); + + it('does NOT widen to type-checking — the CEL-type blind spot stays a blind spot', () => { + // `type == 'grid'` PARSES; only `celEngine.compile`'s type checker rejects + // it (`no such overload: type == string`). Routing this rule through + // `compile` / `validateExpression` would silently overturn the deliberate, + // separately-pinned decision to stay conservative there — and would widen + // an error-level gate from "does not parse" to "does not type-check" on a + // surface whose predicates are overwhelmingly `dyn`. The ruling said + // syntax; the parse verdict is exactly syntax. + expect(validateVisibilityPredicates(formStack("type == 'grid'"))).toEqual([]); + // The legitimate CEL the overload message cannot be told apart from. + 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. + 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('...'); + }); + + it.each([ + ['record.a == 1 && record.b == 2', 'the `&&` CEL spells `and` as'], + ['record.a == 1 || record.b == 2', 'the `||` CEL spells `or` as'], + ['!record.archived', 'the `!` CEL spells `not` as'], + ["record.status != 'open'", 'the `!=` CEL spells `<>` as'], + ["record.tags.all(t, t != '')", 'a comprehension macro'], + ["record.type == 'a' ? record.x > 1 : record.y == 'b'", 'a ternary'], + ["record.type in ['lookup', 'master_detail']", 'lowercase `in` — a REAL CEL operator, unlike SQL `IN`'], + ["has(record.a) && record.b != null", 'both guard idioms at once'], + ])('%s parses and is not reported (%s)', (predicate) => { + expect(syntaxFindings(formStack(predicate))).toEqual([]); + }); + }); + + describe('every carrier the schema declares, and both layers', () => { + it('a form SECTION predicate', () => { + const stack = { views: [{ name: 'f', sections: [{ visibleWhen: 'country === "USA"', fields: [] }] }] }; + expect(syntaxFindings(stack).map((f) => f.path)).toEqual(['views[0].sections[0]']); + }); + + it('a PAGE COMPONENT predicate', () => { + const stack = { + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: 'kind === "a"' }] }] }], + }; + const findings = syntaxFindings(stack); + expect(findings.map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']); + expect(findings[0].where).toBe('page "p"'); + }); + + it('reads the value through the deprecated `visibleOn` alias (alias + syntax, both reported)', () => { + // Two independent defects on one element, so unlike the syntax/bare-ref + // pair these DO both report. + const stack = { views: [{ name: 'f', sections: [{ visibleOn: 'status === "x"', fields: [] }] }] }; + expect(validateVisibilityPredicates(stack).map((f) => f.rule).sort()) + .toEqual([VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_PREDICATE_SYNTAX].sort()); + }); + + it('reads the value through the deprecated page-side `visibility` alias', () => { + const stack = { + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibility: 'shown === true' }] }] }], + }; + expect(syntaxFindings(stack)).toHaveLength(1); + }); + + it('resolves a `{ dialect, source }` envelope the same as a bare string', () => { + expect(syntaxFindings(formStack({ dialect: 'cel', source: 'country === "USA"' }))).toHaveLength(1); + }); + + it('reaches a container\'s `formViews.` — the shape a real stack emits', () => { + const stack = { + views: [{ object: 'showcase_task', formViews: { edit: { sections: [{ fields: [{ field: 'n', visibleWhen: 'country === "USA"' }] }] } } }], + }; + expect(syntaxFindings(stack).map((f) => f.path)).toEqual([ + 'views[0].formViews.edit.sections[0].fields[0]', + ]); + }); + + it('is layer-agnostic — a syntax fault is a syntax fault on a metadata form too', () => { + // Unlike the root rules, nothing about "does it parse" depends on which + // namespace the surface binds. + expect(syntaxFindings(formStack('country === "USA"'), { layer: 'metadata' })).toHaveLength(1); + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); + }); + }); + + it('the registry entry already gates, so this `error` reaches all three commands', () => { + // `severity: 'error'` only fails a build because `authoring-rules.ts` marks + // the family `gating` and runs it on validate/build/lint alike. That entry + // was already `gating` (#6128 promoted it), so #6253 needed no registry + // change — this pins that it is still true rather than assuming it. + const entry = AUTHORING_RULES.find((r) => r.name === 'validateVisibilityPredicates'); + expect(entry, 'validateVisibilityPredicates must be registered').toBeDefined(); + expect(entry!.tier).toBe('gating'); + expect([...entry!.commands].sort()).toEqual(['build', 'lint', 'validate']); + }); +}); diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index 56a09f06ab..29ad7af423 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -28,12 +28,16 @@ * fold carries into `visibleWhen` intact — both still report normally on the * normalized tier, and neither is affected by the above. * - * Two advisory rules (both `warning` — nothing is broken, the alias still works - * and a mis-rooted predicate just never matches) plus one **gating** rule + * One advisory rule pair (both `warning` — nothing is broken, the alias still + * works and a mis-rooted predicate just never matches) plus TWO **gating** rules * (`error` — the predicate can never evaluate at all): * * - `visibility-alias-deprecated` — a `visibleOn` / `visibility` key in authored * source. Autofix intent: rename the key to `visibleWhen` (same value). + * - `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. * - `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 @@ -63,6 +67,54 @@ * (`view.zod.ts:1418` / `:1512`) and `visibility` the page-side one * (`page.zod.ts:145`). There is no fourth spelling on this surface. * + * ## Syntax — the one surface where "don't judge syntax" meant nobody judged it + * + * `visibility-predicate-syntax` exists because a policy that is right at three + * call sites was wrong at this one. Both this rule and `validate-null-guards.ts` + * used to skip a source `parseCelToAst` refuses, on the stated grounds that a + * second syntax verdict must not be invented. That reasoning holds wherever + * `validateExpression` (ADR-0032) runs over the same predicate — it does the + * judging, and a duplicate would only disagree. It does NOT hold here: + * `validate-expressions.ts` walks objects / flows / actions / sharingRules / + * hooks and never `views` or `pages`, so on this surface "leave it to the gate + * that owns it" resolved to no gate at all. `country === "USA"` — the spelling + * `packages/spec/src/ui/view.test.ts` fixtures prove authors reach for — passed + * the build with zero diagnostics, and then failed OPEN in the console for the + * #5149 reason the whole family is about: eval faults, `evalFieldPredicate` + * returns its fallback, the visibility fallback is `true`, and the element + * renders unconditionally, pixel-identical to one carrying no predicate. + * + * Maintainer ruling (#6253, 2026-08-07): **blocking error**, at the same + * severity every other predicate surface already applies to a syntax fault. No + * warning tier and no documented exception for this surface — a warning that + * does not fail CI is silence with extra steps. + * + * ### The verdict is still not ours (which is what the old policy was protecting) + * + * The policy's real content was "one answer to what parses", and that survives + * intact: the verdict is `parseCelToAst`'s — the canonical front end, carrying + * the #3306 rewrite and `DEFAULT_LIMITS` (#4812) — and this rule builds no + * `Environment` and hand-rolls no tokenizer to second-guess it. What #6253 adds + * is a REPORT of a verdict already reached, plus the corrective wording the raw + * parser message lacks: cel-js says `Unexpected character: =` with a caret, which + * names neither the operator the author actually typed nor the CEL spelling of + * it. {@link NON_CEL_SPELLINGS} supplies that, and cannot change any verdict — + * it is consulted only after the parse has already failed. + * + * Deliberately NOT `validateExpression` / `celEngine.compile`, though those are + * the ADR-0032 entries and the temptation is obvious. `compile()` is parse **+ + * type-check**, and the difference is not theoretical: measured, it rejects + * `type == 'grid'` with `no such overload: type == string`. That shape is this + * file's pinned blind spot (see the CEL-TYPE bullet below) — a deliberate, + * test-documented decision to stay conservative — and routing this rule through + * `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. + * * ## Bare identifiers — the gap between two gates that both wave it through * * `visibility-bare-identifier` exists because #5149 Repro 1 measured a predicate @@ -125,14 +177,13 @@ * `record.x != null` and every other guard idiom stay green here, whichever * way #4953 is eventually settled. * - **A predicate the canonical front end will not parse.** `parseCelToAst` - * returns `null` for a syntax fault or a `DEFAULT_LIMITS` overrun, and this - * rule then stays silent rather than inventing a second syntax verdict - * (`validate-null-guards.ts` states the same policy for the same reason). - * Worth knowing where that leaves the surface: unlike the object/flow/action - * sites, NOTHING validates view/page predicate syntax today, so a `=` typo is - * still un-diagnosed here. Widening this rule to own that verdict is a - * separate decision about what authors may write, not a wiring gap to close - * in passing. + * 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. * - **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 @@ -149,7 +200,7 @@ * build error — pinned by a test so it reads as a decision. */ -import { firstUndeclaredReference, parseCelToAst } from '@objectstack/formula'; +import { collectCelRootIdentifiers, firstUndeclaredReference, parseCelToAst } from '@objectstack/formula'; import type { CelAstNode } from '@objectstack/formula'; import { walkPageComponents } from './page-walk.js'; @@ -157,6 +208,7 @@ import { walkPageComponents } from './page-walk.js'; export const VISIBILITY_ALIAS_DEPRECATED = 'visibility-alias-deprecated'; export const VISIBILITY_ROOT_MISLAYERED = 'visibility-root-mislayered'; export const VISIBILITY_BARE_IDENTIFIER = 'visibility-bare-identifier'; +export const VISIBILITY_PREDICATE_SYNTAX = 'visibility-predicate-syntax'; export type VisibilitySeverity = 'error' | 'warning'; @@ -175,8 +227,9 @@ export interface VisibilityOptions { export interface VisibilityFinding { /** - * `warning` for the two ADR-0089 D3b advisories; `error` for - * `visibility-bare-identifier`, which gates (see module note). + * `warning` for the two ADR-0089 D3b advisories; `error` for the two rules + * that gate — `visibility-predicate-syntax` and `visibility-bare-identifier` + * (see module note). */ severity: VisibilitySeverity; /** Diagnostic rule id, e.g. `visibility-alias-deprecated`. */ @@ -240,6 +293,106 @@ function usesRoot(source: string, root: string): boolean { return new RegExp(`(^|[^.\\w$])${root}\\.\\w`).test(source); } +// ── `visibility-predicate-syntax` (#6253) ─────────────────────────── + +/** + * `source` with every string literal blanked out — same length, so nothing else + * shifts. The spelling scan below must never read INSIDE a literal, because the + * two things are independent: `record.msg == 'a === b'` parses fine and never + * reaches this code at all, while `record.msg == 'x' and record.n > 1` fails on + * `and` and would otherwise be blamed on the `===`-free string beside it. Only + * the hint could ever be wrong that way — the verdict is already in — but a hint + * naming the wrong token is worse than no hint. + * + * The alternation is unambiguous by construction (`[^'\\]` and `\\.` cannot both + * match at one position), so this scans linearly — the same ReDoS discipline + * `@objectstack/formula`'s `validate.ts` states for its own scanners. + */ +function withoutStringLiterals(source: string): string { + return source.replace(/'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"/g, (lit) => ' '.repeat(lit.length)); +} + +/** + * Non-CEL operator spellings an author — or a model fluent in JavaScript, SQL and + * Python — reaches for, each with the CEL token that means the same thing. + * + * Every row is MEASURED to fail the canonical parse; none is a guess. None can + * change a verdict either: the table is consulted only once `parseCelToAst` has + * already refused the source, so its whole effect is on the wording of a finding + * that was going to be emitted regardless. + * + * Order matters — `===` and `!==` contain shorter operators, so they match first + * and the bare `=` row last. + * + * Deliberately absent, though both are measured to fail the parse and both are + * still reported (with the front end's own diagnostic): `??`, because CEL has no + * null-coalescing operator to swap in, and SQL's `IN (…)`, because the CEL + * spelling changes the list literal too (`('a','b')` → `['a','b']`). A + * "write X instead of Y" hint that is only half the fix sends the author round a + * second lap, which is the opposite of self-correcting. + */ +const NON_CEL_SPELLINGS: ReadonlyArray<{ + wrote: string; + cel: string; + example: string; + re: RegExp; +}> = [ + { wrote: '===', cel: '==', example: "record.country == 'USA'", re: /===/ }, + { wrote: '!==', cel: '!=', example: "record.country != 'USA'", re: /!==/ }, + { wrote: '<>', cel: '!=', example: "record.country != 'USA'", re: /<>/ }, + { wrote: 'and', cel: '&&', example: "record.a == 1 && record.b == 2", re: /(?=`). + { wrote: '=', cel: '==', example: "record.status == 'open'", re: /(?])=(?!=)/ }, +]; + +/** What the canonical front end refused, and the corrective wording for it. */ +interface CelSyntaxFault { + /** 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; +} + +/** 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(); + return flat.length > 120 ? `${flat.slice(0, 117)}...` : flat; +} + +/** + * The canonical front end's refusal of `source`, or `null` when it parses. + * + * Two things are deliberately NOT done here (see the module note's §Syntax): + * 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". + */ +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. + 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 + // 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(); + const scannable = withoutStringLiterals(source); + return { detail, token: NON_CEL_SPELLINGS.find((s) => s.re.test(scannable)) ?? null }; +} + // ── `visibility-bare-identifier` (#6128) ──────────────────────────── /** @@ -407,12 +560,44 @@ function checkElement( }); } - // (3) #6128 — a reference no binding root can resolve. Unlike (2) this one + // (3) #6253 — the canonical CEL front end refuses the source outright. GATES, + // 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 (syntaxFault) { + findings.push({ + severity: 'error', + rule: VISIBILITY_PREDICATE_SYNTAX, + where, + path, + message: + `visibility predicate is not valid CEL — ${syntaxFault.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}\`.` + : `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\`).`, + }); + } + + // (4) #6128 — a reference no binding root can resolve. Unlike (2) this one // GATES: a mis-rooted predicate is at least a statement about a namespace // someone binds somewhere, while a bare identifier resolves nowhere, on no // 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. - if (source) { + // + // Skipped when (3) 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) { const bare = firstBareIdentifier(source); if (bare) { const root = CANONICAL_ROOT_BY_LAYER[layer]; @@ -500,8 +685,9 @@ function formViewSites( * Runs on the **pre-parse** (normalized) stack so it can see the deprecated * `visibleOn` / `visibility` aliases before the schema folds them into * `visibleWhen`. Returns findings (empty = clean). The two ADR-0089 D3b rules - * are advisory (`warning`); `visibility-bare-identifier` is `error` and the - * caller is expected to fail the build on it (#6128). + * are advisory (`warning`); `visibility-predicate-syntax` (#6253) 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 From 43d5ced439309f023fec653a87e8fc9f5438f56a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 00:53:20 +0000 Subject: [PATCH 2/3] =?UTF-8?q?refactor(lint):=20=E7=94=A8=E7=B1=BB?= =?UTF-8?q?=E5=9E=8B=E6=94=B6=E7=AA=84=E6=9B=BF=E4=BB=A3=E9=9D=9E=E7=A9=BA?= =?UTF-8?q?=E6=96=AD=E8=A8=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `quoteSource(source!)` 改为在 `if (source && syntaxFault)` 内由编译器收窄, 去掉本文件注释自己反对的 `!` 断言。行为不变。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --- packages/lint/src/validate-visibility-predicates.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index 29ad7af423..6bc6c48246 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -565,7 +565,7 @@ function checkElement( // 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 (syntaxFault) { + if (source && syntaxFault) { findings.push({ severity: 'error', rule: VISIBILITY_PREDICATE_SYNTAX, @@ -573,7 +573,7 @@ function checkElement( path, message: `visibility predicate is not valid CEL — ${syntaxFault.detail} ` + - `(predicate: \`${quoteSource(source!)}\`). A predicate that does not parse can never ` + + `(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 From d82ebf4eae21e35a8c96920195178feb967a40ac Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 01:25:50 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix(lint):=20=E6=B5=8B=E8=AF=95=E6=96=87?= =?UTF-8?q?=E4=BB=B6=E7=9A=84=E7=9B=B8=E5=AF=B9=E5=AF=BC=E5=85=A5=E8=A1=A5?= =?UTF-8?q?=20`.js`=20=E5=90=8E=E7=BC=80,=E6=B6=88=E9=99=A4=20TS2835=20?= =?UTF-8?q?=E5=8F=8A=E5=85=B6=20TS7006=20=E7=BA=A7=E8=81=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:type-check-debt`(TypeScript Type Check job 的最后一步)红: @objectstack/lint 的 TEST_DEBT 实测 42 → 47(+5)。 根因是一处**既有**缺陷,被本单新增的测试放大:测试文件第 10 行 `from './validate-visibility-predicates'` 缺 `.js` 后缀。在 NodeNext 下这是 TS2835,且该模块因此解析为 `any`,于是文件里每一个 `.map((f) => …)` / `.filter((f) => …)` 回调都级联出 TS7006(`f` 隐式 any)。本单新增的断言带来 更多这类回调,把既有级联乘大了。 补上 `.js` 后(与同包所有测试文件、以及本文件自己的第二条 import `./authoring-rules.js` 一致),该文件的测试层错误 28 → **0**, 整包 47 → **19**,低于台账记录的 42。 **台账未抬**(棘轮只减不增,#5278):entry 仍是 42,门现在把它报成 改进(`ℹ TEST_DEBT records 42, tsc now reports 19 (-23)`)。 **测试未削弱**:本次改动是 1 增 1 删的单行 import,断言一条未动 —— 118 处 expect、86 条用例(其中本单新增 32 条)全绿。 本地未复现是因为检查面不同:@objectstack/lint 的 tsconfig 把 `*.test.ts` 排除在外,所以 `pnpm --filter @objectstack/lint typecheck` 根本不读测试文件, 而该门会解除排除后重测。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --- packages/lint/src/validate-visibility-predicates.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/lint/src/validate-visibility-predicates.test.ts b/packages/lint/src/validate-visibility-predicates.test.ts index 2ec36dcc62..915ad86d53 100644 --- a/packages/lint/src/validate-visibility-predicates.test.ts +++ b/packages/lint/src/validate-visibility-predicates.test.ts @@ -7,7 +7,7 @@ import { VISIBILITY_ROOT_MISLAYERED, VISIBILITY_BARE_IDENTIFIER, VISIBILITY_PREDICATE_SYNTAX, -} from './validate-visibility-predicates'; +} from './validate-visibility-predicates.js'; import { AUTHORING_RULES } from './authoring-rules.js'; describe('validateVisibilityPredicates (ADR-0089 D3b)', () => {