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: teach rulesets to accept exceptions #939

Merged
merged 3 commits into from
Mar 10, 2020
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
22 changes: 22 additions & 0 deletions docs/getting-started/rulesets.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,28 @@ The example above will run all of the rules defined in the `spectral:oas` rulese

- [Rules relevant to OpenAPI v2 and v3](../reference/openapi-rules.md)

## Selectively silencing some results

From time to time, you want to ignore some specific results without turning off
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can we try to pick a word and stick to it? Using both "silencing" and "ignoring" is ok, but then we talk about exceptions.

Can we either ignore, or except? I think personally think ignore is more clear, and in line with .eslintignore, .gitignore, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philsturgeon Fixed. Do you like it better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is great! I'll give it a lick of paint after so you can focus on writing amazing code instead of mucking around with minor tweaks.

the rule entirely. This may happen, for instance, when working with legacy APIs.

The ruleset can be extended for that purpose through the optional `except` property.

`except` describes a map of locations (expressed as Json paths) and rules that should be ignored.

Locations be can either described as relative to the ruleset or absolute paths.

```yaml
extends: spectral:oas

except:
"subfolder/one.yaml#"
- oas3-api-servers
"/tmp/docs/one.yaml#/info":
- info-contact
- info-description
```

## Creating custom functions

Learn more about [custom functions](../guides/custom-functions.md).
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ module.exports = {
testMatch: ['<rootDir>/src/**/__tests__/*.(ts)'],
testPathIgnorePatterns: ['/node_modules/', '\.karma\.test\.ts$'],
coveragePathIgnorePatterns: ['<rootDir>/dist/', '/node_modules/'],
setupFilesAfterEnv: ['./setupJest.ts']
setupFilesAfterEnv: ['./setupJest.ts', './setupTests.ts']
};
3 changes: 2 additions & 1 deletion karma.conf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = (config: any) => {
frameworks: ['jasmine', 'karma-typescript'],

// list of files / patterns to load in the browser
files: ['./karma-jest.ts', './setupKarma.ts', 'src/**/*.ts'],
files: ['./karma-jest.ts', './setupKarma.ts', './setupTests.ts', 'src/**/*.ts'],

// list of files / patterns to exclude
exclude: ['src/cli/**', 'src/formatters/**', 'src/config/__tests__/config.test.ts', 'src/**/*.jest.test.ts'],
Expand All @@ -22,6 +22,7 @@ module.exports = (config: any) => {
'src/**/*.ts': ['karma-typescript'],
'./karma-jest.ts': ['karma-typescript'],
'./setupKarma.ts': ['karma-typescript'],
'./setupTests.ts': ['karma-typescript'],
},

karmaTypescriptConfig: {
Expand Down
10 changes: 10 additions & 0 deletions setupTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { RulesetExceptionCollection } from './src/types/ruleset';

export const buildRulesetExceptionCollectionFrom = (
loc: string,
rules: string[] = ['a'],
): RulesetExceptionCollection => {
const source = {};
source[loc] = rules;
return source;
};
5 changes: 5 additions & 0 deletions src/__tests__/__fixtures__/custom-oas-ruleset.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,10 @@
}
}
}
},
"except": {
"/test/file.json#/info": ["info-contact", "info-description"],
"/test/file.json#": [ "oas3-api-servers"],
"another.yaml#": ["dummy-rule", "info-contact"]
}
}
17 changes: 17 additions & 0 deletions src/__tests__/__fixtures__/exceptions.remote.oas3.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
%YAML 1.2
---
openapi: 3.0.2
info:
title: Random title
description: Random description
version: 0.0.1
contact:
email: random@random.com
paths: {}
servers:
- url: https://www.random.com
components:
schemas:
TheRemoteType:
description: A strongly typed string
type: string
34 changes: 34 additions & 0 deletions src/__tests__/__fixtures__/exceptions.resolving.ruleset.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"extends": [["spectral:oas", "off"]],
"rules": {
"no-yaml-remote-reference": {
"formats": ["oas2", "oas3"],
"message": "$ref must not point at yaml files",
"given": "$..$ref",
"recommended": true,
"resolved": false,
"then": {
"function": "pattern",
"functionOptions": {
"notMatch": "\\.yaml(#.*)$"
}
}
},
"strings-maxLength": {
"formats": ["oas2", "oas3"],
"message": "String typed properties MUST be further described using 'maxLength'. Error: {{error}}",
"given": "$..*[?(@.type === 'string')]",
"recommended": true,
"then": {
"field": "maxLength",
"function": "truthy"
}
},
"oas3-schema": "error"
},
"except": {
"./exceptions.remote.oas3.yaml#/components/schemas/TheRemoteType": ["strings-maxLength"],
"../foo.json#/components/schemas/TheLocalType/$ref": [ "no-yaml-remote-reference"],
"another.yaml#": ["dummy-rule", "info-contact"]
}
}
148 changes: 148 additions & 0 deletions src/__tests__/linter.jest.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { normalize } from '@stoplight/path';
import { DiagnosticSeverity } from '@stoplight/types';
import * as path from 'path';
import { isOpenApiv3 } from '../formats';
import { httpAndFileResolver } from '../resolvers/http-and-file';
import { Spectral } from '../spectral';

import { readRuleset } from '../rulesets';
import { IRuleset, RulesetExceptionCollection } from '../types/ruleset';

const customFunctionOASRuleset = path.join(__dirname, './__fixtures__/custom-functions-oas-ruleset.json');
const customOASRuleset = path.join(__dirname, './__fixtures__/custom-oas-ruleset.json');
const customDirectoryFunctionsRuleset = path.join(__dirname, './__fixtures__/custom-directory-function-ruleset.json');

describe('Linter', () => {
Expand Down Expand Up @@ -170,4 +175,147 @@ describe('Linter', () => {
);
});
});

describe('Exceptions handling', () => {
it('should ignore specified rules violations in a standalone document', async () => {
await spectral.loadRuleset(customOASRuleset);
spectral.registerFormat('oas3', isOpenApiv3);

const res = await spectral.run(
{
openapi: '3.0.2',
info: 17,
},
{
resolve: {
documentUri: '/test/file.json',
},
},
);

expect(res.length).toBeGreaterThan(0);

expect(res).not.toContainEqual(
expect.objectContaining({
code: 'info-contact',
}),
);

expect(res).not.toContainEqual(
expect.objectContaining({
code: 'info-description',
}),
);

expect(res).not.toContainEqual(
expect.objectContaining({
code: 'oas3-api-servers',
}),
);
});

describe('resolving', () => {
const document = {
openapi: '3.0.2',
components: {
schemas: {
TheLocalType: {
$ref: './__fixtures__/exceptions.remote.oas3.yaml#/components/schemas/TheRemoteType',
},
},
},
};

let testRuleset: IRuleset;

beforeAll(async () => {
testRuleset = await readRuleset(path.join(__dirname, './__fixtures__/exceptions.resolving.ruleset.json'));
});

const opts = {
resolve: {
documentUri: path.join(__dirname, './foo.json'),
},
};

const extractExceptionFrom = (ruleset: IRuleset, name: string, position: number): RulesetExceptionCollection => {
const exceptions = {};
const key = Object.keys(ruleset.exceptions)[position];
expect(ruleset.exceptions[key]).toEqual([name]);
exceptions[key] = ruleset.exceptions[key];

return exceptions;
};

it('should ignore specified rules violations in a referenced document', async () => {
spectral = new Spectral({ resolver: httpAndFileResolver });
spectral.registerFormat('oas3', isOpenApiv3);

const rules = {
'strings-maxLength': testRuleset.rules['strings-maxLength'],
'oas3-schema': testRuleset.rules['oas3-schema'],
};

spectral.setRuleset({ rules, exceptions: {}, functions: {} });

const first = await spectral.run(document, opts);

expect(first).toEqual([
expect.objectContaining({
code: 'strings-maxLength',
}),
expect.objectContaining({
code: 'oas3-schema',
}),
]);

const exceptions = extractExceptionFrom(testRuleset, 'strings-maxLength', 0);

spectral.setRuleset({ rules, exceptions, functions: {} });

const second = await spectral.run(document, opts);

expect(second).toEqual([
expect.objectContaining({
code: 'oas3-schema',
}),
]);
});

it('should ignore specified rules violations in "resolved=false" mode', async () => {
spectral = new Spectral({ resolver: httpAndFileResolver });
spectral.registerFormat('oas3', isOpenApiv3);

const rules = {
'no-yaml-remote-reference': testRuleset.rules['no-yaml-remote-reference'],
'oas3-schema': testRuleset.rules['oas3-schema'],
};

spectral.setRuleset({ rules, exceptions: {}, functions: {} });

const first = await spectral.run(document, opts);

expect(first).toEqual([
expect.objectContaining({
code: 'oas3-schema',
}),
expect.objectContaining({
code: 'no-yaml-remote-reference',
}),
]);

const exceptions = extractExceptionFrom(testRuleset, 'no-yaml-remote-reference', 1);

spectral.setRuleset({ rules, exceptions, functions: {} });

const second = await spectral.run(document, opts);

expect(second).toEqual([
expect.objectContaining({
code: 'oas3-schema',
}),
]);
});
});
});
});
8 changes: 8 additions & 0 deletions src/__tests__/spectral.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ describe('Spectral', () => {
},
}),
);

Object.keys(s.exceptions).forEach(p => expect(path.isAbsolute(p)).toEqual(true));

expect(Object.entries(s.exceptions)).toEqual([
[expect.stringMatching('^/test/file.json#/info$'), ['info-contact', 'info-description']],
[expect.stringMatching('^/test/file.json#$'), ['oas3-api-servers']],
[expect.stringMatching('/__tests__/__fixtures__/another.yaml#$'), ['dummy-rule', 'info-contact']],
]);
});

test('should support loading rulesets over http', async () => {
Expand Down
50 changes: 49 additions & 1 deletion src/__tests__/spectral.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { IGraphNodeData } from '@stoplight/json-ref-resolver/types';
import { DiagnosticSeverity, Dictionary } from '@stoplight/types';
import { DepGraph } from 'dependency-graph';
import { merge } from 'lodash';
import { escapeRegExp, merge } from 'lodash';

import { Document } from '../document';
import * as Parsers from '../parsers';
import { Spectral } from '../spectral';
import { IResolver, IRunRule, RuleFunction } from '../types';
import { RulesetExceptionCollection } from '../types/ruleset';

import { buildRulesetExceptionCollectionFrom } from '../../setupTests';

const oasRuleset = JSON.parse(JSON.stringify(require('../rulesets/oas/index.json')));
const oasRulesetRules: Dictionary<IRunRule, string> = oasRuleset.rules;
Expand Down Expand Up @@ -258,4 +261,49 @@ describe('spectral', () => {
});
});
});

describe('setRuleset', () => {
const s = new Spectral();

describe('exceptions handling', () => {
it.each([['one.yaml#'], ['one.yaml#/'], ['one.yaml#/toto'], ['down/one.yaml#/toto'], ['../one.yaml#/toto']])(
'throws on relative locations (location: "%s")',
location => {
const exceptions = buildRulesetExceptionCollectionFrom(location);

expect(() => {
s.setRuleset({ rules: {}, functions: {}, exceptions });
}).toThrow(new RegExp(`.+\`${escapeRegExp(location)}\`.+is not a valid uri.+Only absolute Uris are allowed`));
},
);

it.each([
['https://dot.com/one.yaml#/toto', 'https://dot.com/one.yaml#/toto'],
['/local/one.yaml#/toto', '/local/one.yaml#/toto'],
['c:/one.yaml#/toto', 'c:/one.yaml#/toto'],
['c:\\one.yaml#/toto', 'c:/one.yaml#/toto'],
])('normalizes absolute locations (location: "%s")', (location, expected) => {
const exceptions = buildRulesetExceptionCollectionFrom(location);

s.setRuleset({ rules: {}, functions: {}, exceptions });

const locs = Object.keys(s.exceptions);
expect(locs).toEqual([expected]);
});

it('normalizes exceptions', () => {
const exceptions: RulesetExceptionCollection = {
'/test/file.yaml#/a': ['f', 'c', 'd', 'a'],
'/test/file.yaml#/b': ['1', '3', '3', '2'],
};

s.setRuleset({ rules: {}, functions: {}, exceptions });

expect(s.exceptions).toEqual({
'/test/file.yaml#/a': ['a', 'c', 'd', 'f'],
'/test/file.yaml#/b': ['1', '2', '3'],
});
});
});
});
});
4 changes: 2 additions & 2 deletions src/cli/services/linter/linter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Document } from '../../../document';
import { Document, STDIN } from '../../../document';
import {
isJSONSchema,
isJSONSchemaDraft2019_09,
Expand Down Expand Up @@ -75,7 +75,7 @@ export async function lint(documents: Array<number | string>, flags: ILintConfig
const document = new Document(
await readParsable(targetUri, { encoding: flags.encoding }),
Parsers.Yaml,
typeof targetUri === 'number' ? '<STDIN>' : targetUri,
typeof targetUri === 'number' ? STDIN : targetUri,
);

results.push(
Expand Down
1 change: 1 addition & 0 deletions src/cli/services/linter/utils/getRuleset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ async function loadRulesets(cwd: string, rulesetFiles: string[]): Promise<IRules
return {
functions: {},
rules: {},
exceptions: {},
};
}

Expand Down
Loading