fix(shared): resolve prefixItems alternatives in pattern validation

PATTERN-DECISIONS.md

@@ -0,0 +1,84 @@
+# Pattern decisions
+
+A CALM pattern can offer a choice. This document records what each tool guarantees about
+that choice. It describes behaviour only. It does not describe how a tool is built.
+
+Each section names the tests that hold its guarantees. A guarantee below with no test is a
+gap.
+
+## Terms
+
+| Term | Meaning |
+|---|---|
+| 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:
+
+| Site | Meaning |
+|---|---|
+| a `prefixItems` entry | one node, at that position |
+| `prefixItems[i].oneOf` | alternatives for that position |
+| `prefixItems[i].anyOf` | alternatives for that position |
+
+A pattern declares a relationship at the same three sites.
+
+## Rules that hold across all tools
+
+An id names one kind of thing. A name used for a node is never also used for a relationship
+or an interface, anywhere in the pattern.
+
+A decision names a node or a relationship by its `unique-id`. Two alternatives of one entry
+must therefore have different node ids and different relationship ids. If they did not, no
+answer could select one and not the other.
+
+A decision never names an interface on its own, because a relationship names an interface
+beside its node. Two alternatives may expose the same interface id, because only one of
+them is ever built.
+
+Declare one keyword, not both. An element must satisfy every keyword declared beside it, so
+declaring both `oneOf` and `anyOf` makes some alternatives impossible to select.
+
+Neither keyword controls how many alternatives an architecture includes. A `prefixItems`
+entry is one position, so it takes one alternative. `minItems` and `maxItems` on the array
+set the bounds.
+
+`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.
+
+`calm validate` reports these faults:
+
+| Fault | Severity |
+|---|---|
+| Two declarations that can appear together share a `unique-id` | error |
+| Two alternatives of one entry share a node or relationship `unique-id` | error |
+| 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 |
+| 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.
+
+`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
+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.
+
+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.

cli/test_fixtures/validate_output_junit.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<testsuites tests="32" failures="0" errors="0" skipped="0">
+<testsuites tests="33" 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="31"
+    <testsuite name="Spectral Suite" tests="32"
         failures="0" errors="0" skipped="0">
         <testcase name="architecture-has-nodes-relationships" />
         <testcase name="architecture-has-no-empty-string-properties" />
@@ -41,6 +41,7 @@
         <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-option-relationship-must-have-max-one-item"/>
     </testsuite>
 </testsuites>

shared/src/spectral/functions/helper-functions.ts

@@ -1,6 +1,6 @@
 import { IFunctionResult, RulesetFunctionContext } from '@stoplight/spectral-core';
 
-interface JSONPathMatch {
+export interface JSONPathMatch {
     value: unknown;
     pointer: string;
 }

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

@@ -0,0 +1,50 @@
+import { alternativeIdPaths, containingDeclaration, containingEntry, declarationPaths, declaredId, declaredIdPaths, declaredInterfaceIdPaths, fixedIdPath, isAlternative } 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', () => {
+        expect(declarationPaths('relationships')).toEqual([
+            '$.properties.relationships.prefixItems[*]',
+            '$.properties.relationships.prefixItems[*].oneOf[*]',
+            '$.properties.relationships.prefixItems[*].anyOf[*]'
+        ]);
+    });
+
+    it('builds id paths from the fixed entry followed by the alternatives', () => {
+        expect(declaredIdPaths('nodes')).toEqual([fixedIdPath('nodes'), ...alternativeIdPaths('nodes')]);
+    });
+
+    it('reaches interfaces on every node declaration site', () => {
+        expect(declaredInterfaceIdPaths()).toHaveLength(declarationPaths('nodes').length);
+        expect(declaredInterfaceIdPaths()[0]).toBe('$.properties.nodes.prefixItems[*].properties.interfaces.prefixItems[*].properties.unique-id.const');
+    });
+
+    it('reads the id off a declaration', () => {
+        expect(declaredId({ properties: { 'unique-id': { const: 'webapp' } } })).toBe('webapp');
+        expect(declaredId({ oneOf: [] })).toBeUndefined();
+    });
+});
+
+describe('declaration pointers', () => {
+    it.each([
+        ['a fixed node id', `${ENTRY}${ID}`, ENTRY, ENTRY, false],
+        ['an alternative node id', `${ALTERNATIVE}${ID}`, ALTERNATIVE, ENTRY, true],
+        ['a fixed interface id', `${ENTRY}${INTERFACE_ID}`, ENTRY, ENTRY, false],
+        ['an alternative interface id', `${ALTERNATIVE}${INTERFACE_ID}`, ALTERNATIVE, ENTRY, true],
+        ['a relationship alternative', '/properties/relationships/prefixItems/2/anyOf/0' + ID,
+            '/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(isAlternative(pointer)).toBe(alternative);
+    });
+
+    it('leaves a pointer from outside these paths alone', () => {
+        expect(containingDeclaration('/properties/metadata/0')).toBe('/properties/metadata/0');
+        expect(isAlternative('/properties/metadata/0')).toBe(false);
+    });
+});

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

@@ -0,0 +1,86 @@
+import { get } from 'lodash';
+
+export type CalmType = 'nodes' | 'relationships';
+
+const ALTERNATIVE_KEYWORDS = ['oneOf', 'anyOf'];
+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+$`);
+
+function entryPath(calmType: CalmType): string {
+    return `$.properties.${calmType}.prefixItems[*]`;
+}
+
+function alternativePaths(calmType: CalmType): string[] {
+    return ALTERNATIVE_KEYWORDS.map(keyword => `${entryPath(calmType)}.${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)];
+}
+
+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 declaredIdPaths(calmType: CalmType): string[] {
+    return [fixedIdPath(calmType), ...alternativeIdPaths(calmType)];
+}
+
+export function declaredInterfaceIdPaths(): string[] {
+    return declarationPaths('nodes').map(path => `${path}.${INTERFACES}.${ID}`);
+}
+
+export function declaredId(declaration: object): string | undefined {
+    return get(declaration, ID);
+}
+
+/**
+ * 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;
+}
+
+export function containingEntry(pointer: string): string {
+    return containingDeclaration(pointer).split(ALTERNATIVE_SUFFIX)[0];
+}
+
+export function isAlternative(pointer: string): boolean {
+    return containingDeclaration(pointer) !== containingEntry(pointer);
+}
+
+function declarationIndices(pointer: string): number[] {
+    return (containingDeclaration(pointer).match(/\d+/g) ?? []).map(Number);
+}
+
+/**
+ * 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.
+ */
+export function byBuildOrder(left: string, right: string): number {
+    const [first, second] = [left, right].map(declarationIndices);
+    for (let depth = 0; depth < Math.max(first.length, second.length); depth++) {
+        const difference = (first[depth] ?? -1) - (second[depth] ?? -1);
+        if (difference !== 0) {
+            return difference;
+        }
+    }
+    return 0;
+}