diff --git a/.changeset/lint-literal-empty-combinator-rejection.md b/.changeset/lint-literal-empty-combinator-rejection.md new file mode 100644 index 0000000000..ecee093875 --- /dev/null +++ b/.changeset/lint-literal-empty-combinator-rejection.md @@ -0,0 +1,48 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): literal empty combinators are refused at authoring time, with a per-shape prescription (#5330) + +#5322 settled what an empty combinator MEANS at run time — the boolean identity +reduction — and #5659/PR #6528 made that reduction one implementation +(`reduceFilterVerdict` in `@objectstack/spec/data`, proven against +`FILTER_LOGIC_CASES`, consumed by every backend). This change adds the other half +the ruling deliberately left open: the literal SPELLINGS are now refused where an +author writes them, which is Prime Directive #12's standard shape (reject at the +producer, do not tolerate at the consumer) and #5240's same-direction precedent +one shape over. + +`validateEmptyCombinators` is a new gating rule in `AUTHORING_RULES`, so it runs +on `os validate` / `os build` / `os lint` at once, and on the runtime publish +gate for `flow` writes — the door a Studio tenant, a REST `/meta` client and an +MCP/AI author all use. Two rule ids: + +- `filter-empty-combinator` — a literal `$and: []`, `$or: []` or `$not: {}`. +- `filter-empty-node` — a literal `{}` standing as the whole filter, or as a + branch of `$and` / `$or`. + +**The prescription is per shape, because the identities disagree.** `{$and: []}` +and `{}` reduce to TRUE (match EVERY row); `{$or: []}` and `{$not: {}}` reduce to +FALSE (match NO row). A generic "empty combinator, fix it" message teaches the +wrong fix half the time, so each shape names its own: delete the key to mean "no +filter"; fill the array to mean a constraint; put the negated condition inside +`$not`; and, when zero rows really is the intent, `{ : { $in: [] } }` is +the declared spelling that says so instead of implying it. The row-set wording in +every message is DERIVED from `reduceFilterVerdict` rather than retyped, and a +test drives the four #5322 identity cases straight out of `FILTER_LOGIC_CASES` and +asserts the message agrees with the rows the table says the filter selects. + +**Nothing at run time changed.** No translate or evaluation path is touched, the +conformance matrix is untouched, and a stack that ignores the finding runs exactly +as before. The literal-vs-programmatic boundary the ruling requires is structural, +not heuristic: this rule sees only values that reached the metadata graph, so a +producer that assembles zero disjuncts while serving a request — an RLS lowering, +a CEL `!expr`, a client-built query — never reaches it and keeps the runtime +identity, which is what makes `{$or: []}` = zero rows fail-closed (#5134). + +Also internal: the filter-subtree traversal `validate-filter-tokens.ts` grew for +#3574 moved to a shared `filter-walk.ts` now that it has a second consumer — the +same argument `page-walk.ts` (#3583) and `view-walk.ts` (#6381) make. Each rule +still declares its OWN surface list, so one rule's widening cannot land silently +in the other; `validate-filter-tokens`'s behaviour is unchanged. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index f37a044373..a3c2154945 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -104,6 +104,7 @@ import { validateViewContainers } from './validate-view-containers.js'; import { validateWidgetBindings } from './validate-widget-bindings.js'; import { validateDashboardActionRefs } from './validate-dashboard-action-refs.js'; import { validateFilterTokens } from './validate-filter-tokens.js'; +import { validateEmptyCombinators } from './validate-empty-combinators.js'; import { validateReferenceIntegrity } from './reference-integrity-suite.js'; import { validateComponentProps } from './validate-component-props.js'; import { validateResponsiveStyles } from './validate-responsive-styles.js'; @@ -486,6 +487,31 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateFilterTokens(stack), }, + // #5330 — the LITERAL empty combinators (`$and: []`, `$or: []`, `$not: {}`, + // `{}`). #5322 ruled their RUNTIME meaning to be the boolean identity, and + // this rule does not touch it: it refuses the literal SPELLINGS at authoring + // time with a per-shape prescription, which is Prime Directive #12's standard + // shape (reject at the producer, never tolerate at the consumer) and #5240's + // same-direction precedent one shape over. + { + name: 'validateEmptyCombinators', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-empty-combinators.ts', + // The one type #4463's P1 slice opened, and the one this rule most needs: + // a flow CRUD node's `config.filter` is where an empty combinator has the + // largest blast radius, and the write path is the only door an AI author + // uses. This rule needs NO resolution context at all — it judges the filter + // literal in isolation — so RUNTIME_NEEDS_FULL_SNAPSHOT does not apply to + // it, and widening to the other filter-carrying types (`object`, `view`, + // `page`, `dashboard`) is a one-line `runtimeTypes` edit once #4463 P2 + // opens them at the gate. Making that call here would widen the gate's + // dispatch surface on this rule's authority, which is P2's decision. + surfaces: CLI_AND_RUNTIME, + runtimeTypes: ['flow'], + run: (stack) => validateEmptyCombinators(stack), + }, // The reference-integrity suite (#3583 §5 D5) — itself a registry, of the // rules that answer "does this name resolve to anything?". It reached all // three commands before this file existed; it is an entry here so the two diff --git a/packages/lint/src/filter-walk.ts b/packages/lint/src/filter-walk.ts new file mode 100644 index 0000000000..52d26cded4 --- /dev/null +++ b/packages/lint/src/filter-walk.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Shared traversal: where the AUTHORED filters are in a metadata stack. + * + * Two rules in this package need the same answer to the same question — "which + * values in this stack were authored as a filter?" — and they need it for + * different reasons: `validate-filter-tokens.ts` classifies the STRINGS inside + * those subtrees (#3574), `validate-empty-combinators.ts` classifies their + * SHAPE (#5330). The subtree-finding half is identical for both, and it is the + * half with the interesting failure mode: #3574 happened because a resolver + * enumerated known surfaces and the dashboard was simply never added to the + * list. `page-walk.ts` (#3583/#5405) and `view-walk.ts` (#6381) are the same + * argument on two other traversals — with N copies the next author fixes one of + * N and the survivors keep the old verdict — and this file is written from + * theirs. + * + * ## What is shared, and what deliberately is NOT + * + * The MECHANISM is shared: descend a stack item, recognise a filter KEY, hand + * the subtree to a visitor. The SURFACE LIST is a parameter, not a constant, + * because the two callers genuinely differ: the token rule scans the seven + * presentation collections it has always scanned, and adding an eighth to a + * shared constant would silently widen a live gating rule. A caller declares + * its own {@link FilterSurface} list and owns that decision. + * + * ## Scanning for KEYS rather than enumerating surfaces + * + * Widget filters, list-view filters, dataset and measure filters, report + * runtime filters, flow CRUD node filters and SDUI component filters all spell + * the key the same way, so a new surface that follows the convention is covered + * the day it ships. That is the property #3574 lacked. + * + * Navigation `recordId` / `params` are NOT filter keys and are never visited: + * they resolve an additional vocabulary (`AppContextSelector` ids such as + * `{active_package}`) that is meaningless in a filter, and restricting the walk + * is what holds false positives at zero. + */ + +/** Any plain metadata record. */ +type AnyRec = Record; + +/** Keys whose subtree is a filter. The one place a filter is authored. */ +export const FILTER_KEYS: ReadonlySet = new Set(['filter', 'filters', 'runtimeFilter']); + +/** One stack collection a caller wants walked. */ +export interface FilterSurface { + /** Stack collection key — `dashboards`, `objects`, `flows`, … */ + key: string; + /** Singular noun used in the `where` label — `dashboard`, `object`, `flow`, … */ + kind: string; +} + +/** One authored filter subtree, with everything a finding needs to name it. */ +export interface AuthoredFilter { + /** The value found under the filter key, exactly as authored. */ + value: unknown; + /** Config path, e.g. `dashboards[0].widgets[2].filter`. */ + path: string; + /** Human-readable location, e.g. `dashboard "sales" · widget "my_deals"`. */ + where: string; +} + +/** + * Coerce a collection (array or name-keyed map) to an array of records, + * injecting `name` from the map key — so a rule works on both the parsed + * (array) and normalized (map) stack shapes. + */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +function label(v: unknown, fallback: string): string { + return typeof v === 'string' && v.length > 0 ? v : fallback; +} + +/** + * Find filter subtrees anywhere beneath `node` and hand each to `visit`. + * + * Exported for a caller that already has a single item in hand (the runtime + * publish gate's per-write snapshot arrives that way) rather than a whole stack. + */ +export function scanForFilters( + node: unknown, + path: string, + where: string, + visit: (filter: AuthoredFilter) => void, + seen: Set = new Set(), +): void { + if (!node || typeof node !== 'object') return; + // Metadata graphs can be cyclic once normalized; guard the walk. + if (seen.has(node)) return; + seen.add(node); + + if (Array.isArray(node)) { + node.forEach((v, i) => scanForFilters(v, `${path}[${i}]`, where, visit, seen)); + return; + } + + for (const [k, v] of Object.entries(node as AnyRec)) { + const childPath = `${path}.${k}`; + if (FILTER_KEYS.has(k)) { + visit({ value: v, path: childPath, where }); + continue; + } + scanForFilters(v, childPath, where, visit, seen); + } +} + +/** + * Walk every authored filter in `stack` across the caller's surfaces. + * + * Pure traversal: it holds no judgement and emits no findings. Dashboards get + * a per-widget `where` because that is the surface #3574 was filed against and + * naming the widget is what lets an author jump straight to it; every other + * surface is named by its collection kind and its own `name` / `id`. + */ +export function walkAuthoredFilters( + stack: unknown, + surfaces: readonly FilterSurface[], + visit: (filter: AuthoredFilter) => void, +): void { + if (!stack || typeof stack !== 'object') return; + + for (const { key, kind } of surfaces) { + const items = asArray((stack as AnyRec)[key]); + items.forEach((item, i) => { + const name = label(item.name ?? item.id, `#${i}`); + if (kind === 'dashboard') { + const widgets = Array.isArray(item.widgets) ? (item.widgets as AnyRec[]) : []; + widgets.forEach((w, wi) => { + const wName = label(w.id ?? w.title, `#${wi}`); + scanForFilters( + w, + `${key}[${i}].widgets[${wi}]`, + `dashboard "${name}" · widget "${wName}"`, + visit, + new Set(), + ); + }); + // ...and everything else on the dashboard (globalFilters, header, etc.) + // minus the widgets already covered above. + const { widgets: _skip, ...rest } = item; + scanForFilters(rest, `${key}[${i}]`, `dashboard "${name}"`, visit, new Set()); + return; + } + scanForFilters(item, `${key}[${i}]`, `${kind} "${name}"`, visit, new Set()); + }); + } +} diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index a223a698da..22c96e57f2 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -248,6 +248,22 @@ export type { export { validateFilterTokens, FILTER_TOKEN_UNKNOWN } from './validate-filter-tokens.js'; export type { FilterTokenFinding, FilterTokenSeverity } from './validate-filter-tokens.js'; +// #5330 — the same subtree, judged for SHAPE rather than for its strings. The +// runtime meaning of an empty combinator is settled (#5322: boolean identity, +// one implementation in `@objectstack/spec`'s `reduceFilterVerdict`); this +// refuses the literal spellings at authoring time, with a prescription that is +// per shape because the identities disagree — `{$and: []}` / `{}` are match-ALL +// and `{$or: []}` / `{$not: {}}` are match-NONE. +export { + validateEmptyCombinators, + FILTER_EMPTY_COMBINATOR, + FILTER_EMPTY_NODE, +} from './validate-empty-combinators.js'; +export type { + EmptyCombinatorFinding, + EmptyCombinatorSeverity, +} from './validate-empty-combinators.js'; + export { validateObjectReferences, OBJECT_REFERENCE_UNKNOWN, diff --git a/packages/lint/src/validate-empty-combinators.test.ts b/packages/lint/src/validate-empty-combinators.test.ts new file mode 100644 index 0000000000..5382134a9f --- /dev/null +++ b/packages/lint/src/validate-empty-combinators.test.ts @@ -0,0 +1,321 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #5330 — the authoring-time refusal of LITERAL empty combinators. +// +// Two things are being proven here, and they are different claims: +// +// 1. **The rule fires on the five literal shapes, with the RIGHT prescription +// for each.** A generic "empty combinator, fix it" would pass a test that +// only counted findings, and would teach the wrong fix half the time — +// `{$and: []}` / `{}` are match-ALL and `{$or: []}` / `{$not: {}}` are +// match-NONE. So every rejection case asserts the id AND the severity AND +// the prescription's load-bearing sentence, never just "something was +// reported". +// 2. **The rule's vocabulary is the RUNTIME's.** `identity vocabulary agrees +// with FILTER_LOGIC_CASES` drives the four #5322 identity cases from the +// conformance table itself and asserts that the row-set language in the +// message matches the rows the table says the filter selects. That is the +// pin that would have caught #5388's defect (`{$or: []}` labelled +// "match-all") if it had existed one package over, and it goes red the day +// the messages stop being derived from `reduceFilterVerdict`. + +import { describe, it, expect } from 'vitest'; +import { FILTER_LOGIC_CASES, reduceFilterVerdict } from '@objectstack/spec/data'; + +import { + validateEmptyCombinators, + FILTER_EMPTY_COMBINATOR, + FILTER_EMPTY_NODE, +} from './validate-empty-combinators.js'; + +type AnyRec = Record; + +/** A dashboard widget carrying one authored filter — the #3574 surface shape. */ +const widgetStack = (filter: unknown): AnyRec => ({ + dashboards: [{ name: 'ops', widgets: [{ id: 'open_cases', type: 'metric', filter }] }], +}); + +const WIDGET_PATH = 'dashboards[0].widgets[0].filter'; +const WIDGET_WHERE = 'dashboard "ops" · widget "open_cases"'; + +describe('validateEmptyCombinators — the literal shapes are refused', () => { + it('returns nothing for an absent / empty stack', () => { + expect(validateEmptyCombinators(undefined)).toEqual([]); + expect(validateEmptyCombinators(null)).toEqual([]); + expect(validateEmptyCombinators({})).toEqual([]); + }); + + it('refuses `$and: []` and prescribes deleting the key, not emptying it', () => { + const findings = validateEmptyCombinators(widgetStack({ $and: [] })); + + expect(findings).toHaveLength(1); + // The rejection envelope this package gates on: id + severity + location. + expect(findings[0].rule).toBe(FILTER_EMPTY_COMBINATOR); + expect(findings[0].severity).toBe('error'); + expect(findings[0].where).toBe(WIDGET_WHERE); + expect(findings[0].path).toBe(`${WIDGET_PATH}.$and`); + // Per-shape: the AND identity matches everything. + expect(findings[0].message).toContain('matches EVERY row'); + expect(findings[0].hint).toContain('DELETE the key'); + }); + + it('refuses `$or: []` and says it is the OPPOSITE of "no filter"', () => { + const findings = validateEmptyCombinators(widgetStack({ $or: [] })); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_COMBINATOR); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe(`${WIDGET_PATH}.$or`); + // The half a generic message gets wrong: OR identity is match-NONE. + expect(findings[0].message).toContain('matches NO row'); + expect(findings[0].message).not.toContain('matches EVERY row'); + expect(findings[0].hint).toContain('OPPOSITE'); + }); + + it('refuses `$not: {}` and prescribes putting the negated condition inside', () => { + const findings = validateEmptyCombinators(widgetStack({ $not: {} })); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_COMBINATOR); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe(`${WIDGET_PATH}.$not`); + expect(findings[0].message).toContain('matches NO row'); + expect(findings[0].hint).toContain('inside `$not`'); + }); + + it('refuses a whole-filter `{}` and prescribes omitting the key', () => { + const findings = validateEmptyCombinators(widgetStack({})); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_NODE); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe(WIDGET_PATH); + expect(findings[0].message).toContain('matches EVERY row'); + expect(findings[0].hint).toContain('DELETE the key'); + }); + + it('refuses a `{}` branch of `$or` by naming the branches it kills', () => { + const findings = validateEmptyCombinators( + widgetStack({ $or: [{ status: 'open' }, {}] }), + ); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_NODE); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe(`${WIDGET_PATH}.$or[1]`); + // Absorption, not narrowing — the distinction #5297 was filed about. + expect(findings[0].message).toContain('ABSORBS'); + expect(findings[0].message).toContain('matches EVERY row'); + expect(findings[0].hint).toContain('NARROW'); + }); + + it('refuses a `{}` branch of `$and` with the AND identity wording', () => { + const findings = validateEmptyCombinators( + widgetStack({ $and: [{ status: 'open' }, {}] }), + ); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_NODE); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe(`${WIDGET_PATH}.$and[1]`); + expect(findings[0].message).toContain('AND identity'); + // A dead conjunct is not an absorbed disjunct; the two must not share prose. + expect(findings[0].message).not.toContain('ABSORBS'); + }); + + it('reaches an empty combinator nested inside a live one', () => { + const findings = validateEmptyCombinators( + widgetStack({ $or: [{ status: 'open' }, { $and: [] }, { $not: { $or: [] } }] }), + ); + + expect(findings.map((f) => f.path).sort()).toEqual([ + `${WIDGET_PATH}.$or[1].$and`, + `${WIDGET_PATH}.$or[2].$not.$or`, + ]); + for (const f of findings) expect(f.severity).toBe('error'); + }); +}); + +describe('validateEmptyCombinators — what it leaves alone', () => { + it('says nothing about an absent filter key — the canonical "no filter"', () => { + expect( + validateEmptyCombinators({ + dashboards: [{ name: 'ops', widgets: [{ id: 'w', type: 'metric' }] }], + }), + ).toEqual([]); + }); + + it('says nothing about a filter that carries a real predicate', () => { + expect( + validateEmptyCombinators( + widgetStack({ + $and: [{ status: 'open' }, { $or: [{ owner: 'u1' }, { owner: 'u2' }] }], + $not: { archived: true }, + }), + ), + ).toEqual([]); + }); + + it('says nothing about `{ field: {} }` — that is #5240\'s shape, not this one', () => { + // A field constrained by zero operators is a different defect with a + // different fix, ruled and gated elsewhere. Descending into a comparand + // here would double-report it in a second vocabulary. + expect(validateEmptyCombinators(widgetStack({ status: {} }))).toEqual([]); + }); + + it('says nothing about the declared match-none spelling it prescribes', () => { + // The hint tells authors to write `{ $in: [] }` when they really do want + // zero rows. A rule that then rejected its own prescription would be worse + // than no rule at all. + expect(validateEmptyCombinators(widgetStack({ stage: { $in: [] } }))).toEqual([]); + }); + + it('says nothing about a non-array `$and` or a non-object `$not` operand', () => { + // Refused BY NAME at the schema and at every driver (`assertNodeList` / + // `assertNode`); a second complaint here would describe a run that never + // happens. + expect(validateEmptyCombinators(widgetStack({ $and: 'nope' }))).toEqual([]); + expect(validateEmptyCombinators(widgetStack({ $not: 'nope' }))).toEqual([]); + }); + + it('does not read a non-plain object as an empty node', () => { + // A `Date` / `Map` enumerates to zero own keys. Reporting one would name a + // shape the author never wrote — the mirror of the `isFilterNode` guard the + // shared reduction carries for the opposite reason. + expect(validateEmptyCombinators(widgetStack(new Date()))).toEqual([]); + expect(validateEmptyCombinators(widgetStack({ $or: [{ a: 'x' }, new Date()] }))).toEqual([]); + }); + + it('leaves the array authoring shape alone, empty or not', () => { + // `FilterArray` is lowered by `parseFilterAST` and is not one of the four + // shapes #5322 ruled on — out of scope rather than guessed at. + expect(validateEmptyCombinators(widgetStack([]))).toEqual([]); + expect( + validateEmptyCombinators(widgetStack([{ field: 'owner', operator: 'equals', value: 'u1' }])), + ).toEqual([]); + }); + + it('survives a cyclic metadata graph', () => { + const dash: AnyRec = { name: 'd', widgets: [] }; + dash.self = dash; + expect(() => validateEmptyCombinators({ dashboards: [dash] })).not.toThrow(); + }); +}); + +describe('validateEmptyCombinators — the surfaces it walks', () => { + it('covers a flow CRUD node config, where the blast radius is largest', () => { + const findings = validateEmptyCombinators({ + flows: [ + { + name: 'purge_stale', + nodes: [ + { id: 'start', type: 'start' }, + { + id: 'purge', + type: 'delete_record', + config: { objectName: 'lead', multi: true, filter: { $or: [] } }, + }, + ], + }, + ], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(FILTER_EMPTY_COMBINATOR); + expect(findings[0].severity).toBe('error'); + expect(findings[0].where).toBe('flow "purge_stale"'); + expect(findings[0].path).toBe('flows[0].nodes[1].config.filter.$or'); + expect(findings[0].message).toContain('matches NO row'); + }); + + it('covers objects, views, reports, datasets, pages and apps', () => { + const findings = validateEmptyCombinators({ + objects: [{ name: 'lead', listViews: { mine: { filter: { $or: [] } } } }], + views: [{ name: 'all', list: { filter: { $and: [] } } }], + reports: [{ name: 'weekly', runtimeFilter: { $not: {} } }], + datasets: [{ name: 'cases', filter: {} }], + pages: [ + { + name: 'home', + regions: [{ components: [{ type: 'object-grid', properties: { filter: { $or: [] } } }] }], + }, + ], + apps: [{ name: 'crm', navigation: [{ id: 'n', type: 'object', filters: { $and: [] } }] }], + }); + + expect(findings.map((f) => f.where).sort()).toEqual([ + 'app "crm"', + 'dataset "cases"', + 'object "lead"', + 'page "home"', + 'report "weekly"', + 'view "all"', + ]); + for (const f of findings) expect(f.severity).toBe('error'); + }); +}); + +describe('validateEmptyCombinators — the vocabulary is the runtime\'s (#5322/#5659)', () => { + /** + * The four identity cases, taken from the conformance table rather than + * retyped: `filter-logic-conformance.ts` is what every backend is measured + * against, so a message derived from anything else would be a second opinion. + */ + const identityCases = FILTER_LOGIC_CASES.filter((c) => (c.note ?? '').includes('#5322')); + + it('the identity cases are still findable in the conformance table', () => { + // Guarded body: if the selection ever returns nothing, the loop below would + // pass by vacuity and this file would report coverage it does not have. + expect(identityCases.map((c) => c.name).sort()).toEqual([ + '$not of {} is FALSE — NOT TRUE', + 'a {} branch is a TRUE disjunct and absorbs its $or', + 'empty $and is TRUE — the AND identity', + 'empty $or is FALSE — the OR identity', + ]); + }); + + it('identity vocabulary agrees with FILTER_LOGIC_CASES', () => { + expect(identityCases.length).toBe(4); + + for (const c of identityCases) { + const findings = validateEmptyCombinators(widgetStack(c.filter)); + expect(findings, `${c.name}: exactly one literal shape is reported`).toHaveLength(1); + + // The table says which rows the filter selects; the message must say the + // same thing in words. `expected: []` is match-none, a full row set is + // match-all — there is no third answer among these four. + const selectsEveryRow = c.expected.length === 4; + const selectsNoRow = c.expected.length === 0; + expect(selectsEveryRow || selectsNoRow, `${c.name}: an identity selects all rows or none`).toBe(true); + + expect(findings[0].message, `${c.name}: prescription must match the conformance row set`).toContain( + selectsEveryRow ? 'matches EVERY row' : 'matches NO row', + ); + expect(findings[0].message).not.toContain(selectsEveryRow ? 'matches NO row' : 'matches EVERY row'); + } + }); + + it('the row-set language is DERIVED from the shared reduction, not retyped', () => { + // The same claim from the other side: ask `reduceFilterVerdict` directly. + // If a future edit hand-writes a fourth reduction inside this rule, these + // two views stop agreeing and this case is where it shows. + const probes: ReadonlyArray<[AnyRec, string]> = [ + [{ $and: [] }, 'matches EVERY row'], + [{ $or: [] }, 'matches NO row'], + [{ $not: {} }, 'matches NO row'], + [{}, 'matches EVERY row'], + [{ $or: [{ status: 'open' }, {}] }, 'matches EVERY row'], + ]; + + for (const [filter, prose] of probes) { + const verdict = reduceFilterVerdict(filter); + expect(verdict, `${JSON.stringify(filter)} must reduce to a boolean identity`).not.toBe('clause'); + const expected = verdict === 'true' ? 'matches EVERY row' : 'matches NO row'; + expect(expected, `${JSON.stringify(filter)}`).toBe(prose); + + const findings = validateEmptyCombinators(widgetStack(filter)); + expect(findings, `${JSON.stringify(filter)} is a literal shape this rule reports`).toHaveLength(1); + expect(findings[0].message).toContain(prose); + } + }); +}); diff --git a/packages/lint/src/validate-empty-combinators.ts b/packages/lint/src/validate-empty-combinators.ts new file mode 100644 index 0000000000..7b6eee5a39 --- /dev/null +++ b/packages/lint/src/validate-empty-combinators.ts @@ -0,0 +1,375 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5330 — a LITERAL empty combinator in authored metadata is refused at + * authoring time, with a per-shape prescription. + * + * ## The runtime is not changing, and this file changes nothing about it + * + * #5322 (maintainer ruling, 2026-08-04) settled the RUNTIME semantics of the + * four empty shapes as the boolean identity reduction, and #5659/PR #6528 made + * that reduction one implementation — `reduceFilterVerdict` in + * `@objectstack/spec/data`, proven against `FILTER_LOGIC_CASES` and consumed by + * every backend. This rule touches no translate or evaluation path. It asks the + * SAME predicate what a shape is worth and reports it; a stack that ignores the + * finding runs exactly as it ran before. + * + * ### History note, so the next reader is not misled + * + * `service-analytics`' `filter-normalizer.ts` used to REFUSE `{$and: []}` and + * `{$or: []}` at compile time, and its error argued, verbatim: + * + * > An empty combinator has no defensible reading — dropping it widens the + * > query, and treating it as "match nothing" silently empties a chart. + * + * That wording is GONE from the runtime. #5322 overruled it (only a reduction + * can evaluate a nested tree — a rejection must first reduce to judge `$and: []` + * as the third branch of a `$or`, which concedes the point; and `{$or: []}` = + * zero rows is fail-closed exactly where it matters, #5134), and PR #5365 + * replaced the throw with the identity. The sentiment survives here, one layer + * earlier and with the one thing the runtime version could never have: a + * prescription. If you are reading this because you saw that sentence quoted + * somewhere, the runtime does **not** throw on an empty combinator any more — + * `filter-normalizer.ts`'s own `buildNode` records the same history at its + * combinator branch. + * + * ## The literal-vs-programmatic boundary — this rule's own decision + * + * #5322 kept the runtime identity *because* a programmatic producer needs it: a + * read scope whose disjunct list loops to zero items must fail CLOSED + * (`{$or: []}` = zero rows) rather than expose the table. So the rejection may + * not reach that producer, and the boundary has to be exact. + * + * It is **structural, not heuristic**. This rule reads the AUTHORED METADATA + * GRAPH — the stack a config file declares, and the item the runtime publish + * gate is about to persist. Every value it sees is already in the metadata. A + * value a producer CONSTRUCTS while serving a request never enters that graph + * and therefore never reaches this rule: `objectql-strategy.ts` building + * `{$not: {}}` from a CEL literal, `cel-to-filter` lowering an RLS predicate, a + * client assembling a query — all of them hand a filter straight to a driver, + * where #5322's identity is the single semantics. There is no predicate to get + * wrong here, because a lint rule cannot see those values at all. + * + * The dividing line is therefore "did this value reach the metadata graph", NOT + * "did a human type it". A TS config that computes a filter at `defineStack` + * time and produces `{$or: []}` has produced authored metadata and is judged as + * such — which is the direction that helps, since that author can still see it + * before publishing, and the alternative (a scope that silently returns zero + * rows in production) is the #5134 defect itself. + * + * ## Why `error`, per the severity bar this package states + * + * Gate when no reading of the metadata behaves as written. All five reported + * shapes qualify — a combinator whose operand list is empty, and an empty node + * standing where a condition belongs, both read as "a filter is applied here" + * and apply nothing (or, for `$or: []` / `$not: {}`, apply the OPPOSITE of what + * the shape suggests to a reader who has not memorised the identity table). + * That is the ADR-0049 declared-but-not-enforced shape, and Prime Directive #12 + * puts the refusal at the producer rather than tolerance at the consumer. + * + * The asymmetry is the whole reason the prescriptions are per shape: + * `{$and: []}` and `{}` reduce to TRUE (match EVERY row), `{$or: []}` and + * `{$not: {}}` reduce to FALSE (match NO row). A generic "empty combinator, fix + * it" message teaches the wrong fix half the time. #5388 holds a live instance + * of exactly that confusion one package over (`plugin-sharing`'s + * `isMatchAllCriteria` labels `{$or: []}` "match-all"), so the vocabulary here + * is written to be quoted. + * + * ## What this rule deliberately does NOT judge + * + * - **`{ field: {} }`** — a field constrained by zero operators. #5240 ruled + * it REJECTED and #5327 gated the backends; it is a different shape with a + * different fix, and the walk below never descends into a field key's value + * for that reason. A comparand is data. + * - **A non-array `$and`/`$or`, or a non-object `$not` operand.** The schema + * and the drivers refuse those by name (`assertNodeList` / `assertNode` in + * `filter-verdict.ts`); inventing a second complaint here would describe a + * run that never happens. + * - **The ARRAY authoring shape** (`filter: [{ field, operator, value }]`, + * `filter: []`). That is `FilterArray`, lowered by `parseFilterAST`, and it + * is not one of the four shapes #5322 ruled on. Judging it would need its + * own conformance evidence, so it is out of scope rather than guessed at. + * - **An absent `filter` key.** That is the canonical spelling of "no filter" + * and this rule's own prescription for three of the five shapes. + * + * ## Relationship to `flow-multi-write-unfiltered` (#5482) + * + * `lint-flow-patterns.ts` WARNS when a `multi: true` CRUD node is bounded by a + * filter that reduces to TRUE, and it deliberately does not gate: the engine's + * dispatch case-set grants a whole-object write on purpose, so the shape has a + * legitimate reading. That is not in tension with the gate here, because the two + * judge different facts and the canonical spelling of the legitimate intent — + * omitting the key — is untouched by this rule. On `{ $and: [] }` + `multi` both + * fire, and they are sequential rather than duplicate: delete the `$and` as this + * rule prescribes and the write is still unbounded, so #5482's warning is still + * the right next thing for the author to read. + */ + +import { reduceFilterVerdict, type FilterVerdict } from '@objectstack/spec/data'; + +import { walkAuthoredFilters, type FilterSurface } from './filter-walk.js'; + +/** A literal `$and: []` / `$or: []` / `$not: {}` — a combinator with no operands. */ +export const FILTER_EMPTY_COMBINATOR = 'filter-empty-combinator'; +/** A literal `{}` where a filter node is authored — the whole filter, or a branch. */ +export const FILTER_EMPTY_NODE = 'filter-empty-node'; + +export type EmptyCombinatorSeverity = 'error' | 'warning'; + +export interface EmptyCombinatorFinding { + /** Always `error` — see the severity note in this module's header. */ + severity: EmptyCombinatorSeverity; + /** Diagnostic rule id (`filter-empty-combinator` / `filter-empty-node`). */ + rule: string; + /** Human-readable location, e.g. `dashboard "sales" · widget "my_deals"`. */ + where: string; + /** Config path, e.g. `dashboards[0].widgets[2].filter.$or`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +/** + * The collections whose authored filters this rule judges. + * + * The seven presentation surfaces `validate-filter-tokens.ts` scans, plus + * `flows`: a CRUD node's `config.filter` is authored metadata like any other, + * and it is the surface where an empty combinator has the largest blast radius + * (`{$or: []}` on a `delete_record` deletes nothing and reports `acted: 0`, + * which reads as "there was nothing to purge"). + */ +const EMPTY_COMBINATOR_SURFACES: readonly FilterSurface[] = [ + { key: 'dashboards', kind: 'dashboard' }, + { key: 'objects', kind: 'object' }, + { key: 'views', kind: 'view' }, + { key: 'reports', kind: 'report' }, + { key: 'datasets', kind: 'dataset' }, + { key: 'pages', kind: 'page' }, + { key: 'apps', kind: 'app' }, + { key: 'flows', kind: 'flow' }, +]; + +/** + * Is `value` a Filter Protocol NODE — a plain object, the shape + * `FilterConditionSchema` declares for a `$and`/`$or` element and for the + * operand of `$not`? + * + * The prototype check mirrors `filter-verdict.ts`'s `isFilterNode` and is + * load-bearing for the same reason, in the mirror direction: a `Date`, a `Map` + * or a class instance enumerates to zero own keys, and reading one as "an empty + * node" would report a shape the author did not write. A filter node arrives as + * JSON or as a config module's plain object, so requiring one costs nothing. + */ +function isFilterNode(value: unknown): value is AnyRec { + if (value === null || typeof value !== 'object' || Array.isArray(value)) return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; +} + +/** Where an empty node was authored — the axis the prescription turns on. */ +type EmptyNodePosition = 'root' | 'and-branch' | 'or-branch'; + +/** + * What the shared reduction says each reported shape is worth. + * + * Derived, never asserted: the words "matches EVERY row" / "matches NO row" in + * every message below come from `reduceFilterVerdict` — the same function the + * five backends execute — so a message cannot drift from the ruling it quotes. + * `verdict-agrees-with-the-shared-reduction` in the tests pins that the four + * probes still answer what the prose claims. + */ +const VERDICT_OF = { + $and: reduceFilterVerdict({ $and: [] }), + $or: reduceFilterVerdict({ $or: [] }), + $not: reduceFilterVerdict({ $not: {} }), + node: reduceFilterVerdict({}), + /** One TRUE disjunct absorbs its `$or`: the sibling branches stop mattering. */ + orWithEmptyBranch: reduceFilterVerdict({ $or: [{ status: 'open' }, {}] }), +} as const; + +/** Render a verdict as the row set it selects, for a message an author can act on. */ +function rows(verdict: FilterVerdict): string { + if (verdict === 'true') return 'matches EVERY row'; + if (verdict === 'false') return 'matches NO row'; + // Unreachable for the four literal shapes; kept honest rather than asserted. + return 'carries a real predicate'; +} + +/** + * The one spelling of "match no row" that is a declared predicate rather than + * an identity nobody can read: an empty `$in` list. + * + * Not invented here — `packages/objectql/src/filter-comparand-shape.ts` names + * `$in: []` / `$nin: []` as "a legitimate, declared predicate — matches nothing + * and matches everything respectively", and deliberately does not refuse them. + */ +const MATCH_NONE_SPELLING = + 'If you really do want a predicate that selects nothing, `{ : { $in: [] } }` is the declared ' + + 'spelling for it (an empty `$in` list matches nothing, on every backend) — it says so where an empty ' + + 'combinator only implies it.'; + +/** The prescription every shape shares: an absent key IS "no filter". */ +const OMIT_THE_KEY = + 'To express "no filter", DELETE the key — an absent `filter` and a filter that reduces to TRUE run ' + + 'identically, and only the absent key says so to the next reader (and to the next AI author that ' + + 'copies this metadata).'; + +interface Ctx { + where: string; + out: EmptyCombinatorFinding[]; +} + +function emitEmptyCombinator(key: '$and' | '$or' | '$not', path: string, ctx: Ctx): void { + const spelling = key === '$not' ? '`$not: {}`' : `\`${key}: []\``; + + const message = + key === '$and' + ? '`$and: []` is a conjunction of ZERO conditions. Under the #5322 identity ruling it ' + + `${rows(VERDICT_OF.$and)} — the key is authored, and it constrains nothing, so this surface ` + + 'reads as filtered and is not.' + : key === '$or' + ? '`$or: []` is a disjunction of ZERO branches. Under the #5322 identity ruling it ' + + `${rows(VERDICT_OF.$or)}: this surface renders permanently empty, and on a read scope it hides ` + + 'every row (fail-closed by design — #5134).' + : '`$not: {}` negates an EMPTY node. An empty node is TRUE and NOT TRUE is FALSE, so it ' + + `${rows(VERDICT_OF.$not)} — the opposite of the "no filter" an empty operand looks like.`; + + const hint = + key === '$and' + ? `${OMIT_THE_KEY} To express a constraint, put the conditions in the array. ${MATCH_NONE_SPELLING}` + : key === '$or' + ? 'If you meant "no filter", this is its OPPOSITE: emptying the array does not relax the filter, ' + + `it closes it. ${OMIT_THE_KEY} If you meant to offer alternatives, put the branches in the ` + + `array. ${MATCH_NONE_SPELLING}` + : 'Put the condition you are negating inside `$not` (`{ $not: { status: \'closed\' } }`). ' + + `${OMIT_THE_KEY} ${MATCH_NONE_SPELLING}`; + + ctx.out.push({ + severity: 'error', + rule: FILTER_EMPTY_COMBINATOR, + where: ctx.where, + path, + message: `${message} A literal ${spelling} is not an authoring surface (#5330).`, + hint: + `${hint} A PROGRAMMATIC producer that loops to zero operands keeps the runtime identity ` + + 'unchanged — this rule judges only what is written in the metadata.', + }); +} + +function emitEmptyNode(position: EmptyNodePosition, path: string, ctx: Ctx): void { + if (position === 'root') { + ctx.out.push({ + severity: 'error', + rule: FILTER_EMPTY_NODE, + where: ctx.where, + path, + message: + 'An EMPTY filter node (`{}`) is authored here. Under the #5322 identity ruling an empty node is ' + + `TRUE — it ${rows(VERDICT_OF.node)}, exactly as if the key were absent — so a filter is declared ` + + 'and enforces nothing.', + hint: + `${OMIT_THE_KEY} If you meant to constrain something, write the condition into the node. ` + + `${MATCH_NONE_SPELLING}`, + }); + return; + } + + if (position === 'or-branch') { + ctx.out.push({ + severity: 'error', + rule: FILTER_EMPTY_NODE, + where: ctx.where, + path, + message: + 'An EMPTY branch (`{}`) of a `$or`. An empty node is TRUE, and one TRUE disjunct ABSORBS the ' + + `whole disjunction (\`{ $or: [{ status: 'open' }, {}] }\` ${rows(VERDICT_OF.orWithEmptyBranch)}), ` + + 'so every branch you wrote beside it is dead.', + hint: + 'Delete the empty branch — the `$or` then means what it looks like. If it was meant to carry a ' + + 'condition, write it. (A compiler that DROPPED the empty branch instead would silently NARROW ' + + 'the scope to the surviving branches, which is why the runtime absorbs rather than filters — ' + + '#5297.)', + }); + return; + } + + ctx.out.push({ + severity: 'error', + rule: FILTER_EMPTY_NODE, + where: ctx.where, + path, + message: + 'An EMPTY branch (`{}`) of a `$and`. An empty node is TRUE — the AND identity — so the branch ' + + 'contributes no condition and the conjunction means whatever its other branches mean.', + hint: + 'Delete the empty branch, or write the condition it was meant to carry. A branch that constrains ' + + 'nothing is indistinguishable from one whose condition was lost in an edit.', + }); +} + +/** Recurse into a node whose emptiness has already been decided by the caller. */ +function scanNodeKeys(node: AnyRec, path: string, ctx: Ctx): void { + for (const [key, value] of Object.entries(node)) { + if (key === '$and' || key === '$or') { + // A non-array operand is refused BY NAME at the schema and at every + // driver (`assertNodeList`); it is not this rule's complaint to make. + if (!Array.isArray(value)) continue; + if (value.length === 0) { + emitEmptyCombinator(key, `${path}.${key}`, ctx); + continue; + } + value.forEach((element, index) => { + scanBranch(element, `${path}.${key}[${index}]`, key === '$and' ? 'and-branch' : 'or-branch', ctx); + }); + continue; + } + + if (key === '$not') { + if (!isFilterNode(value)) continue; + if (Object.keys(value).length === 0) { + emitEmptyCombinator('$not', `${path}.$not`, ctx); + continue; + } + scanNodeKeys(value, `${path}.$not`, ctx); + continue; + } + + // A field key. Its value is a comparand or an operator object, never a + // node — `{ field: {} }` is #5240's surface and is judged there, not here. + } +} + +/** A node standing where emptiness is itself the authored evidence. */ +function scanBranch(value: unknown, path: string, position: EmptyNodePosition, ctx: Ctx): void { + if (!isFilterNode(value)) return; + if (Object.keys(value).length === 0) { + emitEmptyNode(position, path, ctx); + return; + } + scanNodeKeys(value, path, ctx); +} + +/** + * Reject literal empty combinators across an authored stack. + * + * Pure `(stack) => Finding[]`; no I/O. Tolerates both authoring tiers — a + * filter subtree survives the Zod parse unchanged, and no filter key in the + * spec carries a `.default({})`, so a `{}` seen here was authored, never filled + * in by a schema. + */ +export function validateEmptyCombinators( + stack: Record | undefined | null, +): EmptyCombinatorFinding[] { + if (!stack || typeof stack !== 'object') return []; + const out: EmptyCombinatorFinding[] = []; + + walkAuthoredFilters(stack, EMPTY_COMBINATOR_SURFACES, ({ value, path, where }) => { + scanBranch(value, path, 'root', { where, out }); + }); + + return out; +} diff --git a/packages/lint/src/validate-filter-tokens.ts b/packages/lint/src/validate-filter-tokens.ts index 75bcb5eefa..5465f87b2f 100644 --- a/packages/lint/src/validate-filter-tokens.ts +++ b/packages/lint/src/validate-filter-tokens.ts @@ -2,6 +2,8 @@ import { classifyFilterToken, CONTEXT_TOKENS } from '@objectstack/spec/data'; +import { walkAuthoredFilters, type FilterSurface } from './filter-walk.js'; + /** * Build-time filter-placeholder diagnostics (issue #3574). * @@ -45,13 +47,16 @@ import { classifyFilterToken, CONTEXT_TOKENS } from '@objectstack/spec/data'; * * ## Scope — filter subtrees only * - * The walk descends into `filter` / `filters` / `runtimeFilter` subtrees and - * classifies string values inside them. It deliberately does NOT check - * navigation `recordId` / `params`, which resolve an additional vocabulary — - * `AppContextSelector` ids such as `{active_package}` — that is meaningless in - * a filter because filters are not evaluated with the sidebar's selector - * state. Restricting the walk keeps that legitimate usage out of the rule and - * holds false positives at zero. + * Finding those subtrees is `filter-walk.ts`'s job since #5330 gave the same + * traversal a second consumer; this file owns only the judgement on the strings + * inside them. The shared walk descends into `filter` / `filters` / + * `runtimeFilter` and deliberately does NOT check navigation `recordId` / + * `params`, which resolve an additional vocabulary — `AppContextSelector` ids + * such as `{active_package}` — that is meaningless in a filter because filters + * are not evaluated with the sidebar's selector state. Restricting the walk + * keeps that legitimate usage out of the rule and holds false positives at + * zero. The seven surfaces below stay THIS rule's declaration, not the walk's: + * a shared surface list would let another rule's widening land here silently. * * Only whole-string placeholders are considered (`'{token}'` / `'${token}'`, * anchored). A value that merely contains braces is left alone. @@ -78,28 +83,22 @@ export interface FilterTokenFinding { type AnyRec = Record; -/** Keys whose subtree is a filter — the only place placeholders resolve. */ -const FILTER_KEYS = new Set(['filter', 'filters', 'runtimeFilter']); +const KNOWN_LIST = CONTEXT_TOKENS.join('}, {'); /** - * Coerce a collection (array or name-keyed map) to an array of records, - * injecting `name` from the map key — mirrors the helper in the sibling - * authoring lints so the rule works on both the parsed (array) and normalized - * (map) stack shapes. + * The presentation collections this rule has scanned since #3574. Declared + * here, handed to the shared walk — see the scope note above for why it is not + * a constant in `filter-walk.ts`. */ -function asArray(v: unknown): AnyRec[] { - if (Array.isArray(v)) return v as AnyRec[]; - if (v && typeof v === 'object') { - return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); - } - return []; -} - -function label(v: unknown, fallback: string): string { - return typeof v === 'string' && v.length > 0 ? v : fallback; -} - -const KNOWN_LIST = CONTEXT_TOKENS.join('}, {'); +const TOKEN_FILTER_SURFACES: readonly FilterSurface[] = [ + { key: 'dashboards', kind: 'dashboard' }, + { key: 'objects', kind: 'object' }, + { key: 'views', kind: 'view' }, + { key: 'reports', kind: 'report' }, + { key: 'datasets', kind: 'dataset' }, + { key: 'pages', kind: 'page' }, + { key: 'apps', kind: 'app' }, +]; /** * Classify every string inside an already-identified filter subtree. @@ -159,43 +158,6 @@ function walkFilterValues( } } -/** - * Find `filter` / `filters` / `runtimeFilter` subtrees anywhere beneath - * `node`, then classify the values inside them. - * - * Scanning for filter KEYS rather than enumerating known surfaces is - * deliberate: widget filters, list-view filters, dataset and measure filters, - * report runtime filters, and SDUI component filters all spell the key the - * same way, and a new surface that follows the convention is covered the day - * it ships. Enumerating surfaces is how #3574 happened — the dashboard was - * simply never added to the list. - */ -function scanForFilters( - node: unknown, - path: string, - where: string, - out: FilterTokenFinding[], - seen: Set, -): void { - if (!node || typeof node !== 'object') return; - if (seen.has(node)) return; - seen.add(node); - - if (Array.isArray(node)) { - node.forEach((v, i) => scanForFilters(v, `${path}[${i}]`, where, out, seen)); - return; - } - - for (const [k, v] of Object.entries(node as AnyRec)) { - const childPath = `${path}.${k}`; - if (FILTER_KEYS.has(k)) { - walkFilterValues(v, childPath, where, out, new Set()); - continue; - } - scanForFilters(v, childPath, where, out, seen); - } -} - /** * Validate filter placeholders across a schema-parsed stack. * @@ -207,43 +169,9 @@ export function validateFilterTokens(stack: Record | undefined if (!stack || typeof stack !== 'object') return []; const out: FilterTokenFinding[] = []; - const surfaces: Array<[key: string, kind: string]> = [ - ['dashboards', 'dashboard'], - ['objects', 'object'], - ['views', 'view'], - ['reports', 'report'], - ['datasets', 'dataset'], - ['pages', 'page'], - ['apps', 'app'], - ]; - - for (const [key, kind] of surfaces) { - const items = asArray((stack as AnyRec)[key]); - items.forEach((item, i) => { - const name = label(item.name ?? item.id, `#${i}`); - // Dashboards are the surface #3574 was filed against; name the widget in - // `where` so the author can jump straight to it. - if (kind === 'dashboard') { - const widgets = Array.isArray(item.widgets) ? (item.widgets as AnyRec[]) : []; - widgets.forEach((w, wi) => { - const wName = label(w.id ?? w.title, `#${wi}`); - scanForFilters( - w, - `${key}[${i}].widgets[${wi}]`, - `dashboard "${name}" · widget "${wName}"`, - out, - new Set(), - ); - }); - // …and everything else on the dashboard (globalFilters, header, etc.) - // minus the widgets already covered above. - const { widgets: _skip, ...rest } = item; - scanForFilters(rest, `${key}[${i}]`, `dashboard "${name}"`, out, new Set()); - return; - } - scanForFilters(item, `${key}[${i}]`, `${kind} "${name}"`, out, new Set()); - }); - } + walkAuthoredFilters(stack, TOKEN_FILTER_SURFACES, ({ value, path, where }) => { + walkFilterValues(value, path, where, out, new Set()); + }); return out; }