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
16 changes: 16 additions & 0 deletions .changeset/27643-validation-rule-finding-path.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@objectstack/lint': patch
---

fix(lint): a validation rule's `expression-invalid` finding carries the slot's key path (`objects[0].validations[1].condition`) beside its `where` text

Clause-②: no
Metadata-protocol: none

An `expression-invalid` finding on an object validation rule's predicate used to be located by its `where` prose alone (`object 'invoice' · validation rule 'amount_rule'`), which is also what its `path` carried, so a reader keyed on paths could not address it. Its `path` is now the judged slot's key path in the spelling a flow edge's findings use, so a draft's `_diagnostics[].path` reports it item-relative (`validations.1.condition`, `validations.2.when`, `validations.2.then.condition`) and the publish refusal's `issues[].path` reads `objects.invoice.validations[1].condition`.

- `where`, the message and the verdict are unchanged: nothing is accepted or refused differently.
- Every finding of the validation-rule pass moves: the expression check, the `parent` root verdict and the null-guard gate, on a rule's `condition` and `when` and on a `conditional` rule's nested `then` / `otherwise` branches.
- The object and the rule are indexed in the lists as authored (a `null` member before them does not renumber them).
- An object in a map-form `objects` collection keeps `where` as its `path`, as before.
- `os lint` prints the key path after `at` for these findings, where it printed the `where` text a second time.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ const WHERE: Readonly<Record<Slot, string>> = {
'validation condition': "object 'fx_probe' · validation 'probe_rule'",
};

/**
* The door finding's `path`: the `where` prose on the two field slots, and on
* the validation rule (#27643) the judged slot's key path, its top-level index
* re-spelled by name at the door.
*/
const DOOR_PATH: Readonly<Record<Slot, string>> = {
...WHERE,
'validation condition': 'objects.fx_probe.validations[0].condition',
};

const SLOTS = Object.keys(WHERE) as Slot[];

/** Every member spelling that names `f`; the dot row is the one that was already refused. */
Expand Down Expand Up @@ -103,7 +113,7 @@ describe('an undeclared field is refused in every member spelling, at both doors
expect(door.rulesRun).toContain('validateStackExpressions');
const errs = expressionFindings(door.errors);
expect(errs, dump(door)).toHaveLength(1);
expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE[slot], path: WHERE[slot] });
expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE[slot], path: DOOR_PATH[slot] });
expect(errs[0]!.message).toContain('unknown field `zz_typo`');
});
}
Expand Down
34 changes: 26 additions & 8 deletions packages/lint/src/runtime-gate.object-validation-writes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ const buildFindings = (body: unknown) => {

const dump = (r: unknown) => JSON.stringify(r, null, 2);

/**
* [#27643] The build's findings as the door spells them: a validation rule's
* finding carries the judged slot's key path, and the door re-spells its
* top-level index by name (`objects[0]` → `objects.fx_rule`); every other key
* is the build's own.
*/
const asAtDoor = <T extends { path: string }>(fs: readonly T[]): T[] =>
fs.map((f) => ({ ...f, path: f.path.replace(/^objects\[0\]/, 'objects.fx_rule') }));

describe('#22032 pass 1 — the object door gives the build\'s validation-rule verdict', () => {
it('needs no registry change: `validateStackExpressions` is already on the object door', () => {
expect(runtimeAuthoringRulesFor('object').map((r) => r.name)).toContain('validateStackExpressions');
Expand All @@ -123,7 +132,7 @@ describe('#22032 pass 1 — the object door gives the build\'s validation-rule v
const errs = expressionFindings(result.errors);
const where = "object 'fx_rule' · validation 'amount_root'";
expect(errs, dump(result)).toHaveLength(1);
expect(errs[0]).toMatchObject({ severity: 'error', where, path: where });
expect(errs[0]).toMatchObject({ severity: 'error', where, path: 'objects.fx_rule.validations[0].condition' });
expect(errs[0]!.message).toContain('`sqrt` is not a callable name here');
});

Expand Down Expand Up @@ -171,7 +180,7 @@ describe('#22032 pass 1 — the object door gives the build\'s validation-rule v

// Non-vacuous: the build refuses each of them.
expect(atBuild.length, dump(rule)).toBeGreaterThan(0);
expect(atDoor, dump(rule)).toEqual(atBuild);
expect(atDoor, dump(rule)).toEqual(asAtDoor(atBuild));
}
});

Expand Down Expand Up @@ -265,7 +274,11 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver
const atBuild = buildFindings(fxRule([NESTED_REFUSED]));

expect(atBuild.map((f) => f.where), dump(atBuild)).toEqual([THEN_WHERE, OTHERWISE_WHERE]);
for (const f of atBuild) expect(f).toMatchObject({ severity: 'error', path: f.where });
expect(atBuild.map((f) => f.path), dump(atBuild)).toEqual([
'objects[0].validations[0].then.condition',
'objects[0].validations[0].otherwise.condition',
]);
for (const f of atBuild) expect(f).toMatchObject({ severity: 'error' });
expect(atBuild[0]!.message).toContain('`sqrt` is not a callable name here');
expect(atBuild[1]!.message).toContain('bare reference `amont`');
});
Expand All @@ -276,7 +289,7 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver
expect(result.rulesRun).toContain('validateStackExpressions');
const atDoor = expressionFindings(result.errors);
expect(atDoor.map((f) => f.where), dump(result)).toEqual([THEN_WHERE, OTHERWISE_WHERE]);
expect(atDoor).toEqual(buildFindings(fxRule([NESTED_REFUSED])));
expect(atDoor).toEqual(asAtDoor(buildFindings(fxRule([NESTED_REFUSED]))));
});

it('⭐ LIT — two levels down: a nested `conditional`\'s `when` and the rule below it are judged', () => {
Expand All @@ -288,7 +301,11 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver
]);
expect(atBuild[0]!.message).toContain('bare reference `amount`');
expect(atBuild[1]!.message).toContain('`sqrt` is not a callable name here');
expect(expressionFindings(gateObject(fxRule([TWO_LEVEL])).errors)).toEqual(atBuild);
expect(atBuild.map((f) => f.path), dump(atBuild)).toEqual([
'objects[0].validations[0].then.when',
'objects[0].validations[0].then.then.condition',
]);
expect(expressionFindings(gateObject(fxRule([TWO_LEVEL])).errors)).toEqual(asAtDoor(atBuild));
});

it('⭐ CONTROL — valid nested predicates publish clean, two levels down and in `otherwise`', () => {
Expand Down Expand Up @@ -326,7 +343,7 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver
const topWhen = buildFindings(fxRef([{ ...HYDRATION, when: MULTI_HOP, then: NESTED_VALID.otherwise, otherwise: undefined }]));
expect(topWhen.map((f) => f.where), dump(topWhen)).toEqual(["object 'fx_rule' · validation 'outer' when"]);
expect(topWhen[0]!.message).toContain('ONE hop');
expect(expressionFindings(gateObject(fxRef([HYDRATION])).errors)).toEqual(atBuild);
expect(expressionFindings(gateObject(fxRef([HYDRATION])).errors)).toEqual(asAtDoor(atBuild));
});

it('a stored sibling\'s nested fault is not this write\'s to answer for (the differential)', () => {
Expand Down Expand Up @@ -364,7 +381,8 @@ describe('#24468 — the object door refuses a rule hop to a field the target do
const errs = expressionFindings(result.errors);
expect(errs, dump(result)).toHaveLength(1);
const where = "object 'fx_order' · validation 'north_only'";
expect(errs[0]).toMatchObject({ severity: 'error', where, path: where });
// [#27643] The second rule's `condition`, by key path; `where` stays the prose.
expect(errs[0]).toMatchObject({ severity: 'error', where, path: 'objects.fx_order.validations[1].condition' });
expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`');
expect(errs[0]!.message).toContain('the rule has no verdict');
});
Expand Down Expand Up @@ -428,7 +446,7 @@ describe('#24512 — the object door judges a `conditional` rule\'s `when` hop a
const errs = expressionFindings(result.errors);
expect(errs, dump(result)).toHaveLength(1);
const where = "object 'fx_order' · validation 'gate' when";
expect(errs[0]).toMatchObject({ rule: EXPRESSION_INVALID, severity: 'error', where, path: where });
expect(errs[0]).toMatchObject({ rule: EXPRESSION_INVALID, severity: 'error', where, path: 'objects.fx_order.validations[0].when' });
expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`');
expect(errs[0]!.message).toContain('the rule has no verdict');
});
Expand Down
49 changes: 44 additions & 5 deletions packages/lint/src/validate-expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,10 @@ export interface ExprIssue {
* [#26858] The judged slot's key path in the stack, in the spelling the other
* rules' findings carry (`flows[0].edges[1].condition`), beside the human
* `where`. Set only where the slot has one: a top-level flow edge's
* `condition`. Absent, the finding is located by `where` alone.
* `condition`, and (#27643) an object validation rule's `condition` / `when`
* (`objects[0].validations[1].condition`), a nested branch's included
* (`objects[0].validations[1].then.condition`). Absent, the finding is
* located by `where` alone.
*/
path?: string;
}
Expand Down Expand Up @@ -573,14 +576,19 @@ interface RulePredicate {
slot: 'condition' | 'when';
/** The name of the rule that carries it (the nested rule's own, below depth 0). */
ruleName: string;
/**
* [#27643] Its key path inside the top-level rule (`condition`, `when`,
* `then.condition`, `otherwise.then.when`), for a finding's `path`.
*/
key: string;
}

/**
* Every predicate a validation rule carries, including the ones nested inside a
* `conditional` rule's `then` / `otherwise` — the trap hides there just as
* happily as at the top level.
*/
function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[] {
function rulePredicates(rule: AnyRec, path: string, depth = 0, keyPrefix = ''): RulePredicate[] {
const out: RulePredicate[] = [];
const name = typeof rule.name === 'string' ? rule.name : '?';
const here = path ? `${path} → '${name}'` : `'${name}'`;
Expand All @@ -593,14 +601,18 @@ function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[]
// author who wrote both `condition` and a rejected alias had their canonical
// predicate short-circuited away and the alias validated instead (#5017).
const main = rule.condition;
if (main != null) out.push({ label: `validation rule ${here}`, raw: main, depth, slot: 'condition', ruleName: name });
if (main != null) {
out.push({ label: `validation rule ${here}`, raw: main, depth, slot: 'condition', ruleName: name, key: `${keyPrefix}condition` });
}
if (rule.when != null) {
out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, depth, slot: 'when', ruleName: name });
out.push({
label: `validation rule ${here} when-predicate`, raw: rule.when, depth, slot: 'when', ruleName: name, key: `${keyPrefix}when`,
});
}
for (const branch of ['then', 'otherwise'] as const) {
const nested = rule[branch];
if (nested && typeof nested === 'object' && !Array.isArray(nested)) {
out.push(...rulePredicates(nested as AnyRec, `${here} ${branch}`, depth + 1));
out.push(...rulePredicates(nested as AnyRec, `${here} ${branch}`, depth + 1, `${keyPrefix}${branch}.`));
}
}
return out;
Expand Down Expand Up @@ -2955,6 +2967,11 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
// ── Object validation-rule + formula predicates ────────────────────
for (const obj of objects) {
const objectName = typeof obj.name === 'string' ? obj.name : undefined;
// [#27643] The object's index in the authored list, for a validation-rule
// finding's key path (`recordsOf` hands an array's own members back). A
// map-form `objects` has no index to spell, so its findings keep `where`
// alone, as a map-form `flows` collection's do (#26858).
const objectIndex = Array.isArray(stack.objects) ? stack.objects.indexOf(obj) : -1;
// `validations` is the key `ObjectSchema` declares; `validationRules` is a
// rejected alias of it (#5017) — see the `## Scope` table above.
const validations = obj.validations;
Expand All @@ -2963,6 +2980,18 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
// position, so the door's findings and their order are the build's.
for (const rule of recordsOf(validations)) {
const where = `object '${objectName}' · validation '${(rule.name as string) ?? '?'}'`;
// [#27643] Every finding below also carries the judged slot's key path
// (`objects[0].validations[1].condition`), indexed over the lists the
// author wrote, in the spelling a flow edge's findings use (#26858), so
// a draft's `_diagnostics` reports it item-relative
// (`validations.1.condition`). `where` stays the human locator.
const ruleIndex = objectIndex >= 0 && Array.isArray(obj.validations) ? obj.validations.indexOf(rule) : -1;
const rulePath = ruleIndex >= 0 ? `objects[${objectIndex}].validations[${ruleIndex}]` : undefined;
const atSlot = (key: string, first: number): void => {
if (!rulePath) return;
const path = `${rulePath}.${key}`;
for (let k = first; k < issues.length; k++) issues[k] = { ...issues[k]!, path };
};
// The declared predicate key is `condition` (see `rulePredicates`).
// Validation predicates are `record`-scoped — no field flattening — so
// bare refs are flagged (#1928).
Expand All @@ -2973,8 +3002,12 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
// `traversalHydration` is passed on those two slots here and nested below
// (#22042), and on a formula field's `expression` (`judgeFieldFormula`,
// #24197), and NOWHERE else.
let first = issues.length;
check(where, rule.condition, objectName, 'record', undefined, true);
atSlot('condition', first);
first = issues.length;
check(`${where} when`, (rule as AnyRec).when, objectName, 'record', undefined, true);
atSlot('when', first);
const predicates = rulePredicates(rule, '');
// [#22042] The same verdict one level down, and every level below it: a
// `conditional` rule's `then` / `otherwise` is a rule the evaluator runs
Expand All @@ -2989,7 +3022,9 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
// meet the traversal checks.
for (const p of predicates) {
if (p.depth === 0) continue;
first = issues.length;
check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, true);
atSlot(p.key, first);
}
// [#24720] The rule's root verdict, over every predicate it carries,
// located where `check()` above located the same predicate. `parent` is
Expand All @@ -3004,12 +3039,16 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
? `object '${objectName}' · ${p.label}`
: p.slot === 'when' ? `${where} when` : where;
// `error`: a rule the runtime cannot evaluate must not pass a door; see the docblock.
first = issues.length;
issues.push({ where: at, message, source: source!, severity: 'error' });
atSlot(p.key, first);
}
// #4763 — null-guard gate over every predicate the rule carries, nested
// `then`/`otherwise` branches included.
for (const p of predicates) {
first = issues.length;
checkNullGuards(`object '${objectName}' · ${p.label}`, p.label, p.raw, objectName);
atSlot(p.key, first);
}
}
// Field-level formulas (computed fields) reference the same object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ describe('#24720 — the object door gives the build\'s verdict', () => {
expect(result.rulesRun).toContain('validateStackExpressions');
const errs = expressionFindings(result.errors);
expect(errs, dump(result)).toHaveLength(1);
expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE, path: WHERE });
// [#27643] Located at the judged slot's key path; `where` stays the prose.
expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE, path: 'objects.fx_line.validations[0].condition' });
expect(errs[0]!.message).toContain("validation rule 'paid_lock' reads `parent`");
// No longer an advisory: the finding moved, it was not duplicated.
expect(expressionFindings(result.advisories), dump(result)).toEqual([]);
Expand All @@ -207,7 +208,12 @@ describe('#24720 — the object door gives the build\'s verdict', () => {
const atDoor = expressionFindings(gateObject(line([script(PARENT)]), [header]).errors);

expect(atBuild, dump(atBuild)).toHaveLength(1);
expect(atDoor, dump(atDoor)).toEqual(atBuild);
// [#27643] Equal once the door's by-name spelling of the top-level index
// is applied to the build's positional key path (`objects[1]`, after the header).
expect(atBuild[0]!.path).toBe('objects[1].validations[0].condition');
expect(atDoor, dump(atDoor)).toEqual(
atBuild.map((f) => ({ ...f, path: f.path.replace(/^objects\[1\]/, 'objects.fx_line') })),
);
});

it('⭐ CONTROL — `parent` on a field\'s own `readonlyWhen` publishes clean, and the build agrees', () => {
Expand Down
Loading
Loading