From 263dc20581c3c24c2903f5522d8b212d15c01df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Thu, 27 Jan 2022 13:03:11 +0100 Subject: [PATCH] feat(core): aliases take only array-ish given (#2033) --- docs/guides/4-custom-rulesets.md | 31 +++--- packages/core/src/meta/ruleset.schema.json | 4 +- packages/core/src/meta/shared.json | 1 + .../__fixtures__/overrides/aliases/scope.ts | 6 +- .../src/ruleset/__tests__/ruleset.test.ts | 66 ++++++------- .../src/ruleset/__tests__/validation.test.ts | 42 ++++---- packages/core/src/ruleset/rule.ts | 95 +++++++++---------- packages/core/src/ruleset/types.ts | 4 +- packages/rulesets/src/oas/index.ts | 4 +- test-harness/scenarios/aliases.scenario | 8 +- .../scenarios/overrides/aliases.scenario | 2 +- 11 files changed, 136 insertions(+), 127 deletions(-) diff --git a/docs/guides/4-custom-rulesets.md b/docs/guides/4-custom-rulesets.md index 60b611caa..14d83d1c8 100644 --- a/docs/guides/4-custom-rulesets.md +++ b/docs/guides/4-custom-rulesets.md @@ -31,7 +31,7 @@ It has a specific syntax known as [JSONPath](https://goessner.net/articles/JsonP Both of them support all the main JSONPath functionality and a little bit more, but this syntax may differ slightly from other JSONPath implementations. Your `given` value can be a string containing any valid JSONPath expression, or an array of expressions to apply a rule to multiple parts of a document. -You can also consume your [aliases][#aliases] here if you have some defined. +You can also consume your (aliases)[#aliases] here if you have some defined. Use the [JSONPath Online Evaluator](http://jsonpath.com/) to determine what `given` path you want. @@ -328,17 +328,22 @@ Targeting certain parts of an OpenAPI spec is powerful but it can become cumbers Define aliases for commonly used JSONPath expressions on a global level which can then be reused across the ruleset. Aliases can be defined in an array of key-value pairs at the root level of the ruleset. -It's a superset of `given`, with the notable difference being the possibility to distinguish between different formats. +It's similar to `given`, with the notable difference being the possibility to distinguish between different formats. **Example** ```yaml aliases: - HeaderNames: "$..parameters.[?(@.in === 'header')].name" - Info: "$..info" - InfoDescription: "#Info.description" - InfoContact: "#Info.contact" - Paths: "$.paths[*]~" + HeaderNames: + - "$..parameters.[?(@.in === 'header')].name" + Info: + - "$..info" + InfoDescription: + - "#Info.description" + InfoContact: + - "#Info.contact" + Paths: + - "$.paths[*]~" ``` If you deal with a variety of different spec, you may find the above approach insufficient, particularly when the shape of the document is notably different. @@ -351,10 +356,12 @@ aliases: targets: - formats: - oas2 - given: $.parameters[*] + given: + - $.parameters[*] - formats: - oas3 - given: $.components.parameters[*] + given: + - $.components.parameters[*] ``` Now, if you referenced `SharedParameterObject` alias, the chosen path would be determined based on the document you use. @@ -366,8 +373,10 @@ To make it more feasible and avoid overly complex JSONPath expressions, `given` ```yaml aliases: - PathItemObject: $.paths[*] - OperationObject: "#PathItem[get,put,post,delete,options,head,patch,trace]" + PathItemObject: + - $.paths[*] + OperationObject: + - "#PathItem[get,put,post,delete,options,head,patch,trace]" ParameterObject: description: an optional property describing the purpose of the alias targets: diff --git a/packages/core/src/meta/ruleset.schema.json b/packages/core/src/meta/ruleset.schema.json index 2ae5dae5e..d567e704d 100644 --- a/packages/core/src/meta/ruleset.schema.json +++ b/packages/core/src/meta/ruleset.schema.json @@ -191,7 +191,7 @@ "$ref": "shared#formats" }, "given": { - "$ref": "shared#given" + "$ref": "shared#arrayish-given" } }, "required": ["formats", "given"], @@ -208,7 +208,7 @@ } }, "else": { - "$ref": "shared#given" + "$ref": "shared#arrayish-given" } } } diff --git a/packages/core/src/meta/shared.json b/packages/core/src/meta/shared.json index 9cacc71ef..7ae2f4496 100644 --- a/packages/core/src/meta/shared.json +++ b/packages/core/src/meta/shared.json @@ -39,6 +39,7 @@ "type": "array" }, "then": { + "$anchor": "arrayish-given", "type": "array", "items": { "$ref": "path-expression" diff --git a/packages/core/src/ruleset/__tests__/__fixtures__/overrides/aliases/scope.ts b/packages/core/src/ruleset/__tests__/__fixtures__/overrides/aliases/scope.ts index ea8828a1c..c0ffad10e 100644 --- a/packages/core/src/ruleset/__tests__/__fixtures__/overrides/aliases/scope.ts +++ b/packages/core/src/ruleset/__tests__/__fixtures__/overrides/aliases/scope.ts @@ -1,12 +1,12 @@ import { DiagnosticSeverity } from '@stoplight/types'; -import {falsy, pattern, truthy} from '@stoplight/spectral-functions'; +import { falsy, pattern, truthy } from '@stoplight/spectral-functions'; import { RulesetDefinition } from '@stoplight/spectral-core'; export { ruleset as default }; const ruleset: RulesetDefinition = { aliases: { - Stoplight: '$..stoplight', + Stoplight: ['$..stoplight'], }, overrides: [ { @@ -29,7 +29,7 @@ const ruleset: RulesetDefinition = { { files: ['**/*.json'], aliases: { - Value: '$..value', + Value: ['$..value'], }, rules: { 'truthy-stoplight-property': { diff --git a/packages/core/src/ruleset/__tests__/ruleset.test.ts b/packages/core/src/ruleset/__tests__/ruleset.test.ts index a0bbdb63b..4353ce3e0 100644 --- a/packages/core/src/ruleset/__tests__/ruleset.test.ts +++ b/packages/core/src/ruleset/__tests__/ruleset.test.ts @@ -1004,10 +1004,10 @@ describe('Ruleset', () => { print( new Ruleset({ aliases: { - Info: '$.info', - PathItem: '$.paths[*][*]', - Description: '$..description', - Name: '$..name', + Info: ['$.info'], + PathItem: ['$.paths[*][*]'], + Description: ['$..description'], + Name: ['$..name'], }, rules: { @@ -1066,10 +1066,10 @@ describe('Ruleset', () => { JSON.stringify( new Ruleset({ aliases: { - Info: '$.info', - PathItem: '$.paths[*][*]', - Description: '$..description', - Name: '$..name', + Info: ['$.info'], + PathItem: ['$.paths[*][*]'], + Description: ['$..description'], + Name: ['$..name'], }, rules: { @@ -1108,10 +1108,10 @@ describe('Ruleset', () => { incompatibleValues: DiagnosticSeverity.Error, }, aliases: { - Info: '$.info', - PathItem: '$.paths[*][*]', - Description: '$..description', - Name: '$..name', + Info: ['$.info'], + PathItem: ['$.paths[*][*]'], + Description: ['$..description'], + Name: ['$..name'], }, rules: { 'valid-path': { @@ -1179,10 +1179,10 @@ describe('Ruleset', () => { print( new Ruleset({ aliases: { - Info: '$.info', - InfoDescription: '#Info.description', - InfoContact: '#Info.contact', - InfoContactName: '#InfoContact.name', + Info: ['$.info'], + InfoDescription: ['#Info.description'], + InfoContact: ['#Info.contact'], + InfoContactName: ['#InfoContact.name'], }, rules: { @@ -1241,7 +1241,7 @@ describe('Ruleset', () => { new Ruleset({ extends: { aliases: { - PathItem: '$.paths[*][*]', + PathItem: ['$.paths[*][*]'], }, rules: {}, }, @@ -1262,10 +1262,10 @@ describe('Ruleset', () => { () => new Ruleset({ aliases: { - Root: '#Info', - Info: '#Root.test', - Contact: '#Info', - Test: '#Contact.test', + Root: ['#Info'], + Info: ['#Root.test'], + Contact: ['#Info'], + Test: ['#Contact.test'], }, rules: { 'valid-path': { @@ -1287,9 +1287,9 @@ describe('Ruleset', () => { new Ruleset({ extends: { aliases: { - PathItem: '$.paths[*][*]', - Description: '$..description', - Name: '$..name', + PathItem: ['$.paths[*][*]'], + Description: ['$..description'], + Name: ['$..name'], }, rules: {}, }, @@ -1331,17 +1331,17 @@ describe('Ruleset', () => { targets: [ { formats: [draft4], - given: '$..id', + given: ['$..id'], }, { formats: [draft6, draft7], - given: '$..$id', + given: ['$..$id'], }, ], }, - PathItem: '$.paths[*]', - OperationObject: '#PathItem[get,put,post,delete,options,head,patch,trace]', + PathItem: ['$.paths[*]'], + OperationObject: ['#PathItem[get,put,post,delete,options,head,patch,trace]'], ParametersDefinitionsObject: { targets: [ { formats: [oas2], given: ['$.parameters'] }, @@ -1461,7 +1461,7 @@ describe('Ruleset', () => { targets: [ { formats: [draft6], - given: '$..$id', + given: ['$..$id'], }, ], }, @@ -1500,11 +1500,11 @@ describe('Ruleset', () => { targets: [ { formats: [draft4], - given: '$..id', + given: ['$..id'], }, { formats: [draft6, draft7], - given: '$..$id', + given: ['$..$id'], }, ], }, @@ -1536,11 +1536,11 @@ describe('Ruleset', () => { targets: [ { formats: ['draft4'], - given: '$..id', + given: ['$..id'], }, { formats: ['draft6', 'draft7'], - given: '$..$id', + given: ['$..$id'], }, ], }, diff --git a/packages/core/src/ruleset/__tests__/validation.test.ts b/packages/core/src/ruleset/__tests__/validation.test.ts index b4bee4cf5..f87d22ea4 100644 --- a/packages/core/src/ruleset/__tests__/validation.test.ts +++ b/packages/core/src/ruleset/__tests__/validation.test.ts @@ -345,19 +345,19 @@ Error at #/rules/rule/formats/1: must be a valid format`, assertValidRuleset.bind(null, { rules: {}, aliases: { - [alias]: '$', + [alias]: ['$'], }, }), ).not.toThrow(); }, ); - it.each(['#Info', '#i', '#Info.contact', '#Info[*]'])('recognizes %s as a valid value of an alias', alias => { + it.each(['#Info', '#i', '#Info.contact', '#Info[*]'])('recognizes %s as a valid value of an alias', value => { expect( assertValidRuleset.bind(null, { rules: {}, aliases: { - alias, + alias: [value], }, }), ).not.toThrow(); @@ -372,17 +372,17 @@ Error at #/rules/rule/formats/1: must be a valid format`, ).toThrow(new RulesetValidationError('Error at #/aliases: must be object')); }); - it.each([null, 5])('recognizes %p as an invalid type of aliases', alias => { + it.each([null, 5])('recognizes %p as an invalid type of aliases', value => { expect( assertValidRuleset.bind(null, { rules: {}, aliases: { - alias, + alias: [value], }, }), ).toThrow( new RulesetValidationError( - 'Error at #/aliases/alias: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', + 'Error at #/aliases/alias/0: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', ), ); }); @@ -392,7 +392,7 @@ Error at #/rules/rule/formats/1: must be a valid format`, assertValidRuleset.bind(null, { rules: {}, aliases: { - [key]: '$.foo', + [key]: ['$.foo'], }, }), ).toThrow( @@ -402,14 +402,14 @@ Error at #/rules/rule/formats/1: must be a valid format`, ); }); - it.each<[unknown, string]>([ + it.each<[unknown[], string]>([ [ - '', - 'Error at #/aliases/PathItem: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', + [''], + 'Error at #/aliases/PathItem/0: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', ], [ - 'foo', - 'Error at #/aliases/PathItem: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', + ['foo'], + 'Error at #/aliases/PathItem/0: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset', ], [[], 'Error at #/aliases/PathItem: must be a non-empty array of expressions'], [ @@ -454,7 +454,7 @@ Error at #/rules/rule/formats/1: must be a valid format`, targets: [ { formats: [formatA], - given: '$.definitions[*]', + given: ['$.definitions[*]'], }, ], }, @@ -464,7 +464,7 @@ Error at #/rules/rule/formats/1: must be a valid format`, }, ); - it.each(['#Info', '#i', '#Info.contact', '#Info[*]'])('recognizes %s as a valid value of an alias', alias => { + it.each(['#Info', '#i', '#Info.contact', '#Info[*]'])('recognizes %s as a valid value of an alias', value => { expect( assertValidRuleset.bind(null, { rules: {}, @@ -473,7 +473,7 @@ Error at #/rules/rule/formats/1: must be a valid format`, targets: [ { formats: [formatA], - given: alias, + given: [value], }, ], }, @@ -512,7 +512,7 @@ Error at #/rules/rule/formats/1: must be a valid format`, ); }); - it.each([{}, { formats: [] }, { given: '$' }])('demands given & formats to be present', targets => { + it.each([{}, { formats: [] }, { given: ['$'] }])('demands given & formats to be present', targets => { expect( assertValidRuleset.bind(null, { rules: {}, @@ -538,11 +538,11 @@ Error at #/rules/rule/formats/1: must be a valid format`, targets: [ { formats: [2], - given: '$.definitions[*]', + given: ['$.definitions[*]'], }, { formats: [formatA, 'formatB'], - given: '$.components.schemas[*]', + given: ['$.components.schemas[*]'], }, ], }, @@ -565,11 +565,11 @@ Error at #/aliases/SchemaObject/targets/1/formats/1: must be a valid format`, targets: [ { formats: [formatA], - given: '#.definitions[*]', + given: ['#.definitions[*]'], }, { formats: [formatA, formatB], - given: '!.components.schemas[*]', + given: ['!.components.schemas[*]'], }, ], }, @@ -577,7 +577,7 @@ Error at #/aliases/SchemaObject/targets/1/formats/1: must be a valid format`, }), ).toThrow( new RulesetValidationError( - `Error at #/aliases/SchemaObject/targets/1/given: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset`, + `Error at #/aliases/SchemaObject/targets/1/given/0: must be a valid JSON Path expression or a reference to the existing Alias optionally paired with a JSON Path expression subset`, ), ); }); diff --git a/packages/core/src/ruleset/rule.ts b/packages/core/src/ruleset/rule.ts index 9adb23dd4..a97d1b03b 100644 --- a/packages/core/src/ruleset/rule.ts +++ b/packages/core/src/ruleset/rule.ts @@ -7,7 +7,13 @@ import { printValue } from '@stoplight/spectral-runtime'; import { DEFAULT_SEVERITY_LEVEL, getDiagnosticSeverity } from './utils/severity'; import { Ruleset } from './ruleset'; import { Format } from './format'; -import type { HumanReadableDiagnosticSeverity, IRuleThen, RuleDefinition, RulesetScopedAliasDefinition } from './types'; +import type { + HumanReadableDiagnosticSeverity, + IRuleThen, + RuleDefinition, + RulesetAliasesDefinition, + RulesetScopedAliasDefinition, +} from './types'; import { minimatch } from './utils/minimatch'; import { FormatsSet } from './utils/formatsSet'; import { isSimpleAliasDefinition } from './utils/guards'; @@ -135,65 +141,58 @@ export class Rule implements IRule { const actualGiven = Array.isArray(given) ? given : [given]; this.#given = this.owner.hasComplexAliases ? actualGiven - : actualGiven.flatMap(expr => this.#resolveAlias(expr, null)).filter(isString); + : actualGiven.flatMap(expr => Rule.#resolveAlias(this.owner.aliases, expr, null, new Set())).filter(isString); } public getGivenForFormats(formats: Set | null): string[] { - return this.owner.hasComplexAliases ? this.#given.flatMap(expr => this.#resolveAlias(expr, formats)) : this.#given; + return this.owner.hasComplexAliases + ? this.#given.flatMap(expr => Rule.#resolveAlias(this.owner.aliases, expr, formats, new Set())) + : this.#given; } - #resolveAlias(expr: string, formats: Set | null): string[] { - const expressions: string[] = [expr]; - const resolvedExpressions = []; - - const stack = new Set(); - resolve: for (const expression of expressions) { - let resolvedExpr = expression; - - while (resolvedExpr.startsWith('#')) { - const alias = ALIAS.exec(resolvedExpr)?.[1]; - - if (alias === void 0 || alias === null) { - throw new ReferenceError(`"${this.name}" rule references an invalid alias`); - } - - if (stack.has(alias)) { - const _stack = [...stack, alias]; - throw new ReferenceError(`Alias "${_stack[0]}" is circular. Resolution stack: ${_stack.join(' -> ')}`); - } + static #resolveAlias( + aliases: RulesetAliasesDefinition | null, + expr: string, + formats: Set | null, + stack: Set, + ): string[] { + const resolvedExpressions: string[] = []; - stack.add(alias); + if (expr.startsWith('#')) { + const alias = ALIAS.exec(expr)?.[1]; - if (this.owner.aliases === null || !(alias in this.owner.aliases)) { - throw new ReferenceError(`Alias "${alias}" does not exist`); - } + if (alias === void 0 || alias === null) { + throw new ReferenceError(`"${this.name}" rule references an invalid alias`); + } - const aliasValue = this.owner.aliases[alias]; - let actualAliasValue; - if (isSimpleAliasDefinition(aliasValue)) { - actualAliasValue = aliasValue; - } else { - actualAliasValue = Rule.#resolveAliasForFormats(aliasValue, formats); - } + if (stack.has(alias)) { + const _stack = [...stack, alias]; + throw new ReferenceError(`Alias "${_stack[0]}" is circular. Resolution stack: ${_stack.join(' -> ')}`); + } - if (actualAliasValue === null) { - continue resolve; - } + stack.add(alias); - if (Array.isArray(actualAliasValue)) { - expressions.push(...actualAliasValue.map(item => item + resolvedExpr.slice(alias.length + 1))); - continue resolve; - } + if (aliases === null || !(alias in aliases)) { + throw new ReferenceError(`Alias "${alias}" does not exist`); + } - if (actualAliasValue.length + 1 === expression.length) { - resolvedExpr = actualAliasValue; - } else { - resolvedExpr = actualAliasValue + resolvedExpr.slice(alias.length + 1); - } + const aliasValue = aliases[alias]; + let actualAliasValue: string[] | null; + if (isSimpleAliasDefinition(aliasValue)) { + actualAliasValue = aliasValue; + } else { + actualAliasValue = Rule.#resolveAliasForFormats(aliasValue, formats); } - stack.clear(); - resolvedExpressions.push(resolvedExpr); + if (actualAliasValue !== null) { + resolvedExpressions.push( + ...actualAliasValue.flatMap(item => + Rule.#resolveAlias(aliases, item + expr.slice(alias.length + 1), formats, new Set([...stack])), + ), + ); + } + } else { + resolvedExpressions.push(expr); } return resolvedExpressions; @@ -202,7 +201,7 @@ export class Rule implements IRule { static #resolveAliasForFormats( { targets }: RulesetScopedAliasDefinition, formats: Set | null, - ): string | string[] | null { + ): string[] | null { if (formats === null || formats.size === 0) { return null; } diff --git a/packages/core/src/ruleset/types.ts b/packages/core/src/ruleset/types.ts index 844d3d67d..f9882ee94 100644 --- a/packages/core/src/ruleset/types.ts +++ b/packages/core/src/ruleset/types.ts @@ -82,11 +82,11 @@ export type RulesetScopedAliasDefinition = { description?: string; targets: { formats: FormatsSet | Format[]; - given: GivenDefinition; + given: string[]; }[]; }; -export type RulesetAliasesDefinition = Record; +export type RulesetAliasesDefinition = Record; export type RulesetDefinition = Readonly< { diff --git a/packages/rulesets/src/oas/index.ts b/packages/rulesets/src/oas/index.ts index 7b86bf12a..da0a61c10 100644 --- a/packages/rulesets/src/oas/index.ts +++ b/packages/rulesets/src/oas/index.ts @@ -32,8 +32,8 @@ const ruleset = { documentationUrl: 'https://meta.stoplight.io/docs/spectral/docs/reference/openapi-rules.md', formats: [oas2, oas3, oas3_0, oas3_1], aliases: { - PathItem: '$.paths[*]', - OperationObject: '#PathItem[get,put,post,delete,options,head,patch,trace]', + PathItem: ['$.paths[*]'], + OperationObject: ['#PathItem[get,put,post,delete,options,head,patch,trace]'], }, rules: { 'operation-success-response': { diff --git a/test-harness/scenarios/aliases.scenario b/test-harness/scenarios/aliases.scenario index 7a5897505..b1a92f573 100644 --- a/test-harness/scenarios/aliases.scenario +++ b/test-harness/scenarios/aliases.scenario @@ -11,10 +11,10 @@ const { truthy, falsy } = require('@stoplight/spectral-functions'); module.exports = { aliases: { - Info: '$..info', - InfoDescription: '#Info.description', - InfoContact: '#Info.contact', - InfoContactName: '#InfoContact.name', + Info: ['$..info'], + InfoDescription: ['#Info.description'], + InfoContact: ['#Info.contact'], + InfoContactName: ['#InfoContact.name'], }, rules: { 'valid-info': { diff --git a/test-harness/scenarios/overrides/aliases.scenario b/test-harness/scenarios/overrides/aliases.scenario index e4f5e17a7..8f8a7a912 100644 --- a/test-harness/scenarios/overrides/aliases.scenario +++ b/test-harness/scenarios/overrides/aliases.scenario @@ -8,7 +8,7 @@ const scenarioId = '{scenarioId}'; module.exports = { aliases: { - Info: '$.info', + Info: ['$.info'], }, rules: { 'description-matches-stoplight': {