From 64fa0df95fdbc89d0a052b4adc8aa3791257d28e Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 1 Feb 2020 11:18:39 +0100 Subject: [PATCH] feat: teach rulesets to accept exceptions --- .../__fixtures__/custom-oas-ruleset.json | 5 + .../__fixtures__/exceptions.remote.oas3.yaml | 17 ++ src/__tests__/linter.jest.test.ts | 180 +++++++++++++++- src/__tests__/spectral.jest.test.ts | 7 + src/__tests__/spectral.test.ts | 50 ++++- src/cli/services/linter/utils/getRuleset.ts | 1 + src/linter.ts | 31 ++- src/meta/ruleset.schema.json | 9 + .../__fixtures__/exceptions/inheriting.yaml | 3 + .../__fixtures__/exceptions/invalid.yaml | 3 + .../__fixtures__/exceptions/simple.yaml | 5 + .../__fixtures__/exceptions/standalone.yaml | 5 + src/rulesets/__tests__/reader.jest.test.ts | 47 ++++ src/rulesets/__tests__/validation.test.ts | 14 ++ .../mergers/__tests__/exceptions.test.ts | 201 ++++++++++++++++++ src/rulesets/mergers/exceptions.ts | 78 +++++++ src/rulesets/reader.ts | 10 + src/runner.ts | 37 +++- src/spectral.ts | 19 +- src/types/ruleset.ts | 3 + src/utils/__tests__/pivotExceptions.test.ts | 51 +++++ src/utils/pivotExceptions.ts | 35 +++ .../scenarios/exceptions.oas3.scenario | 26 +++ 23 files changed, 828 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/__fixtures__/exceptions.remote.oas3.yaml create mode 100644 src/rulesets/__tests__/__fixtures__/exceptions/inheriting.yaml create mode 100644 src/rulesets/__tests__/__fixtures__/exceptions/invalid.yaml create mode 100644 src/rulesets/__tests__/__fixtures__/exceptions/simple.yaml create mode 100644 src/rulesets/__tests__/__fixtures__/exceptions/standalone.yaml create mode 100644 src/rulesets/mergers/__tests__/exceptions.test.ts create mode 100644 src/rulesets/mergers/exceptions.ts create mode 100644 src/utils/__tests__/pivotExceptions.test.ts create mode 100644 src/utils/pivotExceptions.ts create mode 100644 test-harness/scenarios/exceptions.oas3.scenario diff --git a/src/__tests__/__fixtures__/custom-oas-ruleset.json b/src/__tests__/__fixtures__/custom-oas-ruleset.json index 69a541003f..f71f2df672 100644 --- a/src/__tests__/__fixtures__/custom-oas-ruleset.json +++ b/src/__tests__/__fixtures__/custom-oas-ruleset.json @@ -14,5 +14,10 @@ } } } + }, + "except": { + "/test/file.json#/info": ["info-contact", "info-description"], + "/test/file.json#": [ "oas3-api-servers"], + "another.yaml#": ["dummy-rule", "info-contact"] } } diff --git a/src/__tests__/__fixtures__/exceptions.remote.oas3.yaml b/src/__tests__/__fixtures__/exceptions.remote.oas3.yaml new file mode 100644 index 0000000000..3516e68225 --- /dev/null +++ b/src/__tests__/__fixtures__/exceptions.remote.oas3.yaml @@ -0,0 +1,17 @@ +%YAML 1.2 +--- +openapi: 3.0.2 +info: + title: Random title + description: Random description + version: 0.0.1 + contact: + email: random@random.com +paths: {} +servers: + - url: https://www.random.com +components: + schemas: + TheRemoteType: + description: A strongly typed string + type: string diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index 89520bc83c..11a1ca57f0 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -1,10 +1,14 @@ import { normalize } from '@stoplight/path'; -import { DiagnosticSeverity } from '@stoplight/types'; +import { DiagnosticSeverity, Dictionary } from '@stoplight/types'; import * as path from 'path'; +import { isOpenApiv3 } from '../formats'; import { httpAndFileResolver } from '../resolvers/http-and-file'; -import { Spectral } from '../spectral'; +import { Rule, RuleType, Spectral } from '../spectral'; + +import { rules as oasRules } from '../rulesets/oas/index.json'; const customFunctionOASRuleset = path.join(__dirname, './__fixtures__/custom-functions-oas-ruleset.json'); +const customOASRuleset = path.join(__dirname, './__fixtures__/custom-oas-ruleset.json'); const customDirectoryFunctionsRuleset = path.join(__dirname, './__fixtures__/custom-directory-function-ruleset.json'); describe('Linter', () => { @@ -170,4 +174,176 @@ describe('Linter', () => { ); }); }); + + describe('Exceptions handling', () => { + it('should ignore specified rules violations in a standalone document', async () => { + await spectral.loadRuleset(customOASRuleset); + spectral.registerFormat('oas3', isOpenApiv3); + + const res = await spectral.run( + { + openapi: '3.0.2', + info: 17, + }, + { + resolve: { + documentUri: '/test/file.json', + }, + }, + ); + + expect(res.length).toBeGreaterThan(0); + + expect(res).not.toContainEqual( + expect.objectContaining({ + code: 'info-contact', + }), + ); + + expect(res).not.toContainEqual( + expect.objectContaining({ + code: 'info-description', + }), + ); + + expect(res).not.toContainEqual( + expect.objectContaining({ + code: 'oas3-api-servers', + }), + ); + }); + + describe('resolving', () => { + const testRules: Dictionary = { + 'no-yaml-remote-reference': { + formats: ['oas2', 'oas3'], + message: '$ref must not point at yaml files', + given: '$..$ref', + recommended: true, + resolved: false, + then: { + function: 'pattern', + functionOptions: { + notMatch: '\\.yaml(#.*)$', + }, + }, + }, + 'strings-maxLength': { + formats: ['oas2', 'oas3'], + message: "String typed properties MUST be further described using 'maxLength'. Error: {{error}}", + given: "$..*[?(@.type === 'string')]", + recommended: true, + then: { + field: 'maxLength', + function: 'truthy', + }, + }, + 'oas3-schema': { + ...oasRules['oas3-schema'], + type: RuleType[oasRules['oas2-unused-definition'].type], + }, + }; + + const remoteFile = './__fixtures__/exceptions.remote.oas3.yaml'; + const rootedRemoteFile = path.join(__dirname, remoteFile); + const rootedRemoteReference = rootedRemoteFile + '#/components/schemas/TheRemoteType'; + + const localFile = 'foo.json'; + const rootedLocalFile = path.join(__dirname, localFile); + const rootedLocalReference = rootedLocalFile + '#/components/schemas/TheLocalType/$ref'; + + const testExceptions: Dictionary = {}; + testExceptions[rootedRemoteReference] = ['strings-maxLength']; + testExceptions[rootedLocalReference] = ['no-yaml-remote-reference']; + + const document = { + openapi: '3.0.2', + components: { + schemas: { + TheLocalType: { + $ref: remoteFile + '#/components/schemas/TheRemoteType', + }, + }, + }, + }; + + const opts = { + resolve: { + documentUri: rootedLocalFile, + }, + }; + + it('should ignore specified rules violations in a referenced document', async () => { + spectral = new Spectral({ resolver: httpAndFileResolver }); + spectral.registerFormat('oas3', isOpenApiv3); + + const rules = { + 'strings-maxLength': testRules['strings-maxLength'], + 'oas3-schema': testRules['oas3-schema'], + }; + + spectral.setRuleset({ rules, exceptions: {}, functions: {} }); + + const first = await spectral.run(document, opts); + + expect(first).toEqual([ + expect.objectContaining({ + code: 'strings-maxLength', + }), + expect.objectContaining({ + code: 'oas3-schema', + }), + ]); + + const exceptions = {}; + exceptions[rootedRemoteReference] = testExceptions[rootedRemoteReference]; + + spectral.setRuleset({ rules, exceptions, functions: {} }); + + const second = await spectral.run(document, opts); + + expect(second).toEqual([ + expect.objectContaining({ + code: 'oas3-schema', + }), + ]); + }); + + it('should ignore specified rules violations in "resolved=false" mode', async () => { + spectral = new Spectral({ resolver: httpAndFileResolver }); + spectral.registerFormat('oas3', isOpenApiv3); + + const rules = { + 'no-yaml-remote-reference': testRules['no-yaml-remote-reference'], + 'oas3-schema': testRules['oas3-schema'], + }; + + spectral.setRuleset({ rules, exceptions: {}, functions: {} }); + + const first = await spectral.run(document, opts); + + expect(first).toEqual([ + expect.objectContaining({ + code: 'oas3-schema', + }), + expect.objectContaining({ + code: 'no-yaml-remote-reference', + }), + ]); + + const exceptions = {}; + exceptions[rootedLocalReference] = testExceptions[rootedLocalReference]; + + spectral.setRuleset({ rules, exceptions, functions: {} }); + + const second = await spectral.run(document, opts); + + expect(second).toEqual([ + expect.objectContaining({ + code: 'oas3-schema', + }), + ]); + }); + }); + }); }); diff --git a/src/__tests__/spectral.jest.test.ts b/src/__tests__/spectral.jest.test.ts index 1261669105..e135db3ae1 100644 --- a/src/__tests__/spectral.jest.test.ts +++ b/src/__tests__/spectral.jest.test.ts @@ -45,6 +45,13 @@ describe('Spectral', () => { }, }), ); + + Object.keys(s.exceptions).forEach(p => expect(path.isAbsolute(p)).toEqual(true)); + + expect(Object.entries(s.exceptions)).toEqual([ + [expect.stringMatching('^/test/file.json#/info$'), ['info-contact', 'info-description']], + [expect.stringMatching('/__tests__/__fixtures__/another.yaml#$'), ['dummy-rule', 'info-contact']], + ]); }); test('should support loading rulesets over http', async () => { diff --git a/src/__tests__/spectral.test.ts b/src/__tests__/spectral.test.ts index 8fe27ba1c8..284ba638f2 100644 --- a/src/__tests__/spectral.test.ts +++ b/src/__tests__/spectral.test.ts @@ -1,12 +1,15 @@ import { IGraphNodeData } from '@stoplight/json-ref-resolver/types'; import { DiagnosticSeverity, Dictionary } from '@stoplight/types'; import { DepGraph } from 'dependency-graph'; -import { merge } from 'lodash'; +import { escapeRegExp, merge } from 'lodash'; import { Document } from '../document'; import * as Parsers from '../parsers'; import { Spectral } from '../spectral'; import { IResolver, IRunRule, RuleFunction } from '../types'; +import { RulesetExceptionCollection } from '../types/ruleset'; + +import { buildRulesetExceptionCollectionFrom } from '../rulesets/mergers/__tests__/exceptions.test'; const oasRuleset = JSON.parse(JSON.stringify(require('../rulesets/oas/index.json'))); const oasRulesetRules: Dictionary = oasRuleset.rules; @@ -258,4 +261,49 @@ describe('spectral', () => { }); }); }); + + describe('setRuleset', () => { + const s = new Spectral(); + + describe('exceptions handling', () => { + it.each([['one.yaml#'], ['one.yaml#/'], ['one.yaml#/toto'], ['down/one.yaml#/toto'], ['../one.yaml#/toto']])( + 'throws on relative locations (location: "%s")', + location => { + const exceptions = buildRulesetExceptionCollectionFrom(location); + + expect(() => { + s.setRuleset({ rules: {}, functions: {}, exceptions }); + }).toThrow(new RegExp(`.+\`${escapeRegExp(location)}\`.+is not a valid uri.+Only absolute Uris are allowed`)); + }, + ); + + it.each([ + ['https://dot.com/one.yaml#/toto', 'https://dot.com/one.yaml#/toto'], + ['/local/one.yaml#/toto', '/local/one.yaml#/toto'], + ['c:/one.yaml#/toto', 'c:/one.yaml#/toto'], + ['c:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ])('normalizes absolute locations (location: "%s")', (location, expected) => { + const exceptions = buildRulesetExceptionCollectionFrom(location); + + s.setRuleset({ rules: {}, functions: {}, exceptions }); + + const locs = Object.keys(s.exceptions); + expect(locs).toEqual([expected]); + }); + + it('normalizes exceptions', () => { + const exceptions: RulesetExceptionCollection = { + '/test/file.yaml#/a': ['f', 'c', 'd', 'a'], + '/test/file.yaml#/b': ['1', '3', '3', '2'], + }; + + s.setRuleset({ rules: {}, functions: {}, exceptions }); + + expect(s.exceptions).toEqual({ + '/test/file.yaml#/a': ['a', 'c', 'd', 'f'], + '/test/file.yaml#/b': ['1', '2', '3'], + }); + }); + }); + }); }); diff --git a/src/cli/services/linter/utils/getRuleset.ts b/src/cli/services/linter/utils/getRuleset.ts index 223bdd53e1..cdee8b3220 100644 --- a/src/cli/services/linter/utils/getRuleset.ts +++ b/src/cli/services/linter/utils/getRuleset.ts @@ -9,6 +9,7 @@ async function loadRulesets(cwd: string, rulesetFiles: string[]): Promise, apply: IFunction, inventory: DocumentInventory, + exceptionLocations: IExceptionLocation[] | undefined, ): IRuleResult[] => { const givenPath = node.path[0] === '$' ? node.path.slice(1) : node.path; const targets = getLintTargets(node.value, then.field); @@ -76,5 +78,32 @@ export const lintNode = ( ); } - return results; + const isAKnownException = (violation: IRuleResult, locations: IExceptionLocation[]): boolean => { + for (const location of locations) { + if (violation.source !== location.source) { + continue; + } + + if (violation.path.length !== location.path.length) { + continue; + } + + for (let i = 0; i < violation.path.length; i++) { + if (location.path[i] !== violation.path[i]) { + continue; + } + } + + return true; + } + + return false; + }; + + if (exceptionLocations === undefined) { + return results; + } + + const filtered = results.filter(r => !isAKnownException(r, exceptionLocations)); + return filtered; }; diff --git a/src/meta/ruleset.schema.json b/src/meta/ruleset.schema.json index 9cab5e7122..55a8062806 100644 --- a/src/meta/ruleset.schema.json +++ b/src/meta/ruleset.schema.json @@ -64,6 +64,15 @@ } ] } + }, + "except": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } } }, "anyOf": [ diff --git a/src/rulesets/__tests__/__fixtures__/exceptions/inheriting.yaml b/src/rulesets/__tests__/__fixtures__/exceptions/inheriting.yaml new file mode 100644 index 0000000000..25f408d397 --- /dev/null +++ b/src/rulesets/__tests__/__fixtures__/exceptions/inheriting.yaml @@ -0,0 +1,3 @@ +extends: ["./standalone.yaml"] +except: + "four.yaml#": ["my-rule-4"] diff --git a/src/rulesets/__tests__/__fixtures__/exceptions/invalid.yaml b/src/rulesets/__tests__/__fixtures__/exceptions/invalid.yaml new file mode 100644 index 0000000000..2179af1180 --- /dev/null +++ b/src/rulesets/__tests__/__fixtures__/exceptions/invalid.yaml @@ -0,0 +1,3 @@ +extends: [[spectral:oas, off]] +except: + "one.yaml": ["my-rule"] diff --git a/src/rulesets/__tests__/__fixtures__/exceptions/simple.yaml b/src/rulesets/__tests__/__fixtures__/exceptions/simple.yaml new file mode 100644 index 0000000000..dac26fc011 --- /dev/null +++ b/src/rulesets/__tests__/__fixtures__/exceptions/simple.yaml @@ -0,0 +1,5 @@ +extends: [[spectral:oas, off]] +except: + "one.yaml#": ["my-rule-1"] + "../two.yaml#": ["my-rule-2"] + "sub/three.yaml#": ["my-rule-3"] diff --git a/src/rulesets/__tests__/__fixtures__/exceptions/standalone.yaml b/src/rulesets/__tests__/__fixtures__/exceptions/standalone.yaml new file mode 100644 index 0000000000..02defd9496 --- /dev/null +++ b/src/rulesets/__tests__/__fixtures__/exceptions/standalone.yaml @@ -0,0 +1,5 @@ +rules: {} +except: + "one.yaml#": ["my-rule-1"] + "../two.yaml#": ["my-rule-2"] + "sub/three.yaml#": ["my-rule-3"] diff --git a/src/rulesets/__tests__/reader.jest.test.ts b/src/rulesets/__tests__/reader.jest.test.ts index fae9dc292e..953be8ed11 100644 --- a/src/rulesets/__tests__/reader.jest.test.ts +++ b/src/rulesets/__tests__/reader.jest.test.ts @@ -29,6 +29,10 @@ const rulesetWithMissingFunctions = path.join(__dirname, './__fixtures__/ruleset const fooExtendsBarRuleset = path.join(__dirname, './__fixtures__/foo-extends-bar-ruleset.json'); const selfExtendingRuleset = path.join(__dirname, './__fixtures__/self-extending-ruleset.json'); const simpleDisableRuleset = path.join(__dirname, './__fixtures__/simple-disable-ruleset.yaml'); +const standaloneExceptRuleset = path.join(__dirname, './__fixtures__/exceptions/standalone.yaml'); +const simpleExceptRuleset = path.join(__dirname, './__fixtures__/exceptions/simple.yaml'); +const inheritingExceptRuleset = path.join(__dirname, './__fixtures__/exceptions/inheriting.yaml'); +const invalidExceptRuleset = path.join(__dirname, './__fixtures__/exceptions/invalid.yaml'); const fooCJSFunction = fs.readFileSync(path.join(__dirname, './__fixtures__/functions/foo.cjs.js'), 'utf8'); const barFunction = fs.readFileSync(path.join(__dirname, './__fixtures__/customFunctions/bar.js'), 'utf8'); const truthyFunction = fs.readFileSync(path.join(__dirname, './__fixtures__/customFunctions/truthy.js'), 'utf8'); @@ -366,6 +370,7 @@ describe('Rulesets reader', () => { return expect( readRuleset(path.join(__dirname, './__fixtures__/inheritanceRulesets/my-ruleset.json')), ).resolves.toStrictEqual({ + exceptions: {}, functions: {}, rules: { 'contact-name-matches-stoplight': { @@ -417,6 +422,7 @@ describe('Rulesets reader', () => { return expect( readRuleset(path.join(__dirname, './__fixtures__/inheritanceRulesets/my-ruleset-recommended.json')), ).resolves.toStrictEqual({ + exceptions: {}, functions: {}, rules: { 'contact-name-matches-stoplight': { @@ -468,6 +474,7 @@ describe('Rulesets reader', () => { return expect( readRuleset(path.join(__dirname, './__fixtures__/inheritanceRulesets/ruleset-c.json')), ).resolves.toStrictEqual({ + exceptions: {}, functions: {}, rules: { 'contact-name-matches-stoplight': { @@ -645,11 +652,13 @@ describe('Rulesets reader', () => { return expect(readRuleset(rulesetWithMissingFunctions)).resolves.toEqual({ rules: {}, functions: {}, + exceptions: {}, }); }); it('should handle ruleset with circular extensions', () => { return expect(readRuleset(fooExtendsBarRuleset)).resolves.toEqual({ + exceptions: {}, functions: {}, rules: { 'bar-rule': { @@ -674,6 +683,7 @@ describe('Rulesets reader', () => { it('should handle ruleset that extends itself', () => { return expect(readRuleset(selfExtendingRuleset)).resolves.toEqual({ + exceptions: {}, functions: {}, rules: { 'foo-rule': { @@ -750,4 +760,41 @@ describe('Rulesets reader', () => { expect(readFileSpy).not.toBeCalled(); readFileSpy.mockRestore(); }); + + describe('Exceptions loading', () => { + it('should handle loading a standalone ruleset', async () => { + const ruleset = await readRuleset(standaloneExceptRuleset); + + expect(Object.entries(ruleset.exceptions)).toEqual([ + [expect.stringMatching('/__tests__/__fixtures__/exceptions/one.yaml#$'), ['my-rule-1']], + [expect.stringMatching('/__tests__/__fixtures__/two.yaml#$'), ['my-rule-2']], + [expect.stringMatching('/__tests__/__fixtures__/exceptions/sub/three.yaml#$'), ['my-rule-3']], + ]); + }); + + it('should throw when ruleset contains invalid exceptions', () => { + expect(readRuleset(invalidExceptRuleset)).rejects.toThrow('is not a valid uri'); + }); + + it('should handle loading a ruleset deriving from a built-in one', async () => { + const ruleset = await readRuleset(simpleExceptRuleset); + + expect(Object.entries(ruleset.exceptions)).toEqual([ + [expect.stringMatching('/__tests__/__fixtures__/exceptions/one.yaml#$'), ['my-rule-1']], + [expect.stringMatching('/__tests__/__fixtures__/two.yaml#$'), ['my-rule-2']], + [expect.stringMatching('/__tests__/__fixtures__/exceptions/sub/three.yaml#$'), ['my-rule-3']], + ]); + }); + + it('should handle loading a ruleset deriving from another one', async () => { + const ruleset = await readRuleset(inheritingExceptRuleset); + + expect(Object.entries(ruleset.exceptions)).toEqual([ + [expect.stringMatching('/__tests__/__fixtures__/exceptions/one.yaml#$'), ['my-rule-1']], + [expect.stringMatching('/__tests__/__fixtures__/two.yaml#$'), ['my-rule-2']], + [expect.stringMatching('/__tests__/__fixtures__/exceptions/sub/three.yaml#$'), ['my-rule-3']], + [expect.stringMatching('/__tests__/__fixtures__/exceptions/four.yaml#$'), ['my-rule-4']], + ]); + }); + }); }); diff --git a/src/rulesets/__tests__/validation.test.ts b/src/rulesets/__tests__/validation.test.ts index ac78e8dea2..640e680a34 100644 --- a/src/rulesets/__tests__/validation.test.ts +++ b/src/rulesets/__tests__/validation.test.ts @@ -242,6 +242,20 @@ describe('Ruleset Validation', () => { }), ).toThrow(ValidationError); }); + + describe('Exceptions validation', () => { + const rulesetsWithInvalidExceptStructures = [ + { extends: ['foo'], except: '' }, + { extends: ['foo'], except: { one: null } }, + { extends: ['foo'], except: { one: [1] } }, + ]; + + it.each(rulesetsWithInvalidExceptStructures)('throws when defined "except" do not match schema', async r => { + expect(() => { + assertValidRuleset(r); + }).toThrow(ValidationError); + }); + }); }); describe('Function Validation', () => { diff --git a/src/rulesets/mergers/__tests__/exceptions.test.ts b/src/rulesets/mergers/__tests__/exceptions.test.ts new file mode 100644 index 0000000000..eb07b4edec --- /dev/null +++ b/src/rulesets/mergers/__tests__/exceptions.test.ts @@ -0,0 +1,201 @@ +import { escapeRegExp } from 'lodash'; +import { RulesetExceptionCollection } from '../../../types/ruleset'; +import { mergeExceptions } from '../exceptions'; + +export const buildRulesetExceptionCollectionFrom = ( + loc: string, + rules: string[] = ['a'], +): RulesetExceptionCollection => { + const source = {}; + source[loc] = rules; + return source; +}; + +describe('Ruleset exceptions merging', () => { + describe('when loaded from a ruleset', () => { + const dummyRulesetUri = './ruleset.yaml'; + + it('includes new exceptions', () => { + const target: RulesetExceptionCollection = { + 'file.yaml#/a': [], + 'file.yaml#/b': ['1', '2'], + }; + + const source: RulesetExceptionCollection = { + 'file.yaml#/c': ['3'], + 'file.yaml#/d': ['4', '5'], + }; + + mergeExceptions(target, source, dummyRulesetUri); + + expect(target).toEqual({ + 'file.yaml#/a': [], + 'file.yaml#/b': ['1', '2'], + 'file.yaml#/c': ['3'], + 'file.yaml#/d': ['4', '5'], + }); + }); + + it('merges existing exceptions', () => { + const target: RulesetExceptionCollection = { + 'file.yaml#/a': [], + 'file.yaml#/b': ['1', '3'], + }; + + const source: RulesetExceptionCollection = { + 'file.yaml#/a': ['0'], + 'file.yaml#/b': ['2', '4'], + }; + + mergeExceptions(target, source, dummyRulesetUri); + + expect(target).toEqual({ + 'file.yaml#/a': ['0'], + 'file.yaml#/b': ['1', '2', '3', '4'], + }); + }); + + it('deduplicates exceptions', () => { + const target: RulesetExceptionCollection = { + 'file.yaml#/a': [], + 'file.yaml#/b': ['1', '3'], + }; + + const source: RulesetExceptionCollection = { + 'file.yaml#/a': ['0', '0'], + 'file.yaml#/b': ['2', '4', '2', '3'], + }; + + mergeExceptions(target, source, dummyRulesetUri); + + expect(target).toEqual({ + 'file.yaml#/a': ['0'], + 'file.yaml#/b': ['1', '2', '3', '4'], + }); + }); + + describe('Validation', () => { + describe('Invalid locations', () => { + const invalidLocations = [ + 'where', + '123.yaml', + '../123.yaml', + '##where', + '#where', + '#', + '#/where', + '../123.yaml#where', + ]; + + it.each(invalidLocations)( + 'throws when locations are not valid uris (including fragment): "%s"', + async location => { + const source = buildRulesetExceptionCollectionFrom(location); + + expect(() => { + mergeExceptions({}, source, dummyRulesetUri); + }).toThrow( + new RegExp( + `.+\`${escapeRegExp(dummyRulesetUri)}\`.+\\(key \`${escapeRegExp(location)}\`.+is not a valid uri`, + ), + ); + }, + ); + }); + + describe('throws on empty rules array', () => { + const location = 'one.yaml#/'; + const source = buildRulesetExceptionCollectionFrom(location, []); + + expect(() => { + mergeExceptions({}, source, dummyRulesetUri); + }).toThrow( + new RegExp( + `.+\`${escapeRegExp(dummyRulesetUri)}\`.+\\(key \`${escapeRegExp( + location, + )}\`.+An empty array of rules has been provided`, + ), + ); + }); + + describe('throws on empty rule name', () => { + const location = 'one.yaml#/'; + const source = buildRulesetExceptionCollectionFrom(location, ['b', '']); + + expect(() => { + mergeExceptions({}, source, dummyRulesetUri); + }).toThrow( + new RegExp( + `.+\`${escapeRegExp(dummyRulesetUri)}\`.+\\(key \`${escapeRegExp( + location, + )}\`.+A rule with an empty name has been provided`, + ), + ); + }); + }); + + describe('Normalization', () => { + const relativeLocations: Array<[string, string, string]> = [ + ['./ruleset.yaml', 'one.yaml#', 'one.yaml#'], + ['./ruleset.yaml', 'one.yaml#/', 'one.yaml#/'], + ['./ruleset.yaml', 'one.yaml#/toto', 'one.yaml#/toto'], + ['./ruleset.yaml', 'down/one.yaml#/toto', 'down/one.yaml#/toto'], + ['./ruleset.yaml', '../one.yaml#/toto', '../one.yaml#/toto'], + ['../ruleset.yaml', 'one.yaml#', '../one.yaml#'], + ['../ruleset.yaml', 'one.yaml#/', '../one.yaml#/'], + ['../ruleset.yaml', 'one.yaml#/toto', '../one.yaml#/toto'], + ['../ruleset.yaml', 'down/one.yaml#/toto', '../down/one.yaml#/toto'], + ['../ruleset.yaml', '../one.yaml#/toto', '../../one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'one.yaml#', 'https://dot.com/r/one.yaml#'], + ['https://dot.com/r/ruleset.yaml', 'one.yaml#/', 'https://dot.com/r/one.yaml#/'], + ['https://dot.com/r/ruleset.yaml', 'one.yaml#/toto', 'https://dot.com/r/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'down/one.yaml#/toto', 'https://dot.com/r/down/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', '../one.yaml#/toto', 'https://dot.com/one.yaml#/toto'], + ]; + + it.each(relativeLocations)( + 'combines relative locations with ruleset uri (ruleset: "%s", location: "%s")', + (rulesetUri, location, expectedLocation) => { + const source = buildRulesetExceptionCollectionFrom(location); + const target = {}; + + mergeExceptions(target, source, rulesetUri); + + const expected = buildRulesetExceptionCollectionFrom(expectedLocation); + expect(target).toEqual(expected); + }, + ); + + const absoluteLocations: Array<[string, string, string]> = [ + ['./ruleset.yaml', 'https://dot.com/one.yaml#/toto', 'https://dot.com/one.yaml#/toto'], + ['../ruleset.yaml', 'https://dot.com/one.yaml#/toto', 'https://dot.com/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'https://dot.com/one.yaml#/toto', 'https://dot.com/one.yaml#/toto'], + ['./ruleset.yaml', '/local/one.yaml#/toto', '/local/one.yaml#/toto'], + ['../ruleset.yaml', '/local/one.yaml#/toto', '/local/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', '/local/one.yaml#/toto', '/local/one.yaml#/toto'], + ['./ruleset.yaml', 'c:/one.yaml#/toto', 'c:/one.yaml#/toto'], + ['../ruleset.yaml', 'c:/one.yaml#/toto', 'c:/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'c:/one.yaml#/toto', 'c:/one.yaml#/toto'], + ['./ruleset.yaml', 'c:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ['../ruleset.yaml', 'c:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'c:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ['./ruleset.yaml', 'C:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ['../ruleset.yaml', 'C:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ['https://dot.com/r/ruleset.yaml', 'C:\\one.yaml#/toto', 'c:/one.yaml#/toto'], + ]; + + it.each(absoluteLocations)( + 'normalize absolute locations (ruleset: "%s", location: "%s")', + (rulesetUri, location, expectedLocation) => { + const source = buildRulesetExceptionCollectionFrom(location); + const target = {}; + + mergeExceptions(target, source, rulesetUri); + + const expected = buildRulesetExceptionCollectionFrom(expectedLocation); + expect(target).toEqual(expected); + }, + ); + }); + }); +}); diff --git a/src/rulesets/mergers/exceptions.ts b/src/rulesets/mergers/exceptions.ts new file mode 100644 index 0000000000..16c952dbfe --- /dev/null +++ b/src/rulesets/mergers/exceptions.ts @@ -0,0 +1,78 @@ +import { extractPointerFromRef, extractSourceFromRef, pointerToPath } from '@stoplight/json'; +import { isAbsolute, join, normalize as pathNormalize } from '@stoplight/path'; +import { RulesetExceptionCollection } from '../../types/ruleset'; + +const normalize = ($ref: string, rulesetUri?: string): string => { + const source = extractSourceFromRef($ref); + + if (typeof source !== 'string') { + throw new Error(buildInvalidUriErrorMessage($ref, rulesetUri, 'Missing source')); + } + + if (rulesetUri === undefined && !isAbsolute(source)) { + throw new Error( + buildInvalidUriErrorMessage( + $ref, + rulesetUri, + 'Only absolute Uris are allowed when no base ruleset uri has been provided', + ), + ); + } + + const pointer = extractPointerFromRef($ref); + + if (typeof pointer !== 'string') { + throw new Error(buildInvalidUriErrorMessage($ref, rulesetUri, 'Missing pointer fragment')); + } + + try { + pointerToPath(pointer); + } catch { + throw new Error(buildInvalidUriErrorMessage($ref, rulesetUri)); + } + + const path = rulesetUri === undefined || isAbsolute(source) ? source : join(rulesetUri, '..', source); + + return pathNormalize(path) + pointer; +}; + +const buildErrorMessagePrefix = ($ref: string, rulesetUri?: string): string => { + let prefix = ''; + + if (rulesetUri !== undefined) prefix += `in ruleset \`${rulesetUri}\`, `; + + return prefix + `\`except\` entry (key \`${$ref}\`) is malformed. `; +}; + +const buildInvalidUriErrorMessage = ($ref: string, rulesetUri?: string, precision?: string): string => { + return ( + buildErrorMessagePrefix($ref, rulesetUri) + + `Key \`${$ref}\` is not a valid uri${precision ? ` (${precision})` : ''}.` + ); +}; + +export function mergeExceptions( + target: RulesetExceptionCollection, + source: RulesetExceptionCollection, + baseUri?: string, +): void { + for (const [location, sourceRules] of Object.entries(source)) { + const normalizedLocation = normalize(location, baseUri); + const targetRules = target[normalizedLocation] !== undefined ? target[normalizedLocation] : []; + + const set = new Set(targetRules); + + if (sourceRules.length === 0) { + throw new Error(buildErrorMessagePrefix(location, baseUri) + 'An empty array of rules has been provided.'); + } + + sourceRules.forEach(r => { + if (r.length === 0) { + throw new Error(buildErrorMessagePrefix(location, baseUri) + 'A rule with an empty name has been provided.'); + } + set.add(r); + }); + + target[normalizedLocation] = [...set].sort(); + } +} diff --git a/src/rulesets/reader.ts b/src/rulesets/reader.ts index d424da611b..166a58526d 100644 --- a/src/rulesets/reader.ts +++ b/src/rulesets/reader.ts @@ -8,6 +8,7 @@ import { httpAndFileResolver } from '../resolvers/http-and-file'; import { FileRulesetSeverity, IRuleset, RulesetFunctionCollection } from '../types/ruleset'; import { findFile } from './finder'; import { mergeFormats, mergeFunctions, mergeRules } from './mergers'; +import { mergeExceptions } from './mergers/exceptions'; import { assertValidRuleset } from './validation'; export interface IRulesetReadOptions { @@ -18,6 +19,7 @@ export async function readRuleset(uris: string | string[], opts?: IRulesetReadOp const base: IRuleset = { rules: {}, functions: {}, + exceptions: {}, }; const processedRulesets = new Set(); @@ -29,6 +31,7 @@ export async function readRuleset(uris: string | string[], opts?: IRulesetReadOp if (resolvedRuleset === null) continue; Object.assign(base.rules, resolvedRuleset.rules); Object.assign(base.functions, resolvedRuleset.functions); + Object.assign(base.exceptions, resolvedRuleset.exceptions); } return base; @@ -74,9 +77,11 @@ const createRulesetProcessor = ( const ruleset = assertValidRuleset(JSON.parse(JSON.stringify(result))); const rules = {}; const functions = {}; + const exceptions = {}; const newRuleset: IRuleset = { rules, functions, + exceptions, }; const extendedRulesets = ruleset.extends; @@ -97,6 +102,7 @@ const createRulesetProcessor = ( if (extendedRuleset !== null) { mergeRules(rules, extendedRuleset.rules, parentSeverity); Object.assign(functions, extendedRuleset.functions); + mergeExceptions(exceptions, extendedRuleset.exceptions, baseUri); } } } @@ -105,6 +111,10 @@ const createRulesetProcessor = ( mergeRules(rules, ruleset.rules, severity === undefined ? 'recommended' : severity); } + if (ruleset.except !== void 0) { + mergeExceptions(exceptions, ruleset.except, baseUri); + } + if (Array.isArray(ruleset.formats)) { mergeFormats(rules, ruleset.formats); } diff --git a/src/runner.ts b/src/runner.ts index fe96b2e67e..ad2ba60e32 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -4,7 +4,9 @@ import { DocumentInventory } from './documentInventory'; import { lintNode } from './linter'; import { getDiagnosticSeverity } from './rulesets/severity'; import { FunctionCollection, IGivenNode, IRule, IRuleResult, IRunRule, RunRuleCollection } from './types'; +import { RulesetExceptionCollection } from './types/ruleset'; import { hasIntersectingElement } from './utils/'; +import { IExceptionLocation, pivotExceptions } from './utils/pivotExceptions'; export const isRuleEnabled = (rule: IRule) => rule.severity !== void 0 && getDiagnosticSeverity(rule.severity) !== -1; @@ -12,9 +14,26 @@ export const runRules = ( documentInventory: DocumentInventory, rules: RunRuleCollection, functions: FunctionCollection, + exceptions: RulesetExceptionCollection, ): IRuleResult[] => { const results: IRuleResult[] = []; + // TODO: + // Ruleset + // - To be investigated: How to cope with STDIN as a source and exception relative locations? + // LoadRuleSet + // - TODO: Find out how paths that escape out of the root behave + // Spectral + // - idea: report exceptions hit through a new 'Silenced' Severy level (-2) -> This opens up a wide range of possible user friendly reporting + // - (optional) report the exceptions that haven't been triggered (to potentially help the user find out wrong paths) + // - (optional) report the exceptions that point at unknown (or disabled) rules (to potentially help the user find out wrong ruleset/excepts) + // Cli + // - (optional) before the start of analysis, should display something like "x loaded exceptions (spanning y files)" + // - (optional) should diplay a warning related to orphaned exceptions (exceptions that haven't been been triggered by the last run) in order to clean up obsolete exceptions + // - (optional) report issues that have been silenced + + const exceptRuleByLocations = pivotExceptions(exceptions, rules); + for (const name in rules) { if (!rules.hasOwnProperty(name)) continue; @@ -33,17 +52,26 @@ export const runRules = ( continue; } + let ruleResults: IRuleResult[] = []; + try { - results.push(...runRule(documentInventory, rule, functions)); + ruleResults = runRule(documentInventory, rule, functions, exceptRuleByLocations[name]); } catch (e) { console.error(`Unable to run rule '${name}':\n${e}`); } + + results.push(...ruleResults); } return results; }; -const runRule = (resolved: DocumentInventory, rule: IRunRule, functions: FunctionCollection): IRuleResult[] => { +const runRule = ( + resolved: DocumentInventory, + rule: IRunRule, + functions: FunctionCollection, + exceptionLocations: IExceptionLocation[] | undefined, +): IRuleResult[] => { const target = rule.resolved === false ? resolved.unresolved : resolved.resolved; const results: IRuleResult[] = []; @@ -59,6 +87,7 @@ const runRule = (resolved: DocumentInventory, rule: IRunRule, functions: Functio resolved, rule, functions, + exceptionLocations, results, ); } else { @@ -75,6 +104,7 @@ const runRule = (resolved: DocumentInventory, rule: IRunRule, functions: Functio resolved, rule, functions, + exceptionLocations, results, ); }, @@ -90,6 +120,7 @@ function lint( resolved: DocumentInventory, rule: IRunRule, functions: FunctionCollection, + exceptionLocations: IExceptionLocation[] | undefined, results: IRuleResult[], ): void { try { @@ -100,7 +131,7 @@ function lint( continue; } - const validationResults = lintNode(node, rule, then, func, resolved); + const validationResults = lintNode(node, rule, then, func, resolved, exceptionLocations); if (validationResults.length > 0) { results.push(...validationResults); diff --git a/src/spectral.ts b/src/spectral.ts index 7afae33613..0bde079c64 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -11,6 +11,7 @@ import { functions as defaultFunctions } from './functions'; import * as Parsers from './parsers'; import { readRuleset } from './rulesets'; import { compileExportedFunction } from './rulesets/evaluators'; +import { mergeExceptions } from './rulesets/mergers/exceptions'; import { IRulesetReadOptions } from './rulesets/reader'; import { DEFAULT_SEVERITY_LEVEL, getDiagnosticSeverity } from './rulesets/severity'; import { runRules } from './runner'; @@ -27,7 +28,7 @@ import { RuleCollection, RunRuleCollection, } from './types'; -import { IRuleset } from './types/ruleset'; +import { IRuleset, RulesetExceptionCollection } from './types/ruleset'; import { ComputeFingerprintFunc, defaultComputeResultFingerprint, empty, prepareResults } from './utils'; memoize.Cache = WeakMap; @@ -39,6 +40,7 @@ export class Spectral { public functions: FunctionCollection = { ...defaultFunctions }; public rules: RunRuleCollection = {}; + public exceptions: RulesetExceptionCollection = {}; public formats: RegisteredFormats; private readonly _computeFingerprint: ComputeFingerprintFunc; @@ -94,7 +96,7 @@ export class Spectral { return { resolved: inventory.resolved, results: prepareResults( - [...validationResults, ...runRules(inventory, this.rules, this.functions)], + [...validationResults, ...runRules(inventory, this.rules, this.functions, this.exceptions)], this._computeFingerprint, ), }; @@ -135,6 +137,17 @@ export class Spectral { } } + private setExceptions(exceptions: RulesetExceptionCollection) { + const target: RulesetExceptionCollection = {}; + mergeExceptions(target, exceptions); + + empty(this.exceptions); + + Object.entries(target).forEach(([location, rules]) => { + this.exceptions[location] = [...rules]; + }); + } + public async loadRuleset(uris: string[] | string, options?: IRulesetReadOptions) { this.setRuleset(await readRuleset(Array.isArray(uris) ? uris : [uris], options)); } @@ -164,6 +177,8 @@ export class Spectral { }, ), ); + + this.setExceptions(ruleset.exceptions); } public registerFormat(format: string, fn: FormatLookup) { diff --git a/src/types/ruleset.ts b/src/types/ruleset.ts index e084da8cc3..1651a12989 100644 --- a/src/types/ruleset.ts +++ b/src/types/ruleset.ts @@ -18,10 +18,12 @@ export interface IRulesetFunctionDefinition { } export type RulesetFunctionCollection = Dictionary; +export type RulesetExceptionCollection = Dictionary; export interface IRuleset { rules: RuleCollection; functions: RulesetFunctionCollection; + exceptions: RulesetExceptionCollection; } export interface IRulesetFile { @@ -30,4 +32,5 @@ export interface IRulesetFile { rules?: FileRuleCollection; functionsDir?: string; functions?: Array; + except?: RulesetExceptionCollection; } diff --git a/src/utils/__tests__/pivotExceptions.test.ts b/src/utils/__tests__/pivotExceptions.test.ts new file mode 100644 index 0000000000..a5934e11db --- /dev/null +++ b/src/utils/__tests__/pivotExceptions.test.ts @@ -0,0 +1,51 @@ +import { DiagnosticSeverity, Dictionary } from '@stoplight/types'; +import { IRunRule, RuleFunction } from '../../types'; +import { IExceptionLocation, pivotExceptions } from '../pivotExceptions'; + +describe('pivotExceptions', () => { + const dummyRule: IRunRule = { + name: '', + severity: DiagnosticSeverity.Error, + given: '', + then: { + function: RuleFunction.TRUTHY, + }, + }; + + it('ignores exceptions for rules that are not part of the run', () => { + const exceptions = { + 'one#/1': ['a', 'c'], + }; + + const runRules = { a: dummyRule }; + const expected: Dictionary = { + a: [{ source: 'one', path: ['1'] }], + }; + + expect(pivotExceptions(exceptions, runRules)).toEqual(expected); + }); + + it('returns a rule based dictionary', () => { + const exceptions = { + 'one#/1': ['a', 'c'], + 'two#/2': ['b', 'd'], + 'three#/3': ['b', 'a'], + }; + + const runRules = { a: dummyRule, b: dummyRule, c: dummyRule, d: dummyRule }; + const expected: Dictionary = { + a: [ + { source: 'one', path: ['1'] }, + { source: 'three', path: ['3'] }, + ], + b: [ + { source: 'two', path: ['2'] }, + { source: 'three', path: ['3'] }, + ], + c: [{ source: 'one', path: ['1'] }], + d: [{ source: 'two', path: ['2'] }], + }; + + expect(pivotExceptions(exceptions, runRules)).toEqual(expected); + }); +}); diff --git a/src/utils/pivotExceptions.ts b/src/utils/pivotExceptions.ts new file mode 100644 index 0000000000..b69d4c7aba --- /dev/null +++ b/src/utils/pivotExceptions.ts @@ -0,0 +1,35 @@ +import { extractPointerFromRef, extractSourceFromRef, pointerToPath } from '@stoplight/json'; +import { Dictionary, JsonPath } from '@stoplight/types'; +import { IRunRule } from '../types'; +import { RulesetExceptionCollection } from '../types/ruleset'; + +export interface IExceptionLocation { + source: string; + path: JsonPath; +} + +export const pivotExceptions = ( + exceptions: RulesetExceptionCollection, + runRules: Dictionary, +): Dictionary => { + const dic: Dictionary = {}; + + Object.entries(exceptions).forEach(([location, rules]) => { + const pointer = extractPointerFromRef(location)!; + const source = extractSourceFromRef(location)!; + + rules.forEach(rulename => { + const rule = runRules[rulename]; + + if (rule !== undefined) { + if (!(rulename in dic)) { + dic[rulename] = []; + } + + dic[rulename].push({ source, path: pointerToPath(pointer) }); + } + }); + }); + + return dic; +}; diff --git a/test-harness/scenarios/exceptions.oas3.scenario b/test-harness/scenarios/exceptions.oas3.scenario new file mode 100644 index 0000000000..484a3b4d1d --- /dev/null +++ b/test-harness/scenarios/exceptions.oas3.scenario @@ -0,0 +1,26 @@ +====test==== +Do not report exceptions +====document==== +openapi: 3.0.0 +info: + version: 1.0.0 + title: Stoplight +paths: {} +====asset:ruleset==== +extends: spectral:oas + +except: + "{document}#": + - oas3-api-servers + "{document}#/info": + - info-contact +====command==== +{bin} lint {document} -r {asset:ruleset} +====stdout==== +OpenAPI 3.x detected + +{document} + 1:1 warning openapi-tags OpenAPI object should have non-empty `tags` array. + 2:6 warning info-description OpenAPI object info `description` must be present and non-empty string. + +✖ 2 problems (0 error, 2 warnings, 0 infos, 0 hints)