From 42e16efa5f071c74f265dc417e92894d83578042 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:31:46 +0000 Subject: [PATCH 1/2] fix(lint): read an object's fields through the guarded reader in buildFieldIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildFieldIndex` in `validate-expressions.ts` cast every member of an object's `fields:` list inline (`fields.map(f => (f as AnyRec).name)`). `Array.isArray` proves the LIST, not its MEMBERS: an empty YAML list item deserialises to `null`, and the dereference threw out of the whole rule before the `.filter` two calls later could drop it. The list is now read through `recordsOf` — the single home of that coercion — which drops a non-record array member whole and in silence, the same disposition the two sibling readers in this file (`buildFieldTypeIndex`, `fieldEntries`) already had. The map shape keeps `Object.keys`: there the author's key IS the field name. The sweep's `RESIDUAL_THROWS` rows for `objects[].fields` come out in the same change — it is exact in both directions, so it now asserts the throw is gone. `RESIDUAL_INVENTED` is unchanged, measured: the repaired reader raises no finding about the dropped member. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk --- ...date-expressions-non-record-field-entry.md | 11 ++++++ .../lint/src/non-record-object-entry.test.ts | 28 ++++++------- .../lint/src/validate-expressions.test.ts | 39 +++++++++++++++++++ packages/lint/src/validate-expressions.ts | 12 +++++- 4 files changed, 75 insertions(+), 15 deletions(-) create mode 100644 .changeset/lint-validate-expressions-non-record-field-entry.md diff --git a/.changeset/lint-validate-expressions-non-record-field-entry.md b/.changeset/lint-validate-expressions-non-record-field-entry.md new file mode 100644 index 0000000000..dae0004f6d --- /dev/null +++ b/.changeset/lint-validate-expressions-non-record-field-entry.md @@ -0,0 +1,11 @@ +--- +"@objectstack/lint": patch +--- + +`validateStackExpressions` no longer throws on a non-record entry in an object's `fields:` list. + +An empty item in a YAML `fields:` list deserialises to `null`, and `buildFieldIndex` cast each member of the list inline (`fields.map(f => (f as AnyRec).name)`) before the `.filter` two calls later could drop it. `Array.isArray` proves the LIST, never its MEMBERS, so linting such a stack failed with `TypeError: Cannot read properties of null (reading 'name')` out of the whole rule instead of reporting anything about the file. + +The list is now read through `recordsOf` — the one place that coercion is decided — which drops a non-record member of the array shape whole and in **silence**: it carries no author-written name, so there is nothing to report about it. That matches what the two sibling field readers in the same module (`buildFieldTypeIndex`, `fieldEntries`) already did with the same member, so the three readers now agree. The readable siblings of the junk member are still indexed, so unknown-field findings on that object continue to be reported. + +The map shape (`fields: { amount: { … } }`) is unchanged: there the author's key is the field name, which is what this index needs. diff --git a/packages/lint/src/non-record-object-entry.test.ts b/packages/lint/src/non-record-object-entry.test.ts index e57e02a35a..b9f0fdbf31 100644 --- a/packages/lint/src/non-record-object-entry.test.ts +++ b/packages/lint/src/non-record-object-entry.test.ts @@ -340,22 +340,22 @@ const SWEPT_COLLECTIONS: readonly SweptCollection[] = [ * "nothing throws" would have had to be deleted or weakened on the day it was * written, and would then never have caught the next one. * - * Every entry names a reader OUTSIDE what #15636 could touch: + * It is EMPTY today, and that is a measurement, not an aspiration: no rule in + * the table throws on a non-record member of any collection swept here. Two + * rows have come out since it was written, each because the sweep went red + * demanding a throw that no longer happens — which is the both-directions half + * earning its keep, since neither removal started with anyone going looking: * - * - `objects[].fields` — `buildFieldIndex` in `validate-expressions.ts:137`, - * which casts inline instead of through a helper, so the `asArray` sweeps - * that produced #15552 and #15636 never saw it. Filed as #15742. - * - * `stack.datasets` was here too, for `indexDatasets` in - * `validate-chart-bindings.ts`. #15741 re-pointed that reader and these - * assertions went red demanding a throw that no longer happens, which is the - * both-directions half earning its keep: the rows came out because the sweep - * failed, not because anyone went looking for them. + * - `stack.datasets` — `indexDatasets` in `validate-chart-bindings.ts`, + * re-pointed by #15741. + * - `objects[].fields` — `buildFieldIndex` in `validate-expressions.ts`, which + * cast each member inline instead of reading through a helper, so the + * `asArray` greps that produced #15552 and #15636 never saw it. It now reads + * the list through `recordsOf` (#15742), which drops a non-record member of + * the array shape whole and in silence, exactly as the file's two sibling + * field readers already did. */ -const RESIDUAL_THROWS: Readonly> = { - 'objects[].fields · null': ['validateStackExpressions'], - 'objects[].fields · undefined': ['validateStackExpressions'], -}; +const RESIDUAL_THROWS: Readonly> = {}; /** * Where a junk member still draws a finding no author's file justifies — the diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 8a3544333d..e069fac0eb 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3793,4 +3793,43 @@ describe('assignment value envelope — located findings (#15137)', () => { }); expect(issues.filter((i) => i.where.includes('assignment value'))).toHaveLength(0); }); + + describe("a non-record entry in an object's `fields:` list (#15742)", () => { + // `buildFieldIndex` used to cast each member inline + // (`fields.map(f => (f as AnyRec).name)`), so an empty YAML list item — + // which deserialises to `null` — threw `Cannot read properties of null` + // out of the whole rule. It reads the list through `recordsOf` now, which + // is the one place that decision is made: an array member that is not a + // record carries no author-written name, so it is dropped WHOLE and in + // silence, the same disposition the file's two sibling field readers + // (`buildFieldTypeIndex`, `fieldEntries`) already had. The sweep in + // `non-record-object-entry.test.ts` pins the absence of the throw across + // every rule; these two arms pin what this rule does INSTEAD, which a + // crash-only sweep cannot say. + const stackWith = (fields: unknown[], condition: string): Record => ({ + objects: [{ name: 'crm_account', fields }], + flows: [{ + name: 'account_flow', + nodes: [ + { id: 'start', type: 'start', config: { objectName: 'crm_account' } }, + { id: 'check', type: 'decision', config: { condition } }, + ], + edges: [], + }], + }); + + it('is dropped in silence rather than thrown on, and invents no finding', () => { + expect(validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); + expect(validateStackExpressions(stackWith([undefined, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); + }); + + it('still indexes the readable siblings — the junk member does not blank the index', () => { + // The failure mode a bare `try/catch` repair would have produced: no + // crash, and no field knowledge either, so every unknown-field finding + // on the object silently stops being reported. + const issues = validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amont > 0')); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain('amount'); + }); + }); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index e26427a804..a4cfe89a04 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -134,7 +134,17 @@ function buildFieldIndex(objects: AnyRec[]): Map { if (!name) continue; const fields = obj.fields; let names: string[] = []; - if (Array.isArray(fields)) names = fields.map(f => (f as AnyRec).name).filter((n): n is string => typeof n === 'string'); + // The LIST shape is read through `recordsOf` (#15742). `Array.isArray` + // proves the list, never its members: an empty item in a YAML `fields:` + // list deserialises to `null`, and the cast this replaced dereferenced it + // before the `.filter` two calls later could drop it. The two sibling + // readers below already guard (`buildFieldTypeIndex` reads `(f)?.name`, + // `fieldEntries` filters before mapping) and both drop such a member in + // SILENCE — it carries no author-written name, so there is nothing to + // report about it — which is what `recordsOf` does for the array shape too. + // The MAP shape keeps `Object.keys`: there the author's KEY is the field + // name, which is exactly what this "did you mean?" index needs. + if (Array.isArray(fields)) names = recordsOf(fields).map(f => f.name).filter((n): n is string => typeof n === 'string'); else if (fields && typeof fields === 'object') names = Object.keys(fields as AnyRec); // Injected columns come second, de-duplicated by insertion order: a DECLARED // `owner_id` is the author's field (the registry lets it win), so the From 35dbe54620a6271c9ad7f6ab8c352858397b8bdc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:40:32 +0000 Subject: [PATCH 2/2] test(lint): keep the #15742 arms in their own top-level describe They were appended inside the `#15137` assignment-value describe, which reads as a claim about that suite rather than about `buildFieldIndex`. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk --- .../lint/src/validate-expressions.test.ts | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index e069fac0eb..e18f5a4e6b 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3793,43 +3793,43 @@ describe('assignment value envelope — located findings (#15137)', () => { }); expect(issues.filter((i) => i.where.includes('assignment value'))).toHaveLength(0); }); +}); - describe("a non-record entry in an object's `fields:` list (#15742)", () => { - // `buildFieldIndex` used to cast each member inline - // (`fields.map(f => (f as AnyRec).name)`), so an empty YAML list item — - // which deserialises to `null` — threw `Cannot read properties of null` - // out of the whole rule. It reads the list through `recordsOf` now, which - // is the one place that decision is made: an array member that is not a - // record carries no author-written name, so it is dropped WHOLE and in - // silence, the same disposition the file's two sibling field readers - // (`buildFieldTypeIndex`, `fieldEntries`) already had. The sweep in - // `non-record-object-entry.test.ts` pins the absence of the throw across - // every rule; these two arms pin what this rule does INSTEAD, which a - // crash-only sweep cannot say. - const stackWith = (fields: unknown[], condition: string): Record => ({ - objects: [{ name: 'crm_account', fields }], - flows: [{ - name: 'account_flow', - nodes: [ - { id: 'start', type: 'start', config: { objectName: 'crm_account' } }, - { id: 'check', type: 'decision', config: { condition } }, - ], - edges: [], - }], - }); +describe("validateStackExpressions — a non-record entry in an object's `fields:` list (#15742)", () => { + // `buildFieldIndex` used to cast each member inline + // (`fields.map(f => (f as AnyRec).name)`), so an empty YAML list item — + // which deserialises to `null` — threw `Cannot read properties of null` + // out of the whole rule. It reads the list through `recordsOf` now, which + // is the one place that decision is made: an array member that is not a + // record carries no author-written name, so it is dropped WHOLE and in + // silence, the same disposition the file's two sibling field readers + // (`buildFieldTypeIndex`, `fieldEntries`) already had. The sweep in + // `non-record-object-entry.test.ts` pins the absence of the throw across + // every rule; these two arms pin what this rule does INSTEAD, which a + // crash-only sweep cannot say. + const stackWith = (fields: unknown[], condition: string): Record => ({ + objects: [{ name: 'crm_account', fields }], + flows: [{ + name: 'account_flow', + nodes: [ + { id: 'start', type: 'start', config: { objectName: 'crm_account' } }, + { id: 'check', type: 'decision', config: { condition } }, + ], + edges: [], + }], + }); - it('is dropped in silence rather than thrown on, and invents no finding', () => { - expect(validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); - expect(validateStackExpressions(stackWith([undefined, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); - }); + it('is dropped in silence rather than thrown on, and invents no finding', () => { + expect(validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); + expect(validateStackExpressions(stackWith([undefined, { name: 'amount', type: 'number' }], 'record.amount > 0'))).toHaveLength(0); + }); - it('still indexes the readable siblings — the junk member does not blank the index', () => { - // The failure mode a bare `try/catch` repair would have produced: no - // crash, and no field knowledge either, so every unknown-field finding - // on the object silently stops being reported. - const issues = validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amont > 0')); - expect(issues).toHaveLength(1); - expect(issues[0].message).toContain('amount'); - }); + it('still indexes the readable siblings — the junk member does not blank the index', () => { + // The failure mode a bare `try/catch` repair would have produced: no + // crash, and no field knowledge either, so every unknown-field finding + // on the object silently stops being reported. + const issues = validateStackExpressions(stackWith([null, { name: 'amount', type: 'number' }], 'record.amont > 0')); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain('amount'); }); });