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
55 changes: 55 additions & 0 deletions .changeset/form-layout-view-container-ladder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
"@objectstack/lint": patch
---

fix(lint): `validateFormLayout` walks the view CONTAINER ladder, so both its rules stop reporting clean on every real app (#6251)

`form-field-unknown` and `absolute-colspan-discouraged` read a `sections` array
off the **`views[]` entry itself** and skipped everything else. But a `views[]`
entry is a view CONTAINER, not a view: `ViewSchema` declares exactly `name` /
`label` / `object` / `list` / `form` / `listViews` / `formViews`, and form
sections live one level down, under `form` and each `formViews.<key>`. So the
one shape the traversal read is the one shape strict `ViewSchema` **refuses** —
measured, `unrecognized_keys` naming `sections` — and the shapes every app
actually ships were never inspected at all.

Measured on the three shipped example apps, before and after: `app-showcase`,
`app-crm` and `app-todo` carry **0** form sites at the entry root and **14**
under `form` / `formViews.<key>`. The old traversal therefore had nothing to
read on any of them, and reported clean for that reason — the "ghost check"
shape (#4984 / #5009): a rule that is green because it never read anything is
worse than no rule, because it occupies the slot that would otherwise look
empty.

One broken form, three placements, before → after:

| placement | before | after |
| --- | --- | --- |
| `views[0].sections` (entry IS a bare form view) | reports | reports |
| `views[0].form.sections` (container default form) | silent | reports |
| `views[0].formViews.edit.sections` (named form view) | silent | reports |

What changed, precisely:

- The traversal is the one `validate-visibility-predicates.ts` landed in #6248
for the identical hole on the sibling rule — copied, not re-derived, so two
rules on one surface cannot drift apart about which forms exist. `list` /
`listViews.<key>` are `ObjectListViewSchema` and carry no `sections`, so they
are deliberately not walked; `objects[].views` stays out because
`object.zod.ts` tombstones that key by name.
- The legacy `groups` bucket (`FormSectionSchema[]`, the documented alias of
`sections`) is read too. Measured: it is **not** folded into `sections` at
parse, so a `groups`-authored form was a second silent shape.
- A finding names its sub-container — `view "contact_views" · formViews.create`
— because an artifact-emitted container carries neither `name` nor `object`,
and without it two forms under one view were indistinguishable.
- A sub-container inherits the container's object binding when it declares no
`data.object` of its own, resolved through the same `objectName` → `object` →
`data.object` ladder the other view-walking rules in this package use.
- A map-shaped `views` reports at the key it sits at (`views.contact_views.…`)
rather than a synthetic index, so a finding stays usable as an edit target.

Both rules remain advisory `warning`s and their messages, hints and severities
are unchanged. No new finding appeared on any example app, so nothing that was
green goes red on existing metadata — what changes is that a form defect in the
places apps actually put forms is now reported instead of silently passed.
226 changes: 224 additions & 2 deletions packages/lint/src/validate-form-layout.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import { defineStack, normalizeStackInput } from '@objectstack/spec';
import {
validateFormLayout,
FORM_FIELD_UNKNOWN,
FORM_COLSPAN_ABSOLUTE,
} from './validate-form-layout';
} from './validate-form-layout.js';

type AnyRec = Record<string, unknown>;

const objects = [
{ name: 'contract', fields: { name: {}, amount: {}, status: {}, notes: {} } },
Expand Down Expand Up @@ -112,3 +115,222 @@ describe('validateFormLayout (#2578)', () => {
expect(validateFormLayout({ views: [], objects: [] })).toEqual([]);
});
});

// ───────────────────────────────────────────────────────────────────────────
// #6251 — `views[]` is a view CONTAINER, and both rules above were unreachable
// on the shape real apps actually ship.
//
// The measurement that opened the issue: one broken form, three placements.
// Only the placement the strict schema REFUSES was being read, so an app whose
// forms all live under `form` / `formViews.<key>` — i.e. every app — got a
// clean report from a rule that had read nothing at all. That is the ghost
// check #4984 / #5009 name: green because nothing was inspected.
// ───────────────────────────────────────────────────────────────────────────

/** The one broken form, reused verbatim in every placement below. */
const brokenForm = {
data: { provider: 'object', object: 'contract' },
sections: [{ columns: 2, fields: ['name', 'ghost_field'] }],
};

describe('#6251 — the view CONTAINER ladder', () => {
it('reports the SAME broken form in all three placements', () => {
const at = (view: AnyRec) =>
validateFormLayout({ objects, views: [view] }).map((f) => `${f.rule}@${f.path}`);

// (1) the entry IS a bare form view — the only shape read before #6251.
expect(at({ name: 'contract_form', ...brokenForm })).toEqual([
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[1]`,
]);
// (2) the container's DEFAULT form.
expect(at({ name: 'contract_views', object: 'contract', form: brokenForm })).toEqual([
`${FORM_FIELD_UNKNOWN}@views[0].form.sections[0].fields[1]`,
]);
// (3) a NAMED form view — where `os build` on app-showcase actually puts them.
expect(at({ name: 'contract_views', object: 'contract', formViews: { edit: brokenForm } })).toEqual([
`${FORM_FIELD_UNKNOWN}@views[0].formViews.edit.sections[0].fields[1]`,
]);
});

it('names the sub-container in `where`, so two forms under one view are distinguishable', () => {
const findings = validateFormLayout({
objects,
views: [{
name: 'contract_views',
object: 'contract',
form: brokenForm,
formViews: { edit: brokenForm, create: brokenForm },
}],
});
expect(findings.map((f) => f.where)).toEqual([
'view "contract_views" · form',
'view "contract_views" · formViews.edit',
'view "contract_views" · formViews.create',
]);
});

it('a sub-container INHERITS the container binding when it declares none', () => {
// The canonical container carries `object`; a `formViews.<key>` entry that
// omits its own `data.object` still renders against that object, so a
// dangling field reference there is as real as anywhere else.
const findings = validateFormLayout({
objects,
views: [{
name: 'contract_views',
object: 'contract',
formViews: { edit: { sections: [{ fields: ['ghost_inherited'] }] } },
}],
});
expect(findings.map((f) => f.rule)).toEqual([FORM_FIELD_UNKNOWN]);
expect(findings[0].message).toContain('ghost_inherited');
expect(findings[0].message).toContain('"contract"');
});

it('reads the legacy `groups` bucket too — measured NOT folded into `sections` at parse', () => {
const findings = validateFormLayout({
objects,
views: [{
name: 'contract_views',
object: 'contract',
form: { data: { object: 'contract' }, groups: [{ fields: ['name', { field: 'ghost_g', colSpan: 3 }] }] },
}],
});
expect(findings.map((f) => `${f.rule}@${f.path}`)).toEqual([
`${FORM_FIELD_UNKNOWN}@views[0].form.groups[0].fields[1]`,
`${FORM_COLSPAN_ABSOLUTE}@views[0].form.groups[0].fields[1].colSpan`,
]);
});

it('reports a map-shaped `views` at the key it sits at, not a synthetic index', () => {
const findings = validateFormLayout({
objects,
views: { contract_views: { object: 'contract', formViews: { edit: brokenForm } } },
});
expect(findings.map((f) => f.path)).toEqual(['views.contract_views.formViews.edit.sections[0].fields[1]']);
});

// NEGATIVE polarity — this one cannot go red when the traversal is reverted
// (a narrower walk trivially satisfies "does not walk list views"). It is
// here to pin the SCHEMA fact, not the fix: `list` / `listViews.<key>` are
// `ObjectListViewSchema`, which declares no `sections`, so a `sections` key
// there is not a form and must not be judged as one.
it('does not walk `list` / `listViews.*` (they carry no sections by schema)', () => {
expect(validateFormLayout({
objects,
views: [{ name: 'contract_views', object: 'contract', list: brokenForm, listViews: { all: brokenForm } }],
})).toEqual([]);
});
});

// ───────────────────────────────────────────────────────────────────────────
// The anti-ghost pin: the rule must fire on a stack built through the REAL
// authoring door, not only on a hand-shaped object literal.
//
// `cliTierFor` is exactly what `os validate` / `os compile` hand the registry:
// `defineStack` (which Zod-PARSES) followed by `normalizeStackInput`. So a
// fixture that survives it is a shape an author can really ship — and a rule
// that reports on it is really reachable. Without this, "the fix works" could
// still mean "the fix works on shapes the schema refuses", which is how this
// rule was silently dead for four minor versions.
// ───────────────────────────────────────────────────────────────────────────

/** `defineStack` warns on the D2 conversion channel; keep test output clean. */
function quietly<T>(fn: () => T): { value?: T; error?: Error } {
const spy = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
return { value: fn() };
} catch (e) {
return { error: e as Error };
} finally {
spy.mockRestore();
}
}

const cliTierFor = (stack: AnyRec): AnyRec =>
normalizeStackInput(defineStack(stack as never) as unknown as AnyRec);

describe('#6251 — reachable on a REAL parsed app stack', () => {
const manifest = {
id: 'com.example.formlayout',
namespace: 'fl',
version: '1.0.0',
type: 'app',
name: 'Form Layout Probe',
engines: { protocol: '^17' },
};

const data = { provider: 'object' as const, object: 'fl_contact' };

/**
* The container ladder copied from `examples/app-showcase/src/ui/views/
* contact.view.ts` — default `form` grouped into sections, plus a sparse
* `formViews.create` override — with one dangling field planted in each.
*/
const appShape: AnyRec = {
manifest,
objects: [{
name: 'fl_contact',
label: 'Contact',
fields: {
name: { type: 'text', label: 'Name' },
email: { type: 'email', label: 'Email' },
phone: { type: 'phone', label: 'Phone' },
},
}],
views: [{
name: 'fl_contact',
object: 'fl_contact',
list: { label: 'Contacts', type: 'grid', data, columns: [{ field: 'name' }] },
form: {
type: 'simple',
data,
sections: [{ name: 'contact', label: 'Contact', columns: 2, fields: ['name', 'email', 'ghost_default'] }],
},
formViews: {
create: {
type: 'simple',
data,
title: 'New contact',
sections: [{ label: 'Who is this?', columns: 1, fields: ['name', { field: 'ghost_named', colSpan: 2 }] }],
},
},
}],
};

it('the fixture is a shape the strict schema ACCEPTS (so the pin is not testing a rejected stack)', () => {
const { error, value } = quietly(() => cliTierFor(structuredClone(appShape)));
expect(error).toBeUndefined();
// And it really is the container shape: no `sections` at the entry root.
const view = (value!.views as AnyRec[])[0];
expect(Object.keys(view).sort()).toEqual(['form', 'formViews', 'list', 'name', 'object']);
expect(view.sections).toBeUndefined();
});

it('reports every planted defect on that stack — this is the assertion #6251 exists for', () => {
const { value } = quietly(() => cliTierFor(structuredClone(appShape)));
expect(validateFormLayout(value!).map((f) => `${f.rule}@${f.path}`)).toEqual([
`${FORM_FIELD_UNKNOWN}@views[0].form.sections[0].fields[2]`,
`${FORM_FIELD_UNKNOWN}@views[0].formViews.create.sections[0].fields[1]`,
`${FORM_COLSPAN_ABSOLUTE}@views[0].formViews.create.sections[0].fields[1].colSpan`,
]);
});

// EMPTY-GREEN, declared. Revert the container ladder and this test still
// passes — because nothing was read, not because nothing is wrong. It is kept
// (a false-positive guard is worth having) but it is only meaningful PAIRED
// with the test above, which proves on the same fixture family that the
// traversal does read these sites. If that one is ever weakened, this one
// stops guarding anything; do not treat it as independent cover.
it('a CLEAN app stack of the same shape reports nothing — the fix adds no false positives', () => {
const clean = structuredClone(appShape);
const view = (clean.views as AnyRec[])[0];
(view.form as AnyRec).sections = [{ name: 'contact', label: 'Contact', columns: 2, fields: ['name', 'email', 'phone'] }];
(view.formViews as AnyRec).create = {
type: 'simple', data, title: 'New contact',
sections: [{ label: 'Who is this?', columns: 1, fields: ['name', { field: 'email', span: 'full' }] }],
};
const { error, value } = quietly(() => cliTierFor(clean));
expect(error).toBeUndefined();
expect(validateFormLayout(value!)).toEqual([]);
});
});
Loading
Loading