diff --git a/.changeset/20825-cli-picklist-kind.md b/.changeset/20825-cli-picklist-kind.md new file mode 100644 index 00000000000..e66bb45bbcc --- /dev/null +++ b/.changeset/20825-cli-picklist-kind.md @@ -0,0 +1,38 @@ +--- +'@objectstack/cli': minor +'@objectstack/lint': patch +--- + +feat(cli)!: `objectstack validate` and `objectstack build` refuse a field whose `picklist` names no picklist the stack declares, and lint R8 counts `picklist` as an options source (#20825) + +Clause-②: no (narrowing — `objectstack validate` / `objectstack build` newly refuse a field `picklist` that names no picklist the stack declares; the R8 change removes a false-positive warning and widens no accept set of its own) + + + +**BREAKING** — an accept-set narrowing on two authoring commands, shipped as +`minor` under the launch-window convention. A stack with a select field whose +`picklist` names no picklist the stack declares — `picklist: '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 field +and the list it names (`picklist-reference-unknown`). +**One-line fix:** correct `picklist` to a list the stack declares, or declare the +list it names (`picklists: [{ name, label, options }]`, or a `*.picklist.ts` file the +stack imports). + +**Which references are judged.** The ones the load path registers: the top-level +`objects` and `objectExtensions` of a one-package stack, or each `packages[]` +entry's own. A reference 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 field lists a `manifest.dependencies` entry the stack does not +carry, the list may live there, and these commands cannot read it. That reference is +an `info` notice (`picklist-reference-unverified`) in `warnings` and on the console, +naming the field, the list and the dependencies — never a failure, not even under +`--strict`. + +**Lint R8 (`field/select-missing-options`)** no longer reports a select, multiselect +or radio field that names a `picklist`: the picklist is its options source. The +warning it used to give pointed at `options`, which a field naming a `picklist` +cannot add — the field schema refuses the two together. A select with neither still +warns, and its fix now names both sources as alternatives. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 1e716a6e53f..0a9726d0b3c 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -64,6 +64,9 @@ import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-s // [#20393] The boot registrar's divergent view-container `name` refusal — the // walk `os validate` step 2c runs, over `@objectstack/objectql`'s one judge. import { findViewContainerNameRefusals } from '../utils/view-container-names.js'; +// A field `picklist` that names no picklist the stack declares — the walk +// `os validate` step 2d runs, over the same judge. +import { judgePicklistReferences, printPicklistReferenceNotices } from '../utils/picklist-references.js'; export default class Compile extends Command { static override description = 'Compile ObjectStack configuration to JSON artifact'; @@ -178,6 +181,13 @@ export default class Compile extends Command { // computes the identical record so the residue pin holds, and it reports // without ever refusing. Appended LAST, in `os validate`'s order. let jsxGateNotices: ReturnType['notices'] = []; + // The `info` records of a field `picklist` reference that resolves nowhere + // in the stack while the declaring package depends on packages outside it + // (step 3a-bis). A member of `warningsSoFar()`, ⛔ not a payload key, for + // the reasons the three lists above record: it reports without refusing, + // and `os validate` computes the identical records. Appended LAST, in + // `os validate`'s order. + let picklistReferenceNotices: ReturnType['notices'] = []; const warningsSoFar = () => [ ...ruleAdvisories, ...docWarnings, @@ -186,6 +196,7 @@ export default class Compile extends Command { ...navGroupWarnings, ...permissionSetCollisionWarnings, ...jsxGateNotices, + ...picklistReferenceNotices, ]; // [#18780] ONE rendering of the author-time advisory block, from the // COMPLETE list — hoisted here for the same reason the lists above are. @@ -448,6 +459,36 @@ 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. + // + // A reference the stack cannot resolve while the declaring package + // depends on packages outside the stack is an `info` notice instead + // (see `utils/picklist-references.ts`): the list may live there, and + // this command cannot read it. Printed here, carried in + // `warningsSoFar()`, never gating. + // + // Right after the parse and ahead of every artifact write, like 3a. + // The `--json` face is 3a's envelope (`errors`); the text face is + // `os validate`'s. + const picklistJudgement = judgePicklistReferences(result.data as Record); + picklistReferenceNotices = [...picklistJudgement.notices]; + if (!flags.json) printPicklistReferenceNotices(picklistReferenceNotices); + if (picklistJudgement.refusals.length > 0) { + if (flags.json) { + await emitJson({ success: false, errors: picklistJudgement.refusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); + this.exit(1); + } + const n = picklistJudgement.refusals.length; + console.log(''); + printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`); + printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY }); + this.exit(1); + } + // 3b. The author-time rule registry (#4409) — one table, three commands. // `os build` was the WEAKEST of the three authoring gates before it: // it published stacks `os validate` or `os lint` refuse, because the diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 11b7ea3e79e..dc34ed730d8 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -64,6 +64,10 @@ import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-s // over the parsed stack the way the load path registers it. The verdict is // `@objectstack/objectql`'s; see the module header. import { findViewContainerNameRefusals } from '../utils/view-container-names.js'; +// A field `picklist` that names no picklist the stack declares — refused, or +// reported when the declaring package depends on packages outside the stack. +// Walked the way the load path registers, like the refusal above. +import { judgePicklistReferences, printPicklistReferenceNotices } from '../utils/picklist-references.js'; export default class Validate extends Command { static override description = @@ -166,6 +170,13 @@ export default class Validate extends Command { // of a project without a manifest is unchanged on both faces. `os compile` // computes the identical record, so the residue pin keeps holding. let jsxGateNotices: ReturnType['notices'] = []; + // The `info` records of a field `picklist` reference that resolves nowhere + // in the stack while the declaring package depends on packages outside it + // (step 2d). Same class as the notice above — reports, never refuses, and + // is NOT in the `warnings` list `--strict` reads: the reference may be + // right, and `--strict` failing on it would refuse a correct stack. `os + // compile` computes the identical records, so the residue pin keeps holding. + let picklistReferenceNotices: ReturnType['notices'] = []; const warningsSoFar = () => [ ...ruleAdvisories, ...docWarnings, @@ -184,6 +195,8 @@ export default class Validate extends Command { ...permissionSetCollisionWarnings, // [#20113] APPENDED for the same reason, one member later again. ...jsxGateNotices, + // APPENDED for the same reason, one member later again. + ...picklistReferenceNotices, ]; // [commit 79cf692b0] The ADR-0087 D2 conversion notices, hoisted for the SAME reason // and under the SAME ruling as the five lists above — one field over. The @@ -407,6 +420,46 @@ 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. + // + // 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. + // 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 + // that case is an `info` notice, printed here and carried in + // `warningsSoFar()`, never gating. + // + // Right after the parse, ahead of the rule table, for the reason + // step 2c gives. `os build` runs the same call at its step 3a-bis. + const picklistJudgement = judgePicklistReferences(result.data as Record); + picklistReferenceNotices = [...picklistJudgement.notices]; + if (!flags.json) printPicklistReferenceNotices(picklistReferenceNotices); + if (picklistJudgement.refusals.length > 0) { + if (flags.json) { + await emitJson({ + valid: false, + errors: picklistJudgement.refusals, + // Every exit carries the lists the run has computed so far — the + // pre-parse ones and the notices above. + warnings: warningsSoFar(), + conversions: conversionNotices, + duration: timer.elapsed(), + }); + this.exit(1); + } + const n = picklistJudgement.refusals.length; + console.log(''); + printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`); + printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY }); + this.exit(1); + } + // 3. The author-time rule registry (#4409). Every rule the three authoring // commands share — expressions, view shape, widget/action/filter/name // references, SDUI styling, page sources, security posture, the CLI's diff --git a/packages/cli/src/utils/picklist-references.test.ts b/packages/cli/src/utils/picklist-references.test.ts new file mode 100644 index 00000000000..6fe26142b91 --- /dev/null +++ b/packages/cli/src/utils/picklist-references.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `judgePicklistReferences` — the walk `os validate` (step 2d) and `os build` + * (step 3a-bis) run over a field's `picklist` reference. + * + * 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. + */ + +import { describe, expect, it } from 'vitest'; +import { + judgePicklistReferences, + PICKLIST_REFERENCE_UNKNOWN, + PICKLIST_REFERENCE_UNVERIFIED, +} from './picklist-references.js'; + +const manifest = (id: string, extra: Record = {}) => ({ + id, name: id, version: '1.0.0', type: 'app', ...extra, +}); + +const industry = { + name: 'industry', + label: 'Industry', + options: [{ label: 'Technology', value: 'technology' }], +}; + +const account = (picklist: string) => ({ + name: 'pk_account', + label: 'Account', + fields: { + name: { type: 'text', label: 'Name' }, + industry: { type: 'select', label: 'Industry', picklist }, + }, +}); + +describe('judgePicklistReferences — a one-package stack', () => { + it('CONTROL: a field naming a picklist the stack declares is neither refused nor reported', () => { + expect(judgePicklistReferences({ + manifest: manifest('com.example.pk'), + picklists: [industry], + objects: [account('industry')], + })).toEqual({ refusals: [], notices: [] }); + }); + + it('CONTROL: a select with inline options and no `picklist` is not this judge\'s business', () => { + expect(judgePicklistReferences({ + manifest: manifest('com.example.pk'), + objects: [{ + name: 'pk_account', + fields: { tier: { type: 'select', options: [{ label: 'Gold', value: 'gold' }] } }, + }], + })).toEqual({ refusals: [], notices: [] }); + }); + + it('refuses a `picklist` that names no picklist in the stack, naming the field and the list', () => { + const { refusals, notices } = judgePicklistReferences({ + manifest: manifest('com.example.pk'), + picklists: [industry], + objects: [account('industy')], + }); + expect(notices).toEqual([]); + expect(refusals).toHaveLength(1); + expect(refusals[0]).toMatchObject({ + severity: 'error', + rule: PICKLIST_REFERENCE_UNKNOWN, + where: 'field "pk_account.industry"', + path: 'objects[0].fields.industry.picklist', + }); + 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 reference in a stack that declares no picklist at all', () => { + const { refusals } = judgePicklistReferences({ + manifest: manifest('com.example.pk'), + objects: [account('industry')], + }); + expect(refusals.map((r) => r.rule)).toEqual([PICKLIST_REFERENCE_UNKNOWN]); + }); + + it('judges the fields an `objectExtensions` entry merges into its target too', () => { + const { refusals } = judgePicklistReferences({ + manifest: manifest('com.example.pk'), + picklists: [industry], + objectExtensions: [{ extend: 'crm_account', fields: { segment: { type: 'select', picklist: 'segment' } } }], + }); + expect(refusals).toHaveLength(1); + expect(refusals[0]).toMatchObject({ + where: 'field "crm_account.segment"', + path: 'objectExtensions[0].fields.segment.picklist', + }); + }); + + 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' } }), + objects: [account('industry')], + }); + expect(refusals).toEqual([]); + expect(notices).toHaveLength(1); + expect(notices[0]).toMatchObject({ + severity: 'info', + rule: PICKLIST_REFERENCE_UNVERIFIED, + path: 'objects[0].fields.industry.picklist', + }); + // The notice is printed without its `where`, so it names the field, the + // list and the dependency it could not read. + expect(notices[0].message).toContain('pk_account.industry'); + expect(notices[0].message).toContain("'industry'"); + expect(notices[0].message).toContain("'com.acme.crm'"); + }); +}); + +describe('judgePicklistReferences — a `packages[]` stack', () => { + it('resolves 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' } }), + objects: [account('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' } }), + objects: [account('region')], + }, + }, + ], + }); + expect(notices).toEqual([]); + expect(refusals).toHaveLength(1); + expect(refusals[0]).toMatchObject({ + rule: PICKLIST_REFERENCE_UNKNOWN, + path: 'packages[1].manifest.objects[0].fields.industry.picklist', + }); + }); + + 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'), objects: [account('region')] } }, + { + manifest: { + ...manifest('com.example.billing', { dependencies: { 'com.acme.crm': '^1.0.0' } }), + objects: [account('segment')], + }, + }, + ], + }); + expect(refusals.map((r) => r.path)).toEqual(['packages[1].manifest.objects[0].fields.industry.picklist']); + expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.objects[0].fields.industry.picklist']); + }); +}); diff --git a/packages/cli/src/utils/picklist-references.ts b/packages/cli/src/utils/picklist-references.ts new file mode 100644 index 00000000000..dd2d16dfb46 --- /dev/null +++ b/packages/cli/src/utils/picklist-references.ts @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The author-time judge of a field's `picklist` reference: a field that names + * a shared option list no package in the stack declares is refused, loudly, + * naming the field and the list it names. + * + * ## The defect + * + * `Field.select({ picklist: 'industry' })` takes its options from the + * `picklist` item it names (`data/picklist.zod.ts`). `FieldSchema` checks the + * NAME's spelling (lowercase snake_case) and nothing else — the reference is + * resolved against the stack's picklists by nobody at author time. So a typo + * (`picklist: 'industy'`) parsed, `os validate` exited 0, `os build` wrote the + * artifact, and the field reached the runtime as a choice with nothing to + * choose. A named list exists precisely so a wrong reference is a refusal + * instead of a silently empty control. + * + * ## The walk is the boot path's + * + * The same reading `view-container-names.ts` makes, for the same reason — + * what is judged is what boot registers: + * + * - no `packages[]` → the top-level `objects` / `objectExtensions`, bounded + * by the top-level `manifest.dependencies`; + * - `packages[]` → each body's own `objects` / `objectExtensions`, + * bounded by that body's own `dependencies`, and ⛔ NOT the top level, + * which the load path does not register from. + * + * A reference RESOLVES against every picklist the stack declares — every + * `packages[]` body's `picklists`, or the top-level `picklists` when there is + * no `packages[]`. The release artifact is the co-ownership boundary + * (ADR-0130), so a list a SIBLING package in the same artifact owns resolves + * here exactly as one the declaring package owns. + * + * ## Declared dependencies outside the stack: reported, never refused + * + * A package may use a picklist another package owns — `picklistExtensions` + * exists because one package adds values to a list it does not own. When that + * owner is a declared dependency INSIDE this artifact, its picklists are in + * the resolution set above. When it is a dependency OUTSIDE the artifact, this + * command cannot read it: `manifest.dependencies` maps package IDS to version + * ranges, which the installer resolves, and nothing maps such an id to a file + * the CLI could open. So a reference that resolves nowhere here is: + * + * - REFUSED (`severity: 'error'`) when the declaring package names no + * dependency outside this stack — nothing else could provide the list; + * - REPORTED (`severity: 'info'`, never gating, not even under `--strict`) + * when it does — the reference could be right, and a refusal here would + * reject a stack that is correct. The notice names the dependencies, so the + * skipped judgement is said out loud rather than read as a pass. + * + * ## Why this is not an `@objectstack/lint` registry rule + * + * The verdict depends on WHICH package declares the field: that package's + * declared dependencies decide between a refusal and a notice. A registry rule + * is handed one stack, and on the union run that stack is the flattened top + * level, which carries no package provenance. The walk above is the artifact's + * package reading, the same class as `findViewContainerNameRefusals`, and both + * doors call it (`validate.ts` step 2d, `compile.ts` step 3a-bis). + * + * ⚠️ Bound, stated rather than hidden: the runtime metadata write path (a field + * saved through Studio or REST `/meta`) is not judged here. And a + * `picklistExtensions` entry whose `extend` names no picklist is a different + * reference, which this module does not judge. + * + * Reads the PARSED stack — what `defineStack()` hands the boot wrap, and what + * `os build` serializes. + */ + +import chalk from 'chalk'; +import type { AuthoringFinding } from '@objectstack/lint'; + +import { artifactPackages } from './artifact-packages.js'; +import { printInfo } from './format.js'; + +/** A field's `picklist` names no picklist the stack declares, and nothing outside it could. */ +export const PICKLIST_REFERENCE_UNKNOWN = 'picklist-reference-unknown'; + +/** Same, but the declaring package names dependencies outside the stack, which this command cannot read. */ +export const PICKLIST_REFERENCE_UNVERIFIED = 'picklist-reference-unverified'; + +/** What {@link judgePicklistReferences} hands both doors. */ +export interface PicklistReferenceJudgement { + /** `severity: 'error'` — the door exits 1 with these as its `errors`. */ + readonly refusals: readonly AuthoringFinding[]; + /** `severity: 'info'` — carried in `warnings`, never gating. */ + readonly notices: readonly AuthoringFinding[]; +} + +type AnyRec = Record; + +const asRec = (v: unknown): AnyRec | undefined => + v && typeof v === 'object' && !Array.isArray(v) ? (v as AnyRec) : undefined; + +/** How many declared names the refusal quotes before it counts the rest. */ +const MAX_QUOTED_NAMES = 10; + +/** One body the boot path registers, with the dependencies that bound it. */ +interface JudgedBody { + /** Path prefix of this body in the parsed stack: `''` or `packages[1].manifest.`. */ + prefix: string; + body: AnyRec; + /** Declared dependency ids this artifact does not carry. */ + outside: string[]; +} + +function declaredDependencyIds(manifestLike: AnyRec | undefined): string[] { + const declared = asRec(manifestLike?.dependencies); + return declared ? Object.keys(declared) : []; +} + +function bodiesOf(parsed: AnyRec): JudgedBody[] { + // The resolver's own branch test (`declared === undefined`). On the PARSED + // stack a present `packages` is an array — `null` and every non-array were + // refused at the parse. + if (parsed.packages !== undefined) { + const packages = artifactPackages(parsed); + const inside = new Set(packages.map((pkg) => pkg.id)); + return packages.map((pkg) => ({ + prefix: `packages[${pkg.index}].manifest.`, + body: pkg.body, + outside: declaredDependencyIds(pkg.body).filter((id) => !inside.has(id)), + })); + } + const manifest = asRec(parsed.manifest); + const self = typeof manifest?.id === 'string' ? manifest.id : undefined; + return [{ + prefix: '', + body: parsed, + outside: declaredDependencyIds(manifest).filter((id) => id !== self), + }]; +} + +function picklistNamesOf(body: AnyRec): string[] { + const declared = body.picklists; + if (!Array.isArray(declared)) return []; + return declared + .map((entry) => asRec(entry)?.name) + .filter((name): name is string => typeof name === 'string' && name !== ''); +} + +/** Every `picklist` a body's fields name, located. */ +function referencesOf(judged: JudgedBody): Array<{ where: string; path: string; picklist: string }> { + const out: Array<{ where: string; path: string; picklist: string }> = []; + const walk = (collection: 'objects' | 'objectExtensions', ownerKey: 'name' | 'extend') => { + const entries = judged.body[collection]; + if (!Array.isArray(entries)) return; + entries.forEach((entry, index) => { + const rec = asRec(entry); + const fields = asRec(rec?.fields); + if (!rec || !fields) return; + const owner = typeof rec[ownerKey] === 'string' ? (rec[ownerKey] as string) : `${collection}[${index}]`; + for (const [fieldName, def] of Object.entries(fields)) { + const picklist = asRec(def)?.picklist; + if (typeof picklist !== 'string' || picklist === '') continue; + out.push({ + where: `field "${owner}.${fieldName}"`, + path: `${judged.prefix}${collection}[${index}].fields.${fieldName}.picklist`, + picklist, + }); + } + }); + }; + walk('objects', 'name'); + walk('objectExtensions', 'extend'); + return out; +} + +function quoteNames(names: readonly string[]): string { + if (names.length === 0) return 'this stack declares no picklist'; + const quoted = names.slice(0, MAX_QUOTED_NAMES).map((n) => `'${n}'`).join(', '); + const rest = names.length - MAX_QUOTED_NAMES; + return `declared: ${quoted}${rest > 0 ? ` and ${rest} more` : ''}`; +} + +/** + * Every field `picklist` reference in this stack that resolves to no picklist + * the stack declares — refused, or reported when the declaring package names + * dependencies outside the stack. See the module header for the walk and the + * two verdicts. + * + * Returns two empty lists for a stack whose every reference resolves. + */ +export function judgePicklistReferences(parsed: AnyRec): PicklistReferenceJudgement { + const bodies = bodiesOf(parsed); + const declared = [...new Set(bodies.flatMap((b) => picklistNamesOf(b.body)))]; + const known = new Set(declared); + + const refusals: AuthoringFinding[] = []; + const notices: AuthoringFinding[] = []; + for (const judged of bodies) { + for (const ref of referencesOf(judged)) { + if (known.has(ref.picklist)) continue; + const names = `\`picklist: '${ref.picklist}'\` names no picklist this stack declares`; + if (judged.outside.length === 0) { + // `where` names the field and every printer leads the line with it, so + // the message does not repeat it. + refusals.push({ + severity: 'error', + rule: PICKLIST_REFERENCE_UNKNOWN, + where: ref.where, + path: ref.path, + message: + `${names} (${quoteNames(declared)}), so the field has no list to take its ` + + 'options from.', + hint: + `Declare the list — \`picklists: [{ name: '${ref.picklist}', label, options }]\` in the stack, or a ` + + '`*.picklist.ts` file the stack imports — or correct `picklist` to a list the stack declares. ' + + 'Or drop `picklist` and give the field inline `options` of its own.', + }); + } else { + notices.push({ + severity: 'info', + rule: PICKLIST_REFERENCE_UNVERIFIED, + where: ref.where, + path: ref.path, + // The notice printer shows no `where`, so this message names the field. + message: + `${ref.where}: ${names} — not judged: the package declaring the field depends on ` + + `${judged.outside.map((id) => `'${id}'`).join(', ')}, which this stack does not carry, ` + + 'and whose picklists this command cannot read.', + hint: + `If one of those packages declares '${ref.picklist}', nothing is wrong. Otherwise declare the list ` + + 'in this stack or correct the name.', + }); + } + } + } + return { refusals, notices }; +} + +/** + * The text face of the notices for `os validate` and `os build`, printed at the + * step that computes them so they show on the failing paths too. + */ +export function printPicklistReferenceNotices(notices: readonly AuthoringFinding[]): void { + for (const n of notices) { + printInfo(`[${n.rule}] ${n.message}`); + console.log(chalk.dim(` → ${n.hint}`)); + } +} diff --git a/packages/cli/test/picklist-reference-doors.test.ts b/packages/cli/test/picklist-reference-doors.test.ts new file mode 100644 index 00000000000..e6c2adb5f9a --- /dev/null +++ b/packages/cli/test/picklist-reference-doors.test.ts @@ -0,0 +1,175 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `picklist` kind through the three authoring doors, over the real CLI. + * + * Three facts, each read off a real `os` process: + * + * 1. A stack declaring a picklist and a select field that names it validates, + * builds and lints with no refusal — and the artifact `os build` writes + * carries the list and the reference as authored. + * 2. A field whose `picklist` names no picklist the stack declares is REFUSED + * by `os validate`, naming the field and the list it names; `os build` + * refuses the same stack and writes no artifact. Before this, both doors + * exited 0 and the misspelt reference shipped. + * 3. `os lint`'s R8 (`field/select-missing-options`) does not report the + * picklist-bound field — it names its options source. + * + * The walk and both verdicts are pinned in `src/utils/picklist-references.test.ts` + * and R8 in `@objectstack/lint`; this file holds the DOORS to them. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { execFile } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { writeDefineStackConfig } from './helpers/define-stack-fixture.js'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** oclif + tsx cold start, with every command module loaded. */ +const RUN_TIMEOUT_MS = 180_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err + ? typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1 + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** The last line of stdout that parses as a JSON object — the `--json` document. */ +function jsonOf(run: Run): Record { + const text = run.stdout.trim(); + const start = text.indexOf('{'); + if (start < 0) throw new Error(`no JSON document on stdout:\n${run.stdout}\n--- stderr ---\n${run.stderr}`); + return JSON.parse(text.slice(start)) as Record; +} + +const stack = (picklist: string) => ({ + manifest: { id: 'com.example.pickdoor', name: 'pickdoor', version: '1.0.0', type: 'app', namespace: 'pickdoor' }, + picklists: [{ + name: 'industry', + label: 'Industry', + options: [ + { label: 'Technology', value: 'technology' }, + { label: 'Finance', value: 'finance' }, + ], + }], + objects: [{ + name: 'pickdoor_account', + label: 'Account', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + industry: { type: 'select', label: 'Industry', picklist }, + }, + }], + apps: [{ name: 'pickdoor_app', label: 'Pick Door' }], +}); + +let okDir: string; +let danglingDir: string; + +beforeAll(() => { + okDir = mkdtempSync(join(tmpdir(), 'os-picklist-ok-')); + writeDefineStackConfig(okDir, stack('industry')); + danglingDir = mkdtempSync(join(tmpdir(), 'os-picklist-dangling-')); + writeDefineStackConfig(danglingDir, stack('industy')); +}); + +afterAll(() => { + for (const dir of [okDir, danglingDir]) if (dir) rmSync(dir, { recursive: true, force: true }); +}); + +const picklistRules = (list: unknown): string[] => + (Array.isArray(list) ? list : []) + .map((w) => (w && typeof w === 'object' ? (w as { rule?: unknown }).rule : undefined)) + .filter((r): r is string => typeof r === 'string' && r.startsWith('picklist-reference-')); + +describe('a stack with a picklist and a field naming it passes all three doors', () => { + it('os validate accepts it, with no picklist-reference finding', async () => { + const run = await runCli(['validate', '--json'], okDir); + expect(run.code, run.stdout + run.stderr).toBe(0); + const doc = jsonOf(run); + expect(doc.valid).toBe(true); + expect(picklistRules(doc.warnings)).toEqual([]); + }, RUN_TIMEOUT_MS); + + it('os build writes the artifact, carrying the list and the reference as authored', async () => { + const out = join(okDir, 'dist', 'objectstack.json'); + const run = await runCli(['build', '--json', '-o', out], okDir); + expect(run.code, run.stdout + run.stderr).toBe(0); + expect(picklistRules(jsonOf(run).warnings)).toEqual([]); + const artifact = JSON.parse(readFileSync(out, 'utf8')) as { + picklists?: Array<{ name?: string }>; + objects?: Array<{ fields?: Record }>; + }; + expect(artifact.picklists?.map((p) => p.name)).toEqual(['industry']); + expect(artifact.objects?.[0]?.fields?.industry?.picklist).toBe('industry'); + }, RUN_TIMEOUT_MS); + + it('os lint reports no error, and R8 does not report the picklist-bound field', async () => { + const run = await runCli(['lint', '--json'], okDir); + expect(run.code, run.stdout + run.stderr).toBe(0); + const doc = jsonOf(run); + expect(doc.errors).toBe(0); + const issues = (doc.issues as Array<{ rule?: string }>) ?? []; + expect(issues.map((i) => i.rule)).not.toContain('field/select-missing-options'); + }, RUN_TIMEOUT_MS); +}); + +describe('a field naming a picklist the stack does not declare is refused', () => { + it('os validate refuses it, naming the field and the missing list', async () => { + const run = await runCli(['validate', '--json'], danglingDir); + expect(run.code, run.stdout + run.stderr).toBe(1); + const doc = jsonOf(run); + expect(doc.valid).toBe(false); + const errors = doc.errors as Array<{ rule?: string; where?: string; message?: string; path?: string }>; + expect(errors).toHaveLength(1); + expect(errors[0].rule).toBe('picklist-reference-unknown'); + expect(errors[0].where).toContain('pickdoor_account.industry'); + expect(errors[0].message).toContain("'industy'"); + expect(errors[0].path).toBe('objects[0].fields.industry.picklist'); + }, RUN_TIMEOUT_MS); + + it('the text face says so too, with the same exit status', async () => { + const run = await runCli(['validate'], danglingDir); + expect(run.code).toBe(1); + expect(run.stdout).toContain('pickdoor_account.industry'); + expect(run.stdout).toContain("'industy'"); + }, RUN_TIMEOUT_MS); + + it('os build refuses the same stack and writes no artifact', async () => { + const out = join(danglingDir, 'dist', 'objectstack.json'); + const run = await runCli(['build', '--json', '-o', out], danglingDir); + expect(run.code, run.stdout + run.stderr).toBe(1); + const errors = jsonOf(run).errors as Array<{ rule?: string }>; + expect(errors.map((e) => e.rule)).toEqual(['picklist-reference-unknown']); + expect(existsSync(out)).toBe(false); + }, RUN_TIMEOUT_MS); +}); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index ebd134d64d8..b0587fd3035 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -138,6 +138,15 @@ const SHARED_NON_REGISTRY_GATES: readonly string[] = [ // it — deleted there rather than reworded, for the reason the // `runPerPackageAuthoringRules` row above gives. 'findViewContainerNameRefusals', + // A field `picklist` that names no picklist the stack declares is refused + // (an `info` notice when the declaring package depends on packages outside + // the stack, whose picklists no command can read). Not a registry rule: the + // verdict depends on WHICH package declares the field — that package's + // declared dependencies decide refusal against notice — and the union run a + // registry rule is handed is the flattened top level, which carries no + // package provenance. The WALK is the load path's package reading, the same + // class as the row above; both doors make the call, right after the parse. + 'judgePicklistReferences', // [#20367 ruling B] One authoring shape: a default export no stack producer // built (`defineStack` / `composeStacks`) is refused right after load, before // any other judgement — the `STACK_*` cross-field refusals run inside the @@ -278,6 +287,10 @@ const NOT_A_GATE: Readonly> = { // gate above) already produced; the same record rides `--json` whether // this runs or not. 'printJsxGateNotices', + // Renders the `info` notices `judgePicklistReferences` (a gate above) + // already produced; the same records ride `--json` whether this runs or + // not. + 'printPicklistReferenceNotices', // [#18780] `compile.ts`' local once-guard around the line above it. It // decides WHEN that printer is called — after the per-package pass has // appended its survivors, so the closing `N author-time warning(s) — see diff --git a/packages/lint/src/data-model-rules.picklist-options-source.test.ts b/packages/lint/src/data-model-rules.picklist-options-source.test.ts new file mode 100644 index 00000000000..425f0d7d982 --- /dev/null +++ b/packages/lint/src/data-model-rules.picklist-options-source.test.ts @@ -0,0 +1,48 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * R8 `field/select-missing-options` counts a `picklist` reference as an options + * source, and its fix never prescribes `options` beside `picklist`. + * + * A field that names a shared list (`Field.select({ picklist: 'industry' })`) + * takes its options from that list and declares none of its own — and + * `FieldSchema` refuses `options` declared beside `picklist`. So R8 warning + * "has no options" on such a field, with a remedy of adding `options`, told + * the author to write the one shape the schema door refuses. + * + * The control is the half that keeps this from being a deletion: a select + * with NEITHER source still warns, and its fix names both sources as + * alternatives. + */ + +import { describe, expect, it } from 'vitest'; +import { lintDataModel } from './data-model-rules.js'; + +const R8 = 'field/select-missing-options'; + +const r8 = (fields: Record) => + lintDataModel([{ name: 'pk_account', nameField: 'name', fields: { name: { type: 'text' }, ...fields } }]) + .filter((issue) => issue.rule === R8); + +describe('R8 — a `picklist` reference is an options source', () => { + it.each(['select', 'multiselect', 'radio'])('a %s naming a picklist is not reported', (type) => { + expect(r8({ industry: { type, picklist: 'industry' } })).toEqual([]); + }); + + it('CONTROL: a select with neither `options` nor `picklist` still warns, at the field', () => { + const issues = r8({ industry: { type: 'select' } }); + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ severity: 'warning', path: 'objects[0].fields.industry.options' }); + }); + + it('the fix names both sources — `options` and `picklist` — not `options` alone', () => { + const [issue] = r8({ industry: { type: 'select' } }); + expect(issue.fix).toBeDefined(); + expect(issue.fix).toContain('`options'); + expect(issue.fix).toContain('`picklist'); + }); + + it('CONTROL: an empty `picklist` string is no source', () => { + expect(r8({ industry: { type: 'select', picklist: '' } })).toHaveLength(1); + }); +}); diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 23c933289bb..02b300d1bd6 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -627,9 +627,18 @@ export function lintDataModel(objects: any[]): LintIssue[] { const type = def.type; // R8 — option fields need options (or an options source). + // + // A `picklist` reference IS an options source: the field takes its + // options from the shared list it names (`data/picklist.zod.ts`), the + // same reading `FIELD_CHOICE_WITHOUT_OPTIONS` in the spec's + // functional-completeness rules makes. And the fix names the two + // sources as ALTERNATIVES: `FieldSchema` refuses `options` declared + // beside `picklist`, so a fix that said "add options" to a + // picklist-bound field would prescribe the schema's own refusal. if (OPTION_FIELD_TYPES.has(type)) { const hasOptions = (Array.isArray(def.options) && def.options.length > 0) || + (typeof def.picklist === 'string' && def.picklist !== '') || !!def.optionsFrom || !!def.dataSource || !!refOf(def); if (!hasOptions) { issues.push({ @@ -637,6 +646,10 @@ export function lintDataModel(objects: any[]): LintIssue[] { rule: 'field/select-missing-options', message: `${type} field "${obj.name}.${fieldName}" has no options`, path: `${fieldPath}.options`, + fix: + "Give the field ONE options source: inline `options: [{ label: '…', value: '…' }]`, or " + + "`picklist: ''` naming a shared list — never both, because a field declaring " + + '`options` beside `picklist` is refused.', }); } }