Merge pull request #856 from stoplightio/fix/improve-results

feat: dedupe results by unique fingerprint

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,17 +153,20 @@ spectral
   .then(result => {
     expect(result).toEqual([
       expect.objectContaining({
-       code: 'rule1',   
+       code: 'rule1',
      }),
    ]);
   });
 ```
 
 ### 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);
+  },
+});
+```

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",

src/__tests__/linter.jest.test.ts

@@ -64,18 +64,18 @@ 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',
+      }),
     ]);
   });
 
-  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 });
 

src/__tests__/linter.test.ts

@@ -97,7 +97,7 @@ describe('linter', () => {
     );
 
     expect(result).toEqual([
-      {
+      expect.objectContaining({
         code: 'rule1',
         message,
         severity: DiagnosticSeverity.Warning,
@@ -112,7 +112,7 @@ describe('linter', () => {
             line: 5,
           },
         },
-      },
+      }),
     ]);
   });
 
@@ -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',
@@ -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'],
+      }),
     ]);
   });
 

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,51 +214,9 @@ describe('Spectral', () => {
 
     const results = await s.run(fs.readFileSync(documentUri, 'utf8'), { resolve: { documentUri } });
 
-    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'),
-        range: {
-          end: {
-            character: 28,
-            line: 23,
-          },
-          start: {
-            character: 21,
-            line: 22,
-          },
-        },
-      }),
-
-      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(results.length).toEqual(3);
 
+    return expect(results).toEqual([
       expect.objectContaining({
         path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
         source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
@@ -273,79 +231,22 @@ describe('Spectral', () => {
           },
         },
       }),
-      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'],
+        path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'],
         source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
         range: {
           end: {
-            character: 18,
-            line: 43,
+            character: 28,
+            line: 23,
           },
           start: {
-            character: 8,
-            line: 42,
+            character: 21,
+            line: 22,
           },
         },
       }),
 
-      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'),

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<number | string>, flags: ILintConfig
     );
   }
 
-  return deduplicateResults(results);
+  return results;
 }

src/cli/services/linter/utils/index.ts

@@ -1,4 +1,3 @@
-export * from './deduplicateResults';
 export * from './getRuleset';
 export * from './listFiles';
 export * from './skipRules';