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
41 changes: 41 additions & 0 deletions .changeset/lint-data-model-refof-canonical-reference.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
---
"@objectstack/lint": patch
---

fix(lint): `lintDataModel` reads only the canonical `reference` target (#13250)

`refOf` in `packages/lint/src/data-model-rules.ts` resolved
`def?.reference || def?.reference_to`, so a relationship field spelled with the
rejected alias resolved a target. #11567 settled that `reference` is the only
relationship spelling `@objectstack/spec` declares — `FieldSchema` answers
`reference_to` with `unrecognized_keys` and *"Did you mean `reference_to` →
`reference`?"* — and put it as "one key, one answer, on both doors".

`@objectstack/lint` runs over an in-memory, schema-parsed stack, so the alias
cannot legitimately appear here at all: the tolerance was inert. Where it did
fire, it made the rule whose entire job is to catch a relationship with no
target — `relationship/missing-reference` — report a valid target for a field
that has none, i.e. the one component that exists to tell an author their
metadata is wrong was the component accepting the wrong spelling.

This mirrors the deliberate canonical-only narrowing already recorded in-file
for `refOf` in `packages/lint/src/validate-security-posture.ts`, including its
`typeof r === 'string'` guard — which also makes the declared
`string | undefined` return type true, where the old `||` chain returned
whatever truthy value it found (a non-string `reference` was reported as a
resolved target).

What changes for a consumer, only for metadata the spec already refuses:
`relationship/missing-reference` (error) now fires on a relationship field
whose only target spelling is `reference_to`, and the rules that need a
resolved target (`relationship/master-detail-required`, `rollup/missing-summary`
and the rest of the relationship family) no longer treat such a field as
pointing anywhere. Canonical `reference` is untouched.

Scope note: the two remaining tolerant readers named in #13250 —
`packages/verify/src/derive.ts` and
`packages/plugins/plugin-security/src/security-plugin.ts` — are deliberately
NOT narrowed here. Both were measured to sit on populations the alias can
actually reach (raw `registerObject`, which skips Zod by design, and an app
config that never passes through a `define*` parse), so narrowing them is a
triage call rather than a defect fix.
107 changes: 107 additions & 0 deletions packages/cli/test/data-model-rules.test.ts
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest';
import { lintDataModel, lintUniqueDeclarations, lintUnscopedDeclaredIndexes, lintLegacyOrganizationComposites } from '@objectstack/lint';
import { FieldSchema } from '@objectstack/spec/data';
import { lintConfig } from '../src/commands/lint';

const rulesOf = (issues: { rule: string }[]) => issues.map((i) => i.rule);
Expand DownExpand Up@@ -696,3 +697,109 @@ describe('lintLegacyOrganizationComposites — S6 respelling nudge (ADR-0120 D5c
expect(has(lintDataModel(objs), 'unique/unscoped-declared-index')).toBe(true);
});
});

/**
* [#13250] `reference_to` is a REJECTED alias of `reference` — these rules must
* not read it as a target.
*
* #11567 settled the ruling ("one key, one answer, on both doors") and
* `packages/lint/src/validate-security-posture.ts` records the same narrowing
* for its own `refOf`, on purpose. `lintDataModel` runs over a schema-parsed
* stack, so the alias cannot legitimately appear here at all; tolerating it
* made `relationship/missing-reference` — the rule whose whole job is to catch
* a relationship with no target — report a valid target for a field that has
* none.
*
* ⚠️ Every assertion below is paired with a POSITIVE CONTROL on the canonical
* spelling. A `refOf` that resolved NOTHING would satisfy the alias half on its
* own, so the canonical half is what makes these measurements rather than
* vacuous passes.
*/
describe('lintDataModel — `reference_to` is a rejected alias, not a tolerated one (#13250)', () => {
it('upstream control: FieldSchema declares `reference` and refuses `reference_to`', () => {
const fieldKeys = Object.keys(FieldSchema.shape);
expect(fieldKeys).toContain('reference');
expect(fieldKeys).not.toContain('reference_to');

const rejected = FieldSchema.safeParse({ name: 'project', type: 'lookup', reference_to: 'project' });
expect(rejected.success).toBe(false);
expect(rejected.error?.issues.map((i) => i.code)).toContain('unrecognized_keys');

// POSITIVE CONTROL — the canonical spelling parses, so the refusal above is
// about the KEY and not about the fixture being malformed some other way.
expect(FieldSchema.safeParse({ name: 'project', type: 'lookup', reference: 'project' }).success).toBe(true);
});

it('R1 fires: a relationship whose only target spelling is `reference_to` has no target', () => {
const issue = lintDataModel([
{ name: 'task', fields: { project: { type: 'lookup', reference_to: 'project' } } },
]).find((i) => i.rule === 'relationship/missing-reference');
expect(issue?.severity).toBe('error');
expect(issue?.message).toContain('is missing a reference target');
expect(issue?.path).toBe('objects[0].fields.project.reference');
});

it('POSITIVE CONTROL — canonical `reference` still resolves, so R1 stays silent', () => {
expect(
has(
lintDataModel([
{ name: 'project', fields: { name: { type: 'text' } } },
{ name: 'task', fields: { project: { type: 'lookup', reference: 'project' } } },
]),
'relationship/missing-reference',
),
).toBe(false);
});

it('the resolved target NAME is read from `reference` (not merely truthiness)', () => {
// `relationship/master-detail-required` renders the target it resolved, so
// this pins that `refOf` returns the VALUE — a guard that returned `true`
// would pass the R1 tests above and fail here.
const canonical = lintDataModel([
{ name: 'invoice_line', fields: { invoice: { type: 'master_detail', reference: 'invoice' } } },
]).find((i) => i.rule === 'relationship/master-detail-required');
expect(canonical?.message).toContain('→ invoice');

// The alias resolves nothing, so the rule that needs a target never runs —
// R1 is what the author hears instead, and the schema names the bad key.
expect(
has(
lintDataModel([
{ name: 'invoice_line', fields: { invoice: { type: 'master_detail', reference_to: 'invoice' } } },
]),
'relationship/master-detail-required',
),
).toBe(false);
});

it('a non-string `reference` is not a target either', () => {
// `refOf` is declared `string | undefined`; the old `||` chain returned
// whatever truthy value was there, so this shape used to be reported as a
// resolved target named `[object Object]`.
//
// ⚠️ The non-string value is BOUND THROUGH A VARIABLE rather than written
// inline, and the binding is a legibility device — not a way past a gate.
// `packages/lint/scripts/check-reference-carrier-shape.mjs` refuses a
// non-string LITERAL at a `reference` carrier position, and it is right to:
// an AUTHORED site of that shape is invisible in both directions at once —
// refused by `ObjectSchema.safeParse` and read as `undefined` by every rule
// that resolves it (#13053), so it reports nothing either way.
//
// That shape is exactly what this case must keep driving, because the whole
// assertion is that such a value resolves to NOTHING: it is a deliberate
// counter-example, not an authored carrier. The gate judges literals and
// leaves a non-literal unjudged, so the binding keeps its authored-site
// sweep honest while the assertion goes on testing the identical shape.
// ⛔ Do not inline this value again; ⛔ do not weaken the gate or add a path
// ignore there — it has no baseline and wants none, by design.
const nonStringReference = { object: 'project' };
expect(
has(
lintDataModel([
{ name: 'task', fields: { project: { type: 'lookup', reference: nonStringReference } } },
]),
'relationship/missing-reference',
),
).toBe(true);
});
});
24 changes: 23 additions & 1 deletion packages/lint/src/data-model-rules.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -163,8 +163,30 @@ function fieldEntries(fields: any): FieldEntry[] {
return Object.entries<any>(fields).map(([name, def]) => ({ name, def }));
}

/**
* The `reference` target a relationship field points at.
*
* `reference` is the only spelling `FieldSchema` declares; `reference_to` (like
* `referenceTo` / `relatedTo` / `target`) is a rejected alias the strict error
* map renames for the author, so a field carrying it does not parse (#5017,
* #11567 — "one key, one answer, on both doors").
*
* The DELIBERATE narrowing here mirrors `refOf` in
* `packages/lint/src/validate-security-posture.ts`, which was narrowed to
* canonical-only on purpose and records the reasoning. It applies with more
* force in THIS file, because these rules exist to tell an author their
* metadata is wrong: `lintDataModel` runs over a schema-parsed stack
* (`@objectstack/lint` is "pure `(stack) => Finding[]` … an in-memory,
* schema-parsed stack object"), where a rejected alias cannot appear — and on
* any pre-parse path the schema already names the real defect (the alias key)
* rather than R1 guessing past it. Tolerating the alias made
* `relationship/missing-reference` report a valid target for a field that has
* none: the one component whose job is to catch the misspelling was the one
* accepting it.
*/
function refOf(def: any): string | undefined {
return def?.reference || def?.reference_to;
const r = def?.reference as unknown;
return typeof r === 'string' && r ? r : undefined;
}

// ─── Uniqueness declarations (ADR-0120) ─────────────────────────────
Expand Down
Loading