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
32 changes: 32 additions & 0 deletions .changeset/20825-cli-picklist-extend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
'@objectstack/cli': minor
---

feat(cli)!: `objectstack validate` and `objectstack build` refuse a `picklistExtensions` entry whose `extend` names no picklist the stack declares (#20825)

Clause-②: no (narrowing — `objectstack validate` / `objectstack build` newly refuse a `picklistExtensions[].extend` that names no picklist the stack declares; nothing is accepted that was refused before)

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable changes spelling or type: `packages/spec` is untouched, and `picklistExtensions[].extend` stays the snake_case name it was. What changes is that two authoring commands now refuse one authored shape, an extension whose `extend` names no picklist in the stack. `objectstack migrate meta` could not rewrite that shape even in principle, because which list the author meant is not in the metadata. Nothing here judges a stored row. -->

**BREAKING** — an accept-set narrowing on two authoring commands, shipped as
`minor` under the launch-window convention. A stack with a `picklistExtensions`
entry whose `extend` names no picklist the stack declares — `extend: 'industy'`
beside a `picklists: [{ name: 'industry', … }]` — used to pass `objectstack
validate` and `objectstack build` (which wrote the artifact). Both now exit 1 and
name the extension and the list it names (`picklist-reference-unknown`, the rule a
field's dangling `picklist` already gets).
**One-line fix:** correct `extend` to the picklist the entry adds options to, declare
the list it names (`picklists: [{ name, label, options }]`, or a `*.picklist.ts` file
the stack imports), or remove the entry.

**Which extensions are judged.** The ones the load path registers: the top-level
`picklistExtensions` of a one-package stack, or each `packages[]` entry's own. An
`extend` resolves against every picklist the stack declares, including one a sibling
package in the same artifact owns.

**A list from a package outside the stack is reported, not refused.** When the
package declaring the extension lists a `manifest.dependencies` entry the stack does
not carry, the list may live there, and these commands cannot read it. That
extension is an `info` notice (`picklist-reference-unverified`) in `warnings` and on
the console, naming the extension, the list and the dependencies — never a failure,
not even under `--strict`.
13 changes: 7 additions & 6 deletions packages/cli/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,12 @@ export default class Compile extends Command {
this.exit(1);
}

// 3a-bis. A field `picklist` that names no picklist the stack declares is
// REFUSED — the SAME call `os validate` makes at its step 2d, so the
// two doors cannot disagree about which references resolve. Without
// it this door wrote the artifact carrying the misspelt reference, and
// the command that ships shipped a choice with nothing to choose.
// 3a-bis. A field `picklist`, or a `picklistExtensions` entry's `extend`,
// that names no picklist the stack declares is REFUSED — the SAME call
// `os validate` makes at its step 2d, so the two doors cannot disagree
// about which references resolve. Without it this door wrote the
// artifact carrying the misspelt reference, and the command that ships
// shipped a choice with nothing to choose.
//
// A reference the stack cannot resolve while the declaring package
// depends on packages outside the stack is an `info` notice instead
Expand All @@ -484,7 +485,7 @@ export default class Compile extends Command {
}
const n = picklistJudgement.refusals.length;
console.log('');
printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}
Expand Down
19 changes: 11 additions & 8 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,15 +420,18 @@ export default class Validate extends Command {
this.exit(1);
}

// 2d. A field `picklist` that names no picklist the stack declares is
// REFUSED, naming the field and the list. `FieldSchema` judges the
// name's spelling only, so a misspelt reference parsed, passed this
// door at exit 0 and reached the runtime as a choice with nothing to
// choose — the silence a NAMED list exists to remove.
// 2d. A field `picklist`, or a `picklistExtensions` entry's `extend`, that
// names no picklist the stack declares is REFUSED, naming the field
// (or the extension) and the list. `FieldSchema` and
// `PicklistExtensionSchema` judge the name's spelling only, so a
// misspelt reference parsed, passed this door at exit 0 and reached
// the runtime as a choice with nothing to choose — the silence a NAMED
// list exists to remove.
//
// The walk is the load path's (see `utils/picklist-references.ts`):
// each `packages[]` body's fields, or the top level's when there is
// no `packages[]`, resolved against every picklist the stack declares.
// each `packages[]` body's fields and extensions, or the top level's
// when there is no `packages[]`, resolved against every picklist the
// stack declares.
// A reference that resolves nowhere is refused only when the
// declaring package depends on no package outside the stack; when it
// does, the list may live there, and this command cannot read it — so
Expand All @@ -455,7 +458,7 @@ export default class Validate extends Command {
}
const n = picklistJudgement.refusals.length;
console.log('');
printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}
Expand Down
167 changes: 163 additions & 4 deletions packages/cli/src/utils/picklist-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@

/**
* `judgePicklistReferences` — the walk `os validate` (step 2d) and `os build`
* (step 3a-bis) run over a field's `picklist` reference.
* (step 3a-bis) run over a field's `picklist` reference and a
* `picklistExtensions` entry's `extend`.
*
* Pinned here: which references are judged (the load path's reading — the top
* level when there is no `packages[]`, each body's own otherwise), what they
* resolve against (every picklist the stack declares), and which of the two
* verdicts an unresolved one gets (refused when the declaring package depends
* on nothing outside the stack; an `info` notice when it does). Asserted by
* rule id, severity, the field named in `where`, the list named in `message`
* and the `path` — never by the sentence around them.
* on nothing outside the stack; an `info` notice when it does) — the same two
* verdicts for a field and for an extension. Asserted by rule id, severity,
* the field or extension named in `where`, the list named in `message` and the
* `path` — never by the sentence around them.
*/

import { describe, expect, it } from 'vitest';
Expand All @@ -30,6 +32,12 @@ const industry = {
options: [{ label: 'Technology', value: 'technology' }],
};

/** An extension adding one option to the list it names. */
const extension = (extend: string) => ({
extend,
options: [{ label: 'Healthcare', value: 'healthcare' }],
});

const account = (picklist: string) => ({
name: 'pk_account',
label: 'Account',
Expand Down Expand Up @@ -170,3 +178,154 @@ describe('judgePicklistReferences — a `packages[]` stack', () => {
expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.objects[0].fields.industry.picklist']);
});
});

describe('judgePicklistReferences — a `picklistExtensions[].extend` in a one-package stack', () => {
it('CONTROL: an extension naming a picklist the stack declares is neither refused nor reported', () => {
expect(judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
picklistExtensions: [extension('industry')],
})).toEqual({ refusals: [], notices: [] });
});

it('refuses an `extend` that names no picklist in the stack, naming the extension and the list', () => {
const { refusals, notices } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
picklistExtensions: [extension('industry'), extension('industy')],
});
expect(notices).toEqual([]);
// Only the second entry: the first resolves.
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
severity: 'error',
rule: PICKLIST_REFERENCE_UNKNOWN,
where: 'picklist extension "industy"',
path: 'picklistExtensions[1].extend',
});
expect(refusals[0].message).toContain("'industy'");
// The lists it could have meant are named, so the typo is visible.
expect(refusals[0].message).toContain("'industry'");
});

it('refuses a dangling `extend` in a stack that declares no picklist at all', () => {
const { refusals } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklistExtensions: [extension('industry')],
});
expect(refusals.map((r) => r.rule)).toEqual([PICKLIST_REFERENCE_UNKNOWN]);
expect(refusals[0].path).toBe('picklistExtensions[0].extend');
});

it('judges a field and an extension in one pass: each dangling reference is its own refusal', () => {
const { refusals } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objects: [account('industy')],
picklistExtensions: [extension('regoin')],
});
expect(refusals.map((r) => r.path)).toEqual([
'objects[0].fields.industry.picklist',
'picklistExtensions[0].extend',
]);
expect(new Set(refusals.map((r) => r.rule))).toEqual(new Set([PICKLIST_REFERENCE_UNKNOWN]));
});

it('a resolving extension does not hide a dangling field reference, nor the reverse', () => {
const fieldOnly = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objects: [account('industy')],
picklistExtensions: [extension('industry')],
});
expect(fieldOnly.refusals.map((r) => r.where)).toEqual(['field "pk_account.industry"']);

const extensionOnly = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objects: [account('industry')],
picklistExtensions: [extension('industy')],
});
expect(extensionOnly.refusals.map((r) => r.where)).toEqual(['picklist extension "industy"']);
});

it('REPORTS, never refuses, when the stack depends on a package it does not carry', () => {
const { refusals, notices } = judgePicklistReferences({
manifest: manifest('com.example.pk', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
picklistExtensions: [extension('industry')],
});
expect(refusals).toEqual([]);
expect(notices).toHaveLength(1);
expect(notices[0]).toMatchObject({
severity: 'info',
rule: PICKLIST_REFERENCE_UNVERIFIED,
path: 'picklistExtensions[0].extend',
});
// The notice is printed without its `where`, so it names the extension, the
// list and the dependency it could not read.
expect(notices[0].message).toContain('picklist extension "industry"');
expect(notices[0].message).toContain("'industry'");
expect(notices[0].message).toContain("'com.acme.crm'");
});
});

describe('judgePicklistReferences — a `picklistExtensions[].extend` in a `packages[]` stack', () => {
it('resolves an extension against a picklist a SIBLING package in the same artifact declares', () => {
expect(judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
{
manifest: {
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
picklistExtensions: [extension('industry')],
},
},
],
})).toEqual({ refusals: [], notices: [] });
});

it('refuses when every declared dependency is inside the artifact — none of them declares the list', () => {
const { refusals, notices } = judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
{
manifest: {
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
picklistExtensions: [extension('region')],
},
},
],
});
expect(notices).toEqual([]);
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
rule: PICKLIST_REFERENCE_UNKNOWN,
where: 'picklist extension "region"',
path: 'packages[1].manifest.picklistExtensions[0].extend',
});
});

it('reads the declaring package\'s OWN dependencies: an outside dependency of a sibling does not soften it', () => {
const { refusals, notices } = judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core', { dependencies: { 'com.acme.crm': '^1.0.0' } }) } },
{ manifest: { ...manifest('com.example.orders'), picklistExtensions: [extension('region')] } },
{
manifest: {
...manifest('com.example.billing', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
picklistExtensions: [extension('segment')],
},
},
],
});
expect(refusals.map((r) => r.path)).toEqual(['packages[1].manifest.picklistExtensions[0].extend']);
expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.picklistExtensions[0].extend']);
});

it('does not judge a TOP-LEVEL `picklistExtensions` once `packages[]` carries the bodies: the load path does not register from it', () => {
expect(judgePicklistReferences({
packages: [{ manifest: { ...manifest('com.example.core'), picklists: [industry] } }],
picklistExtensions: [extension('industy')],
})).toEqual({ refusals: [], notices: [] });
});
});
Loading
Loading