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
47 changes: 47 additions & 0 deletions .changeset/index-rule-where-slot-names-the-object.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/lint": patch
---

fix(lint): the three ADR-0120 uniqueness rules name the object in the `where` slot instead of repeating the config path (#9600)

`AuthoringFinding` declares two location slots with different jobs — `where`
("human-readable location", e.g. `object "leave_request"`) and `path` ("config
path", e.g. `objects[3].sharingModel`). Three registry adapters set the first
from the second (`where: f.path`), so every CLI command printed the same
positional string twice and the only human-readable slot said nothing the `at`
clause did not already say:

```
• objects[44].indexes[1]: "sys_account" declares index [provider_id, account_id] with bare `unique: true` …
rule: unique/unscoped-declared-index at objects[44].indexes[1]
```

That index is a position in the MERGED object array, which appears in no file
the author wrote. `unique/unscoped-declared-index`, `unique/double-declaration`
and `unique/legacy-organization-composite` now spell it the way the rest of the
table does:

```
• object "sys_account" · index [provider_id, account_id]: "sys_account" declares index …
rule: unique/unscoped-declared-index at objects[44].indexes[1]
```

An index is identified by its `name` when it has one, and otherwise by the
columns the author actually wrote (`· index [provider_id, account_id]`) — both
searchable in their source, which a bare ordinal is not.

`where` is stated by the rule functions themselves rather than reconstructed in
the adapter, because only the rule still holds the object it walked. Their
return type is now `LocatedLintIssue` (a `LintIssue` with a REQUIRED `where`),
newly exported, so a fourth rule joining this family cannot reach the adapter
without one — a `f.where ?? f.path` fallback at the adapter would have let the
positional spelling ship again silently.

Display text only, and the rules' population is unchanged: measured over the 45
object declarations `@objectstack/platform-objects` and
`@objectstack/metadata-core` ship, the registry produced 1050 findings from the
same 5 rules before and after, with the count of findings whose `where` was a
bare config path going 72 to 0. `path` is deliberately untouched and stays
positional — it is the slot that is supposed to be a config path, and the
runtime gate's `fingerprint` reads `where` and `path` together, so making
`where` more specific cannot merge two findings that were distinct.
6 changes: 3 additions & 3 deletions packages/lint/src/authoring-rules.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1121,7 +1121,7 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
lintUnscopedDeclaredIndexes(Array.isArray(stack.objects) ? (stack.objects as unknown[]) : []).map((f) => ({
severity: f.severity === 'suggestion' ? ('info' as const) : f.severity,
rule: f.rule,
where: f.path,
where: f.where,
path: f.path,
message: f.message,
hint: f.fix ?? '',
Expand All@@ -1146,7 +1146,7 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
lintUniqueDeclarations(Array.isArray(stack.objects) ? (stack.objects as unknown[]) : []).map((f) => ({
severity: f.severity === 'suggestion' ? ('info' as const) : f.severity,
rule: f.rule,
where: f.path,
where: f.where,
path: f.path,
message: f.message,
hint: f.fix ?? '',
Expand All@@ -1172,7 +1172,7 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
lintLegacyOrganizationComposites(Array.isArray(stack.objects) ? (stack.objects as unknown[]) : []).map((f) => ({
severity: f.severity === 'suggestion' ? ('info' as const) : f.severity,
rule: f.rule,
where: f.path,
where: f.where,
path: f.path,
message: f.message,
hint: f.fix ?? '',
Expand Down
174 changes: 174 additions & 0 deletions packages/lint/src/data-model-rule-where-slot.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
//
// `AuthoringFinding` declares two location slots with different jobs:
//
// where — "Human-readable location, e.g. `object \"leave_request\"`"
// path — "Config path, e.g. `objects[3].sharingModel`"
//
// Every CLI command renders the FIRST as the line's prefix. `os validate`
// prints `• ${where}: ${message}` and then `rule: ${rule} at ${path}`, and
// `os lint` prints `${where}: ${message}` with `path` as the machine slot — so
// the two slots print side by side and a `where` copied from `path` spends the
// human-readable one saying what the `at` clause already said.
//
// Scope, measured rather than assumed: all three of these rules are
// `surfaces: CLI_ONLY`, and `runtime-gate.ts` dispatches only rules whose
// `surfaces` include `runtime-publish`. They therefore do NOT reach
// `SaveMetaItemResponseSchema.advisories` and Studio does not render them
// today — the card that filed this predates the #4716 split, which crossed the
// five GATING object rules and deliberately left the six advisory-tier ones
// (these among them) behind the door. This is a CLI diagnostic defect now, and
// the guard below is what keeps it fixed if that door later opens.
//
// The three ADR-0120 uniqueness rules used to set `where: f.path`, so an author
// read the same positional string twice and the location slot carried nothing:
//
// • objects[44].indexes[1]: "sys_account" declares index [provider_id, …
// rule: unique/unscoped-declared-index at objects[44].indexes[1]
//
// That index is a position in the MERGED object array (44 objects from
// `@objectstack/platform-objects` plus one from `@objectstack/metadata-core` in
// the measurement that filed this), which appears in no file the author wrote.
//
// This file pins BOTH halves, because pinning only the three would leave the
// class open: the second test runs the whole registry and fails on any rule
// that puts a bare config path in `where`. It is a guard on the shape of the
// corpus, not on these three rules — measured when this landed, the registry
// held 41 rules and exactly these 3 carried the shape, with no rule building a
// positional `where` in its own module.
//
// The fix changes the `where` STRING only, never which code a rule matches:
// over the same 45 shipped object declarations the card measured, the registry
// produced 1050 findings from 5 rules both before and after, and the count of
// findings whose `where` was a bare config path went 72 → 0.
import { describe, expect, it } from 'vitest';
import { AUTHORING_RULES, runAuthoringRules, type AuthoringFinding } from './authoring-rules.js';
import {
lintLegacyOrganizationComposites,
lintUniqueDeclarations,
lintUnscopedDeclaredIndexes,
} from './data-model-rules.js';

/**
* Filler so the object under test does NOT sit at `objects[0]`. The defect is
* invisible at index 0 — `objects[0].indexes[0]` reads plausibly enough that a
* fixture rooted there would pass a human review of the old spelling too.
*/
const filler = (n: number) =>
Array.from({ length: n }, (_, k) => ({ name: `filler_${k}`, fields: { id: { type: 'text' } } }));

const SYS_ACCOUNT = {
name: 'sys_account',
fields: {
email: { type: 'text', unique: true },
provider_id: { type: 'text' },
account_id: { type: 'text' },
organization_id: { type: 'text' },
},
indexes: [
// named + lists the organization column → R11 and R12
{ name: 'uniq_org_email', unique: true, fields: ['organization_id', 'email'] },
// unnamed composite → R11, and exercises the column-list `where` label
{ unique: true, fields: ['provider_id', 'account_id'] },
// single column that ALSO carries a field-level `unique` → R11 and R10
{ unique: true, fields: ['email'] },
],
};

const objects = [...filler(44), SYS_ACCOUNT];
const stack = { objects } as Record<string, unknown>;

const whereOf = (rule: string, findings: readonly AuthoringFinding[]) =>
findings.filter((f) => f.rule === rule).map((f) => f.where);

describe('the three ADR-0120 uniqueness rules name the object in `where`', () => {
// Through the REGISTRY, not the rule functions: the defect lived in the
// registry adapter (`where: f.path`), so a test that called the rule directly
// would have stayed green through the whole bug.
const findings = runAuthoringRules('validate', { normalized: stack, parsed: stack });

it('R11 `unique/unscoped-declared-index` names the object and the index', () => {
expect(whereOf('unique/unscoped-declared-index', findings)).toEqual([
`object "sys_account" · index 'uniq_org_email'`,
'object "sys_account" · index [provider_id, account_id]',
'object "sys_account" · index [email]',
]);
});

it('R10 `unique/double-declaration` names the object and the column', () => {
expect(whereOf('unique/double-declaration', findings)).toEqual([
`object "sys_account" · field 'email'`,
]);
});

it('R12 `unique/legacy-organization-composite` names the object and the index', () => {
expect(whereOf('unique/legacy-organization-composite', findings)).toEqual([
`object "sys_account" · index 'uniq_org_email'`,
]);
});

// The card that filed this asked for `where` only. `path` is the slot that is
// SUPPOSED to be positional, `os validate` prints it after `at`, and the
// runtime gate's `fingerprint` reads `where` and `path` together — so a
// consumer diffing findings across two stack shapes still sees the index
// move. Pinned so a later "clean up the indexes" pass has to be deliberate.
it('leaves `path` positional', () => {
expect(findings.filter((f) => f.rule === 'unique/double-declaration').map((f) => f.path)).toEqual([
'objects[44]',
]);
expect(
findings.filter((f) => f.rule === 'unique/legacy-organization-composite').map((f) => f.path),
).toEqual(['objects[44].indexes[0]']);
});

// The rules' own return type carries `where`, so a fourth rule joining this
// family cannot reach the adapter without one. Reading it off the direct call
// proves the producer states it — not the adapter reconstructing it.
it('the rule functions themselves state `where`', () => {
for (const issue of [
...lintUnscopedDeclaredIndexes(objects),
...lintUniqueDeclarations(objects),
...lintLegacyOrganizationComposites(objects),
]) {
expect(issue.where).toMatch(/^object "sys_account" /);
expect(issue.path).toMatch(/^objects\[44\]/);
}
});
});

describe('no authoring rule puts a bare config path in the `where` slot', () => {
/** `objects[3]`, `flows[0].nodes[2].config` — a config path, not a location. */
const BARE_CONFIG_PATH = /^[A-Za-z_$][\w$]*\[\d+\]/;

it('holds across every rule the violating stack triggers', () => {
const findings = runAuthoringRules('validate', { normalized: stack, parsed: stack });

// Non-vacuous: the stack must actually trip the three rules this guard was
// written for, or the sweep below is asserting over an empty list.
const rules = new Set(findings.map((f) => f.rule));
expect(rules).toContain('unique/unscoped-declared-index');
expect(rules).toContain('unique/double-declaration');
expect(rules).toContain('unique/legacy-organization-composite');

const offenders = findings
.filter((f) => BARE_CONFIG_PATH.test(f.where))
.map((f) => `${f.rule}: where = ${f.where}`);
expect(
offenders,
'`where` is the slot every CLI command prints as the line prefix. A config path ' +
'belongs in `path`, which the same commands print separately — putting one here ' +
'spends the only human-readable slot on a number the `at` clause already carries.',
).toEqual([]);
});

// The sweep above only sees rules this one stack happens to trip. This second
// assertion is static and covers all 41 entries: no registry adapter may map
// the `where` slot from a source finding's `path`.
it('holds for every registry adapter, including rules this stack does not trip', () => {
const source = AUTHORING_RULES.map((r) => r.run.toString()).join('\n');
expect(
source.match(/where:\s*\w+\.path\b/g) ?? [],
'a registry adapter is mapping `where` from a positional config path again',
).toEqual([]);
});
});
88 changes: 79 additions & 9 deletions packages/lint/src/data-model-rules.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,6 +26,68 @@ export interface LintIssue {
fix?: string;
}

/**
* A {@link LintIssue} that also states the HUMAN-READABLE location, for rules
* that are adapted into `AuthoringFinding` (`packages/lint/src/authoring-rules.ts`).
*
* `AuthoringFinding` declares two location slots with different jobs — `where`
* ("human-readable location", e.g. `object "leave_request"`) and `path` ("config
* path", e.g. `objects[3].sharingModel`) — and every CLI command renders the
* first as the line's prefix: `os validate` prints
* `• ${where}: ${message}` and then `rule: ${rule} at ${path}`.
*
* `LintIssue` carries only the positional `path`, so the three ADR-0120
* uniqueness rules' registry adapters had nothing else to map and set
* `where: f.path`. Measured on `origin/main`: an author saw
* `• objects[44].indexes[1]: "sys_account" declares index … at objects[44].indexes[1]`
* — the same positional string twice, and an index into the MERGED object array
* that appears in no file the author wrote (44 objects from
* `@objectstack/platform-objects` plus one from `@objectstack/metadata-core`).
* No information was lost — the object's name is in the message — but the
* location slot said nothing the `at` clause did not already say, and every
* other rule in the table spells it `object "sys_account"`.
*
* Stating `where` at the PRODUCER rather than reconstructing it in the adapter
* is deliberate: only the rule still holds the object it walked. Making it
* REQUIRED here (rather than adding `where?` to `LintIssue` and writing
* `f.where ?? f.path` at the adapter) is the same discipline — a consumer-side
* fallback would let the next rule ship the positional spelling again, silently.
*
* `path` is unchanged and stays positional: it is the slot that is SUPPOSED to
* be a config path, `os validate` prints it after `at`, and the runtime gate's
* `fingerprint` reads `where` and `path` together (making `where` more specific
* cannot merge two findings that were distinct).
*/
export interface LocatedLintIssue extends LintIssue {
/** Human-readable location, e.g. `object "sys_account" · index 'uniq_org_email'`. */
where: string;
}

/** `object "sys_account"` — the spelling the rest of the authoring table uses. */
function objectWhere(obj: any): string {
return `object "${obj?.name}"`;
}

/**
* `object "sys_account" · index 'uniq_org_email'`.
*
* A declared index's `name` is optional, so an unnamed one is identified by the
* columns the author actually wrote (`· index [provider_id, account_id]`) — searchable
* in their source, which a bare ordinal is not. The ordinal is the last resort
* for an index that has neither, and is scoped to the named object rather than
* to the merged stack array.
*/
function indexWhere(obj: any, idx: any, j: number, cols: readonly string[]): string {
const named = typeof idx?.name === 'string' && idx.name.trim() ? `'${idx.name.trim()}'` : '';
const label = named || (cols.length > 0 ? `[${cols.join(', ')}]` : `#${j}`);
return `${objectWhere(obj)} · index ${label}`;
}

/** `object "sys_account" · field 'email'` — the field-scoped form of the above. */
function fieldWhere(obj: any, fieldName: string): string {
return `${objectWhere(obj)} · field '${fieldName}'`;
}

// ─── Heuristics ─────────────────────────────────────────────────────

const RELATIONSHIP_TYPES = new Set(['lookup', 'master_detail']);
Expand DownExpand Up@@ -162,8 +224,8 @@ function indexUniqueScope(u: unknown): 'organization' | 'global' {
* Wiring: own AUTHORING_RULES entry (validate/build), and `lintDataModel`
* calls it for `os lint` — each command reports each finding exactly once.
*/
export function lintUnscopedDeclaredIndexes(objects: any[]): LintIssue[] {
const issues: LintIssue[] = [];
export function lintUnscopedDeclaredIndexes(objects: any[]): LocatedLintIssue[] {
const issues: LocatedLintIssue[] = [];
if (!Array.isArray(objects) || objects.length === 0) return issues;

for (let i = 0; i < objects.length; i++) {
Expand All@@ -173,13 +235,15 @@ export function lintUnscopedDeclaredIndexes(objects: any[]): LintIssue[] {
for (let j = 0; j < declaredIndexes.length; j++) {
const idx = declaredIndexes[j];
if (idx?.unique !== true) continue; // fires on the bare spelling only
const cols = Array.isArray(idx?.fields)
? idx.fields.filter((f: unknown) => typeof f === 'string').join(', ')
: '';
const colList: string[] = Array.isArray(idx?.fields)
? idx.fields.filter((f: unknown) => typeof f === 'string')
: [];
const cols = colList.join(', ');
const indexLabel = typeof idx?.name === 'string' && idx.name.trim() ? ` '${idx.name.trim()}'` : '';
issues.push({
severity: 'warning',
rule: UNIQUE_UNSCOPED_DECLARED_INDEX,
where: indexWhere(obj, idx, j, colList),
message:
`"${obj.name}" declares index${indexLabel} [${cols}] with bare \`unique: true\` — a unique index whose scope is ` +
`unstated (ADR-0120). Today the bare spelling materializes over exactly its \`fields\`, i.e. installation-wide; ` +
Expand DownExpand Up@@ -225,8 +289,8 @@ export function lintUnscopedDeclaredIndexes(objects: any[]): LintIssue[] {
* Advisory. The resulting stack is well-defined — the cost is an intent that
* never takes effect, not a broken artifact — so this never fails a build.
*/
export function lintUniqueDeclarations(objects: any[]): LintIssue[] {
const issues: LintIssue[] = [];
export function lintUniqueDeclarations(objects: any[]): LocatedLintIssue[] {
const issues: LocatedLintIssue[] = [];
if (!Array.isArray(objects) || objects.length === 0) return issues;

for (let i = 0; i < objects.length; i++) {
Expand DownExpand Up@@ -290,6 +354,11 @@ export function lintUniqueDeclarations(objects: any[]): LintIssue[] {
issues.push({
severity: 'warning',
rule: UNIQUE_DOUBLE_DECLARATION,
// More specific than `path` on purpose: this rule's finding is about ONE
// column, but its `path` has always been the whole object (`objects[i]`)
// because the defect straddles `fields.<name>.unique` and an entry of
// `indexes`. `where` can name the column without picking one of the two.
where: fieldWhere(obj, name),
message,
path: `objects[${i}]`,
fix,
Expand DownExpand Up@@ -325,8 +394,8 @@ export function lintUniqueDeclarations(objects: any[]): LintIssue[] {
* unique declared on the organization column ALONE, which is not a composite and
* has no per-organization reading to recover.
*/
export function lintLegacyOrganizationComposites(objects: any[]): LintIssue[] {
const issues: LintIssue[] = [];
export function lintLegacyOrganizationComposites(objects: any[]): LocatedLintIssue[] {
const issues: LocatedLintIssue[] = [];
if (!Array.isArray(objects) || objects.length === 0) return issues;

for (let i = 0; i < objects.length; i++) {
Expand All@@ -351,6 +420,7 @@ export function lintLegacyOrganizationComposites(objects: any[]): LintIssue[] {
issues.push({
severity: 'warning',
rule: UNIQUE_LEGACY_ORGANIZATION_COMPOSITE,
where: indexWhere(obj, idx, j, cols),
message:
`"${obj.name}" declares index${indexLabel} [${cols.join(', ')}] with ${spelling} and lists the organization ` +
`column '${tenantColumn}' itself — the hand-written per-organization composite that predates the scope ` +
Expand Down
2 changes: 1 addition & 1 deletion packages/lint/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -628,7 +628,7 @@ export {
UNIQUE_UNSCOPED_DECLARED_INDEX,
UNIQUE_LEGACY_ORGANIZATION_COMPOSITE,
} from './data-model-rules.js';
export type { LintIssue, Severity } from './data-model-rules.js';
export type { LintIssue, LocatedLintIssue, Severity } from './data-model-rules.js';

// ─── The registry itself (#4409, relocated #4463) ────────────────────
//
Expand Down
Loading