From e6f040a6d24c4b7cf7137e75dfe842ddc1c86830 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Sun, 15 Dec 2019 18:33:47 -0600 Subject: [PATCH 1/7] feat: dedupe results by unique fingerprint --- package.json | 1 + src/spectral.ts | 30 +++++++++++++++++++++++------- src/types/spectral.ts | 2 ++ src/utils/fingerprint.ts | 17 +++++++++++++++++ src/utils/index.ts | 1 + yarn.lock | 5 +++++ 6 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 src/utils/fingerprint.ts diff --git a/package.json b/package.json index a171aa43e..ba52717c0 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "ajv": "^6.10", "ajv-oai": "^1.1.5", "better-ajv-errors": "^0.6.7", + "blueimp-md5": "^2.12.0", "chalk": "^3.0.0", "eol": "^0.9.1", "fast-glob": "^3.1.0", diff --git a/src/spectral.ts b/src/spectral.ts index bab55ead8..492546975 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -1,10 +1,12 @@ +const md5 = require('blueimp-md5'); + import { getLocationForJsonPath as getLocationForJsonPathJson, JsonParserResult, safeStringify } from '@stoplight/json'; import { Resolver } from '@stoplight/json-ref-resolver'; import { ICache, IUriParser } from '@stoplight/json-ref-resolver/types'; import { extname, normalize } from '@stoplight/path'; import { Dictionary, IDiagnostic, Optional } from '@stoplight/types'; import { getLocationForJsonPath as getLocationForJsonPathYaml, YamlParserResult } from '@stoplight/yaml'; -import { merge } from 'lodash'; +import { memoize, merge, uniqBy } from 'lodash'; import { STATIC_ASSETS } from './assets'; import { formatParserDiagnostics, formatResolverErrors } from './error-messages'; @@ -31,20 +33,24 @@ import { RunRuleCollection, } from './types'; import { IRuleset } from './types/ruleset'; -import { empty } from './utils'; +import { ComputeFingerprintFunc, defaultComputeResultFingerprint, empty } from './utils'; + +memoize.Cache = WeakMap; export * from './types'; export class Spectral { - private readonly _resolver: IResolver; - private readonly _parsedRefs: Dictionary; - private static readonly _parsedCache = new WeakMap>(); public functions: FunctionCollection = { ...defaultFunctions }; public rules: RunRuleCollection = {}; - public formats: RegisteredFormats; + private readonly _computeFingerprint: ComputeFingerprintFunc; + private readonly _resolver: IResolver; + private readonly _parsedRefs: Dictionary; + private static readonly _parsedCache = new WeakMap>(); + constructor(opts?: IConstructorOpts) { + this._computeFingerprint = memoize(opts?.computeFingerprint || defaultComputeResultFingerprint); this._resolver = opts && opts.resolver ? opts.resolver : new Resolver(); this.formats = {}; @@ -108,9 +114,19 @@ export class Spectral { ...runRules(resolved, this.rules, this.functions), ]; + // add fingerprint func to each result, to uniquely identify and group them + const computeFingerprint = this._computeFingerprint; + for (const r of validationResults) { + Object.defineProperty(r, 'fingerprint', { + get() { + return computeFingerprint(r, md5); + }, + }); + } + return { resolved: resolved.resolved, - results: validationResults, + results: uniqBy(validationResults, 'fingerprint'), }; } diff --git a/src/types/spectral.ts b/src/types/spectral.ts index 323526ce4..e7c975eee 100644 --- a/src/types/spectral.ts +++ b/src/types/spectral.ts @@ -9,6 +9,7 @@ import { } from '@stoplight/types'; import { JSONSchema4, JSONSchema6, JSONSchema7 } from 'json-schema'; import { IFunction, IRule, Rule } from '.'; +import { ComputeFingerprintFunc } from '../utils'; export type FunctionCollection = Dictionary; export type RuleCollection = Dictionary; @@ -31,6 +32,7 @@ export type RuleDeclarationCollection = Dictionary; export interface IConstructorOpts { resolver?: IResolver; + computeFingerprint?: ComputeFingerprintFunc; } export interface IRunOpts { diff --git a/src/utils/fingerprint.ts b/src/utils/fingerprint.ts new file mode 100644 index 000000000..27e739a80 --- /dev/null +++ b/src/utils/fingerprint.ts @@ -0,0 +1,17 @@ +import { IRuleResult } from '../types'; + +export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => string) => string; + +export const defaultComputeResultFingerprint: ComputeFingerprintFunc = (rule, hash) => { + let id = String(rule.code); + + if (rule.path && rule.path.length) { + id += JSON.stringify(rule.path); + } else if (rule.range) { + id += JSON.stringify(rule.range); + } + + if (rule.source) id += rule.source; + + return hash(id); +}; diff --git a/src/utils/index.ts b/src/utils/index.ts index 60a050ee6..e072201fc 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -2,3 +2,4 @@ export * from './empty'; export * from './hasIntersectingElement'; export * from './isObject'; export * from './refs'; +export * from './fingerprint'; diff --git a/yarn.lock b/yarn.lock index bd9df7129..a257e8878 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1105,6 +1105,11 @@ bluebird@^3.3.0: resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.7.1.tgz#df70e302b471d7473489acf26a93d63b53f874de" integrity sha512-DdmyoGCleJnkbp3nkbxTLJ18rjDsE4yCggEwKNXkeV123sPNfOCYeDoeuOY+F2FrSjO1YXcTU+dsy96KMy+gcg== +blueimp-md5@^2.12.0: + version "2.12.0" + resolved "https://registry.yarnpkg.com/blueimp-md5/-/blueimp-md5-2.12.0.tgz#be7367938a889dec3ffbb71138617c117e9c130a" + integrity sha512-zo+HIdIhzojv6F1siQPqPFROyVy7C50KzHv/k/Iz+BtvtVzSHXiMXOpq2wCfNkeBqdCv+V8XOV96tsEt2W/3rQ== + bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.1.1, bn.js@^4.4.0: version "4.11.8" resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.11.8.tgz#2cde09eb5ee341f484746bb0309b3253b1b1442f" From 35053b514e01e951fd9ca269cdc316aeb2493e88 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Sun, 15 Dec 2019 18:34:14 -0600 Subject: [PATCH 2/7] test: fix them --- src/__tests__/linter.jest.test.ts | 2 +- src/__tests__/linter.test.ts | 4 +- src/__tests__/spectral.jest.test.ts | 106 +----------------- .../oas/__tests__/contact-properties.ts | 33 ------ .../functions/__tests__/oasOpParams.test.ts | 2 +- 5 files changed, 7 insertions(+), 140 deletions(-) diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index c74fa51c8..ffe8e6de6 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -78,7 +78,7 @@ describe('Linter', () => { ]); }); - describe('evaluate {{value}} in validation messages', () => { + describe('evaluate "value" in validation messages', () => { test('should print correct values for referenced files', async () => { spectral = new Spectral({ resolver: httpAndFileResolver }); diff --git a/src/__tests__/linter.test.ts b/src/__tests__/linter.test.ts index 6233e99b1..e0d0efdb4 100644 --- a/src/__tests__/linter.test.ts +++ b/src/__tests__/linter.test.ts @@ -96,7 +96,7 @@ describe('linter', () => { }); expect(result).toEqual([ - { + expect.objectContaining({ code: 'rule1', message, severity: DiagnosticSeverity.Warning, @@ -111,7 +111,7 @@ describe('linter', () => { line: 5, }, }, - }, + }), ]); }); diff --git a/src/__tests__/spectral.jest.test.ts b/src/__tests__/spectral.jest.test.ts index 23bcbd28b..dfea56cca 100644 --- a/src/__tests__/spectral.jest.test.ts +++ b/src/__tests__/spectral.jest.test.ts @@ -195,7 +195,7 @@ describe('Spectral', () => { ); }); - test('should recognize the source of remote $refs', async () => { + test('should recognize the source of remote $refs, and de-dupe results by fingerprint', async () => { const s = new Spectral({ resolver: httpAndFileResolver }); const documentUri = path.join(__dirname, './__fixtures__/gh-658/URIError.yaml'); @@ -214,6 +214,8 @@ describe('Spectral', () => { const results = await s.run(fs.readFileSync(documentUri, 'utf8'), { resolve: { documentUri } }); + expect(results.length).toEqual(3); + return expect(results).toEqual([ expect.objectContaining({ path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'], @@ -263,108 +265,6 @@ describe('Spectral', () => { }, }, }), - - expect.objectContaining({ - path: ['components', 'schemas', 'Error', 'properties', 'status_code'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'), - range: { - end: { - character: 22, - line: 21, - }, - start: { - character: 20, - line: 20, - }, - }, - }), - expect.objectContaining({ - path: ['components', 'schemas', 'Foo'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), - range: { - end: { - character: 18, - line: 43, - }, - start: { - character: 8, - line: 42, - }, - }, - }), - - expect.objectContaining({ - path: ['components', 'schemas', 'Foo'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), - range: { - end: { - character: 18, - line: 43, - }, - start: { - character: 8, - line: 42, - }, - }, - }), - - expect.objectContaining({ - path: ['components', 'schemas', 'Error', 'properties', 'status_code'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'), - range: { - end: { - character: 22, - line: 21, - }, - start: { - character: 20, - line: 20, - }, - }, - }), - expect.objectContaining({ - path: ['components', 'schemas', 'Foo'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), - range: { - end: { - character: 18, - line: 43, - }, - start: { - character: 8, - line: 42, - }, - }, - }), - - expect.objectContaining({ - path: ['components', 'schemas', 'Error', 'properties', 'status_code'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'), - range: { - end: { - character: 22, - line: 21, - }, - start: { - character: 20, - line: 20, - }, - }, - }), - expect.objectContaining({ - path: ['components', 'schemas', 'Foo'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), - range: { - end: { - character: 18, - line: 43, - }, - start: { - character: 8, - line: 42, - }, - }, - }), ]); }); }); diff --git a/src/rulesets/oas/__tests__/contact-properties.ts b/src/rulesets/oas/__tests__/contact-properties.ts index bac9b051f..f458dd6cc 100644 --- a/src/rulesets/oas/__tests__/contact-properties.ts +++ b/src/rulesets/oas/__tests__/contact-properties.ts @@ -33,39 +33,6 @@ describe('contact-properties', () => { info: { contact: {} }, }); expect(results).toEqual([ - { - code: 'contact-properties', - message: 'Contact object should have `name`, `url` and `email`.', - path: ['info', 'contact'], - range: { - end: { - character: 17, - line: 4, - }, - start: { - character: 14, - line: 4, - }, - }, - severity: 1, - source: undefined, - }, - { - code: 'contact-properties', - message: 'Contact object should have `name`, `url` and `email`.', - path: ['info', 'contact'], - range: { - end: { - character: 17, - line: 4, - }, - start: { - character: 14, - line: 4, - }, - }, - severity: DiagnosticSeverity.Warning, - }, { code: 'contact-properties', message: 'Contact object should have `name`, `url` and `email`.', diff --git a/src/rulesets/oas/functions/__tests__/oasOpParams.test.ts b/src/rulesets/oas/functions/__tests__/oasOpParams.test.ts index 66df95dc6..1fe2674e7 100644 --- a/src/rulesets/oas/functions/__tests__/oasOpParams.test.ts +++ b/src/rulesets/oas/functions/__tests__/oasOpParams.test.ts @@ -144,7 +144,7 @@ describe('oasOpParams', () => { }, }, }); - expect(results.length).toEqual(2); + expect(results.length).toEqual(1); }); test('Error if multiple in:body', async () => { From b780e7fac7b0bb950aaf8811e5ca800008ce3664 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Sun, 15 Dec 2019 18:45:12 -0600 Subject: [PATCH 3/7] docs: document computeFingerprint option --- docs/guides/javascript.md | 46 +++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/guides/javascript.md b/docs/guides/javascript.md index a2b3445e6..3cf73fb47 100644 --- a/docs/guides/javascript.md +++ b/docs/guides/javascript.md @@ -76,10 +76,10 @@ spectral.registerFormat('oas3', isOpenApiv3); const { Spectral, isJSONSchema, - isJSONSchemaDraft4, + isJSONSchemaDraft4, isJSONSchemaDraft6, - isJSONSchemaDraft7, - isJSONSchemaDraft2019_09, + isJSONSchemaDraft7, + isJSONSchemaDraft2019_09, isJSONSchemaLoose, } = require('@stoplight/spectral'); @@ -96,7 +96,7 @@ Learn more about predefined formats in the (ruleset documentation)[../getting-st ## Loading Rules -Spectral comes with some rulesets that are very specific to OpenAPI v2/v3, and they can be loaded using `Spectral.loadRuleset()`. +Spectral comes with some rulesets that are very specific to OpenAPI v2/v3, and they can be loaded using `Spectral.loadRuleset()`. ```js const { Spectral, isOpenApiv2, isOpenApiv3 } = require('@stoplight/spectral'); @@ -114,7 +114,7 @@ spectral.loadRuleset('spectral:oas') .then(results => { console.log('here are the results', results); }); -``` +``` The OpenAPI rules are opinionated. There might be some rules that you prefer to change, or disable. We encourage you to create your rules to fit your use case, and we welcome additions to the existing rulesets as well! @@ -153,7 +153,7 @@ spectral .then(result => { expect(result).toEqual([ expect.objectContaining({ - code: 'rule1', + code: 'rule1', }), ]); }); @@ -161,9 +161,12 @@ spectral ### Using custom resolver -Spectral lets you provide any custom $ref resolver. By default, http(s) and file protocols are resolved, relatively to the document Spectral lints against. -If you'd like support any additional protocol or adjust the resolution, you are absolutely fine to do it. -In order to achieve that, you need to create a custom json-ref-resolver instance. +Spectral lets you provide any custom $ref resolver. By default, http(s) and file protocols are resolved, relatively to +the document Spectral lints against. If you'd like support any additional protocol or adjust the resolution, you are +absolutely fine to do it. In order to achieve that, you need to create a custom json-ref-resolver instance. + +You can find more information about how to create custom resolvers in +the [@stoplight/json-ref-resolver](https://github.com/stoplightio/json-ref-resolver) repository. ```js const path = require('path'); @@ -199,3 +202,28 @@ const spectral = new Spectral({ resolver: customFileResolver }); The custom resolver we've just created will resolve all remote file refs relatively to the current working directory. More on that can be found in the [json-ref-resolver repo](https://github.com/stoplightio/json-ref-resolver). + +### Using custom de-duplication strategy + +By default, Spectral will de-duplicate results based on the result code and document location. You can customize this +behavior with the `computeFingerprint` option. For example, here is the default fingerprint implementation: + +The final reported results are de-duplicated based on their computed fingerprint. + +```ts +const spectral = new Spectral({ + computeFingerprint: (rule: IRuleResult, hash) => { + let id = String(rule.code); + + if (rule.path && rule.path.length) { + id += JSON.stringify(rule.path); + } else if (rule.range) { + id += JSON.stringify(rule.range); + } + + if (rule.source) id += rule.source; + + return hash(id); + }, +}); +``` From df2db687dca3b19466f71325c592550cc6fc5195 Mon Sep 17 00:00:00 2001 From: Marc MacLeod Date: Wed, 18 Dec 2019 18:37:25 -0600 Subject: [PATCH 4/7] chore: consolidate deduplicateResults --- src/__tests__/linter.jest.test.ts | 8 ++-- src/__tests__/linter.test.ts | 32 ++++++------- src/__tests__/spectral.jest.test.ts | 25 +++++----- src/cli/services/linter/linter.ts | 4 +- .../linter/utils/deduplicateResults.ts | 26 ---------- src/cli/services/linter/utils/index.ts | 1 - src/spectral.ts | 18 ++----- .../duplicate-validation-results.json | 0 .../__tests__/prepareResults.spec.ts} | 10 ++-- src/utils/fingerprint.ts | 17 ------- src/utils/index.ts | 2 +- src/utils/prepareResults.ts | 48 +++++++++++++++++++ 12 files changed, 92 insertions(+), 99 deletions(-) delete mode 100644 src/cli/services/linter/utils/deduplicateResults.ts rename src/{cli/services/linter => }/utils/__tests__/__fixtures__/duplicate-validation-results.json (100%) rename src/{cli/services/linter/utils/__tests__/deduplicateResults.spec.ts => utils/__tests__/prepareResults.spec.ts} (70%) delete mode 100644 src/utils/fingerprint.ts create mode 100644 src/utils/prepareResults.ts diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index 9767b739c..e0cd212c7 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -64,14 +64,14 @@ describe('Linter', () => { it('should respect the scope of defined functions (ruleset-based)', async () => { await spectral.loadRuleset(customDirectoryFunctionsRuleset); expect(await spectral.run({})).toEqual([ - expect.objectContaining({ - code: 'has-info-property', - message: 'info property is missing', - }), expect.objectContaining({ code: 'has-field-property', message: 'Object does not have field property', }), + expect.objectContaining({ + code: 'has-info-property', + message: 'info property is missing', + }), ]); }); diff --git a/src/__tests__/linter.test.ts b/src/__tests__/linter.test.ts index d5122537c..97fe7c5c0 100644 --- a/src/__tests__/linter.test.ts +++ b/src/__tests__/linter.test.ts @@ -162,15 +162,15 @@ describe('linter', () => { expect(result).toEqual([ expect.objectContaining({ - code: 'invalid-ref', + code: 'oas3-schema', + message: "/paths//pets/get/responses/200 should have required property '$ref'", + path: ['paths', '/pets', 'get', 'responses', '200'], }), expect.objectContaining({ code: 'invalid-ref', }), expect.objectContaining({ - code: 'oas3-schema', - message: "/paths//pets/get/responses/200 should have required property '$ref'", - path: ['paths', '/pets', 'get', 'responses', '200'], + code: 'invalid-ref', }), expect.objectContaining({ code: 'oas3-unused-components-schema', @@ -605,7 +605,7 @@ responses:: !!foo ); }); - test('should remove all redundant ajv errors', async () => { + test.only('should remove all redundant ajv errors', async () => { spectral.registerFormat('oas2', isOpenApiv2); spectral.registerFormat('oas3', isOpenApiv3); await spectral.loadRuleset('spectral:oas'); @@ -614,26 +614,21 @@ responses:: !!foo expect(result).toEqual([ expect.objectContaining({ - code: 'invalid-ref', - }), - expect.objectContaining({ - code: 'invalid-ref', + code: 'openapi-tags', }), expect.objectContaining({ code: 'operation-tag-defined', }), expect.objectContaining({ - code: 'openapi-tags', + code: 'oas3-schema', + message: "/paths//pets/get/responses/200 should have required property '$ref'", + path: ['paths', '/pets', 'get', 'responses', '200'], }), expect.objectContaining({ - code: 'oas3-valid-schema-example', - message: '"foo.example" property type should be number', - path: ['components', 'schemas', 'foo', 'example'], + code: 'invalid-ref', }), expect.objectContaining({ - code: 'oas3-schema', - message: "/paths//pets/get/responses/200 should have required property '$ref'", - path: ['paths', '/pets', 'get', 'responses', '200'], + code: 'invalid-ref', }), expect.objectContaining({ code: 'oas3-unused-components-schema', @@ -645,6 +640,11 @@ responses:: !!foo message: 'Potentially unused components schema has been detected.', path: ['components', 'schemas', 'foo'], }), + expect.objectContaining({ + code: 'oas3-valid-schema-example', + message: '"foo.example" property type should be number', + path: ['components', 'schemas', 'foo', 'example'], + }), ]); }); diff --git a/src/__tests__/spectral.jest.test.ts b/src/__tests__/spectral.jest.test.ts index c9481e254..2398c2a49 100644 --- a/src/__tests__/spectral.jest.test.ts +++ b/src/__tests__/spectral.jest.test.ts @@ -218,34 +218,35 @@ describe('Spectral', () => { return expect(results).toEqual([ expect.objectContaining({ - path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), + path: ['components', 'schemas', 'Error', 'properties', 'status_code'], + source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'), range: { end: { - character: 28, - line: 23, + character: 22, + line: 21, }, start: { - character: 21, - line: 22, + character: 20, + line: 20, }, }, }), expect.objectContaining({ - path: ['components', 'schemas', 'Error', 'properties', 'status_code'], - source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'), + path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'], + source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), range: { end: { - character: 22, - line: 21, + character: 28, + line: 23, }, start: { - character: 20, - line: 20, + character: 21, + line: 22, }, }, }), + expect.objectContaining({ path: ['components', 'schemas', 'Foo'], source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'), diff --git a/src/cli/services/linter/linter.ts b/src/cli/services/linter/linter.ts index 4b3f4e1ec..c00236a07 100644 --- a/src/cli/services/linter/linter.ts +++ b/src/cli/services/linter/linter.ts @@ -17,7 +17,7 @@ import { isRuleEnabled } from '../../../runner'; import { IRuleResult, Spectral } from '../../../spectral'; import { FormatLookup, IParsedResult } from '../../../types'; import { ILintConfig } from '../../../types/config'; -import { deduplicateResults, getRuleset, listFiles, skipRules } from './utils'; +import { getRuleset, listFiles, skipRules } from './utils'; import { getResolver } from './utils/getResolver'; const KNOWN_FORMATS: Array<[string, FormatLookup, string]> = [ @@ -92,5 +92,5 @@ export async function lint(documents: Array, flags: ILintConfig ); } - return deduplicateResults(results); + return results; } diff --git a/src/cli/services/linter/utils/deduplicateResults.ts b/src/cli/services/linter/utils/deduplicateResults.ts deleted file mode 100644 index 88a76cd01..000000000 --- a/src/cli/services/linter/utils/deduplicateResults.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { compareResults } from '../../../../formatters/utils/sortResults'; -import { IRuleResult } from '../../../../types'; - -export const deduplicateResults = (results: IRuleResult[]): IRuleResult[] => { - const filtered: IRuleResult[] = []; - - const totalResults = results.length; - - if (totalResults < 2) { - return [...results]; - } - - const sorted = [...results].sort(compareResults); - - filtered.push(sorted[0]); - - for (let i = 1; i < totalResults; i++) { - if (compareResults(sorted[i], sorted[i - 1]) === 0) { - continue; - } - - filtered.push(sorted[i]); - } - - return filtered; -}; diff --git a/src/cli/services/linter/utils/index.ts b/src/cli/services/linter/utils/index.ts index 64d27e7d0..4912e9c0c 100644 --- a/src/cli/services/linter/utils/index.ts +++ b/src/cli/services/linter/utils/index.ts @@ -1,4 +1,3 @@ -export * from './deduplicateResults'; export * from './getRuleset'; export * from './listFiles'; export * from './skipRules'; diff --git a/src/spectral.ts b/src/spectral.ts index 3bf6b6884..db7f22c43 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -1,12 +1,10 @@ -const md5 = require('blueimp-md5'); - import { getLocationForJsonPath as getLocationForJsonPathJson, JsonParserResult, safeStringify } from '@stoplight/json'; import { Resolver } from '@stoplight/json-ref-resolver'; import { ICache, IUriParser } from '@stoplight/json-ref-resolver/types'; import { extname, normalize } from '@stoplight/path'; import { DiagnosticSeverity, Dictionary, IDiagnostic, Optional } from '@stoplight/types'; import { getLocationForJsonPath as getLocationForJsonPathYaml, YamlParserResult } from '@stoplight/yaml'; -import { memoize, merge, uniqBy } from 'lodash'; +import { memoize, merge } from 'lodash'; import { STATIC_ASSETS } from './assets'; import { formatParserDiagnostics, formatResolverErrors } from './error-messages'; @@ -33,7 +31,7 @@ import { RunRuleCollection, } from './types'; import { IRuleset } from './types/ruleset'; -import { ComputeFingerprintFunc, defaultComputeResultFingerprint, empty } from './utils'; +import { ComputeFingerprintFunc, defaultComputeResultFingerprint, empty, prepareResults } from './utils'; memoize.Cache = WeakMap; @@ -119,19 +117,9 @@ export class Spectral { validationResults = validationResults.concat(runRules(resolved, this.rules, this.functions)); - // add fingerprint func to each result, to uniquely identify and group them - const computeFingerprint = this._computeFingerprint; - for (const r of validationResults) { - Object.defineProperty(r, 'fingerprint', { - get() { - return computeFingerprint(r, md5); - }, - }); - } - return { resolved: resolved.resolved, - results: uniqBy(validationResults, 'fingerprint'), + results: prepareResults(validationResults, this._computeFingerprint), }; } diff --git a/src/cli/services/linter/utils/__tests__/__fixtures__/duplicate-validation-results.json b/src/utils/__tests__/__fixtures__/duplicate-validation-results.json similarity index 100% rename from src/cli/services/linter/utils/__tests__/__fixtures__/duplicate-validation-results.json rename to src/utils/__tests__/__fixtures__/duplicate-validation-results.json diff --git a/src/cli/services/linter/utils/__tests__/deduplicateResults.spec.ts b/src/utils/__tests__/prepareResults.spec.ts similarity index 70% rename from src/cli/services/linter/utils/__tests__/deduplicateResults.spec.ts rename to src/utils/__tests__/prepareResults.spec.ts index bdf4d0242..07960785e 100644 --- a/src/cli/services/linter/utils/__tests__/deduplicateResults.spec.ts +++ b/src/utils/__tests__/prepareResults.spec.ts @@ -1,10 +1,10 @@ -import { deduplicateResults } from '../deduplicateResults'; +import { defaultComputeResultFingerprint, prepareResults } from '../prepareResults'; import * as duplicateValidationResults from './__fixtures__/duplicate-validation-results.json'; -describe('deduplicateResults util', () => { +describe('prepareResults util', () => { it('deduplicate exact validation results', () => { - expect(deduplicateResults(duplicateValidationResults)).toEqual([ + expect(prepareResults(duplicateValidationResults, defaultComputeResultFingerprint)).toEqual([ expect.objectContaining({ code: 'valid-example-in-schemas', }), @@ -20,7 +20,7 @@ describe('deduplicateResults util', () => { source: void 0, })); - expect(deduplicateResults(duplicateValidationResultsWithNoSource)).toEqual([ + expect(prepareResults(duplicateValidationResultsWithNoSource, defaultComputeResultFingerprint)).toEqual([ expect.objectContaining({ code: 'valid-example-in-schemas', }), @@ -38,6 +38,6 @@ describe('deduplicateResults util', () => { { ...duplicateValidationResults[0] }, ]; - expect(deduplicateResults(onlyDuplicates).length).toBe(1); + expect(prepareResults(onlyDuplicates, defaultComputeResultFingerprint).length).toBe(1); }); }); diff --git a/src/utils/fingerprint.ts b/src/utils/fingerprint.ts deleted file mode 100644 index 27e739a80..000000000 --- a/src/utils/fingerprint.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { IRuleResult } from '../types'; - -export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => string) => string; - -export const defaultComputeResultFingerprint: ComputeFingerprintFunc = (rule, hash) => { - let id = String(rule.code); - - if (rule.path && rule.path.length) { - id += JSON.stringify(rule.path); - } else if (rule.range) { - id += JSON.stringify(rule.range); - } - - if (rule.source) id += rule.source; - - return hash(id); -}; diff --git a/src/utils/index.ts b/src/utils/index.ts index e072201fc..78005416a 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -2,4 +2,4 @@ export * from './empty'; export * from './hasIntersectingElement'; export * from './isObject'; export * from './refs'; -export * from './fingerprint'; +export * from './prepareResults'; diff --git a/src/utils/prepareResults.ts b/src/utils/prepareResults.ts new file mode 100644 index 000000000..adf8abbd8 --- /dev/null +++ b/src/utils/prepareResults.ts @@ -0,0 +1,48 @@ +const md5 = require('blueimp-md5'); + +import { uniqBy } from 'lodash'; + +import { compareResults } from '../formatters/utils/sortResults'; +import { IRuleResult } from '../types'; + +export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => string) => string; + +export const defaultComputeResultFingerprint: ComputeFingerprintFunc = (rule, hash) => { + let id = String(rule.code); + + if (rule.path && rule.path.length) { + id += JSON.stringify(rule.path); + } else if (rule.range) { + id += JSON.stringify(rule.range); + } + + if (rule.source) id += rule.source; + + return hash(id); +}; + +export const prepareResults = (results: IRuleResult[], computeFingerprint: ComputeFingerprintFunc) => { + decorateResultsWithFingerprint(results, computeFingerprint); + + return sortResults(deduplicateResults(results)); +}; + +const decorateResultsWithFingerprint = (results: IRuleResult[], computeFingerprint: ComputeFingerprintFunc) => { + for (const r of results) { + Object.defineProperty(r, 'fingerprint', { + get() { + return computeFingerprint(r, md5); + }, + }); + } + + return results; +}; + +const deduplicateResults = (results: IRuleResult[]): IRuleResult[] => { + return uniqBy([...results], 'fingerprint'); +}; + +const sortResults = (results: IRuleResult[]): IRuleResult[] => { + return [...results].sort(compareResults); +}; From e3ffa7a1dbbabb22a0ba9db503a6a31967b20064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Thu, 19 Dec 2019 01:56:11 +0100 Subject: [PATCH 5/7] chore: move sortResults and update tests --- src/__tests__/linter.test.ts | 2 +- src/formatters/__tests__/html.test.ts | 3 ++- src/formatters/__tests__/json.test.ts | 5 +++-- src/formatters/__tests__/junit.test.ts | 5 +++-- src/formatters/__tests__/stylish.test.ts | 5 +++-- src/formatters/__tests__/teamcity.test.ts | 3 ++- src/formatters/__tests__/text.test.ts | 3 ++- src/formatters/html/index.ts | 11 ++--------- src/formatters/stylish.ts | 11 ++--------- src/formatters/teamcity.ts | 6 ++---- src/formatters/text.ts | 4 ++-- src/formatters/utils/index.ts | 1 - .../utils/__tests__/sortResults.test.ts | 2 +- src/utils/index.ts | 1 + src/utils/prepareResults.ts | 2 +- src/{formatters => }/utils/sortResults.ts | 2 +- 16 files changed, 28 insertions(+), 38 deletions(-) rename src/{formatters => }/utils/__tests__/sortResults.test.ts (99%) rename src/{formatters => }/utils/sortResults.ts (97%) diff --git a/src/__tests__/linter.test.ts b/src/__tests__/linter.test.ts index 97fe7c5c0..5ac5ed73a 100644 --- a/src/__tests__/linter.test.ts +++ b/src/__tests__/linter.test.ts @@ -605,7 +605,7 @@ responses:: !!foo ); }); - test.only('should remove all redundant ajv errors', async () => { + test('should remove all redundant ajv errors', async () => { spectral.registerFormat('oas2', isOpenApiv2); spectral.registerFormat('oas3', isOpenApiv3); await spectral.loadRuleset('spectral:oas'); diff --git a/src/formatters/__tests__/html.test.ts b/src/formatters/__tests__/html.test.ts index 640a6f606..ec67396a7 100644 --- a/src/formatters/__tests__/html.test.ts +++ b/src/formatters/__tests__/html.test.ts @@ -1,7 +1,8 @@ import { HTMLElement, parse } from 'node-html-parser'; +import { sortResults } from '../../utils'; import { html } from '../html'; -const mixedErrors = require('./__fixtures__/mixed-errors.json'); +const mixedErrors = sortResults(require('./__fixtures__/mixed-errors.json')); describe('HTML formatter', () => { test('should display proper severity levels', () => { diff --git a/src/formatters/__tests__/json.test.ts b/src/formatters/__tests__/json.test.ts index d75f8e08a..8aaab4510 100644 --- a/src/formatters/__tests__/json.test.ts +++ b/src/formatters/__tests__/json.test.ts @@ -1,7 +1,8 @@ import { IRuleResult } from '../../types'; +import { sortResults } from '../../utils'; import { json } from '../json'; -const results: IRuleResult[] = [ +const results: IRuleResult[] = sortResults([ { code: 'operation-description', message: 'paths./pets.get.description is not truthy', @@ -36,7 +37,7 @@ const results: IRuleResult[] = [ }, }, }, -]; +]); describe('JSON formatter', () => { test('should include ranges', () => { diff --git a/src/formatters/__tests__/junit.test.ts b/src/formatters/__tests__/junit.test.ts index 1019b62a0..b224898dc 100644 --- a/src/formatters/__tests__/junit.test.ts +++ b/src/formatters/__tests__/junit.test.ts @@ -1,9 +1,10 @@ import { promisify } from 'util'; import { Parser } from 'xml2js'; +import { sortResults } from '../../utils'; import { junit } from '../junit'; -const oas3SchemaErrors = require('./__fixtures__/oas3-schema-errors.json'); -const mixedErrors = require('./__fixtures__/mixed-errors-with-different-paths.json'); +const oas3SchemaErrors = sortResults(require('./__fixtures__/oas3-schema-errors.json')); +const mixedErrors = sortResults(require('./__fixtures__/mixed-errors.json')); describe('JUnit formatter', () => { let parse: Parser['parseString']; diff --git a/src/formatters/__tests__/stylish.test.ts b/src/formatters/__tests__/stylish.test.ts index 95374692a..b262c3a94 100644 --- a/src/formatters/__tests__/stylish.test.ts +++ b/src/formatters/__tests__/stylish.test.ts @@ -1,8 +1,9 @@ import * as chalk from 'chalk'; +import { sortResults } from '../../utils'; import { stylish } from '../stylish'; -const oas3SchemaErrors = require('./__fixtures__/oas3-schema-errors.json'); -const mixedErrors = require('./__fixtures__/mixed-errors.json'); +const oas3SchemaErrors = sortResults(require('./__fixtures__/oas3-schema-errors.json')); +const mixedErrors = sortResults(require('./__fixtures__/mixed-errors.json')); describe('Stylish formatter', () => { test('should prefer message for oas-schema errors', () => { diff --git a/src/formatters/__tests__/teamcity.test.ts b/src/formatters/__tests__/teamcity.test.ts index 929611966..d3b4f7026 100644 --- a/src/formatters/__tests__/teamcity.test.ts +++ b/src/formatters/__tests__/teamcity.test.ts @@ -1,6 +1,7 @@ +import { sortResults } from '../../utils'; import { teamcity } from '../teamcity'; -const mixedErrors = require('./__fixtures__/mixed-errors.json'); +const mixedErrors = sortResults(require('./__fixtures__/mixed-errors.json')); describe('Teamcity formatter', () => { test('should format messages', () => { diff --git a/src/formatters/__tests__/text.test.ts b/src/formatters/__tests__/text.test.ts index 3cb5f772c..7afcd452a 100644 --- a/src/formatters/__tests__/text.test.ts +++ b/src/formatters/__tests__/text.test.ts @@ -1,6 +1,7 @@ +import { sortResults } from '../../utils'; import { text } from '../text'; -const mixedErrors = require('./__fixtures__/mixed-errors.json'); +const mixedErrors = sortResults(require('./__fixtures__/mixed-errors.json')); describe('Text formatter', () => { test('should format messages', () => { diff --git a/src/formatters/html/index.ts b/src/formatters/html/index.ts index 3e1e20fa5..505df7e64 100644 --- a/src/formatters/html/index.ts +++ b/src/formatters/html/index.ts @@ -29,14 +29,7 @@ import * as fs from 'fs'; import { template } from 'lodash'; import { IRuleResult } from '../../types'; import { Formatter } from '../types'; -import { - getHighestSeverity, - getSeverityName, - getSummary, - getSummaryForSource, - groupBySource, - sortResults, -} from '../utils'; +import { getHighestSeverity, getSeverityName, getSummary, getSummaryForSource, groupBySource } from '../utils'; // ------------------------------------------------------------------------------ // Helpers @@ -47,7 +40,7 @@ const messageTemplate = template(eol.lf(fs.readFileSync(path.join(__dirname, 'ht const resultTemplate = template(eol.lf(fs.readFileSync(path.join(__dirname, 'html-template-result.html'), 'utf8'))); function renderMessages(messages: IRuleResult[], parentIndex: number) { - return sortResults(messages) + return messages .map(message => { const line = message.range.start.line + 1; const character = message.range.start.character + 1; diff --git a/src/formatters/stylish.ts b/src/formatters/stylish.ts index 12804cbf4..20a5d0ad9 100644 --- a/src/formatters/stylish.ts +++ b/src/formatters/stylish.ts @@ -30,14 +30,7 @@ import * as table from 'text-table'; import { IRuleResult } from '../types'; import { Formatter } from './types'; -import { - getColorForSeverity, - getHighestSeverity, - getSeverityName, - getSummary, - groupBySource, - sortResults, -} from './utils'; +import { getColorForSeverity, getHighestSeverity, getSeverityName, getSummary, groupBySource } from './utils'; // ----------------------------------------------------------------------------- // Helpers @@ -71,7 +64,7 @@ export const stylish: Formatter = results => { output += `${chalk.underline(path)}\n`; - const pathTableData = sortResults(pathResults).map((result: IRuleResult) => [ + const pathTableData = pathResults.map((result: IRuleResult) => [ formatRange(result.range), getMessageType(result.severity), result.code ?? '', diff --git a/src/formatters/teamcity.ts b/src/formatters/teamcity.ts index 58861035a..50b54e9b5 100644 --- a/src/formatters/teamcity.ts +++ b/src/formatters/teamcity.ts @@ -1,7 +1,7 @@ import { Dictionary, Optional } from '@stoplight/types'; import { IRuleResult } from '../types'; import { Formatter } from './types'; -import { getSeverityName, groupBySource, sortResults } from './utils'; +import { getSeverityName, groupBySource } from './utils'; function escapeString(str: Optional) { if (str === void 0) { @@ -35,9 +35,7 @@ function inspection(result: IRuleResult) { } function renderResults(results: IRuleResult[], parentIndex: number) { - return sortResults(results) - .map(result => `${inspectionType(result)}\n${inspection(result)}`) - .join('\n'); + return results.map(result => `${inspectionType(result)}\n${inspection(result)}`).join('\n'); } function renderGroupedResults(groupedResults: Dictionary) { diff --git a/src/formatters/text.ts b/src/formatters/text.ts index 045f87357..f6be00ce5 100644 --- a/src/formatters/text.ts +++ b/src/formatters/text.ts @@ -1,10 +1,10 @@ import { Dictionary } from '@stoplight/types'; import { IRuleResult } from '../types'; import { Formatter } from './types'; -import { getSeverityName, groupBySource, sortResults } from './utils'; +import { getSeverityName, groupBySource } from './utils'; function renderResults(results: IRuleResult[], parentIndex: number) { - return sortResults(results) + return results .map(result => { const line = result.range.start.line + 1; const character = result.range.start.character + 1; diff --git a/src/formatters/utils/index.ts b/src/formatters/utils/index.ts index 7a4a92c0f..7733c6615 100644 --- a/src/formatters/utils/index.ts +++ b/src/formatters/utils/index.ts @@ -5,5 +5,4 @@ export * from './getSummary'; export * from './groupBySeverity'; export * from './groupBySource'; export * from './pluralize'; -export * from './sortResults'; export * from './xmlEscape'; diff --git a/src/formatters/utils/__tests__/sortResults.test.ts b/src/utils/__tests__/sortResults.test.ts similarity index 99% rename from src/formatters/utils/__tests__/sortResults.test.ts rename to src/utils/__tests__/sortResults.test.ts index 44113e561..00b2441fc 100644 --- a/src/formatters/utils/__tests__/sortResults.test.ts +++ b/src/utils/__tests__/sortResults.test.ts @@ -1,5 +1,5 @@ import { DiagnosticSeverity } from '@stoplight/types'; -import { IRuleResult } from '../../../types'; +import { IRuleResult } from '../../types'; import { compareResults, sortResults } from '../sortResults'; const results: IRuleResult[] = [ diff --git a/src/utils/index.ts b/src/utils/index.ts index 78005416a..db6cc12f7 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -3,3 +3,4 @@ export * from './hasIntersectingElement'; export * from './isObject'; export * from './refs'; export * from './prepareResults'; +export * from './sortResults'; diff --git a/src/utils/prepareResults.ts b/src/utils/prepareResults.ts index adf8abbd8..b5e0575bc 100644 --- a/src/utils/prepareResults.ts +++ b/src/utils/prepareResults.ts @@ -2,8 +2,8 @@ const md5 = require('blueimp-md5'); import { uniqBy } from 'lodash'; -import { compareResults } from '../formatters/utils/sortResults'; import { IRuleResult } from '../types'; +import { compareResults } from './sortResults'; export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => string) => string; diff --git a/src/formatters/utils/sortResults.ts b/src/utils/sortResults.ts similarity index 97% rename from src/formatters/utils/sortResults.ts rename to src/utils/sortResults.ts index 02c5bd37b..29238078d 100644 --- a/src/formatters/utils/sortResults.ts +++ b/src/utils/sortResults.ts @@ -1,4 +1,4 @@ -import { IRuleResult } from '../../types'; +import { IRuleResult } from '../types'; const compareCode = (left: string | number | undefined, right: string | number | undefined): number => { if (left === void 0 && right === void 0) { From 3f2c731548b59c90c1b8e6b6898c2c24547c8c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Fri, 20 Dec 2019 12:14:57 +0100 Subject: [PATCH 6/7] Update src/utils/prepareResults.ts --- src/utils/prepareResults.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/prepareResults.ts b/src/utils/prepareResults.ts index b5e0575bc..54e65e488 100644 --- a/src/utils/prepareResults.ts +++ b/src/utils/prepareResults.ts @@ -10,7 +10,7 @@ export type ComputeFingerprintFunc = (rule: IRuleResult, hash: (val: string) => export const defaultComputeResultFingerprint: ComputeFingerprintFunc = (rule, hash) => { let id = String(rule.code); - if (rule.path && rule.path.length) { + if (rule.path.length) { id += JSON.stringify(rule.path); } else if (rule.range) { id += JSON.stringify(rule.range); From 334bf493b0102a14a78fbbd7fee7efb7b587dfd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Fri, 20 Dec 2019 12:17:09 +0100 Subject: [PATCH 7/7] chore: PR feedback --- src/spectral.ts | 9 +++++---- src/utils/prepareResults.ts | 4 +--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/spectral.ts b/src/spectral.ts index db7f22c43..bdff90447 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -100,7 +100,7 @@ export class Spectral { this._parsedRefs, ); - let validationResults = [...refDiagnostics, ...results, ...formatResolverErrors(resolved)]; + const validationResults = [...refDiagnostics, ...results, ...formatResolverErrors(resolved)]; if (resolved.formats === void 0) { const registeredFormats = Object.keys(this.formats); @@ -115,11 +115,12 @@ export class Spectral { } } - validationResults = validationResults.concat(runRules(resolved, this.rules, this.functions)); - return { resolved: resolved.resolved, - results: prepareResults(validationResults, this._computeFingerprint), + results: prepareResults( + [...validationResults, ...runRules(resolved, this.rules, this.functions)], + this._computeFingerprint, + ), }; } diff --git a/src/utils/prepareResults.ts b/src/utils/prepareResults.ts index 54e65e488..26001c541 100644 --- a/src/utils/prepareResults.ts +++ b/src/utils/prepareResults.ts @@ -30,9 +30,7 @@ export const prepareResults = (results: IRuleResult[], computeFingerprint: Compu const decorateResultsWithFingerprint = (results: IRuleResult[], computeFingerprint: ComputeFingerprintFunc) => { for (const r of results) { Object.defineProperty(r, 'fingerprint', { - get() { - return computeFingerprint(r, md5); - }, + value: computeFingerprint(r, md5), }); }