Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/22161-lint-slice-8-one-line.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@objectstack/lint": patch
---

fix(lint): the dashboard-action, empty-filter, list-view-field and translation findings print one verdict line, and `os explain <rule-id>` carries their reasoning

Clause-②: no

- **Shorter verdicts.** Each finding of these 9 rule ids now prints a `message` of one verdict sentence (followed by `Did you mean "…"?` where the rule offers the nearest declared name). Every finding the rules' own test suites fire is at most 199 characters, where the longest of each id ran from 226 to 553 before. The ids:
- dashboard header actions (`dashboards[].header.actions[]`): `dashboard-action-target-undefined` (the `script` and `modal` arms), `dashboard-action-route-unresolved` (one finding per unregistered `COLLECTION/NAME` segment of a `url` target);
- authored filters: `filter-empty-combinator` (`$and: []`, `$or: []`, `$not: {}`), `filter-empty-node` (`{}` as the whole filter, as a `$or` branch, as a `$and` branch);
- list-view field references: `list-view-field-unknown`, `list-view-field-dotted` (the projection door and the filter door's three refused head classes);
- translation bundles (`translations[]`): `translation-target-unknown` (every leg: objects and their fields, views, sections, tabs, validations and actions; global actions; apps and navigation; dashboards, widgets and header actions; flows, toasts, screens, screen descriptions, screen fields and refusals; action params, outcomes and result dialogs), `translation-option-key-unknown` (field, action-param and screen-field options), `translation-section-name-missing`.

The empty-filter verdicts still take their row-set words ("matches EVERY row" / "matches NO row") from the shared filter reduction. `list-view-field-unknown` keeps the shared field-path sentence and, for a dotted reference, which written name it judged. The `fix` (the CLI's `fix:` line, the runtime issue's `hint`), every rule id, severity and `path`, and what each rule accepts or refuses are unchanged. A tool that matched the old message text should match on `rule` and `path` instead.
- **`os explain <rule-id>` takes these 9 ids**, for example `os explain translation-target-unknown`. It prints the reasoning the verdicts no longer carry: what a `script` or `modal` header-action target must name and how a `url` route is resolved and skipped; the empty-filter identity table, what each shape does on a read scope or beside other branches, and which filters are judged; what a dangling list-view field costs at each position, why only the head of a dotted name is judged, and which positions reach which query door; how the translation resolver reads a key, why an orphan key is an error while a mis-keyed option is a warning, what every bundle leg may name, and why a section with no `name` can never be translated. Paragraphs shared across ids are one text, printed under every id they explain. The `rule:` line under each of these findings now ends with `` — `os explain <rule-id>` for … ``. The no-argument listing and its `--json` `rules` array list the 9 ids, and so does the unknown-id error's `Rules with an explanation:` line. `RULE_EXPLANATIONS` in `@objectstack/lint` gains the 9 entries.
- **Where the new text prints.** On the CLI: `os validate`, `os build` (and `os compile`, which `os dev` runs on every compile), `os lint`, `os verify` and the scaffold check `os init` runs print the new `message` on the text face for all 9 ids, and `os validate --json` and `os build --json` carry it in their `errors` and author-time `issues`. At the runtime publish gate (Studio, REST `/meta`, MCP): on a `flow` or `report` write, `filter-empty-combinator` and `filter-empty-node` change the 422 issue's `message` and the refusal log line under `OS_ALLOW_UNLINTED_METADATA_WRITES`; on the write types the reference-integrity suite dispatches the list-view rule on (`view`, `object` and `flow`), `list-view-field-dotted` and the error-tier positions of `list-view-field-unknown` change the 422 issue's `message` and the refusal log line, and its warning-tier positions ride the 2xx `advisories` and the `[Protocol] authoring advisory` log line. Each issue's `hint` is unchanged.
- **Never at the runtime gate:** the two dashboard-action ids (a CLI-only rule: a single published item cannot see the actions and pages it resolves against) and the three translation ids (the rules run there only on a `flow` write, whose snapshot carries no translation bundles).
289 changes: 289 additions & 0 deletions packages/lint/src/rule-explanations.ts

Large diffs are not rendered by default.

101 changes: 100 additions & 1 deletion packages/lint/src/validate-dashboard-action-refs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,24 @@

import { describe, it, expect } from 'vitest';
import {
validateDashboardActionRefs,
validateDashboardActionRefs as validateDashboardActionRefsUnrecorded,
DASHBOARD_ACTION_TARGET_UNDEFINED,
DASHBOARD_ACTION_ROUTE_UNRESOLVED,
} from './validate-dashboard-action-refs';
import { explainRule } from './rule-explanations.js';

// [#22161] Each finding of the two ids is one verdict sentence; the reasoning
// it used to carry is the id's `os explain` entry. Every direct call below
// records what it fired, and the last cases in this file hold each recorded
// verdict to one line of at most 200 characters — every firing variant this
// suite exercises, not a chosen few. Run the whole file: those cases read what
// the cases above fired.
const fired: Array<{ rule: string; message: string }> = [];
const validateDashboardActionRefs: typeof validateDashboardActionRefsUnrecorded = (...args) => {
const findings = validateDashboardActionRefsUnrecorded(...args);
fired.push(...findings);
return findings;
};

/** Build a stack with a single dashboard whose header carries `actions`. */
function dashWithHeaderActions(actions: unknown[], extra: Record<string, unknown> = {}) {
Expand Down Expand Up @@ -349,3 +363,88 @@ describe('validateDashboardActionRefs (ADR-0049 references / #3367)', () => {
).toEqual([]);
});
});

describe('[#22161] one-line verdicts — dashboard-action-target-undefined, dashboard-action-route-unresolved', () => {
const CONVERTED = [DASHBOARD_ACTION_TARGET_UNDEFINED, DASHBOARD_ACTION_ROUTE_UNRESOLVED];

it('every verdict the cases above fired is one line of at most 200 characters', () => {
const converted = fired.filter((f) => CONVERTED.includes(f.rule));
// The coverage control first: both ids fired, the target id on both arms,
// and the route id on a path with two bad segments.
expect([...new Set(converted.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort());
const target = converted.filter((f) => f.rule === DASHBOARD_ACTION_TARGET_UNDEFINED);
expect(target.some((f) => f.message.startsWith('modal '))).toBe(true);
expect(target.some((f) => f.message.startsWith('script '))).toBe(true);
expect(
converted.some((f) => f.message.includes('"/apps/no_such_app_nope/dashboard/no_such_dashboard_nope"')),
).toBe(true);
for (const f of converted) {
expect(f.message, f.rule).not.toContain('\n');
expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200);
}
});

it('reads as one sentence per arm', () => {
const findings = validateDashboardActionRefs(
dashWithHeaderActions([
{ label: 'Export PDF', actionType: 'script', actionUrl: 'export_dashboard_pdf' },
{ label: 'New Deal', actionType: 'modal', actionUrl: 'create_opportunity' },
{ label: 'Forecast', actionType: 'url', actionUrl: '/reports/forecast' },
]),
);
expect(findings.map((f) => f.message)).toEqual([
'script action target "export_dashboard_pdf" names no defined action, so the button renders and ' +
'does nothing when clicked',
'modal action target "create_opportunity" names no declared page (a modal target names a page, ' +
'only), so the button renders and the runtime refuses the dispatch on click',
'url action target "/reports/forecast": no report named "forecast" is registered in this stack, so ' +
'the button likely opens a dead route',
]);
});

it('`os explain` carries what the verdicts no longer say', () => {
const facts: Record<string, string[]> = {
[DASHBOARD_ACTION_TARGET_UNDEFINED]: [
'ADR-0049',
'`stack.actions`',
'`stack.pages`',
'bare object name',
'`VERB_OBJECT` convention',
"`actionType: 'form'`",
],
[DASHBOARD_ACTION_ROUTE_UNRESOLVED]: [
'no `actionType`',
'`apps`, `objects`, `reports`, `dashboards`, `pages` and `views`',
'does not hide a bad dashboard name',
'another installed package',
'`http(s)://`',
'opaque route',
],
};
for (const [rule, list] of Object.entries(facts)) {
const entry = explainRule(rule);
expect(entry, `no \`os explain ${rule}\` entry`).toBeDefined();
const text = entry!.paragraphs.join('\n');
for (const fact of list) expect(text, `${rule} explanation names ${fact}`).toContain(fact);
// The scope paragraph is one text under both ids.
expect(text).toContain('`header.actions[]`');
expect(text).toContain('retired in `@objectstack/spec` 17');
}
const scopeOf = (rule: string) => explainRule(rule)!.paragraphs.at(-1);
expect(scopeOf(DASHBOARD_ACTION_TARGET_UNDEFINED)).toBe(scopeOf(DASHBOARD_ACTION_ROUTE_UNRESOLVED));
});

it('the collections the route explanation lists are the ones the rule resolves', () => {
// The explanation module imports nothing, so its collection list is held
// to the rule by running the rule: each named collection, singular and
// plural, reports an unknown name.
for (const plural of ['apps', 'objects', 'reports', 'dashboards', 'pages', 'views']) {
for (const segment of [plural, plural.replace(/s$/, '')]) {
const findings = validateDashboardActionRefsUnrecorded(
dashWithHeaderActions([{ label: 'Go', actionType: 'url', actionUrl: `/${segment}/zz_nope` }]),
);
expect(findings.map((f) => f.rule), segment).toEqual([DASHBOARD_ACTION_ROUTE_UNRESOLVED]);
}
}
});
});
21 changes: 11 additions & 10 deletions packages/lint/src/validate-dashboard-action-refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,15 +293,14 @@ export function validateDashboardActionRefs(stack: AnyRec): DashboardActionRefFi
rule: DASHBOARD_ACTION_TARGET_UNDEFINED,
where,
path,
// [#22161] One verdict sentence; what a target may name, and why a
// dangling one gates, is `os explain dashboard-action-target-undefined`.
message:
actionType === 'modal'
? `modal action target "${target}" names no declared page — a modal target ` +
`names a PAGE, only. The button renders but the runtime ` +
`refuses the dispatch when clicked — a dangling reference ` +
`(ADR-0049: a declared reference must resolve).`
: `script action target "${target}" resolves to no defined action. ` +
`The button renders but does nothing when clicked — a dangling reference ` +
`the runtime cannot dispatch (ADR-0049: a declared reference must resolve).`,
? `modal action target "${target}" names no declared page (a modal target names a ` +
`page, only), so the button renders and the runtime refuses the dispatch on click`
: `script action target "${target}" names no defined action, so the button renders ` +
`and does nothing when clicked`,
hint:
actionType === 'modal'
? `Point actionUrl at a declared page (stack.pages), or use ` +
Expand All @@ -324,10 +323,12 @@ export function validateDashboardActionRefs(stack: AnyRec): DashboardActionRefFi
rule: DASHBOARD_ACTION_ROUTE_UNRESOLVED,
where,
path,
// [#22161] One verdict sentence, naming the one segment that misses;
// what is resolved and what is skipped is
// `os explain dashboard-action-route-unresolved`.
message:
`url action target "${target}" points at ${route.collection}/${route.name}, ` +
`but no ${route.collection.replace(/s$/, '')} named "${route.name}" is registered ` +
`in this stack — the button likely navigates to a dead route.`,
`url action target "${target}": no ${route.collection.replace(/s$/, '')} named ` +
`"${route.name}" is registered in this stack, so the button likely opens a dead route`,
hint:
`Check the path for a typo, define the referenced ${route.collection.replace(/s$/, '')}, ` +
`or ignore this if the route is served by another installed package or a host/console route.`,
Expand Down
110 changes: 109 additions & 1 deletion packages/lint/src/validate-empty-combinators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,25 @@ import { describe, it, expect } from 'vitest';
import { FILTER_LOGIC_CASES, reduceFilterVerdict } from '@objectstack/spec/data';

import {
validateEmptyCombinators,
validateEmptyCombinators as validateEmptyCombinatorsUnrecorded,
FILTER_EMPTY_COMBINATOR,
FILTER_EMPTY_NODE,
} from './validate-empty-combinators.js';
import { FILTER_KEYS } from './filter-walk.js';
import { explainRule } from './rule-explanations.js';

// [#22161] Each finding of the two ids is one verdict sentence; the reasoning
// it used to carry is the id's `os explain` entry. Every direct call below
// records what it fired, and the last cases in this file hold each recorded
// verdict to one line of at most 200 characters — every firing variant this
// suite exercises, not a chosen few. Run the whole file: those cases read what
// the cases above fired.
const fired: Array<{ rule: string; message: string }> = [];
const validateEmptyCombinators: typeof validateEmptyCombinatorsUnrecorded = (...args) => {
const findings = validateEmptyCombinatorsUnrecorded(...args);
fired.push(...findings);
return findings;
};

type AnyRec = Record<string, unknown>;

Expand Down Expand Up @@ -330,3 +345,96 @@ describe('validateEmptyCombinators — the vocabulary is the runtime\'s (#5322/#
}
});
});

describe('[#22161] one-line verdicts — filter-empty-combinator, filter-empty-node', () => {
const CONVERTED = [FILTER_EMPTY_COMBINATOR, FILTER_EMPTY_NODE];

it('every verdict the cases above fired is one line of at most 200 characters', () => {
const converted = fired.filter((f) => CONVERTED.includes(f.rule));
// The coverage control first: both ids fired, the combinator id on all
// three shapes and the node id at all three positions.
expect([...new Set(converted.map((f) => f.rule))].sort()).toEqual([...CONVERTED].sort());
const combinator = converted.filter((f) => f.rule === FILTER_EMPTY_COMBINATOR);
for (const shape of ['`$and: []`', '`$or: []`', '`$not: {}`']) {
expect(combinator.some((f) => f.message.startsWith(shape)), shape).toBe(true);
}
const node = converted.filter((f) => f.rule === FILTER_EMPTY_NODE);
for (const opening of ['An EMPTY filter node', 'An EMPTY branch (`{}`) of a `$or`', 'An EMPTY branch (`{}`) of a `$and`']) {
expect(node.some((f) => f.message.startsWith(opening)), opening).toBe(true);
}
for (const f of converted) {
expect(f.message, f.rule).not.toContain('\n');
expect(f.message.length, `${f.rule}: ${f.message}`).toBeLessThanOrEqual(200);
}
});

it('reads as one sentence per shape and position', () => {
const verdict = (filter: unknown) => validateEmptyCombinators(widgetStack(filter))[0]!.message;
expect(verdict({ $and: [] })).toBe(
'`$and: []` is a conjunction of ZERO conditions, which every backend reduces to its identity, so it ' +
'matches EVERY row: this surface reads as filtered and is not',
);
expect(verdict({ $or: [] })).toBe(
'`$or: []` is a disjunction of ZERO branches, which every backend reduces to its identity, so it ' +
'matches NO row: this surface renders permanently empty',
);
expect(verdict({ $not: {} })).toBe(
'`$not: {}` negates an EMPTY node, which is TRUE, so it matches NO row: the opposite of the ' +
'"no filter" an empty operand looks like',
);
expect(verdict({})).toBe(
'An EMPTY filter node (`{}`) is TRUE, so it matches EVERY row exactly as if the key were absent: a ' +
'filter is declared and enforces nothing',
);
expect(verdict({ $or: [{ status: 'open' }, {}] })).toBe(
'An EMPTY branch (`{}`) of a `$or` is TRUE, and one TRUE disjunct ABSORBS the disjunction (it ' +
'matches EVERY row), so every branch written beside it is dead',
);
expect(verdict({ $and: [{ status: 'open' }, {}] })).toBe(
'An EMPTY branch (`{}`) of a `$and` is TRUE, the AND identity, so it contributes no condition and ' +
'the conjunction means whatever its other branches mean',
);
});

it('`os explain` carries what the verdicts no longer say', () => {
const facts: Record<string, string[]> = {
[FILTER_EMPTY_COMBINATOR]: [
'on a read scope, hides every row',
'fail-closed by design',
'not an authoring surface',
],
[FILTER_EMPTY_NODE]: [
'silently narrow the scope',
'lost in an edit',
],
};
for (const [rule, list] of Object.entries(facts)) {
const entry = explainRule(rule);
expect(entry, `no \`os explain ${rule}\` entry`).toBeDefined();
const text = entry!.paragraphs.join('\n');
for (const fact of list) expect(text, `${rule} explanation names ${fact}`).toContain(fact);
}
// The identity table and the scope are one text each under both ids.
const [combinator, node] = CONVERTED.map((rule) => explainRule(rule)!.paragraphs);
expect(combinator![0]).toBe(node![0]);
expect(combinator!.at(-1)).toBe(node!.at(-1));
});

it('the explanation\'s identity table is the shared reduction\'s', () => {
// The explanation module imports nothing, so the row sets it writes out
// are held to `reduceFilterVerdict` here, the same function the verdicts
// derive their words from.
const identities = explainRule(FILTER_EMPTY_COMBINATOR)!.paragraphs[0]!;
expect(reduceFilterVerdict({ $and: [] })).toBe('true');
expect(reduceFilterVerdict({})).toBe('true');
expect(identities).toContain('`{ $and: [] }` and `{}` reduce to TRUE and match EVERY row');
expect(reduceFilterVerdict({ $or: [] })).toBe('false');
expect(reduceFilterVerdict({ $not: {} })).toBe('false');
expect(identities).toContain('`{ $or: [] }` and `{ $not: {} }` reduce to FALSE and match NO row');
});

it('the explanation names every filter key the walk visits', () => {
const scope = explainRule(FILTER_EMPTY_COMBINATOR)!.paragraphs.at(-1)!;
for (const key of FILTER_KEYS) expect(scope, key).toContain(`\`${key}\``);
});
});
Loading
Loading