From 153d68557da4bcffd6d2ed2261bcdb6a8324cdb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Mon, 28 Jun 2021 12:58:47 +0200 Subject: [PATCH] feat(core): support overrides in rulesets (#1684) Co-authored-by: William Hilton --- .../src/services/linter/utils/getRuleset.ts | 8 +- packages/core/package.json | 2 + packages/core/src/meta/ruleset.schema.json | 57 +++ .../__tests__/__fixtures__/overrides/_base.ts | 43 ++ .../__fixtures__/overrides/extends/all.ts | 14 + .../__fixtures__/overrides/extends/both.ts | 15 + .../overrides/extends/multiple-extends.ts | 18 + .../__fixtures__/overrides/formats.ts | 49 ++ .../__fixtures__/overrides/hierarchy.ts | 34 ++ .../overrides/new-definitions-error.ts | 27 ++ .../__fixtures__/overrides/new-definitions.ts | 30 ++ .../__fixtures__/overrides/only-overrides.ts | 10 + .../__fixtures__/overrides/only-rules.ts | 26 + .../__fixtures__/overrides/parser-options.ts | 24 + .../ruleset/__tests__/__helpers__/print.ts | 4 +- .../src/ruleset/__tests__/ruleset.test.ts | 451 +++++++++++++++++- .../src/ruleset/__tests__/validation.test.ts | 36 +- packages/core/src/ruleset/mergers/rules.ts | 8 +- packages/core/src/ruleset/mergers/rulesets.ts | 61 +++ packages/core/src/ruleset/ruleset.ts | 83 +++- packages/core/src/ruleset/types.ts | 32 +- packages/core/src/ruleset/validation.ts | 4 +- packages/core/src/runner/runner.ts | 2 +- packages/core/src/spectral.ts | 76 +-- test-harness/helpers.ts | 59 ++- test-harness/index.ts | 11 +- .../scenarios/overrides/extends.scenario | 103 ++++ .../scenarios/overrides/rules.scenario | 97 ++++ test-harness/spawn.ts | 10 +- yarn.lock | 2 +- 30 files changed, 1293 insertions(+), 103 deletions(-) create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/_base.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/extends/all.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/extends/both.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/extends/multiple-extends.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/formats.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/hierarchy.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/new-definitions-error.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/new-definitions.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/only-overrides.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/only-rules.ts create mode 100644 packages/core/src/ruleset/__tests__/__fixtures__/overrides/parser-options.ts create mode 100644 packages/core/src/ruleset/mergers/rulesets.ts create mode 100644 test-harness/scenarios/overrides/extends.scenario create mode 100644 test-harness/scenarios/overrides/rules.scenario diff --git a/packages/cli/src/services/linter/utils/getRuleset.ts b/packages/cli/src/services/linter/utils/getRuleset.ts index 380f55fd6..d0d7b2ab2 100644 --- a/packages/cli/src/services/linter/utils/getRuleset.ts +++ b/packages/cli/src/services/linter/utils/getRuleset.ts @@ -1,10 +1,9 @@ import { Optional } from '@stoplight/types'; -import { Ruleset } from '@stoplight/spectral-core'; +import { Ruleset, RulesetDefinition } from '@stoplight/spectral-core'; import * as fs from 'fs'; import * as path from '@stoplight/path'; import { isAbsolute } from '@stoplight/path'; import * as process from 'process'; -import { RulesetDefinition } from '@stoplight/spectral-core'; async function getDefaultRulesetFile(): Promise> { const cwd = process.cwd(); @@ -30,5 +29,8 @@ export async function getRuleset(rulesetFile: Optional): Promise 0 ? { formats: printFormats([...rule.formats]) } : null), severity: String(rule.severity), ...(rule.documentationUrl !== null ? { documentationUrl: rule.documentationUrl } : null), }; diff --git a/packages/core/src/ruleset/__tests__/ruleset.test.ts b/packages/core/src/ruleset/__tests__/ruleset.test.ts index 8aef20fba..86d62dea3 100644 --- a/packages/core/src/ruleset/__tests__/ruleset.test.ts +++ b/packages/core/src/ruleset/__tests__/ruleset.test.ts @@ -1,13 +1,15 @@ import { oas2 } from '@stoplight/spectral-formats'; import { truthy } from '@stoplight/spectral-functions'; +import * as path from '@stoplight/path'; +import { DiagnosticSeverity } from '@stoplight/types'; import { Ruleset } from '../ruleset'; import { RulesetDefinition } from '../types'; import { print } from './__helpers__/print'; import { RulesetValidationError } from '../validation'; -async function loadRuleset(mod: Promise<{ default: RulesetDefinition }>): Promise { - return new Ruleset((await mod).default); +async function loadRuleset(mod: Promise<{ default: RulesetDefinition }>, source?: string): Promise { + return new Ruleset((await mod).default, { source }); } describe('Ruleset', () => { @@ -185,7 +187,7 @@ describe('Ruleset', () => { // @ts-expect-error: invalid ruleset {}, ), - ).toThrowError(new RulesetValidationError('Ruleset must have rules or extends property')); + ).toThrowError(new RulesetValidationError('Ruleset must have rules or extends or overrides defined')); }); }); @@ -237,4 +239,447 @@ describe('Ruleset', () => { incompatibleValues: 'off', }); }); + + describe('overrides', () => { + const cwd = path.join(__dirname, './__fixtures__/overrides/'); + + it('given no overrides, should return the initial ruleset', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/_base'), path.join(cwd, 'hierarchy')); + + expect(ruleset.fromSource(null)).toBe(ruleset); + }); + + it('given a ruleset with overrides only, should consider rule as empty for unmatched files', async () => { + const ruleset = await loadRuleset( + import('./__fixtures__/overrides/only-overrides'), + path.join(cwd, 'only-overrides'), + ); + + expect(ruleset.rules).toEqual({}); + expect(ruleset.fromSource(path.join(cwd, 'spec.yaml')).rules).toEqual({}); + }); + + it('rules', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/only-rules'), path.join(cwd, 'only-rules')); + + expect(print(ruleset)).toEqual(print(await loadRuleset(import('./__fixtures__/overrides/_base')))); + + expect(print(ruleset)).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: false + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: false + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: false + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'unmatched/spec.json')))).toEqual(print(ruleset)); + + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: -1 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/test/spec.json')))).toEqual( + print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json'))), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'v2/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 3 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + }); + + it('should respect the hierarchy of overrides', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/hierarchy'), path.join(cwd, 'hierarchy')); + + expect(print(ruleset)).toEqual(print(await loadRuleset(import('./__fixtures__/overrides/_base')))); + + expect(print(ruleset)).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: false + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: false + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: false + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'unmatched/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 2 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: -1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 2 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: -1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: true + ├─ inherited: true + └─ severity: 1 +`); + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/test/spec.json')))).toEqual( + print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json'))), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'v2/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 2 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: -1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + }); + + it('should support new rule definitions', async () => { + const ruleset = await loadRuleset( + import('./__fixtures__/overrides/new-definitions'), + path.join(cwd, 'new-definitions'), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + ├─ contact-name-matches-stoplight + │ ├─ name: contact-name-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 1 + └─ value-matches-stoplight + ├─ name: value-matches-stoplight + ├─ enabled: true + ├─ inherited: false + └─ severity: 0 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'legacy/test/spec.json')))).toEqual( + print(ruleset.fromSource(path.join(cwd, 'legacy/spec.json'))), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'v2/spec.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + }); + + it('should respect parserOptions', async () => { + const ruleset = await loadRuleset( + import('./__fixtures__/overrides/parser-options'), + path.join(cwd, 'parser-options'), + ); + + expect(ruleset.fromSource(path.join(cwd, 'document.json')).parserOptions).toStrictEqual({ + duplicateKeys: DiagnosticSeverity.Error, + incompatibleValues: DiagnosticSeverity.Error, + }); + + expect(ruleset.fromSource(path.join(cwd, 'legacy/apis/document.json')).parserOptions).toStrictEqual({ + duplicateKeys: DiagnosticSeverity.Error, + incompatibleValues: DiagnosticSeverity.Hint, + }); + + expect(ruleset.fromSource(path.join(cwd, 'v2/document.json')).parserOptions).toStrictEqual({ + incompatibleValues: DiagnosticSeverity.Warning, + duplicateKeys: DiagnosticSeverity.Information, + }); + }); + + it('should respect formats', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/formats'), path.join(cwd, 'formats')); + + expect(print(ruleset.fromSource(path.join(cwd, 'schemas/common/user.draft7.json')))) + .toEqual(`├─ formats: JSON Schema Draft 7 +└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + ├─ contact-name-matches-stoplight + │ ├─ name: contact-name-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 1 + └─ valid-number-validation + ├─ name: valid-number-validation + ├─ enabled: true + ├─ inherited: false + ├─ formats: JSON Schema Draft 7 + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'schemas/user.draft4.json')))) + .toEqual(`├─ formats: JSON Schema Draft 4 +└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + ├─ contact-name-matches-stoplight + │ ├─ name: contact-name-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 1 + └─ valid-number-validation + ├─ name: valid-number-validation + ├─ enabled: true + ├─ inherited: false + ├─ formats: JSON Schema Draft 4 + └─ severity: 1 +`); + }); + + describe('extends', () => { + const cwd = path.join(__dirname, './__fixtures__/overrides/extends'); + + it('given local extend with severity set to all, should mark all rules as enabled for matching files', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/extends/all'), path.join(cwd, 'all')); + + expect(print(ruleset.fromSource(path.join(cwd, 'document.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: true + ├─ inherited: true + └─ severity: 1 +`); + }); + + it('given multiple extends, should merge them respecting the hierarchy', async () => { + const ruleset = await loadRuleset( + import('./__fixtures__/overrides/extends/multiple-extends'), + path.join(cwd, 'multiple-extends'), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'document.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: true + ├─ inherited: true + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'v2/document.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + }); + + it('given presence of extends in both the ruleset and the override, should always prioritize the override', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/extends/both'), path.join(cwd, 'both')); + + expect(print(ruleset.fromSource(path.join(cwd, 'document.json')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: true + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: true + ├─ inherited: true + └─ severity: 1 +`); + + expect(print(ruleset.fromSource(path.join(cwd, 'v2/document.json')))).toEqual( + print(ruleset.fromSource(path.join(cwd, 'document.json'))), + ); + + expect(print(ruleset.fromSource(path.join(cwd, 'document.yaml')))).toEqual(`└─ rules + ├─ description-matches-stoplight + │ ├─ name: description-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 0 + ├─ title-matches-stoplight + │ ├─ name: title-matches-stoplight + │ ├─ enabled: false + │ ├─ inherited: true + │ └─ severity: 1 + └─ contact-name-matches-stoplight + ├─ name: contact-name-matches-stoplight + ├─ enabled: false + ├─ inherited: true + └─ severity: 1 +`); + }); + }); + + describe('error handling', () => { + it('given document with no source, should throw an error', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/hierarchy'), path.join(cwd, 'hierarchy')); + + expect(ruleset.fromSource.bind(ruleset, null)).toThrowError( + Error( + 'Document must have some source assigned. If you use Spectral programmatically make sure to pass the source to Document', + ), + ); + }); + + it('given ruleset with no source, should throw an error', async () => { + const ruleset = await loadRuleset(import('./__fixtures__/overrides/hierarchy')); + + expect(ruleset.fromSource.bind(ruleset, path.join(cwd, 'v2/spec.json'))).toThrowError( + Error( + 'Ruleset must have some source assigned. If you use Spectral programmatically make sure to pass the source to Ruleset', + ), + ); + }); + + it('given no local or extended rule, should throw an error', async () => { + const ruleset = await loadRuleset( + import('./__fixtures__/overrides/new-definitions-error'), + path.join(cwd, 'new-definitions-error'), + ); + + expect(ruleset.fromSource.bind(ruleset, path.join(cwd, 'v2/spec.json'))).toThrowError( + ReferenceError('Cannot extend non-existing rule: "new-definition"'), + ); + }); + }); + }); }); diff --git a/packages/core/src/ruleset/__tests__/validation.test.ts b/packages/core/src/ruleset/__tests__/validation.test.ts index be32cf612..4a429b469 100644 --- a/packages/core/src/ruleset/__tests__/validation.test.ts +++ b/packages/core/src/ruleset/__tests__/validation.test.ts @@ -15,8 +15,10 @@ describe('Ruleset Validation', () => { }); it('given object with no rules and no extends properties, throws', () => { - expect(assertValidRuleset.bind(null, {})).toThrow('Ruleset must have rules or extends property'); - expect(assertValidRuleset.bind(null, { rule: {} })).toThrow('Ruleset must have rules or extends property'); + expect(assertValidRuleset.bind(null, {})).toThrow('Ruleset must have rules or extends or overrides defined'); + expect(assertValidRuleset.bind(null, { rule: {} })).toThrow( + 'Ruleset must have rules or extends or overrides defined', + ); }); it('given object with extends property only, emits no errors', () => { @@ -223,6 +225,36 @@ Error at #/rules/rule/formats/1: must be a valid format`, ).toThrow(new RulesetValidationError(error)); }); + describe('overrides validation', () => { + it('given an invalid overrides, throws', () => { + expect( + assertValidRuleset.bind(null, { + overrides: null, + }), + ).toThrow(new RulesetValidationError('Error at #/overrides: must be an array')); + }); + + it('given an empty overrides, throws', () => { + expect( + assertValidRuleset.bind(null, { + overrides: [], + }), + ).toThrow(new RulesetValidationError('Error at #/overrides: must not be empty')); + }); + + it('given an invalid pattern, throws', () => { + expect( + assertValidRuleset.bind(null, { + overrides: [2], + }), + ).toThrow( + new RulesetValidationError( + 'Error at #/overrides/0: must be a override, i.e. { "files": ["v2/**/*.json"], "rules": {} }', + ), + ); + }); + }); + describe('then validation', () => { describe('custom function', () => { it('given valid then, does not complain', () => { diff --git a/packages/core/src/ruleset/mergers/rules.ts b/packages/core/src/ruleset/mergers/rules.ts index 47deac8bd..c195e5e22 100644 --- a/packages/core/src/ruleset/mergers/rules.ts +++ b/packages/core/src/ruleset/mergers/rules.ts @@ -4,9 +4,9 @@ import { Rule } from '../rule/rule'; import type { Ruleset } from '../ruleset'; import { FileRuleDefinition } from '../types'; -function assertExistingRule(maybeRule: Optional): asserts maybeRule is Rule { +function assertExistingRule(maybeRule: Optional, name: string): asserts maybeRule is Rule { if (maybeRule === void 0) { - throw new ReferenceError('Cannot extend non-existing rule'); + throw new ReferenceError(`Cannot extend non-existing rule: "${name}"`); } } @@ -24,12 +24,12 @@ export function mergeRule( ): Rule { switch (typeof rule) { case 'boolean': - assertExistingRule(existingRule); + assertExistingRule(existingRule, name); existingRule.enabled = rule; break; case 'string': case 'number': - assertExistingRule(existingRule); + assertExistingRule(existingRule, name); existingRule.severity = rule; if (rule === 'off') { existingRule.enabled = false; diff --git a/packages/core/src/ruleset/mergers/rulesets.ts b/packages/core/src/ruleset/mergers/rulesets.ts new file mode 100644 index 000000000..9dabbe93b --- /dev/null +++ b/packages/core/src/ruleset/mergers/rulesets.ts @@ -0,0 +1,61 @@ +import { + FileRuleDefinition, + FileRulesetSeverityDefinition, + RulesetDefinition, + RulesetExtendsDefinition, + RulesetOverrideDefinition, +} from '../types'; + +type MergeableRuleset = Omit | RulesetOverrideDefinition; + +function getExtension( + extension: RulesetDefinition | [RulesetDefinition, FileRulesetSeverityDefinition], +): Readonly { + return Array.isArray(extension) ? extension[0] : extension; +} + +function getExtensions(extensions: RulesetExtendsDefinition): ReadonlyArray { + return (Array.isArray(extensions) ? extensions : [extensions]).map(getExtension); +} + +export function mergeRulesets(left: MergeableRuleset, right: MergeableRuleset, isOverride: boolean): RulesetDefinition { + const ruleset: MergeableRuleset = { + ...left, + ...right, + }; + + if ('extends' in ruleset && 'extends' in ruleset) { + const rightExtensions = getExtensions(ruleset.extends); + (ruleset as Omit & { extends: RulesetExtendsDefinition }).extends = [ + ...(Array.isArray(ruleset.extends) ? ruleset.extends : [ruleset.extends]).filter( + ext => !rightExtensions.includes(getExtension(ext)), + ), + ...(Array.isArray(ruleset.extends) ? ruleset.extends : [ruleset.extends]), + ]; + } + + if (!('rules' in left) || !('rules' in right)) return ruleset as RulesetDefinition; + + if (isOverride) { + (ruleset as Omit & { rules: Record }).rules = { + ...left.rules, + ...right.rules, + }; + } else { + const _ruleset = { + rules: left.rules, + } as RulesetDefinition; + + const r = ruleset as Omit & { extends?: RulesetExtendsDefinition }; + + if (!('extends' in r)) { + r.extends = _ruleset; + } else if (Array.isArray(r.extends)) { + r.extends = [...r.extends, _ruleset]; + } else { + r.extends = [r.extends as RulesetDefinition, _ruleset]; + } + } + + return ruleset as RulesetDefinition; +} diff --git a/packages/core/src/ruleset/ruleset.ts b/packages/core/src/ruleset/ruleset.ts index 6980c2abf..c7aabb6c3 100644 --- a/packages/core/src/ruleset/ruleset.ts +++ b/packages/core/src/ruleset/ruleset.ts @@ -1,25 +1,35 @@ +import { dirname, relative } from '@stoplight/path'; +import * as minimatch from 'minimatch'; import { Rule } from './rule/rule'; -import { FileRulesetSeverityDefinition, ParserOptions, RulesetDefinition } from './types'; +import { FileRulesetSeverityDefinition, ParserOptions, RulesetDefinition, RulesetOverridesDefinition } from './types'; import { assertValidRuleset } from './validation'; import { Format } from './format'; import { mergeRule } from './mergers/rules'; import { DEFAULT_PARSER_OPTIONS } from '..'; +import { mergeRulesets } from './mergers/rulesets'; const STACK_SYMBOL = Symbol('@stoplight/spectral/ruleset/#stack'); +const DEFAULT_RULESET_FILE = /^\.?spectral\.(ya?ml|json|m?js)$/; type RulesetContext = { - readonly severity: FileRulesetSeverityDefinition; + readonly severity?: FileRulesetSeverityDefinition; + readonly source?: string; readonly [STACK_SYMBOL]?: Map; }; export class Ruleset { protected extends!: Ruleset[]; public readonly formats = new Set(); + public readonly overrides: RulesetOverridesDefinition | null; + + readonly #context: RulesetContext & { severity: FileRulesetSeverityDefinition }; + + constructor(public readonly definition: RulesetDefinition, context?: RulesetContext) { + this.#context = { + severity: 'recommended', + ...context, + }; - constructor( - protected readonly definition: RulesetDefinition, - protected readonly context: RulesetContext = { severity: 'recommended' }, - ) { const stack = context?.[STACK_SYMBOL] ?? new Map(); stack.set(this.definition, this); @@ -58,6 +68,12 @@ export class Ruleset { ) : []; + if (stack.size === 1 && definition.overrides) { + this.overrides = definition.overrides; + } else { + this.overrides = null; + } + stack.delete(this.definition); if (Array.isArray(this.definition.formats)) { @@ -75,6 +91,55 @@ export class Ruleset { this.rules; } + get source(): string | null { + return this.#context.source ?? null; + } + + public fromSource(source: string | null): Ruleset { + if (this.overrides === null) { + return this; + } + + if (source === null) { + throw new Error( + 'Document must have some source assigned. If you use Spectral programmatically make sure to pass the source to Document', + ); + } + + if (this.source === null) { + throw new Error( + 'Ruleset must have some source assigned. If you use Spectral programmatically make sure to pass the source to Ruleset', + ); + } + + const relativeSource = relative(dirname(this.source), source); + const minimatchOpts = { matchBase: true }; + const overrides = this.overrides + .filter(({ files }) => files.some(pattern => minimatch(relativeSource, pattern, minimatchOpts))) + .map(({ files, ...ruleset }) => ruleset); + + const { overrides: _, ...definition } = this.definition; + + if (overrides.length === 0) { + return this; + } + + const mergedOverrides = + overrides.length > 1 + ? overrides + .slice(1) + .reduce( + (left, right) => mergeRulesets(left, right, true), + overrides[0] as RulesetDefinition, + ) + : overrides[0]; + + return new Ruleset(mergeRulesets(definition, mergedOverrides, false), { + severity: 'recommended', + source: this.source, + }); + } + public get rules(): Record { const rules: Record = {}; @@ -94,7 +159,7 @@ export class Ruleset { if (rule.owner === this) { rule.enabled = - this.context.severity === 'all' || (this.context.severity === 'recommended' && rule.recommended); + this.#context.severity === 'all' || (this.#context.severity === 'recommended' && rule.recommended); } if (rule.formats !== null) { @@ -102,7 +167,7 @@ export class Ruleset { this.formats.add(format); } } else if (rule.owner !== this) { - rule.formats = new Set(rule.owner.definition.formats); + rule.formats = rule.owner.definition.formats === void 0 ? null : new Set(rule.owner.definition.formats); } else if (this.definition.formats !== void 0) { rule.formats = new Set(this.definition.formats); } @@ -121,8 +186,6 @@ export class Ruleset { } public static isDefaultRulesetFile(uri: string): boolean { - const DEFAULT_RULESET_FILE = /^\.?spectral\.(ya?ml|json|m?js)$/; - return DEFAULT_RULESET_FILE.test(uri); } } diff --git a/packages/core/src/ruleset/types.ts b/packages/core/src/ruleset/types.ts index 2ac3327e0..7af0a8458 100644 --- a/packages/core/src/ruleset/types.ts +++ b/packages/core/src/ruleset/types.ts @@ -8,8 +8,6 @@ export type FileRulesetSeverityDefinition = 'off' | 'recommended' | 'all'; export type FileRuleDefinition = RuleDefinition | FileRuleSeverityDefinition; -export type FileRuleCollection = Record; - export type ParserOptions = { duplicateKeys: DiagnosticSeverity | HumanReadableDiagnosticSeverity; incompatibleValues: DiagnosticSeverity | HumanReadableDiagnosticSeverity; @@ -58,21 +56,45 @@ export interface IRuleThen { functionOptions?: unknown; } +export type RulesetExtendsDefinition = + | RulesetDefinition + | (RulesetDefinition | [RulesetDefinition, FileRulesetSeverityDefinition])[]; + +export type RulesetOverrideDefinition = Pick & + ( + | { + extends: RulesetExtendsDefinition; + } + | { + rules: Record>; + } + | { + extends: RulesetExtendsDefinition; + rules: Record>; + } + ); + +export type RulesetOverridesDefinition = ReadonlyArray<{ files: string[] } & RulesetOverrideDefinition>; + export type RulesetDefinition = Readonly< { documentationUrl?: string; formats?: Format[]; parserOptions?: Partial; + overrides?: RulesetOverridesDefinition; } & Readonly< | { - extends: RulesetDefinition | Array; + overrides: RulesetOverridesDefinition; + } + | { + extends: RulesetExtendsDefinition; } | { rules: Record>; } | { - extends: RulesetDefinition | Array; - rules: FileRuleCollection; + extends: RulesetExtendsDefinition; + rules: Record>; } > >; diff --git a/packages/core/src/ruleset/validation.ts b/packages/core/src/ruleset/validation.ts index 5d65a2463..f24ed9446 100644 --- a/packages/core/src/ruleset/validation.ts +++ b/packages/core/src/ruleset/validation.ts @@ -108,8 +108,8 @@ export function assertValidRuleset(ruleset: unknown): asserts ruleset is Ruleset throw new Error('Provided ruleset is not an object'); } - if (!('rules' in ruleset) && !('extends' in ruleset)) { - throw new RulesetValidationError('Ruleset must have rules or extends property'); + if (!('rules' in ruleset) && !('extends' in ruleset) && !('overrides' in ruleset)) { + throw new RulesetValidationError('Ruleset must have rules or extends or overrides defined'); } if (!validate(ruleset)) { diff --git a/packages/core/src/runner/runner.ts b/packages/core/src/runner/runner.ts index 2c4c66378..c1c82e433 100644 --- a/packages/core/src/runner/runner.ts +++ b/packages/core/src/runner/runner.ts @@ -52,7 +52,7 @@ export class Runner { public readonly results: IRuleResult[]; constructor(protected readonly runtime: RunnerRuntime, protected readonly inventory: DocumentInventory) { - this.results = [...this.inventory.diagnostics, ...this.document.diagnostics, ...(this.inventory.errors ?? [])]; + this.results = [...this.inventory.diagnostics, ...(this.inventory.errors ?? [])]; } protected get document(): IDocument { diff --git a/packages/core/src/spectral.ts b/packages/core/src/spectral.ts index d97574db6..28631c42a 100644 --- a/packages/core/src/spectral.ts +++ b/packages/core/src/spectral.ts @@ -11,7 +11,7 @@ import { IConstructorOpts, IRunOpts, ISpectralDiagnostic, ISpectralFullResult } import { ComputeFingerprintFunc, defaultComputeResultFingerprint } from './utils'; import { Ruleset } from './ruleset/ruleset'; import { generateDocumentWideResult } from './utils/generateDocumentWideResult'; -import { RulesetDefinition } from './ruleset/types'; +import { ParserOptions, RulesetDefinition } from './ruleset/types'; import { Format } from './ruleset/format'; import { getDiagnosticSeverity } from './ruleset'; @@ -41,53 +41,28 @@ export class Spectral { } protected parseDocument(target: IParsedResult | IDocument | Record | string): IDocument { - const document = - target instanceof Document - ? target - : isParsedResult(target) - ? new ParsedDocument(target) - : new Document>( - typeof target === 'string' ? target : stringify(target, void 0, 2), - Parsers.Yaml, - ); - - let i = -1; - for (const diagnostic of document.diagnostics.slice()) { - i++; - if (diagnostic.code !== 'parser') continue; - - let severity; - - if (diagnostic.message.startsWith('Mapping key must be a string scalar rather than')) { - severity = getDiagnosticSeverity(this.ruleset.parserOptions.incompatibleValues); - } else if (diagnostic.message.startsWith('Duplicate key')) { - severity = getDiagnosticSeverity(this.ruleset.parserOptions.duplicateKeys); - } else { - continue; - } - - if (severity === -1) { - document.diagnostics.splice(i, 1); - i--; - } else { - diagnostic.severity = severity; - } - } - - return document; + return target instanceof Document + ? target + : isParsedResult(target) + ? new ParsedDocument(target) + : new Document>( + typeof target === 'string' ? target : stringify(target, void 0, 2), + Parsers.Yaml, + ); } public async runWithResolved( target: IParsedResult | IDocument | Record | string, opts: IRunOpts = {}, ): Promise { - const { ruleset } = this; - const document = this.parseDocument(target); + const ruleset = this.ruleset.fromSource(document.source); + const inventory = new DocumentInventory(document, this._resolver); await inventory.resolve(); const runner = new Runner(this.runtime, inventory); + runner.results.push(...this._filterParserErrors(document.diagnostics, ruleset.parserOptions)); if (document.formats === void 0) { const foundFormats = [...ruleset.formats].filter(format => format(inventory.resolved, document.source)); @@ -132,4 +107,31 @@ export class Spectral { 'unrecognized-format', ); } + + private _filterParserErrors( + diagnostics: ReadonlyArray, + parserOptions: ParserOptions, + ): ISpectralDiagnostic[] { + return diagnostics.reduce((diagnostics, diagnostic) => { + if (diagnostic.code !== 'parser') return diagnostics; + + let severity; + + if (diagnostic.message.startsWith('Mapping key must be a string scalar rather than')) { + severity = getDiagnosticSeverity(parserOptions.incompatibleValues); + } else if (diagnostic.message.startsWith('Duplicate key')) { + severity = getDiagnosticSeverity(parserOptions.duplicateKeys); + } else { + diagnostics.push(diagnostic); + return diagnostics; + } + + if (severity !== -1) { + diagnostics.push(diagnostic); + diagnostic.severity = severity; + } + + return diagnostics; + }, []); + } } diff --git a/test-harness/helpers.ts b/test-harness/helpers.ts index 0384cf79d..bd554fd90 100644 --- a/test-harness/helpers.ts +++ b/test-harness/helpers.ts @@ -9,6 +9,7 @@ export interface IScenarioFile { test: string; assets: string[][]; command: Optional; + cwd: Optional; status: Optional; stdout: Optional; stderr: Optional; @@ -31,7 +32,8 @@ function getItem(input: string[], key: string, required?: boolean): Optional { - await fs.promises.mkdir(path.join(__dirname, 'tmp'), { recursive: true }); +const TMP_DIR = path.join(__dirname, 'tmp'); +if (fs.existsSync(TMP_DIR)) { + fs.rmSync(TMP_DIR, { recursive: true }); +} + +export async function tmpFile(opts?: tmp.TmpNameOptions): Promise { return new Promise((resolve, reject) => { - tmp.file( - { - tmpdir: path.join(__dirname, 'tmp'), - postfix: '.yml', - prefix: 'asset-', - tries: 10, - ...opts, - }, - (err, name, fd, removeCallback) => { - if (err) { - reject(err); - } else { - resolve({ - name, - fd, - removeCallback, - }); - } - }, - ); + fs.promises.mkdir(path.join(TMP_DIR, opts?.tmpdir ?? ''), { recursive: true }).then(() => { + tmp.file( + { + postfix: '.yml', + tries: 10, + ...opts, + tmpdir: path.join(__dirname, 'tmp', opts?.tmpdir ?? ''), + }, + (err, name, fd, removeCallback) => { + if (err) { + reject(err); + } else { + resolve({ + name, + fd, + removeCallback, + }); + } + }, + ); + }); }); } diff --git a/test-harness/index.ts b/test-harness/index.ts index 682314b22..05bc25c2a 100644 --- a/test-harness/index.ts +++ b/test-harness/index.ts @@ -4,10 +4,8 @@ import { Dictionary } from '@stoplight/types'; import * as fg from 'fast-glob'; import * as fs from 'fs'; import * as tmp from 'tmp'; -import { promisify } from 'util'; import { applyReplacements, normalizeLineEndings, parseScenarioFile, tmpFile } from './helpers'; import { spawnNode } from './spawn'; -const writeFileAsync = promisify(fs.writeFile); const spectralBin = path.join(__dirname, '../packages/cli/binaries/spectral'); const cwd = path.join(__dirname, './scenarios'); @@ -33,9 +31,10 @@ describe('cli acceptance tests', () => { beforeAll(async () => { await Promise.all( scenario.assets.map(async ([asset]) => { - const ext = path.extname(asset); + const assetPath = asset.replace(/^asset:/, ''); const tmpFileHandle = await tmpFile({ - ...(ext !== void 0 ? { postfix: ext } : null), + name: path.basename(assetPath), + tmpdir: path.dirname(assetPath), }); tmpFileHandles.set(asset, tmpFileHandle); @@ -51,7 +50,7 @@ describe('cli acceptance tests', () => { await Promise.all( scenario.assets.map(async ([asset, contents]) => { const replaced = applyReplacements(contents, replacements); - await writeFileAsync(replacements[asset], replaced, { + await fs.promises.writeFile(replacements[asset], replaced, { encoding: 'utf8', }); }), @@ -68,7 +67,7 @@ describe('cli acceptance tests', () => { test(scenario.test, async () => { const command = applyReplacements(scenario.command!, replacements); - const { stderr, stdout, status } = await spawnNode(command, scenario.env); + const { stderr, stdout, status } = await spawnNode(command, scenario.env, scenario.cwd); replacements.date = String(new Date()); // this may introduce random failures, but hopefully they don't occur too often const expectedStdout = scenario.stdout === void 0 ? void 0 : applyReplacements(scenario.stdout, replacements); diff --git a/test-harness/scenarios/overrides/extends.scenario b/test-harness/scenarios/overrides/extends.scenario new file mode 100644 index 000000000..49b25b406 --- /dev/null +++ b/test-harness/scenarios/overrides/extends.scenario @@ -0,0 +1,103 @@ +====test==== +Respect overrides with extends +====cwd==== +tmp +====asset:base.js==== +const { DiagnosticSeverity } = require('@stoplight/types'); +const { pattern } = require('@stoplight/spectral-functions'); + +module.exports = { + rules: { + 'description-matches-stoplight': { + message: 'Description must contain Stoplight', + given: '$.info', + recommended: true, + severity: DiagnosticSeverity.Error, + then: { + field: 'description', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + 'title-matches-stoplight': { + message: 'Title must contain Stoplight', + given: '$.info', + then: { + field: 'title', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + 'contact-name-matches-stoplight': { + message: 'Contact name must contain Stoplight', + given: '$.info.contact', + recommended: false, + then: { + field: 'name', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + }, +} +====asset:spectral.js==== +const base = require('{asset:base.js}') + +module.exports = { + extends: base, + overrides: [ + { + files: ['legacy/**/*.json'], + rules: { + 'description-matches-stoplight': 'error', + 'title-matches-stoplight': 'warn', + }, + }, + { + files: ['v2/**/*.json'], + rules: { + 'description-matches-stoplight': 'info', + 'title-matches-stoplight': 'hint', + }, + }, + ], +}; +====asset:v2/document.json==== +{ + "info": { + "description": "", + "title": "", + "contact": { + "name": "" + } + } +} +====asset:legacy/document.json==== +{ + "info": { + "description": "", + "title": "", + "contact": { + "name": "" + } + } +} +====command==== +{bin} lint **/*.json --ruleset {asset:spectral.js} --fail-on-unmatched-globs +====stdout==== + +{asset:legacy/document.json} + 3:20 error description-matches-stoplight Description must contain Stoplight info.description + 4:14 warning title-matches-stoplight Title must contain Stoplight info.title + +{asset:v2/document.json} + 3:20 information description-matches-stoplight Description must contain Stoplight info.description + 4:14 hint title-matches-stoplight Title must contain Stoplight info.title + +✖ 4 problems (1 error, 1 warning, 1 info, 1 hint) diff --git a/test-harness/scenarios/overrides/rules.scenario b/test-harness/scenarios/overrides/rules.scenario new file mode 100644 index 000000000..f9c6f40be --- /dev/null +++ b/test-harness/scenarios/overrides/rules.scenario @@ -0,0 +1,97 @@ +====test==== +Respect overrides with rules-only +====cwd==== +tmp +====asset:spectral.js==== +const { DiagnosticSeverity } = require('@stoplight/types'); +const { pattern } = require('@stoplight/spectral-functions'); + +module.exports = { + rules: { + 'description-matches-stoplight': { + message: 'Description must contain Stoplight', + given: '$.info', + recommended: true, + severity: DiagnosticSeverity.Error, + then: { + field: 'description', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + 'title-matches-stoplight': { + message: 'Title must contain Stoplight', + given: '$.info', + then: { + field: 'title', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + 'contact-name-matches-stoplight': { + message: 'Contact name must contain Stoplight', + given: '$.info.contact', + recommended: false, + then: { + field: 'name', + function: pattern, + functionOptions: { + match: 'Stoplight', + }, + }, + }, + }, + overrides: [ + { + files: ['legacy/**/*.json'], + rules: { + 'description-matches-stoplight': 'error', + 'title-matches-stoplight': 'warn', + }, + }, + { + files: ['v2/**/*.json'], + rules: { + 'description-matches-stoplight': 'info', + 'title-matches-stoplight': 'hint', + }, + }, + ], +}; +====asset:v2/document.json==== +{ + "info": { + "description": "", + "title": "", + "contact": { + "name": "" + } + } +} +====asset:legacy/document.json==== +{ + "info": { + "description": "", + "title": "", + "contact": { + "name": "" + } + } +} +====command==== +{bin} lint **/*.json --ruleset {asset:spectral.js} --fail-on-unmatched-globs +====stdout==== + +{asset:legacy/document.json} + 3:20 error description-matches-stoplight Description must contain Stoplight info.description + 4:14 warning title-matches-stoplight Title must contain Stoplight info.title + +{asset:v2/document.json} + 3:20 information description-matches-stoplight Description must contain Stoplight info.description + 4:14 hint title-matches-stoplight Title must contain Stoplight info.title + +✖ 4 problems (1 error, 1 warning, 1 info, 1 hint) diff --git a/test-harness/spawn.ts b/test-harness/spawn.ts index 6c27e47c2..c16db7952 100644 --- a/test-harness/spawn.ts +++ b/test-harness/spawn.ts @@ -4,15 +4,17 @@ import * as child_process from 'child_process'; import { Transform } from 'stream'; import { normalizeLineEndings } from './helpers'; -const cwd = join(__dirname, 'scenarios'); - export type SpawnReturn = { stdout: string; stderr: string; status: number; }; -export type SpawnFn = (command: string, env: Optional) => Promise; +export type SpawnFn = ( + command: string, + env: Optional, + cwd: Optional, +) => Promise; const createStream = () => new Transform({ @@ -42,7 +44,7 @@ function stringifyStream(stream: Transform) { }); } -export const spawnNode: SpawnFn = async (script, env) => { +export const spawnNode: SpawnFn = async (script, env, cwd = join(__dirname, 'scenarios')) => { const stderr = createStream(); const stdout = createStream(); diff --git a/yarn.lock b/yarn.lock index cf28f8934..4e537caa8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2563,7 +2563,7 @@ resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.170.tgz#0d67711d4bf7f4ca5147e9091b847479b87925d6" integrity sha512-bpcvu/MKHHeYX+qeEN8GE7DIravODWdACVA1ctevD8CN24RhPZIKMn9ntfAsrvLfSX3cR5RrBKAbYm9bGs0A+Q== -"@types/minimatch@^3.0.3": +"@types/minimatch@^3.0.3", "@types/minimatch@^3.0.4": version "3.0.4" resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.4.tgz#f0ec25dbf2f0e4b18647313ac031134ca5b24b21" integrity sha512-1z8k4wzFnNjVK/tlxvrWuK5WMt6mydWWP7+zvH5eFep4oj+UkrfiJTRtjCeBXNpwaA/FYqqtb4/QS4ianFpIRA==