Skip to content

Commit e847324

Browse files
committed
fix(lint,cli): a hint is a fix — expression-invalid's source rides its message; four context-only hints state their fix
`os validate` / `os build` label every finding's hint `fix:`. Five producers put something else there: expression-invalid quoted the authored source (printed as `fix: source: …`), and component-props-invalid, flow-time-relative-descriptor-invalid, react-prop-missing-required (the contract description branch) and liveness-experimental-property carried context only. The source now ends the expression-invalid message (` — source: …`, the flow engine's spelling) with an empty hint; the other four lead with the instruction. Claude-Session: https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn Co-authored-by: Claude <noreply@anthropic.com>
1 parent d33862b commit e847324

7 files changed

Lines changed: 83 additions & 15 deletions

File tree

‎packages/cli/src/utils/format.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,9 +2128,10 @@ export type AuthoringRuleFinding = AuthoringAdvisory;
21282128
*
21292129
* The `hint` line is conditional, which is how `printAuthoringAdvisories` and
21302130
* `init` already rendered it; `compile`/`validate` printed it unconditionally.
2131-
* `AuthoringFinding.hint` is a required non-empty string in every rule the
2132-
* registry ships (checked: no rule emits an empty one), so the two forms
2133-
* differ on no finding this CLI can actually produce.
2131+
* [#22161] The condition is now load-bearing: `hint` prints as the `fix:`
2132+
* line, and `expression-invalid` emits an empty one — its finding carries no
2133+
* fix of its own, and the authored source it used to put there is a quote,
2134+
* which rides the message instead.
21342135
*/
21352136
export function printAuthoringRuleErrors(
21362137
errors: readonly AuthoringRuleFinding[],

‎packages/cli/test/explain-rule-id.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
2727

2828
import * as lint from '@objectstack/lint';
2929
import {
30+
AUTHORING_RULES,
31+
EXPRESSION_INVALID,
3032
FIELD_NO_CONSUMERS,
3133
RULE_EXPLANATIONS,
3234
SECURITY_OWD_UNSET,
@@ -220,4 +222,44 @@ describe('the printed shape of each shortened rule — verdict, fix, rule line w
220222
.toEqual(['fix: Remove it.', 'rule: liveness-dead-property at views[0].name']);
221223
expect(authoringFindingDetailLines({ rule: 'some-rule', path: 'p' })).toEqual(['rule: some-rule at p']);
222224
});
225+
226+
// The `fix:` label is only true of a hint that IS a fix. `expression-invalid`
227+
// used to put the authored source in `hint`, which printed as
228+
// `fix: source: \`status != "resolved"\`` — the expression the author already
229+
// wrote, offered as the fix. The source is a quote: it rides the verdict, so
230+
// it still reaches the text face (and the runtime 422 issue's `message`).
231+
it('expression-invalid: the authored source rides the verdict, and no `fix: source:` line is printed', async () => {
232+
const entry = AUTHORING_RULES.find((r) => r.name === 'validateStackExpressions');
233+
expect(entry, 'the registry entry that adapts ExprIssue into expression-invalid').toBeDefined();
234+
const [finding] = entry!.run(
235+
{
236+
objects: [
237+
{
238+
name: 'my_app_ticket',
239+
fields: {
240+
title: { type: 'text' },
241+
status: { type: 'select', options: [{ label: 'Open', value: 'open' }, { label: 'Resolved', value: 'resolved' }] },
242+
},
243+
},
244+
],
245+
actions: [
246+
{ name: 'resolve_ticket', label: 'Resolve', objectName: 'my_app_ticket', type: 'script', visible: 'status != "resolved"' },
247+
],
248+
},
249+
{},
250+
);
251+
expect(finding?.rule).toBe(EXPRESSION_INVALID);
252+
expect(finding.message).toContain('Write `record.status`');
253+
expect(finding.message).toMatch(/ — source: `status != "resolved"`$/);
254+
expect(finding.hint).toBe('');
255+
256+
const out = await captureStdout(() => printAuthoringRuleErrors([finding]));
257+
const lines = out.split('\n').filter((l) => l.trim() !== '');
258+
expect(lines).toHaveLength(2);
259+
expect(lines[0]).toBe(` • ${finding.where}: ${finding.message}`);
260+
expect(lines[0]).toContain('source: `status != "resolved"`');
261+
expect(lines[1]).toBe(` rule: expression-invalid at ${finding.path}`);
262+
expect(out).not.toMatch(/fix: source:/);
263+
expect(out).not.toMatch(/^\s*fix:/m);
264+
});
223265
});

‎packages/lint/src/authoring-rules.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,15 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
683683
rule: EXPRESSION_INVALID,
684684
where: i.where,
685685
path: i.where,
686-
message: i.message,
687-
hint: `source: \`${i.source}\``,
686+
// [#22161] The authored source is a QUOTE of what the author wrote, not
687+
// a fix, so it rides the verdict — `— source: \`…\`` is the spelling
688+
// the flow engine's runtime refusals already use — and reaches the CLI
689+
// text face and the runtime 422 issue alike. `hint` is the `fix:` line:
690+
// an `ExprIssue` carries no fix of its own (its message prescribes the
691+
// rewrite where there is one, e.g. "Write `record.status`"), so there is
692+
// none to print rather than a quote dressed as one.
693+
message: i.source.trim() ? `${i.message} — source: \`${i.source}\`` : i.message,
694+
hint: '',
688695
})),
689696
},
690697
// ADR-0053 — `userFilters`/`quickFilters` on an object list view ("views"

‎packages/lint/src/lint-liveness-properties.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,11 @@ function describe(entry: LedgerEntry): { kind: string; rule: string; defaultHint
244244
return {
245245
kind: 'is experimental — declared but NOT enforced at runtime',
246246
rule: LIVENESS_EXPERIMENTAL_PROPERTY,
247-
defaultHint: 'It is declared in the spec as an experimental guarantee — not yet enforced at runtime.',
247+
// [#22161] The CLI prints `hint` as the `fix:` line, so it says what to
248+
// do; the fact it used to state alone is the reason.
249+
defaultHint:
250+
'Do not rely on it as a guarantee: it is declared in the spec as an experimental guarantee and ' +
251+
'not yet enforced at runtime.',
248252
};
249253
}
250254
if (entry.status === 'planned') {

‎packages/lint/src/validate-component-props.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,17 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] {
332332
rule: COMPONENT_PROPS_INVALID,
333333
where,
334334
path: at,
335-
message: `${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)}`,
335+
// [#22161] `hint` is the CLI's `fix:` line, so it states the fix; the
336+
// advisory posture it used to explain (judged here as a warning, and
337+
// the props bag is not parsed on the storage path) is the verdict's
338+
// consequence and rides the message.
339+
message:
340+
`${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)} — ` +
341+
'nothing refuses it today, so the renderer receives the props as written',
336342
hint:
337-
`\`${type}\`'s props are declared by ComponentPropsMap (@objectstack/spec/ui) — the ` +
338-
'rejection above carries the fix. Advisory for now: props are judged here, at the authoring ' +
339-
'door, as a warning before they become an error, and the props bag is not parsed on the ' +
340-
'storage path either, so nothing rejects this today.',
343+
`Give \`${type}\` props its schema accepts (ComponentPropsMap, @objectstack/spec/ui) — the ` +
344+
'rejection above names what it expects — or, if the component really does honour what is ' +
345+
'written, correct the schema so the declaration and the renderer agree.',
341346
});
342347
}
343348
}

‎packages/lint/src/validate-flow-trigger-readiness.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -494,10 +494,13 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines
494494
`has a config.timeRelative descriptor the time-relative trigger REFUSES at bind time, so the ` +
495495
`sweep is never installed — the flow declares a time-relative trigger and then never runs ` +
496496
`(the only trace is one warn in the server log). ${problems}`,
497+
// [#22161] `hint` is the CLI's `fix:` line: the instruction first, the
498+
// reason it is sufficient after it.
497499
hint:
498-
`Those messages are TimeRelativeTriggerSchema's own — the same schema the trigger safeParses at ` +
499-
`bind time, so a descriptor that satisfies them binds. An unrecognized key names the declared key ` +
500-
`it was probably meant to be; see content/docs/references/automation/time-relative-trigger.mdx.`,
500+
`Correct config.timeRelative until it satisfies each message above: they are ` +
501+
`TimeRelativeTriggerSchema's own, the same schema the trigger safeParses at bind time, so a ` +
502+
`descriptor that satisfies them binds. An unrecognized key names the declared key it was ` +
503+
`probably meant to be; see content/docs/references/automation/time-relative-trigger.mdx.`,
501504
});
502505
}
503506
}

‎packages/lint/src/validate-react-page-props.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1163,9 +1163,15 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
11631163
: `<${tag}> is missing the required prop "${req}".`,
11641164
// The contract's own description of the binding says how to
11651165
// write it — the author gets the spelling, not a pointer.
1166+
// [#22161] `hint` is the CLI's `fix:` line, and a description
1167+
// alone ("The registered component type to render.") is not a
1168+
// fix: the instruction leads, the description follows it.
11661169
hint: dep
11671170
? dep.note
1168-
: block.requiredDescriptions.get(req) ?? `Pass ${req}={…}. See the react-tier component contract.`,
1171+
: `Pass ${req}={…}` +
1172+
(block.requiredDescriptions.has(req)
1173+
? `: ${block.requiredDescriptions.get(req)}`
1174+
: '. See the react-tier component contract.'),
11691175
});
11701176
}
11711177
}

0 commit comments

Comments
 (0)