Skip to content

Commit 01fd9e1

Browse files
hotlongclaude
andauthored
fix(lint): validateFormLayout 走视图容器阶梯,两条规则不再对真实 app 全盘报绿 (#6251) (#6382)
* fix(lint): validateFormLayout 走视图容器阶梯,两条规则不再对真实 app 全盘报绿 (#6251) `form-field-unknown` / `absolute-colspan-discouraged` 只读 `views[]` 条目 自身的 `sections`,其余一律 `continue`。但 `views[]` 是视图**容器** —— `ViewSchema` 自有键只有 name/label/object/list/form/listViews/formViews, 表单 `sections` 在下一层的 `form` 与 `formViews.<key>` 下。于是唯一被读 的那种形状,恰恰是严格 `ViewSchema` 会拒绝的形状(实测报 `unrecognized_keys` 并点名 `sections`),真实 app 出货的形状一个都没被检查。 三个 example app 实测:app-showcase / app-crm / app-todo 在条目根部有 0 个 表单站点,在 form / formViews.<key> 下有 14 个 —— 旧遍历在它们身上无物可读, 报绿正是因为什么都没读到(#4984 / #5009 的幽灵检查族)。 遍历直接照抄 #6248 落在同包 `validate-visibility-predicates.ts` 里的 `formViewSites`,不另造第三套;list / listViews.<key> 是 `ObjectListViewSchema`,按 schema 不带 sections,故不走;`objects[].views` 已被 `object.zod.ts` 具名墓碑化,同样不走。另补:legacy `groups` 桶(实测 parse 阶段并未折叠进 sections)、finding 的 `where` 标注子容器、子容器缺 `data.object` 时继承容器绑定、map 形态的 `views` 报在真实键上。 两条规则的严重级别、消息与提示一字未改;三个 example app 上新增 finding 数 为 0,即不引入误报。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 * test(lint): 标注 clean-stack 断言的空绿性质与配对依赖 (#6251) 反向验证声明 B 的空绿自查查出:「干净 app 栈报 0」在遍历被退回后照样通过 —— 因为什么都没读到,不是因为没有缺陷。断言保留(误报守卫仍有价值),但把它 只在与「真实 app 形状上确实报了」配对时才成立这件事写进文件,免得日后 后者被削弱而前者被当成独立保障。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 * fix(lint): 测试文件的相对导入补 `.js` 扩展名,修复 TypeScript Type Check 门 (#6251) CI 的 `check:type-check-debt` 把 `packages/lint` 的**测试层**重新纳入 tsc 后 测得 48 个 raw error,超过 TEST_DEBT 冻结的 42(+6)。包自己的 `pnpm typecheck` 看不到这个 —— 它的 tsconfig 把 `**/*.test.ts` 排除在外。 根因是一处 TS2835:`./validate-form-layout` 缺 `.js` 扩展名,在 `moduleResolution: node16` 下解析失败,导入即退化为 `any`,下游每个 `.map(f => …)` 都变成 TS7006。补上扩展名(本包其余测试文件本就都是这个写法, 这一处是异类)后该文件 9 个 error 全清,包的测试层从 48 降到 39 —— 低于 冻结值,ratchet 只降不升,按门的规则属 ℹ 而非 error。 TEST_DEBT 条目保持 42 不动:门明确「改进不必为记账付费」,且并发改动下 下调数字容易与他人的测量赛跑。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDmDsu2575gDxeMCxXhDE3 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 761a0ba commit 01fd9e1

3 files changed

Lines changed: 479 additions & 70 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): `validateFormLayout` walks the view CONTAINER ladder, so both its rules stop reporting clean on every real app (#6251)
6+
7+
`form-field-unknown` and `absolute-colspan-discouraged` read a `sections` array
8+
off the **`views[]` entry itself** and skipped everything else. But a `views[]`
9+
entry is a view CONTAINER, not a view: `ViewSchema` declares exactly `name` /
10+
`label` / `object` / `list` / `form` / `listViews` / `formViews`, and form
11+
sections live one level down, under `form` and each `formViews.<key>`. So the
12+
one shape the traversal read is the one shape strict `ViewSchema` **refuses** —
13+
measured, `unrecognized_keys` naming `sections` — and the shapes every app
14+
actually ships were never inspected at all.
15+
16+
Measured on the three shipped example apps, before and after: `app-showcase`,
17+
`app-crm` and `app-todo` carry **0** form sites at the entry root and **14**
18+
under `form` / `formViews.<key>`. The old traversal therefore had nothing to
19+
read on any of them, and reported clean for that reason — the "ghost check"
20+
shape (#4984 / #5009): a rule that is green because it never read anything is
21+
worse than no rule, because it occupies the slot that would otherwise look
22+
empty.
23+
24+
One broken form, three placements, before → after:
25+
26+
| placement | before | after |
27+
| --- | --- | --- |
28+
| `views[0].sections` (entry IS a bare form view) | reports | reports |
29+
| `views[0].form.sections` (container default form) | silent | reports |
30+
| `views[0].formViews.edit.sections` (named form view) | silent | reports |
31+
32+
What changed, precisely:
33+
34+
- The traversal is the one `validate-visibility-predicates.ts` landed in #6248
35+
for the identical hole on the sibling rule — copied, not re-derived, so two
36+
rules on one surface cannot drift apart about which forms exist. `list` /
37+
`listViews.<key>` are `ObjectListViewSchema` and carry no `sections`, so they
38+
are deliberately not walked; `objects[].views` stays out because
39+
`object.zod.ts` tombstones that key by name.
40+
- The legacy `groups` bucket (`FormSectionSchema[]`, the documented alias of
41+
`sections`) is read too. Measured: it is **not** folded into `sections` at
42+
parse, so a `groups`-authored form was a second silent shape.
43+
- A finding names its sub-container — `view "contact_views" · formViews.create`
44+
— because an artifact-emitted container carries neither `name` nor `object`,
45+
and without it two forms under one view were indistinguishable.
46+
- A sub-container inherits the container's object binding when it declares no
47+
`data.object` of its own, resolved through the same `objectName` → `object` →
48+
`data.object` ladder the other view-walking rules in this package use.
49+
- A map-shaped `views` reports at the key it sits at (`views.contact_views.…`)
50+
rather than a synthetic index, so a finding stays usable as an edit target.
51+
52+
Both rules remain advisory `warning`s and their messages, hints and severities
53+
are unchanged. No new finding appeared on any example app, so nothing that was
54+
green goes red on existing metadata — what changes is that a form defect in the
55+
places apps actually put forms is now reported instead of silently passed.

‎packages/lint/src/validate-form-layout.test.ts‎

Lines changed: 224 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import { describe, it, expect } from 'vitest';
3+
import { describe, it, expect, vi } from 'vitest';
4+
import { defineStack, normalizeStackInput } from '@objectstack/spec';
45
import {
56
validateFormLayout,
67
FORM_FIELD_UNKNOWN,
78
FORM_COLSPAN_ABSOLUTE,
8-
} from './validate-form-layout';
9+
} from './validate-form-layout.js';
10+
11+
type AnyRec = Record<string, unknown>;
912

1013
const objects = [
1114
{ name: 'contract', fields: { name: {}, amount: {}, status: {}, notes: {} } },
@@ -112,3 +115,222 @@ describe('validateFormLayout (#2578)', () => {
112115
expect(validateFormLayout({ views: [], objects: [] })).toEqual([]);
113116
});
114117
});
118+
119+
// ───────────────────────────────────────────────────────────────────────────
120+
// #6251 — `views[]` is a view CONTAINER, and both rules above were unreachable
121+
// on the shape real apps actually ship.
122+
//
123+
// The measurement that opened the issue: one broken form, three placements.
124+
// Only the placement the strict schema REFUSES was being read, so an app whose
125+
// forms all live under `form` / `formViews.<key>` — i.e. every app — got a
126+
// clean report from a rule that had read nothing at all. That is the ghost
127+
// check #4984 / #5009 name: green because nothing was inspected.
128+
// ───────────────────────────────────────────────────────────────────────────
129+
130+
/** The one broken form, reused verbatim in every placement below. */
131+
const brokenForm = {
132+
data: { provider: 'object', object: 'contract' },
133+
sections: [{ columns: 2, fields: ['name', 'ghost_field'] }],
134+
};
135+
136+
describe('#6251 — the view CONTAINER ladder', () => {
137+
it('reports the SAME broken form in all three placements', () => {
138+
const at = (view: AnyRec) =>
139+
validateFormLayout({ objects, views: [view] }).map((f) => `${f.rule}@${f.path}`);
140+
141+
// (1) the entry IS a bare form view — the only shape read before #6251.
142+
expect(at({ name: 'contract_form', ...brokenForm })).toEqual([
143+
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[1]`,
144+
]);
145+
// (2) the container's DEFAULT form.
146+
expect(at({ name: 'contract_views', object: 'contract', form: brokenForm })).toEqual([
147+
`${FORM_FIELD_UNKNOWN}@views[0].form.sections[0].fields[1]`,
148+
]);
149+
// (3) a NAMED form view — where `os build` on app-showcase actually puts them.
150+
expect(at({ name: 'contract_views', object: 'contract', formViews: { edit: brokenForm } })).toEqual([
151+
`${FORM_FIELD_UNKNOWN}@views[0].formViews.edit.sections[0].fields[1]`,
152+
]);
153+
});
154+
155+
it('names the sub-container in `where`, so two forms under one view are distinguishable', () => {
156+
const findings = validateFormLayout({
157+
objects,
158+
views: [{
159+
name: 'contract_views',
160+
object: 'contract',
161+
form: brokenForm,
162+
formViews: { edit: brokenForm, create: brokenForm },
163+
}],
164+
});
165+
expect(findings.map((f) => f.where)).toEqual([
166+
'view "contract_views" · form',
167+
'view "contract_views" · formViews.edit',
168+
'view "contract_views" · formViews.create',
169+
]);
170+
});
171+
172+
it('a sub-container INHERITS the container binding when it declares none', () => {
173+
// The canonical container carries `object`; a `formViews.<key>` entry that
174+
// omits its own `data.object` still renders against that object, so a
175+
// dangling field reference there is as real as anywhere else.
176+
const findings = validateFormLayout({
177+
objects,
178+
views: [{
179+
name: 'contract_views',
180+
object: 'contract',
181+
formViews: { edit: { sections: [{ fields: ['ghost_inherited'] }] } },
182+
}],
183+
});
184+
expect(findings.map((f) => f.rule)).toEqual([FORM_FIELD_UNKNOWN]);
185+
expect(findings[0].message).toContain('ghost_inherited');
186+
expect(findings[0].message).toContain('"contract"');
187+
});
188+
189+
it('reads the legacy `groups` bucket too — measured NOT folded into `sections` at parse', () => {
190+
const findings = validateFormLayout({
191+
objects,
192+
views: [{
193+
name: 'contract_views',
194+
object: 'contract',
195+
form: { data: { object: 'contract' }, groups: [{ fields: ['name', { field: 'ghost_g', colSpan: 3 }] }] },
196+
}],
197+
});
198+
expect(findings.map((f) => `${f.rule}@${f.path}`)).toEqual([
199+
`${FORM_FIELD_UNKNOWN}@views[0].form.groups[0].fields[1]`,
200+
`${FORM_COLSPAN_ABSOLUTE}@views[0].form.groups[0].fields[1].colSpan`,
201+
]);
202+
});
203+
204+
it('reports a map-shaped `views` at the key it sits at, not a synthetic index', () => {
205+
const findings = validateFormLayout({
206+
objects,
207+
views: { contract_views: { object: 'contract', formViews: { edit: brokenForm } } },
208+
});
209+
expect(findings.map((f) => f.path)).toEqual(['views.contract_views.formViews.edit.sections[0].fields[1]']);
210+
});
211+
212+
// NEGATIVE polarity — this one cannot go red when the traversal is reverted
213+
// (a narrower walk trivially satisfies "does not walk list views"). It is
214+
// here to pin the SCHEMA fact, not the fix: `list` / `listViews.<key>` are
215+
// `ObjectListViewSchema`, which declares no `sections`, so a `sections` key
216+
// there is not a form and must not be judged as one.
217+
it('does not walk `list` / `listViews.*` (they carry no sections by schema)', () => {
218+
expect(validateFormLayout({
219+
objects,
220+
views: [{ name: 'contract_views', object: 'contract', list: brokenForm, listViews: { all: brokenForm } }],
221+
})).toEqual([]);
222+
});
223+
});
224+
225+
// ───────────────────────────────────────────────────────────────────────────
226+
// The anti-ghost pin: the rule must fire on a stack built through the REAL
227+
// authoring door, not only on a hand-shaped object literal.
228+
//
229+
// `cliTierFor` is exactly what `os validate` / `os compile` hand the registry:
230+
// `defineStack` (which Zod-PARSES) followed by `normalizeStackInput`. So a
231+
// fixture that survives it is a shape an author can really ship — and a rule
232+
// that reports on it is really reachable. Without this, "the fix works" could
233+
// still mean "the fix works on shapes the schema refuses", which is how this
234+
// rule was silently dead for four minor versions.
235+
// ───────────────────────────────────────────────────────────────────────────
236+
237+
/** `defineStack` warns on the D2 conversion channel; keep test output clean. */
238+
function quietly<T>(fn: () => T): { value?: T; error?: Error } {
239+
const spy = vi.spyOn(console, 'warn').mockImplementation(() => {});
240+
try {
241+
return { value: fn() };
242+
} catch (e) {
243+
return { error: e as Error };
244+
} finally {
245+
spy.mockRestore();
246+
}
247+
}
248+
249+
const cliTierFor = (stack: AnyRec): AnyRec =>
250+
normalizeStackInput(defineStack(stack as never) as unknown as AnyRec);
251+
252+
describe('#6251 — reachable on a REAL parsed app stack', () => {
253+
const manifest = {
254+
id: 'com.example.formlayout',
255+
namespace: 'fl',
256+
version: '1.0.0',
257+
type: 'app',
258+
name: 'Form Layout Probe',
259+
engines: { protocol: '^17' },
260+
};
261+
262+
const data = { provider: 'object' as const, object: 'fl_contact' };
263+
264+
/**
265+
* The container ladder copied from `examples/app-showcase/src/ui/views/
266+
* contact.view.ts` — default `form` grouped into sections, plus a sparse
267+
* `formViews.create` override — with one dangling field planted in each.
268+
*/
269+
const appShape: AnyRec = {
270+
manifest,
271+
objects: [{
272+
name: 'fl_contact',
273+
label: 'Contact',
274+
fields: {
275+
name: { type: 'text', label: 'Name' },
276+
email: { type: 'email', label: 'Email' },
277+
phone: { type: 'phone', label: 'Phone' },
278+
},
279+
}],
280+
views: [{
281+
name: 'fl_contact',
282+
object: 'fl_contact',
283+
list: { label: 'Contacts', type: 'grid', data, columns: [{ field: 'name' }] },
284+
form: {
285+
type: 'simple',
286+
data,
287+
sections: [{ name: 'contact', label: 'Contact', columns: 2, fields: ['name', 'email', 'ghost_default'] }],
288+
},
289+
formViews: {
290+
create: {
291+
type: 'simple',
292+
data,
293+
title: 'New contact',
294+
sections: [{ label: 'Who is this?', columns: 1, fields: ['name', { field: 'ghost_named', colSpan: 2 }] }],
295+
},
296+
},
297+
}],
298+
};
299+
300+
it('the fixture is a shape the strict schema ACCEPTS (so the pin is not testing a rejected stack)', () => {
301+
const { error, value } = quietly(() => cliTierFor(structuredClone(appShape)));
302+
expect(error).toBeUndefined();
303+
// And it really is the container shape: no `sections` at the entry root.
304+
const view = (value!.views as AnyRec[])[0];
305+
expect(Object.keys(view).sort()).toEqual(['form', 'formViews', 'list', 'name', 'object']);
306+
expect(view.sections).toBeUndefined();
307+
});
308+
309+
it('reports every planted defect on that stack — this is the assertion #6251 exists for', () => {
310+
const { value } = quietly(() => cliTierFor(structuredClone(appShape)));
311+
expect(validateFormLayout(value!).map((f) => `${f.rule}@${f.path}`)).toEqual([
312+
`${FORM_FIELD_UNKNOWN}@views[0].form.sections[0].fields[2]`,
313+
`${FORM_FIELD_UNKNOWN}@views[0].formViews.create.sections[0].fields[1]`,
314+
`${FORM_COLSPAN_ABSOLUTE}@views[0].formViews.create.sections[0].fields[1].colSpan`,
315+
]);
316+
});
317+
318+
// EMPTY-GREEN, declared. Revert the container ladder and this test still
319+
// passes — because nothing was read, not because nothing is wrong. It is kept
320+
// (a false-positive guard is worth having) but it is only meaningful PAIRED
321+
// with the test above, which proves on the same fixture family that the
322+
// traversal does read these sites. If that one is ever weakened, this one
323+
// stops guarding anything; do not treat it as independent cover.
324+
it('a CLEAN app stack of the same shape reports nothing — the fix adds no false positives', () => {
325+
const clean = structuredClone(appShape);
326+
const view = (clean.views as AnyRec[])[0];
327+
(view.form as AnyRec).sections = [{ name: 'contact', label: 'Contact', columns: 2, fields: ['name', 'email', 'phone'] }];
328+
(view.formViews as AnyRec).create = {
329+
type: 'simple', data, title: 'New contact',
330+
sections: [{ label: 'Who is this?', columns: 1, fields: ['name', { field: 'email', span: 'full' }] }],
331+
};
332+
const { error, value } = quietly(() => cliTierFor(clean));
333+
expect(error).toBeUndefined();
334+
expect(validateFormLayout(value!)).toEqual([]);
335+
});
336+
});

0 commit comments

Comments
 (0)