Skip to content

Commit 8e76b5c

Browse files
os-litantclaude
andauthored
fix(cli): guard convention/label-case on a localized label (#16280)
* fix(cli): guard `convention/label-case` on a localized label `checkLabelCase` indexed its argument (`label[0].toUpperCase()`) on a parameter annotated `string`, while every call site reaches it through `any`-typed config walking and `I18nLabelSchema` is `z.union([z.string(), InlineLocaleMapSchema])`. On the map form `label[0]` is `undefined`, so the rule threw a `TypeError` that escaped `lintConfig` into the command's catch-all: every `os lint` face exited 1 with `Cannot read properties of undefined (reading 'toUpperCase')`, naming no rule, no path and no remedy, on input `ObjectStackDefinitionSchema` parses clean. The rule now returns early unless `typeof label === 'string'`. The string branch is byte-identical, pinned per carrier. It deliberately says nothing about a localized label rather than resolving the map: picking which locale entry a case verdict is taken against is a product call, not a lint call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(cli): make the schema-valid pins actually parse the fixture The four rows in "the localized fixtures are schema-VALID" called `normalizeStackInput(stack).stack`, but `normalizeStackInput` returns the normalized stack itself, not a `{ stack }` wrapper — so `.stack` was `undefined` and every row was parsing `undefined`, not the fixture. All four were red. Worse, the CONTROL row was passing for the wrong reason: it asserts a number label does NOT parse, and `undefined` does not parse either, so it went green without ever discriminating on the label. The block that exists to prove the localized fixtures are supported authoring input was measuring nothing. Drop the `.stack`. The four positives now parse clean and the control still rejects, so the control discriminates on the label for the first time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * chore(cli): grade the localized-label guard changeset `minor` `Check Changeset`'s level axis refuses a PR that declares clause ② and grades a package it grew `patch`. This PR declares clause ② — the `needs:contract-review` carrier is on it — and the conformance limb fires: `convention/label-case`'s JSON face moves from `{"error": ...}` / exit 1 to `{"passed": true, ...}` / exit 0 on an input class `ObjectStackDefinitionSchema` parses clean. `minor` is the grade that declaration implies — a purely additive widening of a published package's public surface takes at least `minor` (maintainer ruling, 2026-09-04 decision batch #35), and the commit type may raise a bump but never lower it. The two declarations now agree inside one PR. Also replaces a falsification condition in the NO MOVE header that did not falsify. It offered `a label != null guard that changes the empty-string case` as a third way to swallow strings; it is neither. On a lowercase string `label != null` is true, so the row still reports and NO MOVE stays green; on `''` both spellings reach the falsy `label &&` test and return null, so the empty-string case does not move either. Replaced with the guard inverted to `typeof label === 'string'`, which does make every lowercase row below stop reporting — the observable the header claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fe7b880 commit 8e76b5c

4 files changed

Lines changed: 286 additions & 7 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
`os lint` no longer crashes on a localized label.
6+
7+
`convention/label-case` indexed its argument (`label[0].toUpperCase()`) on a parameter annotated `string`, while every call site reaches it through `any`-typed config walking and the spec does not require a label to be a string: `I18nLabelSchema` is `z.union([z.string(), InlineLocaleMapSchema])`. On the map form `label[0]` is `undefined`, the rule threw a `TypeError`, and the throw escaped `lintConfig` into the command's catch-all — so an author who localized an app label or a list-view label could not lint the project at all. Every face exited 1 with `Cannot read properties of undefined (reading 'toUpperCase')`, naming no rule, no path and no remedy, on input `ObjectStackDefinitionSchema` parses clean.
8+
9+
The rule now checks `typeof label === 'string'` first. Two of the four carriers it walks accept the inline locale map — `apps[].label` (`AppSchema`) and a view's `list` / `listViews.*` labels (`ListViewShapeSchema`); the other two are `z.string()` and reject the map at the schema door (`objects[].label`, `objects[].fields.*.label`).
10+
11+
**Nothing about a plain string label moves.** Same warning, same message, same `fix`, same path, on all four carriers — that is pinned per carrier rather than asserted.
12+
13+
**The rule deliberately says nothing about a localized label**, rather than resolving the map and case-checking one of its entries. Case is a property of a literal, and deciding which locale entry a case verdict is taken against is a product call, not a lint call. Widening the rule that way is a separate change.

packages/cli/src/commands/lint.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,31 @@ function checkLabelExists(item: any, path: string, kind: string): LintIssue | nu
9898
return null;
9999
}
100100

101-
function checkLabelCase(label: string, path: string): LintIssue | null {
101+
// A label is not required to be a string. `I18nLabelSchema` (spec
102+
// `ui/i18n.zod`) is `z.union([z.string(), InlineLocaleMapSchema])`, and it is
103+
// the label primitive the whole `ui/` tree imports — so of the four carriers
104+
// this rule is called on, two accept the inline locale map:
105+
// `views[].list.label` / `views[].listViews.*.label` (`ListViewShapeSchema`)
106+
// and `apps[].label` (`AppSchema`). The other two are `z.string()` and reject
107+
// the map at the schema door (`objects[].label`, `objects[].fields.*.label`).
108+
//
109+
// Every call site reaches this function through `any`-typed config walking, so
110+
// the annotation below used to say `string` and be wrong: on a map,
111+
// `label[0]` is `undefined` and `undefined.toUpperCase()` threw. The throw
112+
// escaped `lintConfig` into the command's catch-all, so an author who
113+
// localized an app or list-view label could not lint the project at all —
114+
// every face exited 1 naming no rule, no path and no remedy, on input
115+
// `ObjectStackDefinitionSchema` parses clean.
116+
//
117+
// ⛔ The guard deliberately says NOTHING about a localized label rather than
118+
// resolving the map and case-checking an entry. Case is a property of a
119+
// literal; picking WHICH locale entry a case verdict is taken against is a
120+
// product decision (`resolveI18nLabel` exists, but which entry is
121+
// authoritative for a lint verdict is not this rule's to answer). Widening
122+
// the rule to localized labels is an extension, filed separately; this guard
123+
// is the floor, and it leaves the string branch below byte-identical.
124+
function checkLabelCase(label: unknown, path: string): LintIssue | null {
125+
if (typeof label !== 'string') return null;
102126
if (label && label[0] !== label[0].toUpperCase()) {
103127
return {
104128
severity: 'warning',
@@ -111,7 +135,11 @@ function checkLabelCase(label: string, path: string): LintIssue | null {
111135
return null;
112136
}
113137

114-
function getViewLabel(view: any, viewPath: string): { label?: string; path: string } {
138+
// ⚠️ `label` is `unknown`, not `string`: it is read straight off `any`-typed
139+
// config and `ListViewShapeSchema.label` is `I18nLabelSchema`, so the value
140+
// can legitimately be an inline locale map. Annotating it `string` here is
141+
// what let the map reach `checkLabelCase`'s indexing unchecked.
142+
function getViewLabel(view: any, viewPath: string): { label?: unknown; path: string } {
115143
if (view?.list?.label) {
116144
return { label: view.list.label, path: `${viewPath}.list.label` };
117145
}
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `convention/label-case` must not crash `os lint` on a localized label.
5+
*
6+
* ## What this file pins, and how each half can fail
7+
*
8+
* The rule indexed its argument (`label[0].toUpperCase()`) on a parameter
9+
* annotated `string` that every call site reaches through `any`-typed config
10+
* walking. `I18nLabelSchema` is `z.union([z.string(), InlineLocaleMapSchema])`,
11+
* so on the map form `label[0]` is `undefined` and the rule threw — out of
12+
* `lintConfig`, into the command's catch-all, exit 1 on every face with a
13+
* message naming no rule, no path and no remedy.
14+
*
15+
* Two independent properties, and neither one covers the other:
16+
*
17+
* 1. **NO MOVE** — the check on a plain string label is unchanged. This is
18+
* the property that makes a guard the uncontroversial floor, so it is
19+
* pinned per carrier rather than asserted in prose. Falsified by any guard
20+
* that also swallows strings (a `typeof` typo, an early return placed
21+
* above the string branch, or the guard inverted to
22+
* `typeof label === 'string'`): every lowercase row below stops
23+
* reporting.
24+
* 2. **NO CRASH** — a localized label is walked without throwing, on every
25+
* carrier whose schema accepts the map. Falsified by removing the guard:
26+
* each of those rows throws a `TypeError` instead of returning issues.
27+
*
28+
* ## Why the carriers are enumerated rather than sampled
29+
*
30+
* The filed mutation sweep hit exactly two paths (`apps.0.label`,
31+
* `views.0.list.label`) because the fixture it swept had exactly those two.
32+
* The class is the set of call sites, not the set of sweep hits:
33+
* `lintConfig` calls this rule from four places, and the schema decides which
34+
* of them can carry a map —
35+
*
36+
* | call site | governing schema | map? |
37+
* | `objects[].label` | `ObjectSchema.label` — `z.string()` | no |
38+
* | `objects[].fields.*.label` | field base — `z.string()` | no |
39+
* | `views[].list{,Views.*}.label` | `ListViewShapeSchema` — `I18nLabelSchema` | yes |
40+
* | `apps[].label` | `AppSchema.label` — `I18nLabelSchema` | yes |
41+
*
42+
* — which is why `views[].listViews.*.label` is pinned below even though no
43+
* sweep ever reached it: it is the same primitive behind a second path, and
44+
* `getViewLabel` only falls through to it when `list.label` is absent.
45+
*
46+
* ## What the rule now SAYS about a localized label: nothing
47+
*
48+
* Deliberately. Case is a property of a literal; deciding which locale entry a
49+
* case verdict is taken against is a product call this card does not make. So
50+
* the localized rows assert the ABSENCE of a `convention/label-case` issue —
51+
* if someone later widens the rule to resolve the map, these are the
52+
* assertions that must be rewritten on purpose rather than silently satisfied.
53+
*/
54+
55+
import { describe, expect, it } from 'vitest';
56+
import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/spec';
57+
import { lintConfig } from '../src/commands/lint';
58+
import { scoreMetadata } from '../src/lint/score';
59+
60+
const MANIFEST = {
61+
id: 'todo',
62+
namespace: 'todo',
63+
version: '1.0.0',
64+
name: 'Todo',
65+
type: 'app' as const,
66+
};
67+
68+
/** The card's own fixture value, plus the sweep's literal hit value. */
69+
const LOCALIZED = { en: 'Todos', 'zh-CN': '待办' };
70+
const EMPTY_MAP = {};
71+
72+
const caseIssues = (issues: { rule: string }[]) =>
73+
issues.filter((i) => i.rule === 'convention/label-case');
74+
75+
/** `objects[]` needs fields to avoid drowning the label rows in structure issues. */
76+
const objectWith = (label: unknown) => ({
77+
name: 'invoice',
78+
label,
79+
fields: { name: { type: 'text', label: 'Invoice Number' } },
80+
});
81+
82+
const objectWithFieldLabel = (label: unknown) => ({
83+
name: 'invoice',
84+
label: 'Invoice',
85+
fields: { name: { type: 'text', label } },
86+
});
87+
88+
const stackWithApp = (label: unknown) => ({
89+
manifest: MANIFEST,
90+
apps: [{ name: 'todo_app', label }],
91+
});
92+
93+
const stackWithListLabel = (label: unknown) => ({
94+
manifest: MANIFEST,
95+
views: [{ name: 'invoice_views', object: 'invoice', list: { label, type: 'grid', columns: ['name'] } }],
96+
});
97+
98+
const stackWithNamedListLabel = (label: unknown) => ({
99+
manifest: MANIFEST,
100+
views: [{
101+
name: 'invoice_views',
102+
object: 'invoice',
103+
listViews: { all: { label, type: 'grid', columns: ['name'] } },
104+
}],
105+
});
106+
107+
describe('convention/label-case — the plain-string check does not move', () => {
108+
// Falsification for every row: a guard that swallows strings as well as
109+
// maps makes the lowercase rows report nothing and this whole block red.
110+
const rows: [string, unknown, string][] = [
111+
['objects[].label', { objects: [objectWith('invoice')] }, 'objects[0].label'],
112+
['objects[].fields.*.label', { objects: [objectWithFieldLabel('invoice number')] }, 'objects[0].fields.name.label'],
113+
['views[].list.label', stackWithListLabel('accounts'), 'views[0].list.label'],
114+
['views[].listViews.*.label', stackWithNamedListLabel('all accounts'), 'views[0].listViews.all.label'],
115+
['apps[].label', stackWithApp('todos'), 'apps[0].label'],
116+
];
117+
118+
for (const [carrier, config, path] of rows) {
119+
it(`still warns on a lowercase string at ${carrier}`, () => {
120+
const issues = caseIssues(lintConfig(config as any));
121+
expect(issues).toHaveLength(1);
122+
expect(issues[0]).toEqual({
123+
severity: 'warning',
124+
rule: 'convention/label-case',
125+
message: expect.stringContaining('should start with an uppercase letter'),
126+
path,
127+
fix: expect.any(String),
128+
});
129+
});
130+
}
131+
132+
it('carries the label verbatim in the message and the capitalized value in `fix`', () => {
133+
// The message/fix wording is what an author reads, so it is pinned whole
134+
// on one row rather than left to `stringContaining` everywhere.
135+
const [issue] = caseIssues(lintConfig(stackWithApp('todos') as any));
136+
expect(issue).toMatchObject({
137+
message: 'Label "todos" should start with an uppercase letter',
138+
fix: 'Todos',
139+
});
140+
});
141+
142+
it('stays silent on an already-uppercase string', () => {
143+
expect(caseIssues(lintConfig(stackWithApp('Todos') as any))).toEqual([]);
144+
});
145+
146+
it('stays silent on a label whose first character has no case (unchanged)', () => {
147+
// `'1st quarter'[0].toUpperCase()` === `'1'`, so the rule never fired here
148+
// and must still not. A guard written as `label.length && ...` would keep
149+
// this green; one written as "warn unless the first char is uppercase"
150+
// would flip it. That is the distinction this row protects.
151+
expect(caseIssues(lintConfig(stackWithApp('1st quarter') as any))).toEqual([]);
152+
});
153+
});
154+
155+
describe('convention/label-case — a localized label is walked, not indexed', () => {
156+
// Falsification for every row: drop the `typeof label !== 'string'` guard
157+
// and each of these throws `TypeError: Cannot read properties of undefined
158+
// (reading 'toUpperCase')` instead of returning.
159+
const rows: [string, (label: unknown) => unknown][] = [
160+
['apps[].label', stackWithApp],
161+
['views[].list.label', stackWithListLabel],
162+
['views[].listViews.*.label', stackWithNamedListLabel],
163+
];
164+
165+
for (const [carrier, build] of rows) {
166+
it(`does not throw on an inline locale map at ${carrier}`, () => {
167+
expect(() => lintConfig(build(LOCALIZED) as any)).not.toThrow();
168+
});
169+
170+
it(`does not throw on an EMPTY locale map at ${carrier}`, () => {
171+
// `{}` is the value the filed sweep actually mutated in, and it is a
172+
// valid `InlineLocaleMapSchema` (a `z.record` with no entries).
173+
expect(() => lintConfig(build(EMPTY_MAP) as any)).not.toThrow();
174+
});
175+
176+
it(`reports no case verdict for the localized label at ${carrier}`, () => {
177+
expect(caseIssues(lintConfig(build(LOCALIZED) as any))).toEqual([]);
178+
});
179+
180+
it(`does not report the localized label as MISSING at ${carrier}`, () => {
181+
// The other failure mode a careless guard produces: treat a non-string
182+
// as absent and emit `required/label`, which would be a NEW error on a
183+
// schema-valid config — the opposite of leaving behaviour where it was.
184+
const issues = lintConfig(build(LOCALIZED) as any) as { rule: string }[];
185+
expect(issues.filter((i) => i.rule === 'required/label')).toEqual([]);
186+
});
187+
}
188+
189+
it('does not throw on a non-string, non-map label either', () => {
190+
// Schema-INVALID input (no label carrier accepts a number), so this is
191+
// not the defect's class — but the guard is written on the type, not on
192+
// the map shape, and a linter that dies on bad input still cannot report
193+
// the bad input. Falsified by a guard spelled `if (isLocaleMap(label))`.
194+
expect(() => lintConfig(stackWithApp(42) as any)).not.toThrow();
195+
expect(caseIssues(lintConfig(stackWithApp(42) as any))).toEqual([]);
196+
});
197+
});
198+
199+
describe('the localized fixtures are schema-VALID — this is not bad input', () => {
200+
// Falsification: if any of these stopped parsing, the crash rows above
201+
// would be pinning a diagnostic degrading on bad input (acceptable) rather
202+
// than a tool that cannot walk a supported authoring shape (the defect).
203+
const stacks: [string, unknown][] = [
204+
['apps[].label', stackWithApp(LOCALIZED)],
205+
['views[].list.label', stackWithListLabel(LOCALIZED)],
206+
['views[].listViews.*.label', stackWithNamedListLabel(LOCALIZED)],
207+
['apps[].label, empty map', stackWithApp(EMPTY_MAP)],
208+
];
209+
210+
for (const [carrier, stack] of stacks) {
211+
it(`${carrier} parses clean`, () => {
212+
const parsed = ObjectStackDefinitionSchema.safeParse(normalizeStackInput(stack as any));
213+
expect(parsed.success).toBe(true);
214+
});
215+
}
216+
217+
it('CONTROL: a number label does NOT parse, so the parse check discriminates', () => {
218+
const parsed = ObjectStackDefinitionSchema.safeParse(normalizeStackInput(stackWithApp(42) as any));
219+
expect(parsed.success).toBe(false);
220+
});
221+
});
222+
223+
describe('the scorer reaches a verdict on a localized stack', () => {
224+
// The join with the swallowed-crash repair one module over: that repair
225+
// makes `scoreMetadata` REFUSE when a rule throws. With the guard there is
226+
// no throw, so the refusal must not fire here. Falsification: drop the
227+
// guard and `lintError` is set, `valid` is false, `grade` is 'F'.
228+
it('scores it without a lint crash', () => {
229+
const r = scoreMetadata(stackWithApp(LOCALIZED));
230+
expect(r.lintError).toBeUndefined();
231+
expect(r.valid).toBe(true);
232+
expect(r.counts.schemaErrors).toBe(0);
233+
expect(r.grade).not.toBe('F');
234+
});
235+
});

packages/cli/test/score-lint-crash.test.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66
*
77
* ## Why the linter is mocked here rather than driven
88
*
9-
* The crash IS reachable on a schema-valid stack — a localized `label`
9+
* The crash WAS reachable on a schema-valid stack — a localized `label`
1010
* (`{ en: …, 'zh-CN': … }`) on an app, or on a view's `list`, parses clean and
11-
* makes the label-case rule throw a `TypeError`. That is a defect in the rule,
12-
* filed on its own; pinning it here would make this suite depend on a bug
13-
* staying unfixed, and the day someone repairs the rule these assertions would
14-
* go green for the wrong reason — or be deleted to make them pass.
11+
* made the label-case rule throw a `TypeError`. That was a defect in the rule,
12+
* filed and fixed on its own (`convention/label-case` now guards on
13+
* `typeof label === 'string'`; the pins live in
14+
* `lint-label-case-localized.test.ts`). Pinning it here would have made this
15+
* suite depend on a bug staying unfixed — and that day has since come: the
16+
* repair landed, and had these assertions been driven through the real rule
17+
* they would now be green for the wrong reason, or deleted to make them pass.
1518
*
1619
* What this file pins is the SCORER's contract, which holds for any throw from
1720
* any rule: a crash is recorded, never swallowed into `issues: []`. So the

0 commit comments

Comments
 (0)