Skip to content

Commit

Permalink
feat: register rules against specific formats (#432)
Browse files Browse the repository at this point in the history
* feat: register formats in ruleset and rule schemas

* feat: handle formats in merger

* test(reader): cover formats

* feat: implement registerFormat

* feat: register OAS formats for CLI

* test: oas spec is logged properly now when ruleset with rules is given

* feat: improve naming and lookups

* chore: tweak naming
  • Loading branch information
P0lip committed Aug 13, 2019
1 parent d951bdf commit 027664f
Show file tree
Hide file tree
Showing 22 changed files with 514 additions and 25 deletions.
186 changes: 186 additions & 0 deletions src/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,192 @@ describe('linter', () => {
]);
});

test('should respect the format of data and run rules associated with it', async () => {
spectral.registerFormat('foo-bar', obj => typeof obj === 'object' && obj !== null && 'foo-bar' in obj);

spectral.addFunctions(oas2Functions());

spectral.addRules({
rule1: {
given: '$.x',
formats: ['foo-bar'],
severity: 'error',
then: {
function: 'truthy',
},
},
rule2: {
given: '$.y',
formats: [],
severity: 'warn',
then: {
function: 'truthy',
},
},
});

const result = await spectral.run({
'foo-bar': true,
x: false,
y: '',
});

expect(result).toEqual([
expect.objectContaining({
code: 'rule1',
}),
]);
});

test('should match all formats if rule has no formats defined', async () => {
spectral.registerFormat('foo-bar', obj => typeof obj === 'object' && obj !== null && 'foo-bar' in obj);

spectral.addFunctions(oas2Functions());

spectral.addRules({
rule1: {
given: '$.x',
formats: ['foo-bar'],
severity: 'error',
then: {
function: 'truthy',
},
},
rule2: {
given: '$.y',
severity: 'warn',
then: {
function: 'truthy',
},
},
});

const result = await spectral.run({
'foo-bar': true,
x: false,
y: '',
});

expect(result).toEqual([
expect.objectContaining({
code: 'rule1',
}),
expect.objectContaining({
code: 'rule2',
}),
]);
});

test('should include all rules if document has no formats defined', async () => {
spectral.addFunctions(oas2Functions());

spectral.addRules({
rule1: {
given: '$.x',
formats: ['foo-bar'],
severity: 'error',
then: {
function: 'truthy',
},
},
rule2: {
given: '$.y',
severity: 'warn',
then: {
function: 'truthy',
},
},
});

const result = await spectral.run({
'foo-bar': true,
x: false,
y: '',
});

expect(result).toEqual([
expect.objectContaining({
code: 'rule1',
}),
expect.objectContaining({
code: 'rule2',
}),
]);
});

test('should let a format lookup to be overridden', async () => {
spectral.registerFormat('foo-bar', obj => typeof obj === 'object' && obj !== null && 'foo-bar' in obj);
spectral.registerFormat('foo-bar', () => false);
spectral.registerFormat('baz', () => true);

spectral.addFunctions(oas2Functions());

spectral.addRules({
rule1: {
given: '$.x',
formats: ['foo-bar'],
severity: 'error',
then: {
function: 'truthy',
},
},
rule2: {
formats: ['foo-bar'],
given: '$.y',
severity: 'warn',
then: {
function: 'truthy',
},
},
});

const result = await spectral.run({
'foo-bar': true,
x: false,
y: '',
});

expect(result).toEqual([]);
});

test('should prefer the first matched format', async () => {
spectral.registerFormat('foo-bar', obj => typeof obj === 'object' && obj !== null && 'foo-bar' in obj);
spectral.registerFormat('baz', () => true);

spectral.addFunctions(oas2Functions());

spectral.addRules({
rule1: {
given: '$.x',
formats: ['foo-bar'],
severity: 'error',
then: {
function: 'truthy',
},
},
rule2: {
formats: ['baz'],
given: '$.y',
severity: 'warn',
then: {
function: 'truthy',
},
},
});

const result = await spectral.run({
'foo-bar': true,
x: false,
y: '',
});

expect(result).toEqual([
expect.objectContaining({
code: 'rule1',
}),
]);
});

test('should include parser diagnostics', async () => {
spectral.addRules(oas2Ruleset.rules as RuleCollection);
spectral.addFunctions(oas2Functions());
Expand Down
6 changes: 3 additions & 3 deletions src/cli/commands/__tests__/lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('lint', () => {
.it('outputs warnings in default format', ctx => {
expect(ctx.stdout).toContain('5:10 warning info-matches-stoplight Info must contain Stoplight');
expect(ctx.stdout).not.toContain('Info object should contain `contact` object');
expect(ctx.stdout).not.toContain('OpenAPI 3.x detected');
expect(ctx.stdout).toContain('OpenAPI 3.x detected');
});

test
Expand Down Expand Up @@ -208,7 +208,7 @@ describe('lint', () => {
expect(ctx.stdout).toContain(
'3:6 warning info-description OpenAPI object info `description` must be present and non-empty string',
);
expect(ctx.stdout).not.toContain('OpenAPI 3.x detected');
expect(ctx.stdout).toContain('OpenAPI 3.x detected');
});

test
Expand All @@ -219,7 +219,7 @@ describe('lint', () => {
'46:24 warning operation-description Operation `description` must be present and non-empty string',
);
expect(ctx.stdout).toContain('28 problems (0 errors, 28 warnings, 0 infos)');
expect(ctx.stdout).not.toContain('OpenAPI 2.x detected');
expect(ctx.stdout).toContain('OpenAPI 2.0 (Swagger) detected');
});
});
});
Expand Down
32 changes: 25 additions & 7 deletions src/cli/commands/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { json, stylish } from '../../formatters';
import { readParsable } from '../../fs/reader';
import { httpAndFileResolver } from '../../resolvers/http-and-file';
import { getDefaultRulesetFile } from '../../rulesets/loader';
import { isOpenApiv2, isOpenApiv3 } from '../../rulesets/lookups';
import { oas2Functions, rules as oas2Rules } from '../../rulesets/oas2';
import { oas3Functions, rules as oas3Rules } from '../../rulesets/oas3';
import { readRulesFromRulesets } from '../../rulesets/reader';
Expand Down Expand Up @@ -150,6 +151,24 @@ async function lint(name: string, flags: ILintConfig, command: Lint, rules?: Rul
mergeKeys: true,
});
const spectral = new Spectral({ resolver: httpAndFileResolver });
spectral.registerFormat('oas2', document => {
if (isOpenApiv2(document)) {
command.log('OpenAPI 2.0 (Swagger) detected');
return true;
}

return false;
});

spectral.registerFormat('oas3', document => {
if (isOpenApiv3(document)) {
command.log('OpenAPI 3.x detected');
return true;
}

return false;
});

if (parseInt(spec.data.swagger) === 2) {
command.log('Adding OpenAPI 2.0 (Swagger) functions');
spectral.addFunctions(oas2Functions());
Expand All @@ -166,13 +185,12 @@ async function lint(name: string, flags: ILintConfig, command: Lint, rules?: Rul
if (flags.verbose) {
command.log('No rules loaded, attempting to detect document type');
}
if (parseInt(spec.data.swagger) === 2) {
command.log('OpenAPI 2.0 (Swagger) detected');
rules = await tryReadOrLog(command, async () => await oas2Rules());
} else if (parseInt(spec.data.openapi) === 3) {
command.log('OpenAPI 3.x detected');
rules = await tryReadOrLog(command, async () => await oas3Rules());
}

rules = Object.assign(
{},
await tryReadOrLog(command, async () => await oas2Rules()),
await tryReadOrLog(command, async () => await oas3Rules()),
);
}

if (flags.skipRule) {
Expand Down
7 changes: 7 additions & 0 deletions src/meta/rule.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@
},
"type": "array"
},
"formats": {
"type": "array",
"items": {
"type": "string",
"minItems": 1
}
},
"then": {
"anyOf": [
{
Expand Down
7 changes: 7 additions & 0 deletions src/meta/ruleset.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
"$ref": "rule.schema.json"
}
},
"formats": {
"type": "array",
"items": {
"type": "string",
"minLength": 1
}
},
"extends": {
"type": ["array", "string"],
"items": {
Expand Down
2 changes: 2 additions & 0 deletions src/resolved.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ export class Resolved implements IResolveResult {
public result: unknown;
public errors: IResolveError[];
public runner: IResolveRunner;
public format?: string;

constructor(public spec: IParsedResult, result: IResolveResult, public parsedMap: IParseMap) {
this.refMap = result.refMap;
this.result = result.result;
this.errors = result.errors;
this.runner = result.runner;
this.format = spec.format;
}

public getParsedForJsonPath(path: JsonPath) {
Expand Down
12 changes: 12 additions & 0 deletions src/rulesets/__tests__/__fixtures__/my-open-api-ruleset.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"extends": ["spectral:oas2", "spectral:oas3"],
"rules": {
"valid-rule": {
"message": "should be OK",
"given": "$.info",
"then": {
"function": "truthy"
}
}
}
}
56 changes: 56 additions & 0 deletions src/rulesets/__tests__/lookups.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { isOpenApiv2, isOpenApiv3, isOpenApiv3_1 } from '../lookups';

// @oclif/test packages requires @types/mocha, therefore we have 2 packages coming up with similar typings
// TS is confused and prefers the mocha ones, so we need to instrument it to pick up the Jest ones
declare var it: jest.It;

describe('Format lookups', () => {
describe('OpenAPI 2.0 aka Swagger', () => {
it.each(['2.0.0', '2', '2.0'])('recognizes %s version correctly', version => {
expect(isOpenApiv2({ swagger: version })).toBe(true);
});

it('does not recognize invalid document', () => {
expect(isOpenApiv2({ openapi: '2.0' })).toBe(false);
expect(isOpenApiv2({ openapi: null })).toBe(false);
expect(isOpenApiv2({ swagger: null })).toBe(false);
expect(isOpenApiv2({ swagger: '3.0' })).toBe(false);
expect(isOpenApiv2({ swagger: '1.0' })).toBe(false);
expect(isOpenApiv2({})).toBe(false);
expect(isOpenApiv2(null)).toBe(false);
});
});

describe('OpenAPI 3.0', () => {
it.each(['3.0.0', '3', '3.0'])('recognizes %s version correctly', version => {
expect(isOpenApiv3({ openapi: version })).toBe(true);
});

it('does not recognize invalid document', () => {
expect(isOpenApiv3({ openapi: '4.0' })).toBe(false);
expect(isOpenApiv3({ openapi: '2.0' })).toBe(false);
expect(isOpenApiv3({ openapi: null })).toBe(false);
expect(isOpenApiv3({ swagger: null })).toBe(false);
expect(isOpenApiv3({ swagger: '3.0' })).toBe(false);
expect(isOpenApiv3({})).toBe(false);
expect(isOpenApiv3(null)).toBe(false);
});
});

describe('OpenAPI 3.1', () => {
it.each(['3.1.0', '3.1'])('recognizes %s version correctly', version => {
expect(isOpenApiv3_1({ openapi: version })).toBe(true);
});

it('does not recognize invalid document', () => {
expect(isOpenApiv3_1({ openapi: '4.0' })).toBe(false);
expect(isOpenApiv3_1({ openapi: 3.0 })).toBe(false);
expect(isOpenApiv3_1({ openapi: '2.0' })).toBe(false);
expect(isOpenApiv3_1({ openapi: null })).toBe(false);
expect(isOpenApiv3_1({ swagger: null })).toBe(false);
expect(isOpenApiv3_1({ swagger: '3.0' })).toBe(false);
expect(isOpenApiv3_1({})).toBe(false);
expect(isOpenApiv3_1(null)).toBe(false);
});
});
});

0 comments on commit 027664f

Please sign in to comment.