Skip to content

Commit ea1e65b

Browse files
committed
fix(lint): descend into conditional then/otherwise when building the _validations universe
validate-translation-references built its `_validations` universe with a flat walk of `objects[].validations[]`, so a `conditional` rule's `then` / `otherwise` branch — a full rule carrying its own `name`, which IS the address `checkConditional` delegates to and `authoredRuleMessage` keys on at runtime (packages/objectql/src/validation/rule-validator.ts) — was reported as an orphan `translation-target-unknown`, with inverted advice (acting on it reintroduces the exact defect the entry fixed). The walk now descends into `then` / `otherwise` via a small recursive collector, mirroring `evaluateRule`'s recursion (a branch may itself be a nested `conditional`, so depth is unbounded). The wrapper's own name stays in the universe unchanged, since a bundle entry for it is kept deliberately elsewhere. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV
1 parent e6ac0c6 commit ea1e65b

3 files changed

Lines changed: 156 additions & 5 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
Fix `validate-translation-references` reporting a nested `conditional` validation branch's legitimate `_validations` bundle entry as an orphan `translation-target-unknown`, with inverted advice.
6+
7+
The rule built its `_validations` universe with a flat walk of `objects[].validations[]`. A `conditional` rule's `then` / `otherwise` branch is itself a full rule carrying its own `name`, and that branch name — not the wrapper's — is the address `checkConditional` delegates to and `authoredRuleMessage` keys on at runtime (`packages/objectql/src/validation/rule-validator.ts`). The flat walk never saw a branch name, so a correct bundle entry for one was flagged as an orphan, and the finding's own text ("keeps its source locale in every refusal") was the opposite of the truth for that key — acting on the advice (deleting the entry) reintroduced the exact defect it fixed.
8+
9+
The walk now descends into `then` / `otherwise`, mirroring `evaluateRule`'s recursion (a branch may itself be a nested `conditional`, so depth is unbounded). The wrapper's own name stays in the universe, unchanged: its message is structurally unreachable at runtime, but a bundle entry for it is deliberately kept elsewhere so the bundle mirrors the declared rule set 1:1.

packages/lint/src/validate-translation-references.test.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,120 @@ describe('validateTranslationReferences — orphan keys', () => {
152152
});
153153
});
154154

155+
describe('validateTranslationReferences — nested conditional validation branches (#14700)', () => {
156+
// Mirrors the card's own fixture: `demo_account` with one `conditional`
157+
// rule whose `then` / `otherwise` are each a full, named rule.
158+
const conditionalStack = (validations: unknown[], objectNode: Record<string, unknown>) => ({
159+
objects: [
160+
{
161+
name: 'demo_account',
162+
label: 'Account',
163+
fields: {
164+
status: { type: 'select', label: 'Status' },
165+
churn_reason: { type: 'text', label: 'Churn reason' },
166+
},
167+
validations,
168+
},
169+
],
170+
translations: [{ 'zh-CN': { objects: { demo_account: objectNode } } }],
171+
});
172+
173+
const churnConsistencyRule = {
174+
type: 'conditional',
175+
name: 'churn_reason_consistency',
176+
message: 'Churn reason must match the account state.',
177+
when: "record.status == 'churned'",
178+
then: {
179+
type: 'script',
180+
name: 'churn_reason_present',
181+
message: 'A churned account needs a churn reason.',
182+
condition: 'record.churn_reason == null',
183+
},
184+
otherwise: {
185+
type: 'script',
186+
name: 'churn_reason_absent',
187+
message: 'A non-churned account must not carry a churn reason.',
188+
condition: 'record.churn_reason != null',
189+
},
190+
};
191+
192+
it('accepts bundle entries for both branch names and the wrapper name at once', () => {
193+
// Before the fix, this reported `translation-target-unknown` on BOTH
194+
// branch keys — the card's own measurement — because the flat walk over
195+
// `obj.validations` never descended into `then` / `otherwise`, even
196+
// though `checkConditional` / `authoredRuleMessage` address the branch by
197+
// exactly this name at runtime.
198+
const findings = validateTranslationReferences(
199+
conditionalStack([churnConsistencyRule], {
200+
_validations: {
201+
// The wrapper's own message is never rendered by `checkConditional`
202+
// (#14518 keeps a bundle entry for it anyway, deliberately, so the
203+
// bundle mirrors the declared rule set 1:1).
204+
churn_reason_consistency: { message: 'Wrapper message (never rendered)' },
205+
churn_reason_present: { message: '流失账户需要填写流失原因。' },
206+
churn_reason_absent: { message: '未流失账户不应填写流失原因。' },
207+
},
208+
}),
209+
);
210+
expect(findings).toEqual([]);
211+
});
212+
213+
it('still flags a bundle entry naming no rule at any depth — real orphans stay caught', () => {
214+
const findings = validateTranslationReferences(
215+
conditionalStack([churnConsistencyRule], {
216+
_validations: {
217+
churn_reason_present: { message: '流失账户需要填写流失原因。' },
218+
churn_reason_absent: { message: '未流失账户不应填写流失原因。' },
219+
churn_reason_ghost: { message: 'Nothing declares this.' },
220+
},
221+
}),
222+
);
223+
expect(findings.map((f) => f.path)).toEqual([
224+
'translations[0]["zh-CN"].objects.demo_account._validations.churn_reason_ghost',
225+
]);
226+
expect(findings[0].rule).toBe(TRANSLATION_TARGET_UNKNOWN);
227+
});
228+
229+
it('descends through a branch that is itself a nested conditional', () => {
230+
const outerGate = {
231+
type: 'conditional',
232+
name: 'outer_gate',
233+
message: 'outer',
234+
when: "record.status == 'churned'",
235+
then: {
236+
type: 'conditional',
237+
name: 'inner_gate',
238+
message: 'inner',
239+
when: 'record.churn_reason != null',
240+
then: {
241+
type: 'script',
242+
name: 'innermost_rule',
243+
message: 'deepest branch of all',
244+
condition: 'true',
245+
},
246+
},
247+
};
248+
const findings = validateTranslationReferences(
249+
conditionalStack([outerGate], { _validations: { innermost_rule: { message: '最深层的分支。' } } }),
250+
);
251+
expect(findings).toEqual([]);
252+
});
253+
254+
it('skips an unnamed branch, same as an unnamed top-level rule already was (#14253)', () => {
255+
const gateWithUnnamedBranch = {
256+
type: 'conditional',
257+
name: 'gate',
258+
message: 'gate',
259+
when: "record.status == 'churned'",
260+
then: { type: 'script', message: 'has no name', condition: 'true' },
261+
};
262+
const findings = validateTranslationReferences(
263+
conditionalStack([gateWithUnnamedBranch], { _validations: { gate: { message: '门。' } } }),
264+
);
265+
expect(findings).toEqual([]);
266+
});
267+
});
268+
155269
describe('validateTranslationReferences — option keys', () => {
156270
it('flags an option key that is a near-miss of the stored value', () => {
157271
// The HotCRM instance: `direct-mail` for the value `direct_mail`.

packages/lint/src/validate-translation-references.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,21 @@ function strName(v: unknown): string | undefined {
157157
return typeof v === 'string' && v.length > 0 ? v : undefined;
158158
}
159159

160+
/**
161+
* Add a validation rule's own `name` to the universe, then recurse into a
162+
* `conditional` rule's `then` / `otherwise` branch — each branch is itself a
163+
* full rule (with its own `name`, and possibly its own nested branches), and
164+
* mirrors the recursion `evaluateRule` performs at runtime (#14700). See the
165+
* call site in {@link buildUniverse} for why the wrapper's own name is kept
166+
* too, even though `then` / `otherwise` are what a caller actually reads.
167+
*/
168+
function collectValidationRuleNames(rule: AnyRec, validations: Set<string>): void {
169+
const ruleName = strName(rule.name);
170+
if (ruleName) validations.add(ruleName);
171+
if (isRec(rule.then)) collectValidationRuleNames(rule.then, validations);
172+
if (isRec(rule.otherwise)) collectValidationRuleNames(rule.otherwise, validations);
173+
}
174+
160175
/**
161176
* "Did you mean?" over the known names — a namespace pass the shared helper
162177
* cannot see, falling back to `suggestName`'s containment/edit-distance
@@ -209,9 +224,12 @@ interface ObjectFacts {
209224
sections: Set<string>;
210225
/**
211226
* `_validations` names — the custom validation rules this object declares
212-
* (`objects[].validations[].name`). `objects.<obj>._validations.<rule>.message`
213-
* (#14253) is keyed by that name, so a ghost here is a rule message that
214-
* renders in the source locale inside an otherwise translated refusal.
227+
* (`objects[].validations[].name`, including a nested `conditional` rule's
228+
* `then` / `otherwise` branch — the branch is itself a full rule and is the
229+
* address `checkConditional` / `authoredRuleMessage` actually key on;
230+
* #14700). `objects.<obj>._validations.<rule>.message` (#14253) is keyed by
231+
* that name, so a ghost here is a rule message that renders in the source
232+
* locale inside an otherwise translated refusal.
215233
*/
216234
validations: Set<string>;
217235
/**
@@ -605,9 +623,19 @@ function buildUniverse(stack: AnyRec): Universe {
605623
// #14253: `_validations.<rule>` is keyed by the rule's own `name`. A rule
606624
// without a name has no key and is not registered — the resolver cannot
607625
// address it either, so nothing is lost by skipping it here.
626+
// #14700: a `conditional` rule's `then` / `otherwise` branch is itself a
627+
// full rule, and its `name` — not the wrapper's — is the address
628+
// `checkConditional` delegates to and `authoredRuleMessage` keys on (see
629+
// `packages/objectql/src/validation/rule-validator.ts`). A flat walk over
630+
// `obj.validations` never sees a branch name, so a legitimate bundle
631+
// entry for one was reported as an orphan. Descend into `then` /
632+
// `otherwise`, mirroring `evaluateRule`'s recursion (a branch may itself
633+
// be a nested `conditional`, so depth is unbounded); the wrapper's own
634+
// name stays in the universe too — its message is structurally
635+
// unreachable at runtime, but #14518 keeps a bundle entry for it
636+
// deliberately so the bundle mirrors the declared rule set 1:1.
608637
for (const rule of asArray(obj.validations)) {
609-
const ruleName = strName(rule.name);
610-
if (ruleName) facts.validations.add(ruleName);
638+
collectValidationRuleNames(rule, facts.validations);
611639
}
612640
}
613641

0 commit comments

Comments
 (0)