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
14 changes: 14 additions & 0 deletions .changeset/17135-field-consumers-synthesized-layout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"@objectstack/lint": minor
---

`field-no-consumers` now reads two consumers that name the field nowhere in metadata — a declared field group placing it on the synthesized layout, and the column a seed or import mapping matches on (#17135).

The rule's first run on a real application reported 12 fields, and all 12 were on screen or load-bearing that day. Both misses are now read off the spec rather than off a hand-kept list, the way the rule's other two exemptions already are:

- **The synthesized layout.** `deriveFieldGroupLayout` (ADR-0085 §5) is the one derivation every renderer applies — form, detail, drawer and designer — and it places a field by its `group` membership, not by naming it in a `fields: [...]` array. A field the derivation puts in a **declared** group is therefore drawn, and is credited as a display site. The derivation's trailing untitled bucket is deliberately **not** credited: it collects everything the author did not place, so crediting it would hand the display verdict to every visible field in every app.
- **An upsert identity.** A carrier root holds values that are written and labels that are carried, and the root decided the bucket before anything else could ask. But a seed's `externalId` and an import mapping's `upsertKey` name the column the loader **matches on** — it reads that column on every row to decide insert from update. A seeder-only identity column is consumed by being an identity.

⛔ Nothing exempts `hidden` as a category. A `hidden` field no upsert matches on and nothing reads is still reported, and a `hidden` field in a declared group earns nothing from the layout, because the derivation never draws one.

Measured on `hotcrm@965933b` (the tree the 12 were reported on): **12 findings → 0**, with the synthesized layout accounting for 11 and the upsert identity for 2 (they overlap on one field). Against the same application with six deliberately unconsumed fields injected — ungrouped, undeclared-group, hidden-in-a-group, hidden + readonly, a field on an object declaring no groups, and the matched pair of a seeded identity against an identical declaration nothing matches on — all six are still reported and only the identity goes quiet.
116 changes: 116 additions & 0 deletions packages/lint/src/validate-field-consumers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,122 @@ describe('validateFieldConsumers (#15922)', () => {
});
});

describe('the synthesized layout is a display consumer (#17135)', () => {
/** An object whose only consumer root is an empty view — nothing NAMES a field. */
const grouped = (fields: AnyRec, fieldGroups: unknown): AnyRec => ({
objects: [{ name: 'o', fieldGroups, fields }],
views: [{ list: { data: { object: 'o' }, columns: [] } }],
});
const reported = (stack: AnyRec): string[] => validateFieldConsumers(stack).map((f) => f.field);

it('a field a declared group places on the layout is drawn, so it is not reported', () => {
expect(
validateFieldConsumers(
grouped(
{ name: { type: 'text' }, street: { type: 'text', group: 'address' } },
[{ key: 'address', label: 'Address' }],
),
),
).toEqual([]);
});

it('only a DECLARED group places anything — an undeclared key is not a placement', () => {
expect(
reported(
grouped(
{ name: { type: 'text' }, street: { type: 'text', group: 'no_such_group' } },
[{ key: 'address', label: 'Address' }],
),
),
).toEqual(['street']);
});

it('never the trailing flat bucket: an ungrouped field beside a grouped one is still judged', () => {
expect(
reported(
grouped(
{ name: { type: 'text' }, street: { type: 'text', group: 'address' }, loose: { type: 'text' } },
[{ key: 'address', label: 'Address' }],
),
),
).toEqual(['loose']);
});

it('an object declaring no field groups keeps every verdict it had', () => {
expect(reported(grouped({ name: { type: 'text' }, loose: { type: 'text' } }, undefined))).toEqual(['loose']);
expect(reported(grouped({ name: { type: 'text' }, loose: { type: 'text' } }, []))).toEqual(['loose']);
});

it('a hidden field earns nothing here — the derivation never draws one', () => {
expect(
reported(
grouped(
{
name: { type: 'text' },
street: { type: 'text', group: 'address' },
secret: { type: 'text', group: 'address', hidden: true },
},
[{ key: 'address', label: 'Address' }],
),
),
).toEqual(['secret']);
});

it('reaches the array-shaped field map too', () => {
expect(
validateFieldConsumers({
objects: [
{
name: 'o',
fieldGroups: [{ key: 'address', label: 'Address' }],
fields: [{ name: 'name', type: 'text' }, { name: 'street', type: 'text', group: 'address' }],
},
],
views: [{ list: { data: { object: 'o' }, columns: [] } }],
}),
).toEqual([]);
});
});

describe('an upsert identity inside a carrier root is a read (#17135)', () => {
/** `x` is declared, `hidden` and `readonly` — the seeder-only identity shape. */
const seeded = (extra: AnyRec): AnyRec => ({
objects: [{ name: 'o', fields: { name: { type: 'text' }, x: { type: 'text', hidden: true, readonly: true } } }],
views: [{ list: { data: { object: 'o' }, columns: [] } }],
...extra,
});
const reported = (stack: AnyRec): string[] => validateFieldConsumers(stack).map((f) => f.field);

it("a seed's externalId names the column the loader matches on", () => {
expect(reported(seeded({ data: [{ object: 'o', mode: 'upsert', externalId: 'x', records: [{ x: 'k1' }] }] }))).toEqual([]);
});

it('a composite externalId credits every member', () => {
expect(
validateFieldConsumers({
objects: [{ name: 'o', fields: { name: { type: 'text' }, a: { type: 'text' }, b: { type: 'text' } } }],
views: [{ list: { data: { object: 'o' }, columns: [] } }],
data: [{ object: 'o', mode: 'upsert', externalId: ['a', 'b'], records: [{ a: '1', b: '2' }] }],
}),
).toEqual([]);
});

it("an import mapping's upsertKey is the same read", () => {
expect(
reported(seeded({ mappings: [{ name: 'm', targetObject: 'o', mode: 'upsert', upsertKey: ['x'] }] })),
).toEqual([]);
});

it('⛔ hidden is not exempt — the matched pair differs only in the identity role', () => {
// Same declaration, same object, no upsert matching on it.
expect(reported(seeded({ data: [{ object: 'o', records: [{ x: 'k1' }] }] }))).toEqual(['x']);
// A seeded VALUE stays a carrier even when an upsert matches on ANOTHER column.
expect(
reported(seeded({ data: [{ object: 'o', mode: 'upsert', externalId: 'name', records: [{ x: 'k1' }] }] })),
).toEqual(['x']);
});
});

describe('registry wiring', () => {
const entry = AUTHORING_RULES.find((r) => r.name === 'validateFieldConsumers');

Expand Down
117 changes: 109 additions & 8 deletions packages/lint/src/validate-field-consumers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,34 @@
* must clean up; none of them is evidence that anything reads the field.
* A seeded value nothing reads is precisely the shape being hunted.
*
* ## Two consumers that name the field nowhere
*
* A metadata reference is not the only way a field is read, and the first real
* app to take this rule reported 12 fields that were all on screen or
* load-bearing. Both paths are read off the SPEC, not off a hand-kept list:
*
* - **The synthesized layout** ({@link deriveFieldGroupLayout}, ADR-0085 §5).
* An object's `fieldGroups` plus a field's `group` membership are what the
* form, detail, drawer and designer surfaces all render when no `*.page.ts`
* names the field. The author DID place the field; the placement is spelled
* as a group key, so no `fields: [...]` array anywhere mentions it. The
* derivation is the platform's own, so this rule credits exactly what a
* renderer draws — including the hidden-field exclusion, which is why a
* `hidden` field earns nothing here. Only a KEYED section counts: the
* derivation's trailing untitled bucket is the flat fallback that collects
* everything the author did NOT place, so crediting it would credit every
* visible field on every object and leave the rule judging nothing.
* - **A seed's or an import mapping's upsert identity**
* ({@link CARRIER_IDENTITY_SEGMENTS}). A carrier root is where written and
* carried values live, but `externalId` / `upsertKey` names the column the
* loader MATCHES ON — it reads every row's value to decide insert from
* update (`SeedSchema.externalId`: "Field (or composite list of fields)
* matched for the uniqueness check"). A seeder-only identity column is
* therefore consumed BY BEING an identity: it is `hidden` and `readonly`
* precisely so no real row can acquire one, and a consumer anywhere else
* would defeat it. Nothing here exempts `hidden` as such — a `hidden` field
* that no upsert matches on and nothing reads is still reported.
*
* A field with at least one behaviour OR display site is consumed and gets no
* finding — a field that is only drawn is the ordinary state of most fields
* (`phone` on a contact), not a defect; HotCRM's ledger listed `display-only`
Expand Down Expand Up @@ -91,7 +119,7 @@
* maintainer's call, not this rule's.
*/

import { resolveDisplayField } from '@objectstack/spec/data';
import { deriveFieldGroupLayout, resolveDisplayField } from '@objectstack/spec/data';
import type { DisplayNameObjectMeta } from '@objectstack/spec/data';
import { collectionEntries } from './collection-entries.js';
import { recordsOf } from './object-graph.js';
Expand Down Expand Up @@ -175,6 +203,23 @@ export const CONSUMER_ROOTS: readonly string[] = [
*/
export const CARRIER_ROOTS: readonly string[] = ['translations', 'data', 'mappings', 'permissions'];

/**
* The one thing inside a carrier root that is a READ: the column an upsert
* MATCHES ON. `externalId` is the canonical spelling on a seed
* (`SeedSchema.externalId`, "Field (or composite list of fields) matched for
* the uniqueness check") and `upsertKey` the canonical spelling on an import
* mapping (`MappingSchema`, which aliases `externalId`/`matchOn`/`key` onto
* it). Both are already in {@link BEHAVIOUR_SEGMENTS}; a carrier root just
* never got to ask, because the root decided the bucket first.
*
* A seeded VALUE is still a carrier — the loader writes it and nothing reads
* it back. The identity is the opposite: every replay reads the column on
* every row to decide insert from update. That is what makes a seeder-only
* identity column consumed while being invisible: it has exactly one reader,
* and the reader is the loader.
*/
const CARRIER_IDENTITY_SEGMENTS: ReadonlySet<string> = new Set(['externalId', 'upsertKey']);

/** Roots whose sites are display by default; `BEHAVIOUR_SEGMENTS` earn behaviour back. */
const DISPLAY_ROOTS: ReadonlySet<string> = new Set(['views', 'pages', 'apps']);

Expand Down Expand Up @@ -345,7 +390,12 @@ class ConsumerLedger {
}

function bucketFor(root: string, segments: readonly string[], leafKey: string): SiteKind {
if (CARRIER_ROOTS.includes(root)) return 'carrier';
// An upsert identity inside a carrier root is the one read there: the loader
// matches rows on that column. Everything else a carrier root holds is a
// value it writes or a label it carries.
if (CARRIER_ROOTS.includes(root)) {
return segments.some((s) => CARRIER_IDENTITY_SEGMENTS.has(s)) ? 'behaviour' : 'carrier';
}
if (PROSE_KEYS.has(leafKey)) return 'carrier';
if (segments.some((s) => BEHAVIOUR_SEGMENTS.has(s))) return 'behaviour';
if (DISPLAY_ROOTS.has(root)) return 'display';
Expand Down Expand Up @@ -511,14 +561,61 @@ function walkObject(ledger: ConsumerLedger, obj: AnyRec, objectName: string, obj
}
}

/** Build the display-name meta the spec's ladder reads, whatever shape `fields` was authored in. */
function displayMetaOf(obj: AnyRec, fields: { rec: AnyRec; path: string }[]): DisplayNameObjectMeta {
/** The field map the spec helpers read, whatever shape `fields` was authored in. */
function fieldMapOf(fields: readonly { rec: AnyRec; path: string }[]): Record<string, AnyRec> {
const map: Record<string, AnyRec> = {};
for (const { rec } of fields) {
const n = strName(rec.name);
if (n) map[n] = rec;
}
return { nameField: strName(obj.nameField), displayNameField: strName(obj.displayNameField), fields: map };
return map;
}

/** Build the display-name meta the spec's ladder reads, whatever shape `fields` was authored in. */
function displayMetaOf(obj: AnyRec, fields: { rec: AnyRec; path: string }[]): DisplayNameObjectMeta {
return {
nameField: strName(obj.nameField),
displayNameField: strName(obj.displayNameField),
fields: fieldMapOf(fields),
};
}

/**
* Credit every field the SYNTHESIZED layout places in a declared group.
*
* `deriveFieldGroupLayout` is the platform's own derivation (ADR-0085 §5) — the
* one implementation every renderer applies — so what it returns is what a form
* or detail surface draws when no authored page names the field. Running it
* here rather than re-reading `fieldGroups` by hand is the same discipline the
* other two exemptions follow: the verdict moves when the renderer moves.
*
* Only a KEYED section is a site. The derivation's trailing untitled bucket
* collects what the author did NOT place — every visible field that named no
* group, plus, on an object declaring no groups at all, every field there is.
* Crediting it would hand the display verdict to every visible field in every
* app and leave this rule able to report `hidden` fields only.
*/
function creditFieldGroupLayout(
ledger: ConsumerLedger,
obj: AnyRec,
objectName: string,
fields: readonly { rec: AnyRec; path: string }[],
): void {
const sections = deriveFieldGroupLayout({ fieldGroups: obj.fieldGroups, fields: fieldMapOf(fields) });
if (sections === null) return;
const pathOf = new Map<string, string>();
for (const { rec, path } of fields) {
const n = strName(rec.name);
if (n !== undefined) pathOf.set(n, path);
}
for (const section of sections) {
if (section.key === undefined) continue;
for (const field of section.fields) {
const path = pathOf.get(field);
if (path === undefined) continue;
ledger.record(objectName, field, { root: 'objects', path: `${path}.group`, kind: 'display' });
}
}
}

function listPaths(paths: readonly string[]): string {
Expand Down Expand Up @@ -547,6 +644,7 @@ export function validateFieldConsumers(stack: AnyRec): FieldConsumerFinding[] {
const fields = collectionEntries(obj.fields, `${objPath}.fields`);
const injected = injectedColumnsFor(obj);
const titleField = resolveDisplayField(displayMetaOf(obj, fields));
creditFieldGroupLayout(ledger, obj, objectName, fields);
for (const { rec: field, path: fieldPath } of fields) {
const fieldName = strName(field.name);
if (!fieldName) continue;
Expand Down Expand Up @@ -603,11 +701,14 @@ export function validateFieldConsumers(stack: AnyRec): FieldConsumerFinding[] {
message:
`field "${field}" on object "${object}" is declared but nothing in this stack reads or displays ` +
`it: no view column, form section, page binding, flow node, dataset, widget, formula, validation, ` +
`hook or action names it. A translation label, a seed value, an import mapping, a permission grant ` +
`or a flow that only WRITES it is a carrier, not a consumer. ${verdictClause}${sharedClause}`,
`hook or action names it, no declared field group places it on the synthesized layout, and no ` +
`seed or import mapping matches on it. A translation label, a seed value, an import-mapping ` +
`target, a permission grant or a flow that only WRITES it is a carrier, not a consumer. ` +
`${verdictClause}${sharedClause}`,
hint:
`Give "${field}" a consumer — a view column, a form section, a page binding, a formula, a ` +
`validation, a flow node, a dataset dimension — or remove the declaration` +
`validation, a flow node, a dataset dimension, or a \`group\` naming one of this object's ` +
`declared \`fieldGroups\` so the synthesized layout draws it — or remove the declaration` +
(carriers.length > 0 ? ` together with its ${carriers.length} carrier site(s) listed above` : '') +
`. Ignore this if the field is read only by an API client, by a hook or package this stack does not ` +
`carry, or by a Studio-authored view. Roots scanned: ${CONSUMER_ROOTS.join(', ')} (consumers) · ` +
Expand Down
Loading