From 6f534f7125f306106a9567a1ef17c2a22843c210 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Fri, 11 Sep 2026 00:47:51 +0000 Subject: [PATCH 1/9] feat(shared): read items in pattern validation A pattern may now declare nodes and relationships under items as well as prefixItems, so an architecture can add members rather than only choose between them. declaration-paths.ts gains the two items sites, and every rule that resolves declarations picks them up without an edit of its own, because none of them names a site. items members are not mutually exclusive. Verified with Ajv: items admits any number of matching elements, so two members can both be built, while a prefixItems entry is one position and exactly one of its alternatives is built. Two items members must therefore not share a unique-id or an interface id, and the grouping helper is renamed exclusiveGroup to say which declarations actually compete. Adds pattern-decision-must-be-declared-in-prefix-items. An architecture contains every relationship declared in prefixItems, so a decision there is always asked. An items member may be left out, so a decision declared in items can vanish and an answer never gets to decline it. pattern-prefix-items-must-declare-one-keyword becomes pattern-choice-must-declare-one-keyword, because it now covers items blocks as well as prefixItems entries. Additive: every tracked pattern in the repository produces the same findings as before, because none of them uses items yet. --- PATTERN-DECISIONS.md | 33 +++++- cli/test_fixtures/validate_output_junit.xml | 7 +- ...cision-is-declared-in-prefix-items.spec.ts | 44 ++++++++ .../decision-is-declared-in-prefix-items.ts | 20 ++++ .../pattern/declaration-paths.spec.ts | 21 +++- .../functions/pattern/declaration-paths.ts | 46 +++++--- .../functions/pattern/ids-are-unique.ts | 6 +- .../pattern/is-defined-in-oneof-or-anyof.ts | 6 +- shared/src/spectral/rules-pattern.spec.ts | 106 +++++++++++++++++- shared/src/spectral/rules-pattern.ts | 23 ++-- 10 files changed, 266 insertions(+), 46 deletions(-) create mode 100644 shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.spec.ts create mode 100644 shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index 052b7dfdd..32acdee9e 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -13,15 +13,22 @@ gap. | alternative | One entry in a `oneOf` or an `anyOf` array. | | decision | A relationship that carries `relationship-type.properties.options`. A decision asks which alternatives to include. | -A pattern declares a node at three kinds of site: +A pattern declares a node at five kinds of site: | Site | Meaning | |---|---| | a `prefixItems` entry | one node, at that position | | `prefixItems[i].oneOf` | alternatives for that position | | `prefixItems[i].anyOf` | alternatives for that position | +| `items.oneOf` | nodes an architecture may add, none of them required | +| `items.anyOf` | nodes an architecture may add, none of them required | -A pattern declares a relationship at the same three sites. +A pattern declares a relationship at the same five sites. + +`prefixItems` and `items` differ in how many of their declarations reach one architecture. +A `prefixItems` entry is one position, so exactly one of its alternatives is built. `items` +describes every position after the last entry, so an architecture may build any number of +its members, or none. ## Rules that hold across all tools @@ -54,7 +61,7 @@ Tests: [`shared/src/spectral/rules-pattern.spec.ts`](shared/src/spectral/rules-p and the rule tests beside it in `shared/src/spectral/functions/pattern/`. `calm validate` reads every node and every relationship a pattern declares. It reads all -three declaration sites listed above. +five declaration sites listed above. `calm validate` reports these faults: @@ -65,14 +72,24 @@ three declaration sites listed above. | One name is used for more than one kind of thing | error | | The source of a connects relationship refers to a node that the pattern does not declare | error | | A connects relationship refers to an interface that the named node does not declare | error | -| A `prefixItems` entry declares both `oneOf` and `anyOf` | error | +| A `prefixItems` entry or an `items` block declares both `oneOf` and `anyOf` | error | +| A decision is declared in `items` rather than in `prefixItems` | error | | No relationship and no decision refers to a declared node | warning | `calm validate` does not read the destination of a connects relationship. A typo there is not reported. +Declare a decision in `relationships.prefixItems`. An architecture contains every +relationship a pattern declares there, so the decision is always asked. An architecture may +leave out an `items` member, so a decision declared in `items` can vanish, and an answer +never gets to decline it. + +Two `items` members may both be built, so they must not share a `unique-id`, and two nodes +declared there must not share an interface id. Two alternatives of one `prefixItems` entry +may, because only one of them is built. + `calm validate` reads one level of alternatives. It does not read alternatives declared -inside another alternative. The keyword check reads node and relationship entries, not +inside another alternative. The keyword check reads node and relationship sites, not interface entries. Do not give a `prefixItems` entry its own `properties` as well as alternatives. `calm @@ -82,3 +99,9 @@ the two halves share one. That catches the common case. It does not name the fau A pattern that declares alternatives inside an `allOf` branch is not supported. Two `allOf` branches that declare the same property discard one of the two declarations. + +## What generation guarantees + +`calm generate` does not read `items`. It builds one node per `prefixItems` entry and +nothing else, so a node declared in `items` never reaches the architecture. `calm validate` +accepts the pattern, and the missing node is silent. diff --git a/cli/test_fixtures/validate_output_junit.xml b/cli/test_fixtures/validate_output_junit.xml index a482fbe73..0727a2274 100644 --- a/cli/test_fixtures/validate_output_junit.xml +++ b/cli/test_fixtures/validate_output_junit.xml @@ -1,10 +1,10 @@ - + - @@ -41,7 +41,8 @@ - + + \ No newline at end of file diff --git a/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.spec.ts b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.spec.ts new file mode 100644 index 000000000..33174ae3a --- /dev/null +++ b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.spec.ts @@ -0,0 +1,44 @@ +import { RulesetFunctionContext } from '@stoplight/spectral-core'; +import { decisionIsDeclaredInPrefixItems } from './decision-is-declared-in-prefix-items'; + +const asContext = () => ({ path: ['properties', 'relationships', 'items', 'oneOf', 0] } as unknown as RulesetFunctionContext); + +const decision = (id: string) => ({ + properties: { + 'unique-id': { const: id }, + 'relationship-type': { properties: { options: { prefixItems: [] } } } + } +}); + +const connects = (id: string) => ({ + properties: { + 'unique-id': { const: id }, + 'relationship-type': { const: { connects: { source: { node: 'a' }, destination: { node: 'b' } } } } + } +}); + +describe('decisionIsDeclaredInPrefixItems', () => { + it('reports a decision', () => { + const results = decisionIsDeclaredInPrefixItems(decision('add-ons'), null, asContext()); + expect(results).toHaveLength(1); + expect(results[0].message).toContain('The decision \'add-ons\' is declared in items'); + }); + + it('reports at the path of the declaration', () => { + const results = decisionIsDeclaredInPrefixItems(decision('add-ons'), null, asContext()); + expect(results[0].path).toEqual(['properties', 'relationships', 'items', 'oneOf', 0]); + }); + + it('names the decision as unknown when it declares no id', () => { + const anonymous = { properties: { 'relationship-type': { properties: { options: {} } } } }; + expect(decisionIsDeclaredInPrefixItems(anonymous, null, asContext())[0].message).toContain('\'unknown\''); + }); + + it('accepts a relationship that is not a decision', () => { + expect(decisionIsDeclaredInPrefixItems(connects('w-d'), null, asContext())).toEqual([]); + }); + + it('accepts no input', () => { + expect(decisionIsDeclaredInPrefixItems(undefined, null, asContext())).toEqual([]); + }); +}); diff --git a/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts new file mode 100644 index 000000000..de3067a6f --- /dev/null +++ b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts @@ -0,0 +1,20 @@ +import { JSONPath } from 'jsonpath-plus'; +import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; +import { declaredId } from './declaration-paths'; + +/** + * An architecture always contains every relationship a pattern declares in prefixItems, so + * a decision declared there is always asked. An items member may be left out, so a + * decision declared there can vanish, and declining it stops being a choice. + */ +export function decisionIsDeclaredInPrefixItems(input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] { + const relationship = input as object; + if (!relationship || JSONPath({ path: '$.properties.relationship-type.properties.options', json: relationship }).length === 0) { + return []; + } + + return [{ + message: `The decision '${declaredId(relationship) ?? 'unknown'}' is declared in items. Declare a decision in relationships prefixItems, so that an answer can decline it.`, + path: [...context.path], + }]; +} diff --git a/shared/src/spectral/functions/pattern/declaration-paths.spec.ts b/shared/src/spectral/functions/pattern/declaration-paths.spec.ts index 4413d9532..5e3efe80d 100644 --- a/shared/src/spectral/functions/pattern/declaration-paths.spec.ts +++ b/shared/src/spectral/functions/pattern/declaration-paths.spec.ts @@ -1,4 +1,4 @@ -import { alternativeIdPaths, containingDeclaration, containingEntry, declarationPaths, declaredId, declaredIdPaths, declaredInterfaceIdPaths, fixedIdPath, isAlternative } from './declaration-paths'; +import { choiceIdPaths, containingDeclaration, declarationPaths, declaredId, declaredIdPaths, declaredInterfaceIdPaths, exclusiveGroup, fixedIdPath, isAlternative, twoKeywordSites } from './declaration-paths'; const ENTRY = '/properties/nodes/prefixItems/0'; const ALTERNATIVE = `${ENTRY}/oneOf/1`; @@ -6,16 +6,27 @@ const ID = '/properties/unique-id/const'; const INTERFACE_ID = '/properties/interfaces/prefixItems/0/properties/unique-id/const'; describe('declaration paths', () => { - it('covers the fixed entry and both alternative keywords', () => { + it('covers the fixed entry, both entry keywords and both items keywords', () => { expect(declarationPaths('relationships')).toEqual([ '$.properties.relationships.prefixItems[*]', '$.properties.relationships.prefixItems[*].oneOf[*]', - '$.properties.relationships.prefixItems[*].anyOf[*]' + '$.properties.relationships.prefixItems[*].anyOf[*]', + '$.properties.relationships.items.oneOf[*]', + '$.properties.relationships.items.anyOf[*]' + ]); + }); + + it('names every site a choice can declare both keywords at', () => { + expect(twoKeywordSites()).toEqual([ + '$.properties.nodes.prefixItems[?(@.oneOf && @.anyOf)]', + '$.properties.nodes[?(@property === "items" && @.oneOf && @.anyOf)]', + '$.properties.relationships.prefixItems[?(@.oneOf && @.anyOf)]', + '$.properties.relationships[?(@property === "items" && @.oneOf && @.anyOf)]' ]); }); it('builds id paths from the fixed entry followed by the alternatives', () => { - expect(declaredIdPaths('nodes')).toEqual([fixedIdPath('nodes'), ...alternativeIdPaths('nodes')]); + expect(declaredIdPaths('nodes')).toEqual([fixedIdPath('nodes'), ...choiceIdPaths('nodes')]); }); it('reaches interfaces on every node declaration site', () => { @@ -39,7 +50,7 @@ describe('declaration pointers', () => { '/properties/relationships/prefixItems/2/anyOf/0', '/properties/relationships/prefixItems/2', true], ])('resolves %s', (_name, pointer, declaration, entry, alternative) => { expect(containingDeclaration(pointer)).toBe(declaration); - expect(containingEntry(pointer)).toBe(entry); + expect(exclusiveGroup(pointer)).toBe(entry); expect(isAlternative(pointer)).toBe(alternative); }); diff --git a/shared/src/spectral/functions/pattern/declaration-paths.ts b/shared/src/spectral/functions/pattern/declaration-paths.ts index 3ddad86dd..9bc1c5cc0 100644 --- a/shared/src/spectral/functions/pattern/declaration-paths.ts +++ b/shared/src/spectral/functions/pattern/declaration-paths.ts @@ -7,44 +7,56 @@ const ID = 'properties.unique-id.const'; const INTERFACES = 'properties.interfaces.prefixItems[*]'; const ALTERNATIVES = `(?:${ALTERNATIVE_KEYWORDS.join('|')})`; -const DECLARATION_POINTER = new RegExp(`^/properties/(?:nodes|relationships)/prefixItems/\\d+(?:/${ALTERNATIVES}/\\d+)?`); -const ALTERNATIVE_SUFFIX = new RegExp(`/${ALTERNATIVES}/\\d+$`); +const CALM_TYPE = '(?:nodes|relationships)'; +const DECLARATION = new RegExp(`^/properties/${CALM_TYPE}/(?:prefixItems/\\d+(?:/${ALTERNATIVES}/\\d+)?|items/${ALTERNATIVES}/\\d+)`); +const ENTRY_ALTERNATIVE = new RegExp(`^(/properties/${CALM_TYPE}/prefixItems/\\d+)/${ALTERNATIVES}/\\d+$`); function entryPath(calmType: CalmType): string { return `$.properties.${calmType}.prefixItems[*]`; } -function alternativePaths(calmType: CalmType): string[] { - return ALTERNATIVE_KEYWORDS.map(keyword => `${entryPath(calmType)}.${keyword}[*]`); +function choicePaths(calmType: CalmType): string[] { + return [entryPath(calmType), `$.properties.${calmType}.items`].flatMap(base => + ALTERNATIVE_KEYWORDS.map(keyword => `${base}.${keyword}[*]`)); } /** * Shared so that the rules resolving declarations cannot disagree about where they are. * * The paths below find declarations. A query run with `resultType: 'all'` returns each hit - * with the JSON Pointer it was found at, and the `containing` helpers read that pointer - * back, because it is the only surviving trace of which entry the hit came from. + * with the JSON Pointer it was found at, and the pointer helpers read that pointer back, + * because it is the only surviving trace of which site the hit came from. */ export function declarationPaths(calmType: CalmType): string[] { - return [entryPath(calmType), ...alternativePaths(calmType)]; + return [entryPath(calmType), ...choicePaths(calmType)]; } export function fixedIdPath(calmType: CalmType): string { return `${entryPath(calmType)}.${ID}`; } -export function alternativeIdPaths(calmType: CalmType): string[] { - return alternativePaths(calmType).map(path => `${path}.${ID}`); +export function choiceIdPaths(calmType: CalmType): string[] { + return choicePaths(calmType).map(path => `${path}.${ID}`); } export function declaredIdPaths(calmType: CalmType): string[] { - return [fixedIdPath(calmType), ...alternativeIdPaths(calmType)]; + return [fixedIdPath(calmType), ...choiceIdPaths(calmType)]; } export function declaredInterfaceIdPaths(): string[] { return declarationPaths('nodes').map(path => `${path}.${INTERFACES}.${ID}`); } +/** + * Every site at which a pattern can declare both keywords, as Spectral `given` selectors. + */ +export function twoKeywordSites(): string[] { + return (['nodes', 'relationships'] as CalmType[]).flatMap(calmType => [ + `$.properties.${calmType}.prefixItems[?(@.oneOf && @.anyOf)]`, + `$.properties.${calmType}[?(@property === "items" && @.oneOf && @.anyOf)]`, + ]); +} + export function declaredId(declaration: object): string | undefined { return get(declaration, ID); } @@ -53,15 +65,21 @@ export function declaredId(declaration: object): string | undefined { * A pointer from outside these paths has no declaration, so it stands alone. */ export function containingDeclaration(pointer: string): string { - return pointer.match(DECLARATION_POINTER)?.[0] ?? pointer; + return pointer.match(DECLARATION)?.[0] ?? pointer; } -export function containingEntry(pointer: string): string { - return containingDeclaration(pointer).split(ALTERNATIVE_SUFFIX)[0]; +/** + * Declarations sharing a group never appear in the same architecture. Only the + * alternatives of one prefixItems entry qualify, because the entry is one position and one + * of them wins. An items member competes with nothing, since items admits any number. + */ +export function exclusiveGroup(pointer: string): string { + const declaration = containingDeclaration(pointer); + return declaration.match(ENTRY_ALTERNATIVE)?.[1] ?? declaration; } export function isAlternative(pointer: string): boolean { - return containingDeclaration(pointer) !== containingEntry(pointer); + return containingDeclaration(pointer) !== exclusiveGroup(pointer); } function declarationIndices(pointer: string): number[] { diff --git a/shared/src/spectral/functions/pattern/ids-are-unique.ts b/shared/src/spectral/functions/pattern/ids-are-unique.ts index c7086ce72..0a0d82c68 100644 --- a/shared/src/spectral/functions/pattern/ids-are-unique.ts +++ b/shared/src/spectral/functions/pattern/ids-are-unique.ts @@ -2,7 +2,7 @@ import { JSONPath } from 'jsonpath-plus'; import { groupBy, partition } from 'lodash'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; import { detectDuplicates, JSONPathMatch } from '../helper-functions'; -import { byBuildOrder, containingDeclaration, containingEntry, declaredIdPaths, declaredInterfaceIdPaths, isAlternative } from './declaration-paths'; +import { byBuildOrder, containingDeclaration, declaredIdPaths, declaredInterfaceIdPaths, exclusiveGroup, isAlternative } from './declaration-paths'; /** * The rule blames the second declaration it sees, but one query per declaration site means @@ -22,8 +22,8 @@ function groupMatches(matches: JSONPathMatch[], key: (pointer: string) => string * prefixItems entry is ever chosen, so alternatives may repeat an interface id. */ function detectDuplicateInterfaceIds(matches: JSONPathMatch[], seenIds: Set, messages: IFunctionResult[]) { - for (const entry of groupMatches(matches, containingEntry)) { - const [choices, fixed] = partition(entry, match => isAlternative(match.pointer)); + for (const group of groupMatches(matches, exclusiveGroup)) { + const [choices, fixed] = partition(group, match => isAlternative(match.pointer)); detectDuplicates(fixed, seenIds, messages); groupMatches(choices, containingDeclaration).forEach(choice => detectDuplicates(choice, new Set(seenIds), messages)); diff --git a/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.ts b/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.ts index 265e22408..61f45773e 100644 --- a/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.ts +++ b/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.ts @@ -1,6 +1,6 @@ import { JSONPath } from 'jsonpath-plus'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; -import { alternativeIdPaths, CalmType, fixedIdPath } from './declaration-paths'; +import { CalmType, choiceIdPaths, fixedIdPath } from './declaration-paths'; /** * Checks that the input value should be defined in a oneOf or anyOf block. */ @@ -10,12 +10,12 @@ export function isDefinedInOneOfOrAnyOf(input: unknown, { calmType }: { calmType } const fixed = JSONPath({ path: fixedIdPath(calmType), json: context.document.data as object }); - const inAlternatives = alternativeIdPaths(calmType).flatMap(path => + const inChoices = choiceIdPaths(calmType).flatMap(path => JSONPath({ path, json: context.document.data as object })); const results: IFunctionResult[] = []; - if (fixed.includes(input) && !inAlternatives.includes(input)) { + if (fixed.includes(input) && !inChoices.includes(input)) { results.push({ message: `'${input}' is part of a pattern option and must be defined in a oneOf or anyOf block.`, path: [...context.path], diff --git a/shared/src/spectral/rules-pattern.spec.ts b/shared/src/spectral/rules-pattern.spec.ts index 98533acde..215fdb07d 100644 --- a/shared/src/spectral/rules-pattern.spec.ts +++ b/shared/src/spectral/rules-pattern.spec.ts @@ -75,7 +75,7 @@ describe('pattern ruleset', () => { }); }); - describe('pattern-prefix-items-must-declare-one-keyword', () => { + describe('pattern-choice-must-declare-one-keyword', () => { it('accepts an entry that declares one keyword', async () => { const codes = await codesFor({ properties: { @@ -83,7 +83,7 @@ describe('pattern ruleset', () => { relationships: { prefixItems: [] } } }); - expect(codes).not.toContain('pattern-prefix-items-must-declare-one-keyword'); + expect(codes).not.toContain('pattern-choice-must-declare-one-keyword'); }); it('rejects an entry that declares both keywords', async () => { @@ -93,7 +93,7 @@ describe('pattern ruleset', () => { relationships: { prefixItems: [] } } }); - expect(codes).toContain('pattern-prefix-items-must-declare-one-keyword'); + expect(codes).toContain('pattern-choice-must-declare-one-keyword'); }); it('rejects a relationships entry that declares both keywords', async () => { @@ -103,7 +103,7 @@ describe('pattern ruleset', () => { relationships: { prefixItems: [{ oneOf: [node('a')], anyOf: [node('b')] }] } } }); - expect(codes).toContain('pattern-prefix-items-must-declare-one-keyword'); + expect(codes).toContain('pattern-choice-must-declare-one-keyword'); }); it('reports the location of the offending entry', async () => { @@ -112,7 +112,7 @@ describe('pattern ruleset', () => { nodes: { prefixItems: [{ oneOf: [node('cache')], anyOf: [node('queue')] }] }, relationships: { prefixItems: [] } } - }, 'pattern-prefix-items-must-declare-one-keyword'); + }, 'pattern-choice-must-declare-one-keyword'); expect(path).toBe('/properties/nodes/prefixItems/0'); }); }); @@ -138,4 +138,100 @@ describe('pattern ruleset', () => { expect(codes).toContain('connects-relationship-references-existing-nodes-in-pattern'); }); }); + describe('items', () => { + it('resolves a node declared in items', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [connects('w-c', 'cache', 'webapp'), decision('add-ons', ['cache'])] } + } + }); + expect(codes).not.toContain('connects-relationship-references-existing-nodes-in-pattern'); + expect(codes).not.toContain('pattern-nodes-must-be-referenced'); + }); + + it('warns about an items member that nothing references', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp'), node('database')], items: { oneOf: [node('orphan')] } }, + relationships: { prefixItems: [connects('w-d', 'webapp', 'database')] } + } + }); + expect(codes).toContain('pattern-nodes-must-be-referenced'); + }); + + it('reports an id shared by two items members, which can both appear', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('dup'), node('dup')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain('unique-ids-must-be-unique-in-pattern'); + }); + + it('reports an interface id shared by two items members, which can both appear', async () => { + const cache = { properties: { 'unique-id': { const: 'cache' }, interfaces: { prefixItems: [{ properties: { 'unique-id': { const: 'port' } } }] } } }; + const queue = { properties: { 'unique-id': { const: 'queue' }, interfaces: { prefixItems: [{ properties: { 'unique-id': { const: 'port' } } }] } } }; + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [cache, queue] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain('unique-ids-must-be-unique-in-pattern'); + }); + + it('rejects an items block declaring both keywords', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')], anyOf: [node('queue')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain('pattern-choice-must-declare-one-keyword'); + }); + + it('reports the location of the offending items block', async () => { + const path = await pathFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')], anyOf: [node('queue')] } }, + relationships: { prefixItems: [] } + } + }, 'pattern-choice-must-declare-one-keyword'); + expect(path).toBe('/properties/nodes/items'); + }); + }); + + describe('pattern-decision-must-be-declared-in-prefix-items', () => { + it('accepts a decision declared in relationships prefixItems', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [decision('add-ons', ['cache'])] } + } + }); + expect(codes).not.toContain('pattern-decision-must-be-declared-in-prefix-items'); + }); + + it('rejects a decision declared in relationships items', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [], items: { oneOf: [decision('add-ons', ['cache'])] } } + } + }); + expect(codes).toContain('pattern-decision-must-be-declared-in-prefix-items'); + }); + + it('accepts a plain relationship declared in relationships items', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp'), node('database')] }, + relationships: { prefixItems: [], items: { anyOf: [connects('w-d', 'webapp', 'database')] } } + } + }); + expect(codes).not.toContain('pattern-decision-must-be-declared-in-prefix-items'); + }); + }); }); diff --git a/shared/src/spectral/rules-pattern.ts b/shared/src/spectral/rules-pattern.ts index b2cdbb48a..334d4ad54 100644 --- a/shared/src/spectral/rules-pattern.ts +++ b/shared/src/spectral/rules-pattern.ts @@ -7,7 +7,8 @@ import nodeHasRelationship from './functions/pattern/node-has-relationship'; import { interfaceIdExists } from './functions/pattern/interface-id-exists'; import { interfaceIdExistsOnNode } from './functions/pattern/interface-id-exists-on-node'; import { isDefinedInOneOfOrAnyOf } from './functions/pattern/is-defined-in-oneof-or-anyof'; -import { declaredIdPaths } from './functions/pattern/declaration-paths'; +import { decisionIsDeclaredInPrefixItems } from './functions/pattern/decision-is-declared-in-prefix-items'; +import { declaredIdPaths, twoKeywordSites } from './functions/pattern/declaration-paths'; const patternRules: RulesetDefinition = { @@ -188,18 +189,24 @@ const patternRules: RulesetDefinition = { }, }, }, - 'pattern-prefix-items-must-declare-one-keyword': { - description: 'A prefixItems entry must declare either oneOf or anyOf, not both', + 'pattern-choice-must-declare-one-keyword': { + description: 'A choice must declare either oneOf or anyOf, not both', severity: 'error', - message: 'A prefixItems entry declares both \'oneOf\' and \'anyOf\'. An element must satisfy both, so some alternatives can never be selected. Declare one keyword.', - given: [ - '$.properties.nodes.prefixItems[?(@.oneOf && @.anyOf)]', - '$.properties.relationships.prefixItems[?(@.oneOf && @.anyOf)]', - ], + message: 'A choice declares both \'oneOf\' and \'anyOf\'. An element must satisfy both, so some alternatives can never be selected. Declare one keyword.', + given: twoKeywordSites(), then: { function: falsy, }, }, + 'pattern-decision-must-be-declared-in-prefix-items': { + description: 'A decision must be declared in relationships prefixItems, not in items', + severity: 'error', + message: '{{error}}', + given: ['$.properties.relationships.items.oneOf[*]', '$.properties.relationships.items.anyOf[*]'], + then: { + function: decisionIsDeclaredInPrefixItems, + }, + }, 'pattern-option-relationship-must-have-max-one-item': { description: 'Options relationships must have max one item', severity: 'error', From e03257e74304c60c18e6e51f4a2622180aa8d336 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Fri, 11 Sep 2026 16:12:47 +0000 Subject: [PATCH 2/9] fix(shared): order items declarations after prefixItems entries Duplicate ids were blamed on the earlier declaration whenever the pair spanned prefixItems and items, because nothing ranked the two sites. That broke the contract 960fa4fc set, which is that the rule names the later declaration. Ordering is a property of the site: prefixItems fills the first positions and items fills everything after. The rank joins the parsed indices that byBuildOrder already compares. Every ordering test compared two declarations at the same kind of site, which is why none of them caught it. Adds three that span both. Also records two limits the contract overstated. A decision declared as one alternative of an entry can vanish just as one declared in items can, and calm validate reports only the items case. A node declared under items must sit inside oneOf or anyOf, because items applies one schema to every position after the entries. Renames entryPath to fixedPath so it pairs with fixedIdPath, and states isAlternative against the regex rather than deriving it from two other helpers. --- PATTERN-DECISIONS.md | 13 +++-- .../functions/pattern/declaration-paths.ts | 29 +++++----- .../functions/pattern/ids-are-unique.spec.ts | 56 +++++++++++++++++++ 3 files changed, 80 insertions(+), 18 deletions(-) diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index 32acdee9e..b5b457b27 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -23,6 +23,11 @@ A pattern declares a node at five kinds of site: | `items.oneOf` | nodes an architecture may add, none of them required | | `items.anyOf` | nodes an architecture may add, none of them required | +A node declared under `items` must sit inside `oneOf` or `anyOf`. `items` applies one +schema to every position after the entries, so a node declared directly would force every +added node to be that same node. An `items` schema that declares a node directly is not +read. + A pattern declares a relationship at the same five sites. `prefixItems` and `items` differ in how many of their declarations reach one architecture. @@ -79,10 +84,10 @@ five declaration sites listed above. `calm validate` does not read the destination of a connects relationship. A typo there is not reported. -Declare a decision in `relationships.prefixItems`. An architecture contains every -relationship a pattern declares there, so the decision is always asked. An architecture may -leave out an `items` member, so a decision declared in `items` can vanish, and an answer -never gets to decline it. +Declare a decision as a plain entry in `relationships.prefixItems`. An architecture +contains every relationship declared at a fixed position there, so the decision is always +asked. A decision declared in `items`, or as one alternative of an entry, can be left out, +and an answer never gets to decline it. `calm validate` reports the `items` case only. Two `items` members may both be built, so they must not share a `unique-id`, and two nodes declared there must not share an interface id. Two alternatives of one `prefixItems` entry diff --git a/shared/src/spectral/functions/pattern/declaration-paths.ts b/shared/src/spectral/functions/pattern/declaration-paths.ts index 9bc1c5cc0..9ab1f6790 100644 --- a/shared/src/spectral/functions/pattern/declaration-paths.ts +++ b/shared/src/spectral/functions/pattern/declaration-paths.ts @@ -11,28 +11,24 @@ const CALM_TYPE = '(?:nodes|relationships)'; const DECLARATION = new RegExp(`^/properties/${CALM_TYPE}/(?:prefixItems/\\d+(?:/${ALTERNATIVES}/\\d+)?|items/${ALTERNATIVES}/\\d+)`); const ENTRY_ALTERNATIVE = new RegExp(`^(/properties/${CALM_TYPE}/prefixItems/\\d+)/${ALTERNATIVES}/\\d+$`); -function entryPath(calmType: CalmType): string { +function fixedPath(calmType: CalmType): string { return `$.properties.${calmType}.prefixItems[*]`; } function choicePaths(calmType: CalmType): string[] { - return [entryPath(calmType), `$.properties.${calmType}.items`].flatMap(base => + return [fixedPath(calmType), `$.properties.${calmType}.items`].flatMap(base => ALTERNATIVE_KEYWORDS.map(keyword => `${base}.${keyword}[*]`)); } /** * Shared so that the rules resolving declarations cannot disagree about where they are. - * - * The paths below find declarations. A query run with `resultType: 'all'` returns each hit - * with the JSON Pointer it was found at, and the pointer helpers read that pointer back, - * because it is the only surviving trace of which site the hit came from. */ export function declarationPaths(calmType: CalmType): string[] { - return [entryPath(calmType), ...choicePaths(calmType)]; + return [fixedPath(calmType), ...choicePaths(calmType)]; } export function fixedIdPath(calmType: CalmType): string { - return `${entryPath(calmType)}.${ID}`; + return `${fixedPath(calmType)}.${ID}`; } export function choiceIdPaths(calmType: CalmType): string[] { @@ -61,6 +57,9 @@ export function declaredId(declaration: object): string | undefined { return get(declaration, ID); } +// Reading a pointer back. A query run with `resultType: 'all'` returns each hit with the +// JSON Pointer it was found at, the only surviving trace of which site the hit came from. + /** * A pointer from outside these paths has no declaration, so it stands alone. */ @@ -79,18 +78,20 @@ export function exclusiveGroup(pointer: string): string { } export function isAlternative(pointer: string): boolean { - return containingDeclaration(pointer) !== exclusiveGroup(pointer); + return ENTRY_ALTERNATIVE.test(containingDeclaration(pointer)); } function declarationIndices(pointer: string): number[] { - return (containingDeclaration(pointer).match(/\d+/g) ?? []).map(Number); + const declaration = containingDeclaration(pointer); + const indices = (declaration.match(/\d+/g) ?? []).map(Number); + return [declaration.includes('/items/') ? 1 : 0, ...indices]; } /** - * Orders declarations as an architecture fills the array. The indices decide it, not the - * pointer text: sorting the text puts an alternative ahead of the entry that holds it, - * because "oneOf" precedes "properties". A declaration with fewer indices contains the - * other, so it comes first. + * Orders declarations as an architecture fills the array: every prefixItems entry, then + * every items member. The indices decide it, not the pointer text, which sorts "items" + * ahead of "prefixItems" and an alternative ahead of the entry that holds it. A + * declaration with fewer indices contains the other, so it comes first. */ export function byBuildOrder(left: string, right: string): number { const [first, second] = [left, right].map(declarationIndices); diff --git a/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts b/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts index 8f836b7a8..a839fac15 100644 --- a/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts +++ b/shared/src/spectral/functions/pattern/ids-are-unique.spec.ts @@ -569,4 +569,60 @@ describe('idsAreUnique', () => { const result = idsAreUnique(input, null, asContext(context)); expect(result[0].message).toContain('path: /properties/nodes/prefixItems/11/properties/unique-id/const'); }); + it('should blame the items member when a fixed entry declares the same id', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + prefixItems: [{ 'properties': { 'unique-id': { 'const': 'dup' } } }], + items: { 'oneOf': [{ 'properties': { 'unique-id': { 'const': 'dup' } } }] } + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result[0].message).toContain('path: /properties/nodes/items/oneOf/0/properties/unique-id/const'); + }); + + it('should blame the items member however the pattern orders its keys', () => { + const input = {}; + const context = { + document: { + data: { + properties: { + nodes: { + items: { 'oneOf': [{ 'properties': { 'unique-id': { 'const': 'dup' } } }] }, + prefixItems: [{ 'properties': { 'unique-id': { 'const': 'dup' } } }] + } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result[0].message).toContain('path: /properties/nodes/items/oneOf/0/properties/unique-id/const'); + }); + + it('should blame the items member for an interface id a fixed node already declares', () => { + const input = {}; + const withPort = (id: string) => ({ 'properties': { + 'unique-id': { 'const': id }, + 'interfaces': { prefixItems: [{ 'properties': { 'unique-id': { 'const': 'port' } } }] } } }); + const context = { + document: { + data: { + properties: { + nodes: { prefixItems: [withPort('database')], items: { 'oneOf': [withPort('cache')] } } + } + } + } + }; + + const result = idsAreUnique(input, null, asContext(context)); + expect(result[0].message).toContain('path: /properties/nodes/items/oneOf/0/properties/interfaces/prefixItems/0/properties/unique-id/const'); + }); }); From c6425efecbf3b4b1d45bc00906455bed30c061f4 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Fri, 11 Sep 2026 17:10:39 +0000 Subject: [PATCH 3/9] feat(shared): reject maxItems that leaves no room for items maxItems counts the whole array and the prefixItems entries fill it from the front, so an items member can only be built in what is left over. A pattern that pins maxItems to the number of entries can never build one, and calm generate produced an architecture that failed its own pattern on maxItems. The narrowed pattern cannot fix this. calm generate does not record a decision's options in the architecture, so extractChoicesFromArchitecture finds nothing and validation runs against the pattern as written. --- PATTERN-DECISIONS.md | 4 ++ cli/test_fixtures/validate_output_junit.xml | 5 +- .../items-fit-within-max-items.spec.ts | 47 +++++++++++++++++++ .../pattern/items-fit-within-max-items.ts | 26 ++++++++++ shared/src/spectral/rules-pattern.spec.ts | 31 ++++++++++++ shared/src/spectral/rules-pattern.ts | 10 ++++ 6 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts create mode 100644 shared/src/spectral/functions/pattern/items-fit-within-max-items.ts diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index b5b457b27..a3d694622 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -23,6 +23,9 @@ A pattern declares a node at five kinds of site: | `items.oneOf` | nodes an architecture may add, none of them required | | `items.anyOf` | nodes an architecture may add, none of them required | +`maxItems` counts the whole array and the `prefixItems` entries fill it from the front, so +a pattern that wants `items` must leave room for them. + A node declared under `items` must sit inside `oneOf` or `anyOf`. `items` applies one schema to every position after the entries, so a node declared directly would force every added node to be that same node. An `items` schema that declares a node directly is not @@ -79,6 +82,7 @@ five declaration sites listed above. | A connects relationship refers to an interface that the named node does not declare | error | | A `prefixItems` entry or an `items` block declares both `oneOf` and `anyOf` | error | | A decision is declared in `items` rather than in `prefixItems` | error | +| `maxItems` leaves no room for an `items` member | error | | No relationship and no decision refers to a declared node | warning | `calm validate` does not read the destination of a connects relationship. A typo there is diff --git a/cli/test_fixtures/validate_output_junit.xml b/cli/test_fixtures/validate_output_junit.xml index 0727a2274..b76c34d4b 100644 --- a/cli/test_fixtures/validate_output_junit.xml +++ b/cli/test_fixtures/validate_output_junit.xml @@ -1,10 +1,10 @@ - + - @@ -42,6 +42,7 @@ + diff --git a/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts b/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts new file mode 100644 index 000000000..9e0ba6579 --- /dev/null +++ b/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts @@ -0,0 +1,47 @@ +import { RulesetFunctionContext } from '@stoplight/spectral-core'; +import { itemsFitWithinMaxItems } from './items-fit-within-max-items'; + +const asContext = () => ({ path: ['properties', 'nodes'] } as unknown as RulesetFunctionContext); +const run = (array: unknown) => itemsFitWithinMaxItems(array, null, asContext()); + +const choice = { oneOf: [{}] }; + +describe('itemsFitWithinMaxItems', () => { + it('reports an array whose entries already fill maxItems', () => { + const result = run({ prefixItems: [{}], maxItems: 1, items: choice }); + expect(result).toHaveLength(1); + expect(result[0].message).toContain('maxItems is 1 and 1 prefixItems entries already fill it'); + }); + + it('reports an array with no entries and no room', () => { + expect(run({ maxItems: 0, items: choice })).toHaveLength(1); + }); + + it('reports at the path of the array', () => { + expect(run({ prefixItems: [{}], maxItems: 1, items: choice })[0].path).toEqual(['properties', 'nodes']); + }); + + it('accepts an array with room to spare', () => { + expect(run({ prefixItems: [{}], maxItems: 2, items: choice })).toEqual([]); + }); + + it('accepts an array that declares no maxItems', () => { + expect(run({ prefixItems: [{}], items: choice })).toEqual([]); + }); + + it('accepts an array with no items block', () => { + expect(run({ prefixItems: [{}], maxItems: 1 })).toEqual([]); + }); + + it('accepts an items block that offers no choice', () => { + expect(run({ prefixItems: [{}], maxItems: 1, items: { $ref: 'core.json#/defs/node' } })).toEqual([]); + }); + + it('accepts an anyOf items block with room', () => { + expect(run({ prefixItems: [{}], maxItems: 3, items: { anyOf: [{}] } })).toEqual([]); + }); + + it('accepts no input', () => { + expect(run(undefined)).toEqual([]); + }); +}); diff --git a/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts b/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts new file mode 100644 index 000000000..cf9e62e8c --- /dev/null +++ b/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts @@ -0,0 +1,26 @@ +import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; + +interface CalmArray { + maxItems?: number; + prefixItems?: unknown[]; + items?: { oneOf?: unknown[]; anyOf?: unknown[] }; +} + +/** + * `maxItems` counts the whole array, and the `prefixItems` entries fill it from the front. + * An `items` member can only be built in what is left over. + */ +export function itemsFitWithinMaxItems(input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] { + const array = input as CalmArray; + const offersChoice = Boolean(array?.items?.oneOf || array?.items?.anyOf); + const positions = array?.prefixItems?.length ?? 0; + + if (!offersChoice || array.maxItems === undefined || array.maxItems > positions) { + return []; + } + + return [{ + message: `maxItems is ${array.maxItems} and ${positions} prefixItems entries already fill it, so no items member can ever be built. Raise maxItems or remove items.`, + path: [...context.path], + }]; +} diff --git a/shared/src/spectral/rules-pattern.spec.ts b/shared/src/spectral/rules-pattern.spec.ts index 215fdb07d..7194f6912 100644 --- a/shared/src/spectral/rules-pattern.spec.ts +++ b/shared/src/spectral/rules-pattern.spec.ts @@ -234,4 +234,35 @@ describe('pattern ruleset', () => { expect(codes).not.toContain('pattern-decision-must-be-declared-in-prefix-items'); }); }); + describe('pattern-items-must-fit-within-max-items', () => { + it('rejects a pattern whose maxItems leaves no room for items', async () => { + const codes = await codesFor({ + properties: { + nodes: { minItems: 1, maxItems: 1, prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).toContain('pattern-items-must-fit-within-max-items'); + }); + + it('accepts a pattern whose maxItems leaves room', async () => { + const codes = await codesFor({ + properties: { + nodes: { minItems: 1, maxItems: 2, prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).not.toContain('pattern-items-must-fit-within-max-items'); + }); + + it('accepts a pattern that declares no maxItems', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [] } + } + }); + expect(codes).not.toContain('pattern-items-must-fit-within-max-items'); + }); + }); }); diff --git a/shared/src/spectral/rules-pattern.ts b/shared/src/spectral/rules-pattern.ts index 334d4ad54..f563ee416 100644 --- a/shared/src/spectral/rules-pattern.ts +++ b/shared/src/spectral/rules-pattern.ts @@ -8,6 +8,7 @@ import { interfaceIdExists } from './functions/pattern/interface-id-exists'; import { interfaceIdExistsOnNode } from './functions/pattern/interface-id-exists-on-node'; import { isDefinedInOneOfOrAnyOf } from './functions/pattern/is-defined-in-oneof-or-anyof'; import { decisionIsDeclaredInPrefixItems } from './functions/pattern/decision-is-declared-in-prefix-items'; +import { itemsFitWithinMaxItems } from './functions/pattern/items-fit-within-max-items'; import { declaredIdPaths, twoKeywordSites } from './functions/pattern/declaration-paths'; @@ -198,6 +199,15 @@ const patternRules: RulesetDefinition = { function: falsy, }, }, + 'pattern-items-must-fit-within-max-items': { + description: 'maxItems must leave room for an items member to be built', + severity: 'error', + message: '{{error}}', + given: ['$.properties.nodes', '$.properties.relationships'], + then: { + function: itemsFitWithinMaxItems, + }, + }, 'pattern-decision-must-be-declared-in-prefix-items': { description: 'A decision must be declared in relationships prefixItems, not in items', severity: 'error', From 80cc8fb080af6e5f29b3a3777785610cf848bc3c Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Sat, 12 Sep 2026 14:27:32 +0000 Subject: [PATCH 4/9] docs: record three limits the contract did not state An items block cannot limit how many times one member is used, so two positions may both match it and the architecture holds one unique-id twice. The pattern is correct, so calm validate cannot report it; unique-ids-must-be-unique-in-architecture reports it later. prefixItems is positional, so an architecture lists those elements first and in order. Pre-existing, but items is the first construct that makes an author think about array positions. minItems: 0 is rejected, because pattern-has-no-empty-properties reads a zero as a placeholder. Zero is the default, so the keyword can be dropped. Issue #2859 writes the catalog with minItems: 0. --- PATTERN-DECISIONS.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index a3d694622..865a54c27 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -58,6 +58,10 @@ Neither keyword controls how many alternatives an architecture includes. A `pref entry is one position, so it takes one alternative. `minItems` and `maxItems` on the array set the bounds. +`prefixItems` is positional. An architecture lists the elements a pattern declares there +first, and in the order the pattern declares them. Members built from `items` follow. An +architecture that reorders the positional elements does not match the pattern. + `oneOf` and `anyOf` do not differ for CALM alternatives. Each alternative pins a distinct `unique-id`, so an element matches at most one of them, and "exactly one" and "at least one" become the same test. The visualiser prints the keyword as the label on the decision box, so @@ -65,9 +69,6 @@ the choice is visible to a reader. It changes no validation. ## What validation guarantees -Tests: [`shared/src/spectral/rules-pattern.spec.ts`](shared/src/spectral/rules-pattern.spec.ts) -and the rule tests beside it in `shared/src/spectral/functions/pattern/`. - `calm validate` reads every node and every relationship a pattern declares. It reads all five declaration sites listed above. @@ -97,6 +98,15 @@ Two `items` members may both be built, so they must not share a `unique-id`, and declared there must not share an interface id. Two alternatives of one `prefixItems` entry may, because only one of them is built. +An `items` block cannot limit how many times one member is used. `items` constrains every +position after the entries, so two positions may both match the same member, and the +architecture then holds one `unique-id` twice. `calm validate` cannot report that from the +pattern, because the pattern is correct. The architecture check +`unique-ids-must-be-unique-in-architecture` reports it against the architecture. + +Leave `minItems` out rather than writing `minItems: 0`. Zero is the default, and +`pattern-has-no-empty-properties` reads a zero as a placeholder and reports an error. + `calm validate` reads one level of alternatives. It does not read alternatives declared inside another alternative. The keyword check reads node and relationship sites, not interface entries. From 6e3e2ea745284c0f17b2b10cc122016f0e6e4654 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Sat, 12 Sep 2026 15:55:34 +0000 Subject: [PATCH 5/9] docs: say why a fixed half beside alternatives cannot work Both halves apply to the same element. If each pins a unique-id, no element satisfies both and the position can never be filled. Verified with Ajv, and calm generate still emits the selected alternative, so it produces an architecture that fails its own pattern. The note claimed the duplicate-id rule catches the common case. Sharing an id is not the common case, and that rule reports a duplicate rather than the loss. --- PATTERN-DECISIONS.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index 865a54c27..a77821d14 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -111,10 +111,12 @@ Leave `minItems` out rather than writing `minItems: 0`. Zero is the default, and inside another alternative. The keyword check reads node and relationship sites, not interface entries. -Do not give a `prefixItems` entry its own `properties` as well as alternatives. `calm -generate` keeps the selected alternative and discards the entry's own `properties`, so -whatever the entry declares is lost. `calm validate` reports a duplicate `unique-id` when -the two halves share one. That catches the common case. It does not name the fault. +Do not give a `prefixItems` entry its own `properties` as well as alternatives. Both halves +apply to the same element. If each half pins a `unique-id`, no element can satisfy both, so +nothing can ever fill that position. If they do not conflict, `calm generate` keeps the +selected alternative and discards the entry's own `properties`, so whatever the entry +declares is lost. `calm validate` reports this only when the two halves share a `unique-id`, +and then it reports a duplicate rather than naming the fault. A pattern that declares alternatives inside an `allOf` branch is not supported. Two `allOf` branches that declare the same property discard one of the two declarations. From df040a7ba0cbf2f024a876d7a41c10b5465c2051 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Tue, 15 Sep 2026 21:56:08 +0000 Subject: [PATCH 6/9] refactor(shared): read the decision marker without JSONPath The path has no wildcard, so a lodash read off a named constant does the same work and keeps the marker's location beside the id's. --- .../pattern/decision-is-declared-in-prefix-items.ts | 5 ++--- .../src/spectral/functions/pattern/declaration-paths.ts | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts index de3067a6f..ca04c78df 100644 --- a/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts +++ b/shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts @@ -1,6 +1,5 @@ -import { JSONPath } from 'jsonpath-plus'; import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core'; -import { declaredId } from './declaration-paths'; +import { declaredId, declaresOptions } from './declaration-paths'; /** * An architecture always contains every relationship a pattern declares in prefixItems, so @@ -9,7 +8,7 @@ import { declaredId } from './declaration-paths'; */ export function decisionIsDeclaredInPrefixItems(input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] { const relationship = input as object; - if (!relationship || JSONPath({ path: '$.properties.relationship-type.properties.options', json: relationship }).length === 0) { + if (!relationship || !declaresOptions(relationship)) { return []; } diff --git a/shared/src/spectral/functions/pattern/declaration-paths.ts b/shared/src/spectral/functions/pattern/declaration-paths.ts index 9ab1f6790..17a161d04 100644 --- a/shared/src/spectral/functions/pattern/declaration-paths.ts +++ b/shared/src/spectral/functions/pattern/declaration-paths.ts @@ -4,6 +4,7 @@ export type CalmType = 'nodes' | 'relationships'; const ALTERNATIVE_KEYWORDS = ['oneOf', 'anyOf']; const ID = 'properties.unique-id.const'; +const OPTIONS = 'properties.relationship-type.properties.options'; const INTERFACES = 'properties.interfaces.prefixItems[*]'; const ALTERNATIVES = `(?:${ALTERNATIVE_KEYWORDS.join('|')})`; @@ -57,6 +58,13 @@ export function declaredId(declaration: object): string | undefined { return get(declaration, ID); } +/** + * A relationship that carries options is a decision: it asks which alternatives to include. + */ +export function declaresOptions(relationship: object): boolean { + return get(relationship, OPTIONS) !== undefined; +} + // Reading a pointer back. A query run with `resultType: 'all'` returns each hit with the // JSON Pointer it was found at, the only surviving trace of which site the hit came from. From 8375890a42f8b76fb759446a83f7a93ce1e56d12 Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Thu, 17 Sep 2026 11:17:54 +0000 Subject: [PATCH 7/9] fix(shared): reject decision options declared in items A decision holds its answers in an options array, and calm generate reads options prefixItems alone. Nothing stopped an author declaring the option blocks in options items instead, where validation passed, generation ignored them, and the decision lost its answers with no message. The four rules that read decision options all select options prefixItems, so widening each to reach options items would validate a shape the generator cannot build. The new rule rejects the shape instead. itemsFitWithinMaxItems treated an empty oneOf or anyOf as a choice, because an empty array is truthy. It reported that maxItems left no room for a member the block never declared. Raised in review on #3060. --- PATTERN-DECISIONS.md | 5 +++ .../items-fit-within-max-items.spec.ts | 5 +++ .../pattern/items-fit-within-max-items.ts | 2 +- shared/src/spectral/rules-pattern.spec.ts | 39 +++++++++++++++++++ shared/src/spectral/rules-pattern.ts | 10 +++++ 5 files changed, 60 insertions(+), 1 deletion(-) diff --git a/PATTERN-DECISIONS.md b/PATTERN-DECISIONS.md index a77821d14..3bdb62f09 100644 --- a/PATTERN-DECISIONS.md +++ b/PATTERN-DECISIONS.md @@ -83,6 +83,7 @@ five declaration sites listed above. | A connects relationship refers to an interface that the named node does not declare | error | | A `prefixItems` entry or an `items` block declares both `oneOf` and `anyOf` | error | | A decision is declared in `items` rather than in `prefixItems` | error | +| A decision declares its options in `items` rather than in `prefixItems` | error | | `maxItems` leaves no room for an `items` member | error | | No relationship and no decision refers to a declared node | warning | @@ -94,6 +95,10 @@ contains every relationship declared at a fixed position there, so the decision asked. A decision declared in `items`, or as one alternative of an entry, can be left out, and an answer never gets to decline it. `calm validate` reports the `items` case only. +A decision holds its answers in an `options` array. Declare those option blocks in +`options.prefixItems`. `calm generate` reads that array alone, so a block declared in +`options.items` is never built and the decision loses the answers it holds. + Two `items` members may both be built, so they must not share a `unique-id`, and two nodes declared there must not share an interface id. Two alternatives of one `prefixItems` entry may, because only one of them is built. diff --git a/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts b/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts index 9e0ba6579..af49bb117 100644 --- a/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts +++ b/shared/src/spectral/functions/pattern/items-fit-within-max-items.spec.ts @@ -37,6 +37,11 @@ describe('itemsFitWithinMaxItems', () => { expect(run({ prefixItems: [{}], maxItems: 1, items: { $ref: 'core.json#/defs/node' } })).toEqual([]); }); + it('accepts an items block whose choice is empty, because it declares no member', () => { + expect(run({ prefixItems: [{}], maxItems: 1, items: { oneOf: [] } })).toEqual([]); + expect(run({ prefixItems: [{}], maxItems: 1, items: { anyOf: [] } })).toEqual([]); + }); + it('accepts an anyOf items block with room', () => { expect(run({ prefixItems: [{}], maxItems: 3, items: { anyOf: [{}] } })).toEqual([]); }); diff --git a/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts b/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts index cf9e62e8c..8b7ed2fdb 100644 --- a/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts +++ b/shared/src/spectral/functions/pattern/items-fit-within-max-items.ts @@ -12,7 +12,7 @@ interface CalmArray { */ export function itemsFitWithinMaxItems(input: unknown, _: unknown, context: RulesetFunctionContext): IFunctionResult[] { const array = input as CalmArray; - const offersChoice = Boolean(array?.items?.oneOf || array?.items?.anyOf); + const offersChoice = Boolean(array?.items?.oneOf?.length || array?.items?.anyOf?.length); const positions = array?.prefixItems?.length ?? 0; if (!offersChoice || array.maxItems === undefined || array.maxItems > positions) { diff --git a/shared/src/spectral/rules-pattern.spec.ts b/shared/src/spectral/rules-pattern.spec.ts index 7194f6912..015694ac5 100644 --- a/shared/src/spectral/rules-pattern.spec.ts +++ b/shared/src/spectral/rules-pattern.spec.ts @@ -234,6 +234,45 @@ describe('pattern ruleset', () => { expect(codes).not.toContain('pattern-decision-must-be-declared-in-prefix-items'); }); }); + describe('pattern-decision-options-must-be-declared-in-prefix-items', () => { + const decisionWithOptions = (options: object) => ({ + properties: { + 'unique-id': { const: 'add-ons' }, + description: { const: 'Pick an alternative' }, + 'relationship-type': { properties: { options } } + } + }); + const optionBlock = { + anyOf: [{ + properties: { + description: { const: 'Use cache' }, + nodes: { const: ['cache'] }, + relationships: { const: [] } + } + }] + }; + + it('accepts option blocks declared in options prefixItems', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [decisionWithOptions({ prefixItems: [optionBlock] })] } + } + }); + expect(codes).not.toContain('pattern-decision-options-must-be-declared-in-prefix-items'); + }); + + it('rejects option blocks declared in options items, which calm generate never reads', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [decisionWithOptions({ items: optionBlock })] } + } + }); + expect(codes).toContain('pattern-decision-options-must-be-declared-in-prefix-items'); + }); + }); + describe('pattern-items-must-fit-within-max-items', () => { it('rejects a pattern whose maxItems leaves no room for items', async () => { const codes = await codesFor({ diff --git a/shared/src/spectral/rules-pattern.ts b/shared/src/spectral/rules-pattern.ts index f563ee416..d3cb7a2d1 100644 --- a/shared/src/spectral/rules-pattern.ts +++ b/shared/src/spectral/rules-pattern.ts @@ -217,6 +217,16 @@ const patternRules: RulesetDefinition = { function: decisionIsDeclaredInPrefixItems, }, }, + 'pattern-decision-options-must-be-declared-in-prefix-items': { + description: 'Decision options must be declared in options prefixItems, not in items', + severity: 'error', + message: 'Declare decision options in options prefixItems. calm generate reads prefixItems only, so an options block declared in items is never built.', + given: '$..relationship-type.properties.options', + then: { + field: 'items', + function: falsy, + }, + }, 'pattern-option-relationship-must-have-max-one-item': { description: 'Options relationships must have max one item', severity: 'error', From dbf5bbe9b428c6ff690a81a089a2885e7821188d Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Thu, 17 Sep 2026 11:38:53 +0000 Subject: [PATCH 8/9] test(shared): pin that a decision may reference an items member Nothing asserted that a decision option can name a node or relationship declared in an items catalogue, so the guarantee was free to regress. The permissive half alone would pass trivially, because the rule reports only when the referenced id is a fixed prefixItems entry. The fixed-entry case is pinned beside it so both sides of the condition are covered. Raised in review on #3060. --- .../is-defined-in-oneof-or-anyof.spec.ts | 10 +++++++++ shared/src/spectral/rules-pattern.spec.ts | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.spec.ts b/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.spec.ts index a78e1be41..e0802b05b 100644 --- a/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.spec.ts +++ b/shared/src/spectral/functions/pattern/is-defined-in-oneof-or-anyof.spec.ts @@ -35,6 +35,16 @@ describe('isDefinedInOneOfOrAnyOf', () => { expect(messagesFor(data, 'redis')).toEqual([]); }); + it('accepts an id declared in an items catalogue', () => { + const data = { properties: { nodes: { prefixItems: [], items: { oneOf: [declaration('redis')] } } } }; + expect(messagesFor(data, 'redis')).toEqual([]); + }); + + it('accepts a relationship id declared in an items catalogue', () => { + const data = { properties: { relationships: { prefixItems: [], items: { anyOf: [declaration('cache-link')] } } } }; + expect(messagesFor(data, 'cache-link', 'relationships')).toEqual([]); + }); + it('accepts an id the pattern does not declare', () => { const data = pattern('nodes', [declaration('webapp')]); expect(messagesFor(data, 'redis')).toEqual([]); diff --git a/shared/src/spectral/rules-pattern.spec.ts b/shared/src/spectral/rules-pattern.spec.ts index 015694ac5..2a5eebf4c 100644 --- a/shared/src/spectral/rules-pattern.spec.ts +++ b/shared/src/spectral/rules-pattern.spec.ts @@ -234,6 +234,28 @@ describe('pattern ruleset', () => { expect(codes).not.toContain('pattern-decision-must-be-declared-in-prefix-items'); }); }); + describe('nodes and relationships referenced by a decision', () => { + it('accepts a decision option referencing a node declared in an items catalogue', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp')], items: { oneOf: [node('cache')] } }, + relationships: { prefixItems: [decision('add-ons', ['cache'])] } + } + }); + expect(codes).not.toContain('nodes-referenced-in-pattern-decision-must-be-in-oneof-or-anyof-block'); + }); + + it('reports a decision option referencing a node declared only as a fixed entry', async () => { + const codes = await codesFor({ + properties: { + nodes: { prefixItems: [node('webapp'), node('cache')] }, + relationships: { prefixItems: [decision('add-ons', ['cache'])] } + } + }); + expect(codes).toContain('nodes-referenced-in-pattern-decision-must-be-in-oneof-or-anyof-block'); + }); + }); + describe('pattern-decision-options-must-be-declared-in-prefix-items', () => { const decisionWithOptions = (options: object) => ({ properties: { From af9851fce201347d2d05eb24d55395a87002fb3c Mon Sep 17 00:00:00 2001 From: YoofiTT96 Date: Thu, 17 Sep 2026 11:57:33 +0000 Subject: [PATCH 9/9] test(cli): count the new decision-options rule in the junit fixture The fixture pins the rule inventory, so adding a rule changes both totals and the rule list. --- cli/test_fixtures/validate_output_junit.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/test_fixtures/validate_output_junit.xml b/cli/test_fixtures/validate_output_junit.xml index b76c34d4b..bbed7fb21 100644 --- a/cli/test_fixtures/validate_output_junit.xml +++ b/cli/test_fixtures/validate_output_junit.xml @@ -1,10 +1,10 @@ - + - @@ -44,6 +44,7 @@ + \ No newline at end of file