Skip to content

Commit

Permalink
feat(core): improve alias validation (#2164)
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip authored Aug 24, 2022
1 parent e13c625 commit d953c0a
Show file tree
Hide file tree
Showing 16 changed files with 416 additions and 183 deletions.
44 changes: 43 additions & 1 deletion docs/guides/4-custom-rulesets.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ For now the JSON, YAML, and JS, are all being maintained, and there are no curre
Targeting certain parts of an OpenAPI spec is powerful but it can become cumbersome to write and repeat complex JSONPath expressions across various rules.
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.
Aliases can be defined in an array of key-value pairs at the root level of the ruleset, or alternatively, within an override.
It's similar to `given`, with the notable difference being the possibility to distinguish between different formats.

**Example**
Expand Down Expand Up @@ -415,6 +415,48 @@ aliases:

Rulesets can then reference aliases in the [given](#given) keyword, either in full: `"given": "#Paths"`, or use it as a prefix for further JSONPath syntax, like dot notation: `"given": "#ParameterObject.name"`.

Bear in mind that an alias has to be explicitly defined in either at the top-level or inside an override.
This is to avoid ambiguity.

```yaml
aliases:
Stoplight:
- "$..stoplight"
overrides:
- files:
- "*.yaml"
rules:
value-matches-stoplight:
message: Value must contain Stoplight
given: "#Stoplight" # valid because declared at the root
severity: error
then:
field: description
function: pattern
functionOptions:
match: Stoplight
- files:
- "**/*.json"
aliases:
Value:
- "$..value"
rules:
truthy-stoplight-property:
message: Value must contain Stoplight
given: "#Value" # valid because declared within the override block
severity: error
then:
function: truthy
- files:
- legacy/**/*.json
rules:
falsy-value:
given: "#Value" # invalid because undeclared both at the top-level and the override. Note that this could be technically resolvable for some JSON documents, because the previous override block has the alias, but to spare some headaches, we demand an alias to be explicitly defined.
severity: error
then:
function: falsy
```

> This will be followed by our core rulesets providing a common set of aliases for OpenAPI and AsyncAPI so that our users don't have to do the work at all. If you have ideas about what kind of aliases could be useful leave your thoughts [here](https://roadmap.stoplight.io).

## Overrides
Expand Down
1 change: 1 addition & 0 deletions docs/reference/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- a rule in the ruleset:
- had an invalid `given`, i.e. the JSON Path expression is not valid from syntax's standpoint
- the ruleset contains `except` entries and the input is passed through stdin
- a JSON Path alias cannot be resolved

### Runtime

Expand Down
16 changes: 0 additions & 16 deletions packages/cli/src/services/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,6 @@ describe('Linter service', () => {
run(`lint ${validCustomOas3SpecPath} -r ${invalidNestedRulesetPath}`),
).rejects.toThrowAggregateError(
new AggregateError([
new RulesetValidationError('must be equal to one of the allowed values', [
'rules',
'rule-with-invalid-enum',
]),
new RulesetValidationError('the rule must have at least "given" and "then" properties', [
'rules',
'rule-without-given-nor-them',
Expand All @@ -242,10 +238,6 @@ describe('Linter service', () => {
'rule-with-invalid-enum',
'type',
]),
new RulesetValidationError('must be equal to one of the allowed values', [
'rules',
'rule-without-given-nor-them',
]),
new RulesetValidationError(
'the value has to be one of: 0, 1, 2, 3 or "error", "warn", "info", "hint", "off"',
['rules', 'rule-with-invalid-enum', 'severity'],
Expand All @@ -265,10 +257,6 @@ describe('Linter service', () => {
it('outputs "invalid ruleset" error', () => {
return expect(run(`lint ${validOas3SpecPath} -r ${invalidRulesetPath}`)).rejects.toThrowAggregateError(
new AggregateError([
new RulesetValidationError('must be equal to one of the allowed values', [
'rules',
'rule-with-invalid-enum',
]),
new RulesetValidationError('the rule must have at least "given" and "then" properties', [
'rules',
'rule-without-given-nor-them',
Expand All @@ -278,10 +266,6 @@ describe('Linter service', () => {
'rule-with-invalid-enum',
'type',
]),
new RulesetValidationError('must be equal to one of the allowed values', [
'rules',
'rule-without-given-nor-them',
]),
new RulesetValidationError(
'the value has to be one of: 0, 1, 2, 3 or "error", "warn", "info", "hint", "off"',
['rules', 'rule-with-invalid-enum', 'severity'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const ruleset: RulesetDefinition = {
},
{
files: ['legacy/**/*.json'],
aliases: {
Value: ['$..value'],
},
rules: {
'falsy-value': {
given: '#Value',
Expand Down
30 changes: 26 additions & 4 deletions packages/core/src/ruleset/__tests__/ruleset.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import '@stoplight/spectral-test-utils/matchers';

import { oas2 } from '@stoplight/spectral-formats';
import { pattern, truthy } from '@stoplight/spectral-functions';
import * as path from '@stoplight/path';
import { DiagnosticSeverity } from '@stoplight/types';
import AggregateError = require('es-aggregate-error');

import { Ruleset } from '../ruleset';
import { RulesetDefinition } from '../types';
Expand Down Expand Up @@ -1266,7 +1269,11 @@ describe('Ruleset', () => {
},
},
}),
).toThrowError(ReferenceError('Alias "PathItem-" does not exist'));
).toThrowAggregateError(
new AggregateError([
new RulesetValidationError('Alias "PathItem-" does not exist', ['rules', 'valid-path', 'given']),
]),
);
});

it('given circular alias, should throw', () => {
Expand All @@ -1288,8 +1295,13 @@ describe('Ruleset', () => {
},
},
}),
).toThrowError(
ReferenceError('Alias "Test" is circular. Resolution stack: Test -> Contact -> Info -> Root -> Info'),
).toThrowAggregateError(
new AggregateError([
new RulesetValidationError(
'Alias "Test" is circular. Resolution stack: Test -> Contact -> Info -> Root -> Info',
['rules', 'valid-path', 'given'],
),
]),
);
});

Expand Down Expand Up @@ -1321,7 +1333,17 @@ describe('Ruleset', () => {
},
},
}),
).toThrowError(ReferenceError('Alias "PathItem" does not exist'));
).toThrowAggregateError(
new AggregateError([
new RulesetValidationError('Alias "PathItem" does not exist', ['rules', 'valid-path', 'given']),
new RulesetValidationError('Alias "Name" does not exist', ['rules', 'valid-name-and-description', 'given']),
new RulesetValidationError(`Alias "Description" does not exist`, [
'rules',
'valid-name-and-description',
'given',
]),
]),
);
});

describe('scoped aliases', () => {
Expand Down
83 changes: 83 additions & 0 deletions packages/core/src/ruleset/alias.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { isScopedAliasDefinition, isSimpleAliasDefinition } from './utils/guards';
import type { RulesetScopedAliasDefinition } from './types';

const ALIAS = /^#([A-Za-z0-9_-]+)/;

export function resolveAliasForFormats(
{ targets }: RulesetScopedAliasDefinition,
formats: Set<unknown> | null,
): string[] | null {
if (formats === null || formats.size === 0) {
return null;
}

// we start from the end to be consistent with overrides etc. - we generally tend to pick the "last" value.
for (let i = targets.length - 1; i >= 0; i--) {
const target = targets[i];
for (const format of target.formats) {
if (formats.has(format)) {
return target.given;
}
}
}

return null;
}

export function resolveAlias(
aliases: Record<string, unknown> | null,
expression: string,
formats: Set<unknown> | null,
): string[] {
return _resolveAlias(aliases, expression, formats, new Set());
}

function _resolveAlias(
aliases: Record<string, unknown> | null,
expression: string,
formats: Set<unknown> | null,
stack: Set<string>,
): string[] {
const resolvedExpressions: string[] = [];

if (expression.startsWith('#')) {
const alias = ALIAS.exec(expression)?.[1];

if (alias === void 0 || alias === null) {
throw new ReferenceError(`Alias must match /^#([A-Za-z0-9_-]+)/`);
}

if (stack.has(alias)) {
const _stack = [...stack, alias];
throw new ReferenceError(`Alias "${_stack[0]}" is circular. Resolution stack: ${_stack.join(' -> ')}`);
}

stack.add(alias);

if (aliases === null || !(alias in aliases)) {
throw new ReferenceError(`Alias "${alias}" does not exist`);
}

const aliasValue = aliases[alias];
let actualAliasValue: string[] | null;
if (isSimpleAliasDefinition(aliasValue)) {
actualAliasValue = aliasValue;
} else if (isScopedAliasDefinition(aliasValue)) {
actualAliasValue = resolveAliasForFormats(aliasValue, formats);
} else {
actualAliasValue = null;
}

if (actualAliasValue !== null) {
resolvedExpressions.push(
...actualAliasValue.flatMap(item =>
_resolveAlias(aliases, item + expression.slice(alias.length + 1), formats, new Set([...stack])),
),
);
}
} else {
resolvedExpressions.push(expression);
}

return resolvedExpressions;
}
120 changes: 63 additions & 57 deletions packages/core/src/ruleset/meta/rule.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,72 +21,78 @@
"$ref": "shared#severity"
}
},
"oneOf": [
{
"properties": {
"description": {
"type": "string"
},
"documentationUrl": {
"type": "string",
"format": "url",
"errorMessage": "must be a valid URL"
},
"recommended": {
"type": "boolean"
},
"given": {
"$ref": "shared#given"
},
"resolved": {
"type": "boolean"
},
"severity": {
"$ref": "#/$defs/Severity"
},
"message": {
"if": {
"type": "object"
},
"then": {
"type": "object",
"properties": {
"description": {
"type": "string"
},
"documentationUrl": {
"type": "string",
"format": "url",
"errorMessage": "must be a valid URL"
},
"recommended": {
"type": "boolean"
},
"given": {
"$ref": "shared#given"
},
"resolved": {
"type": "boolean"
},
"severity": {
"$ref": "#/$defs/Severity"
},
"message": {
"type": "string"
},
"tags": {
"items": {
"type": "string"
},
"tags": {
"items": {
"type": "string"
},
"type": "array"
},
"formats": {
"$ref": "shared#formats"
},
"then": {
"if": {
"type": "array"
},
"formats": {
"$ref": "shared#formats"
},
"then": {
"anyOf": [
{
"$ref": "#/$defs/Then"
},
{
"items": {
"$ref": "#/$defs/Then"
},
"type": "array"
}
]
"type": "array",
"items": {
"$ref": "#/$defs/Then"
}
},
"type": {
"enum": ["style", "validation"],
"type": "string",
"errorMessage": "allowed types are \"style\" and \"validation\""
"else": {
"$ref": "#/$defs/Then"
}
},
"required": ["given", "then"],
"type": "object",
"additionalProperties": false,
"errorMessage": {
"required": "the rule must have at least \"given\" and \"then\" properties"
"type": {
"enum": ["style", "validation"],
"type": "string",
"errorMessage": "allowed types are \"style\" and \"validation\""
}
},
{
"$ref": "shared#/$defs/HumanReadableSeverity"
},
{
"type": "boolean"
"required": ["given", "then"],
"additionalProperties": false,
"errorMessage": {
"required": "the rule must have at least \"given\" and \"then\" properties"
}
]
},
"else": {
"oneOf": [
{
"$ref": "shared#/$defs/HumanReadableSeverity"
},
{
"type": "boolean"
}
]
}
}
Loading

0 comments on commit d953c0a

Please sign in to comment.