Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: dedupe results by unique fingerprint #856

Merged
merged 9 commits into from
Dec 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions docs/guides/javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ spectral.registerFormat('oas3', isOpenApiv3);
const {
Spectral,
isJSONSchema,
isJSONSchemaDraft4,
isJSONSchemaDraft4,
isJSONSchemaDraft6,
isJSONSchemaDraft7,
isJSONSchemaDraft2019_09,
isJSONSchemaDraft7,
isJSONSchemaDraft2019_09,
isJSONSchemaLoose,
} = require('@stoplight/spectral');

Expand All @@ -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');
Expand All @@ -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!

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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
marbemac marked this conversation as resolved.
Show resolved Hide resolved
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);
},
});
```
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/linter.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand Down
34 changes: 17 additions & 17 deletions src/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('linter', () => {
);

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

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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'],
}),
]);
});

Expand Down
115 changes: 8 additions & 107 deletions src/__tests__/spectral.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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'),
Expand All @@ -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'),
Expand Down
4 changes: 2 additions & 2 deletions src/cli/services/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]> = [
Expand Down Expand Up @@ -92,5 +92,5 @@ export async function lint(documents: Array<number | string>, flags: ILintConfig
);
}

return deduplicateResults(results);
return results;
}
26 changes: 0 additions & 26 deletions src/cli/services/linter/utils/deduplicateResults.ts

This file was deleted.

1 change: 0 additions & 1 deletion src/cli/services/linter/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export * from './deduplicateResults';
export * from './getRuleset';
export * from './listFiles';
export * from './skipRules';
Loading