|
6 | 6 | VISIBILITY_ALIAS_DEPRECATED, |
7 | 7 | VISIBILITY_ROOT_MISLAYERED, |
8 | 8 | VISIBILITY_BARE_IDENTIFIER, |
9 | | -} from './validate-visibility-predicates'; |
| 9 | + VISIBILITY_PREDICATE_SYNTAX, |
| 10 | +} from './validate-visibility-predicates.js'; |
10 | 11 | import { AUTHORING_RULES } from './authoring-rules.js'; |
11 | 12 |
|
12 | 13 | describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { |
@@ -459,13 +460,16 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { |
459 | 460 | expect(bareFindings(formStack("record.data == 1"))).toEqual([]); |
460 | 461 | }); |
461 | 462 |
|
462 | | - it('a predicate the canonical front end will not parse is left to the syntax verdict', () => { |
463 | | - // `===` is not CEL. `parseCelToAst` returns null and this rule stays |
464 | | - // silent rather than inventing a second syntax verdict — the same policy |
465 | | - // `validate-null-guards.ts` states. (Documented gap: nothing validates |
466 | | - // view/page predicate SYNTAX today, so this one is currently un-reported.) |
467 | | - expect(validateVisibilityPredicates(formStack('country === "USA"'))).toEqual([]); |
468 | | - expect(validateVisibilityPredicates(formStack('status =='))).toEqual([]); |
| 463 | + it('a predicate that does not parse yields no BARE-IDENTIFIER verdict (the syntax rule owns it)', () => { |
| 464 | + // This case used to assert whole-rule SILENCE on an unparseable source, |
| 465 | + // on the policy that a second syntax verdict must not be invented. #6253 |
| 466 | + // ruled that policy wrong for this surface specifically — nothing else |
| 467 | + // judges view/page syntax — so the source is now reported, by |
| 468 | + // `visibility-predicate-syntax`. What survives from the old assertion is |
| 469 | + // the division of labour: the declaredness check needs an AST and gets |
| 470 | + // none, so `country` is NOT also reported as a bare identifier. |
| 471 | + expect(bareFindings(formStack('country === "USA"'))).toEqual([]); |
| 472 | + expect(bareFindings(formStack('status =='))).toEqual([]); |
469 | 473 | }); |
470 | 474 |
|
471 | 475 | it('an absent / empty predicate is not a finding', () => { |
@@ -502,3 +506,233 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { |
502 | 506 | }); |
503 | 507 | }); |
504 | 508 | }); |
| 509 | + |
| 510 | +// ───────────────────────────────────────────────────────────────────── |
| 511 | +// `visibility-predicate-syntax` — #6253 (maintainer ruling 2026-08-07: |
| 512 | +// blocking error, same severity as every other predicate surface; no warning |
| 513 | +// tier and no exception for this surface). |
| 514 | +// |
| 515 | +// The surface this closes: `validate-expressions.ts` (ADR-0032) runs |
| 516 | +// `validateExpression` over every predicate it walks, but it walks objects / |
| 517 | +// flows / actions / sharingRules / hooks and never `views` / `pages`. The rules |
| 518 | +// that DO walk views/pages all declined the syntax verdict so as not to invent a |
| 519 | +// second one — correct wherever `validateExpression` runs alongside, and wrong |
| 520 | +// here, where nothing did. Net effect before this rule: `country === "USA"` |
| 521 | +// built clean and then failed OPEN at runtime (#5149). |
| 522 | +// ───────────────────────────────────────────────────────────────────── |
| 523 | + |
| 524 | +/** Only the syntax findings, for assertions that ignore the other three rules. */ |
| 525 | +function syntaxFindings(stack: Record<string, unknown>, opts?: { layer: 'runtime' | 'metadata' }) { |
| 526 | + return validateVisibilityPredicates(stack, opts).filter((f) => f.rule === VISIBILITY_PREDICATE_SYNTAX); |
| 527 | +} |
| 528 | + |
| 529 | +describe('visibility-predicate-syntax (#6253)', () => { |
| 530 | + describe('the acceptance pair', () => { |
| 531 | + it('`===` is an ERROR whose message names the token and shows the CEL spelling', () => { |
| 532 | + // `country === "USA"` is the fixture-proven shape: `packages/spec/src/ui/ |
| 533 | + // view.test.ts` writes it at :1126 / :1240 / :1245 / :1291 / :1373, which |
| 534 | + // is what the issue cites as evidence that authors reach for it. |
| 535 | + const findings = validateVisibilityPredicates(formStack('country === "USA"')); |
| 536 | + |
| 537 | + // The WHOLE reported set, not just "a syntax finding is present" — so the |
| 538 | + // bare-identifier rule staying out of the way is pinned here too. |
| 539 | + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_PREDICATE_SYNTAX]); |
| 540 | + expect(findings[0].severity).toBe('error'); |
| 541 | + expect(findings[0].path).toBe('views[0].sections[0].fields[0]'); |
| 542 | + expect(findings[0].where).toBe('view "task_form"'); |
| 543 | + |
| 544 | + // Self-correcting, per the ruling: name the offending token, show the CEL |
| 545 | + // spelling. The raw parser message does neither — cel-js says |
| 546 | + // `Unexpected character: =` and points a caret, which tells an author |
| 547 | + // nothing about `==`. |
| 548 | + expect(findings[0].hint).toContain('`===`'); |
| 549 | + expect(findings[0].hint).toContain('`==`'); |
| 550 | + // The front end's own diagnostic is quoted rather than paraphrased, and |
| 551 | + // the predicate is echoed so the finding is self-contained. |
| 552 | + expect(findings[0].message).toContain('Unexpected character: ='); |
| 553 | + expect(findings[0].message).toContain('country === "USA"'); |
| 554 | + // The consequence is stated, because on screen it is invisible. |
| 555 | + expect(findings[0].message).toContain('#5149'); |
| 556 | + }); |
| 557 | + |
| 558 | + it('the CEL spelling of the SAME predicate is clean — paired so it cannot pass vacuously', () => { |
| 559 | + // A lone "no syntax finding is reported" assertion is green whenever the |
| 560 | + // feature is absent, so it can never detect a regression. Pairing it with |
| 561 | + // the positive case in one test fixes that: delete the rule and the FIRST |
| 562 | + // expectation goes red. |
| 563 | + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); |
| 564 | + expect(validateVisibilityPredicates(formStack("record.country == 'USA'"))).toEqual([]); |
| 565 | + }); |
| 566 | + }); |
| 567 | + |
| 568 | + describe('every non-CEL spelling in the table names its own token', () => { |
| 569 | + // Each row is measured against the canonical front end — `parseCelToAst` |
| 570 | + // really does refuse all of these — so none of them is a guessed hint. |
| 571 | + it.each([ |
| 572 | + ['country === "USA"', '===', '=='], |
| 573 | + ['country !== "USA"', '!==', '!='], |
| 574 | + ["record.country <> 'USA'", '<>', '!='], |
| 575 | + ["record.a == 1 and record.b == 2", 'and', '&&'], |
| 576 | + ["record.a == 1 or record.b == 2", 'or', '||'], |
| 577 | + ['not record.archived', 'not', '!'], |
| 578 | + ["record.status = 'open'", '=', '=='], |
| 579 | + ])('%s → names `%s`, prescribes `%s`', (predicate, wrote, cel) => { |
| 580 | + const findings = syntaxFindings(formStack(predicate)); |
| 581 | + expect(findings).toHaveLength(1); |
| 582 | + expect(findings[0].severity).toBe('error'); |
| 583 | + expect(findings[0].hint).toContain(`\`${wrote}\``); |
| 584 | + expect(findings[0].hint).toContain(`\`${cel}\``); |
| 585 | + }); |
| 586 | + |
| 587 | + it('a fault with no single-token equivalent still reports, with the parser\'s own words', () => { |
| 588 | + // `status ==` is a truncated expression: nothing to swap, so the hint |
| 589 | + // falls back to the general shape instead of inventing a token. |
| 590 | + const findings = syntaxFindings(formStack('status ==')); |
| 591 | + expect(findings).toHaveLength(1); |
| 592 | + expect(findings[0].message).toContain('Unexpected token: EOF'); |
| 593 | + expect(findings[0].hint).toContain("record.status == 'open'"); |
| 594 | + }); |
| 595 | + |
| 596 | + it('blames the operator that actually broke it, not one quoted inside a string', () => { |
| 597 | + // The predicate fails on `and`; the `===` sits inside a string literal and |
| 598 | + // is none of the reason. Blaming it would send the author to edit a |
| 599 | + // perfectly good literal. (String literals are blanked before the scan.) |
| 600 | + const findings = syntaxFindings(formStack("record.msg == 'a === b' and record.n > 1")); |
| 601 | + expect(findings).toHaveLength(1); |
| 602 | + expect(findings[0].hint).toContain('`and`'); |
| 603 | + expect(findings[0].hint).not.toContain('`===`'); |
| 604 | + }); |
| 605 | + |
| 606 | + it('a non-CEL operator INSIDE a string literal is not a fault at all', () => { |
| 607 | + // Same string, no `and` — this parses, so there is no finding to word. |
| 608 | + expect(validateVisibilityPredicates(formStack("record.msg == 'a === b'"))).toEqual([]); |
| 609 | + }); |
| 610 | + }); |
| 611 | + |
| 612 | + describe('the boundaries', () => { |
| 613 | + it('an absent / blank predicate is NOT a syntax fault', () => { |
| 614 | + // `parseCelToAst` returns null for an empty source too, so without an |
| 615 | + // explicit guard this rule would report "no predicate" as broken CEL. |
| 616 | + // Paired with a live case so the assertion can actually fail. |
| 617 | + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); |
| 618 | + expect(validateVisibilityPredicates(formStack(undefined))).toEqual([]); |
| 619 | + expect(validateVisibilityPredicates(formStack(' '))).toEqual([]); |
| 620 | + expect(validateVisibilityPredicates(formStack(''))).toEqual([]); |
| 621 | + }); |
| 622 | + |
| 623 | + it('exactly ONE finding per broken predicate — the syntax rule, not also the bare-ref rule', () => { |
| 624 | + // `country` is rootless as well as mis-spelled, but a source with no AST |
| 625 | + // yields no identifiers to judge. Asserting the whole set (not just "a |
| 626 | + // syntax finding exists") is what pins the exclusivity. |
| 627 | + expect(validateVisibilityPredicates(formStack('country === "USA"')).map((f) => f.rule)) |
| 628 | + .toEqual([VISIBILITY_PREDICATE_SYNTAX]); |
| 629 | + // …and the converse: a source that PARSES is judged by the bare-ref rule |
| 630 | + // and never by this one. |
| 631 | + expect(validateVisibilityPredicates(formStack("status == 'active'")).map((f) => f.rule)) |
| 632 | + .toEqual([VISIBILITY_BARE_IDENTIFIER]); |
| 633 | + }); |
| 634 | + |
| 635 | + it('does NOT widen to type-checking — the CEL-type blind spot stays a blind spot', () => { |
| 636 | + // `type == 'grid'` PARSES; only `celEngine.compile`'s type checker rejects |
| 637 | + // it (`no such overload: type == string`). Routing this rule through |
| 638 | + // `compile` / `validateExpression` would silently overturn the deliberate, |
| 639 | + // separately-pinned decision to stay conservative there — and would widen |
| 640 | + // an error-level gate from "does not parse" to "does not type-check" on a |
| 641 | + // surface whose predicates are overwhelmingly `dyn`. The ruling said |
| 642 | + // syntax; the parse verdict is exactly syntax. |
| 643 | + expect(validateVisibilityPredicates(formStack("type == 'grid'"))).toEqual([]); |
| 644 | + // The legitimate CEL the overload message cannot be told apart from. |
| 645 | + expect(validateVisibilityPredicates(formStack('type(record.x) == string'))).toEqual([]); |
| 646 | + }); |
| 647 | + |
| 648 | + it('a `DEFAULT_LIMITS` overrun is reported too, in the front end\'s own words', () => { |
| 649 | + // `parseCelToAst` also returns null for a source over the platform bounds. |
| 650 | + // That is a bounds fault, not a syntax one, and the message says so rather |
| 651 | + // than pretending to have found a typo — the same way ADR-0032 already |
| 652 | + // reports it under the "invalid CEL predicate" heading. |
| 653 | + const overrun = `record.a${' + record.b'.repeat(400)}`; |
| 654 | + const findings = syntaxFindings(formStack(overrun)); |
| 655 | + expect(findings).toHaveLength(1); |
| 656 | + expect(findings[0].message).toContain('Exceeded maxAstNodes'); |
| 657 | + // The echoed predicate is elided, so one runaway expression cannot flood |
| 658 | + // the console with a 4KB finding. |
| 659 | + expect(findings[0].message).not.toContain(overrun); |
| 660 | + expect(findings[0].message).toContain('...'); |
| 661 | + }); |
| 662 | + |
| 663 | + it.each([ |
| 664 | + ['record.a == 1 && record.b == 2', 'the `&&` CEL spells `and` as'], |
| 665 | + ['record.a == 1 || record.b == 2', 'the `||` CEL spells `or` as'], |
| 666 | + ['!record.archived', 'the `!` CEL spells `not` as'], |
| 667 | + ["record.status != 'open'", 'the `!=` CEL spells `<>` as'], |
| 668 | + ["record.tags.all(t, t != '')", 'a comprehension macro'], |
| 669 | + ["record.type == 'a' ? record.x > 1 : record.y == 'b'", 'a ternary'], |
| 670 | + ["record.type in ['lookup', 'master_detail']", 'lowercase `in` — a REAL CEL operator, unlike SQL `IN`'], |
| 671 | + ["has(record.a) && record.b != null", 'both guard idioms at once'], |
| 672 | + ])('%s parses and is not reported (%s)', (predicate) => { |
| 673 | + expect(syntaxFindings(formStack(predicate))).toEqual([]); |
| 674 | + }); |
| 675 | + }); |
| 676 | + |
| 677 | + describe('every carrier the schema declares, and both layers', () => { |
| 678 | + it('a form SECTION predicate', () => { |
| 679 | + const stack = { views: [{ name: 'f', sections: [{ visibleWhen: 'country === "USA"', fields: [] }] }] }; |
| 680 | + expect(syntaxFindings(stack).map((f) => f.path)).toEqual(['views[0].sections[0]']); |
| 681 | + }); |
| 682 | + |
| 683 | + it('a PAGE COMPONENT predicate', () => { |
| 684 | + const stack = { |
| 685 | + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: 'kind === "a"' }] }] }], |
| 686 | + }; |
| 687 | + const findings = syntaxFindings(stack); |
| 688 | + expect(findings.map((f) => f.path)).toEqual(['pages[0].regions[0].components[0]']); |
| 689 | + expect(findings[0].where).toBe('page "p"'); |
| 690 | + }); |
| 691 | + |
| 692 | + it('reads the value through the deprecated `visibleOn` alias (alias + syntax, both reported)', () => { |
| 693 | + // Two independent defects on one element, so unlike the syntax/bare-ref |
| 694 | + // pair these DO both report. |
| 695 | + const stack = { views: [{ name: 'f', sections: [{ visibleOn: 'status === "x"', fields: [] }] }] }; |
| 696 | + expect(validateVisibilityPredicates(stack).map((f) => f.rule).sort()) |
| 697 | + .toEqual([VISIBILITY_ALIAS_DEPRECATED, VISIBILITY_PREDICATE_SYNTAX].sort()); |
| 698 | + }); |
| 699 | + |
| 700 | + it('reads the value through the deprecated page-side `visibility` alias', () => { |
| 701 | + const stack = { |
| 702 | + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibility: 'shown === true' }] }] }], |
| 703 | + }; |
| 704 | + expect(syntaxFindings(stack)).toHaveLength(1); |
| 705 | + }); |
| 706 | + |
| 707 | + it('resolves a `{ dialect, source }` envelope the same as a bare string', () => { |
| 708 | + expect(syntaxFindings(formStack({ dialect: 'cel', source: 'country === "USA"' }))).toHaveLength(1); |
| 709 | + }); |
| 710 | + |
| 711 | + it('reaches a container\'s `formViews.<key>` — the shape a real stack emits', () => { |
| 712 | + const stack = { |
| 713 | + views: [{ object: 'showcase_task', formViews: { edit: { sections: [{ fields: [{ field: 'n', visibleWhen: 'country === "USA"' }] }] } } }], |
| 714 | + }; |
| 715 | + expect(syntaxFindings(stack).map((f) => f.path)).toEqual([ |
| 716 | + 'views[0].formViews.edit.sections[0].fields[0]', |
| 717 | + ]); |
| 718 | + }); |
| 719 | + |
| 720 | + it('is layer-agnostic — a syntax fault is a syntax fault on a metadata form too', () => { |
| 721 | + // Unlike the root rules, nothing about "does it parse" depends on which |
| 722 | + // namespace the surface binds. |
| 723 | + expect(syntaxFindings(formStack('country === "USA"'), { layer: 'metadata' })).toHaveLength(1); |
| 724 | + expect(syntaxFindings(formStack('country === "USA"'))).toHaveLength(1); |
| 725 | + }); |
| 726 | + }); |
| 727 | + |
| 728 | + it('the registry entry already gates, so this `error` reaches all three commands', () => { |
| 729 | + // `severity: 'error'` only fails a build because `authoring-rules.ts` marks |
| 730 | + // the family `gating` and runs it on validate/build/lint alike. That entry |
| 731 | + // was already `gating` (#6128 promoted it), so #6253 needed no registry |
| 732 | + // change — this pins that it is still true rather than assuming it. |
| 733 | + const entry = AUTHORING_RULES.find((r) => r.name === 'validateVisibilityPredicates'); |
| 734 | + expect(entry, 'validateVisibilityPredicates must be registered').toBeDefined(); |
| 735 | + expect(entry!.tier).toBe('gating'); |
| 736 | + expect([...entry!.commands].sort()).toEqual(['build', 'lint', 'validate']); |
| 737 | + }); |
| 738 | +}); |
0 commit comments