feat(shared): read items in pattern validation

PATTERN-DECISIONS.md

@@ -13,15 +13,30 @@ 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.
+`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
+read.
+
+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
 
@@ -43,18 +58,19 @@ 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
 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
-three declaration sites listed above.
+five declaration sites listed above.
 
 `calm validate` reports these faults:
 
@@ -65,20 +81,53 @@ 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 |
+| 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 |
 
 `calm validate` does not read the destination of a connects relationship. A typo there is
 not reported.
 
+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.
+
+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.
+
+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 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
-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.
+
+## 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.

cli/test_fixtures/validate_output_junit.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<testsuites tests="33" failures="0" errors="0" skipped="0">
+<testsuites tests="36" failures="0" errors="0" skipped="0">
     <testsuite name="JSON Schema Validation" tests="1" failures="0"
         errors="0" skipped="0">
         <testcase name="JSON Schema Validation succeeded" />
     </testsuite>
-    <testsuite name="Spectral Suite" tests="32"
+    <testsuite name="Spectral Suite" tests="35"
         failures="0" errors="0" skipped="0">
         <testcase name="architecture-has-nodes-relationships" />
         <testcase name="architecture-has-no-empty-string-properties" />
@@ -41,7 +41,10 @@
         <testcase name="nodes-referenced-in-pattern-decision-must-be-in-oneof-or-anyof-block"/>
         <testcase name="relationships-referenced-in-pattern-decision-must-be-in-oneof-or-anyof-block"/>
         <testcase name="pattern-option-relationship-must-only-have-oneof-or-anyof-items"/>
-        <testcase name="pattern-prefix-items-must-declare-one-keyword"/>
+        <testcase name="pattern-choice-must-declare-one-keyword"/>
+        <testcase name="pattern-items-must-fit-within-max-items"/>
+        <testcase name="pattern-decision-must-be-declared-in-prefix-items"/>
+        <testcase name="pattern-decision-options-must-be-declared-in-prefix-items"/>
         <testcase name="pattern-option-relationship-must-have-max-one-item"/>
     </testsuite>
 </testsuites>

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([]);
+    });
+});

shared/src/spectral/functions/pattern/decision-is-declared-in-prefix-items.ts

@@ -0,0 +1,19 @@
+import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core';
+import { declaredId, declaresOptions } 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 || !declaresOptions(relationship)) {
+        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],
+    }];
+}

shared/src/spectral/functions/pattern/declaration-paths.spec.ts

@@ -1,21 +1,32 @@
-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`;
 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);
     });
 

shared/src/spectral/functions/pattern/declaration-paths.ts

@@ -4,75 +4,102 @@ 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('|')})`;
-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 {
+function fixedPath(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 [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 `containing` helpers read that pointer
- * back, because it is the only surviving trace of which entry the hit came from.
  */
 export function declarationPaths(calmType: CalmType): string[] {
-    return [entryPath(calmType), ...alternativePaths(calmType)];
+    return [fixedPath(calmType), ...choicePaths(calmType)];
 }
 
 export function fixedIdPath(calmType: CalmType): string {
-    return `${entryPath(calmType)}.${ID}`;
+    return `${fixedPath(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);
 }
 
+/**
+ * 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.
+
 /**
  * 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 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);