Skip to content

Commit

Permalink
Merge pull request #614 from stoplightio/feat/remove-when
Browse files Browse the repository at this point in the history 
feat(rule): remove 'when'
  • Loading branch information
Phil Sturgeon authored Oct 24, 2019
2 parents 846e8d1 + 92f06c4 commit 8c1aedd
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 235 deletions.
147 changes: 18 additions & 129 deletions src/__tests__/linter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('linter', () => {
expect(result).toHaveLength(0);
});

test('should return all properties', async () => {
test('should return all properties matching 4xx response code', async () => {
const message = '4xx responses require a description';

spectral.setFunctions({
Expand All @@ -76,11 +76,7 @@ describe('linter', () => {

spectral.setRules({
rule1: {
given: '$.responses[*]',
when: {
field: '@key',
pattern: '^4.*',
},
given: '$.responses.[?(@property >= 400 && @property < 500)]',
then: {
field: 'description',
function: 'func1',
Expand All @@ -99,22 +95,24 @@ describe('linter', () => {
},
});

expect(result[0]).toMatchObject({
code: 'rule1',
message,
severity: DiagnosticSeverity.Warning,
path: ['responses', '404'],
range: {
end: {
line: 6,
character: 25,
},
start: {
character: 10,
line: 5,
expect(result).toEqual([
{
code: 'rule1',
message,
severity: DiagnosticSeverity.Warning,
path: ['responses', '404'],
range: {
end: {
line: 6,
character: 25,
},
start: {
character: 10,
line: 5,
},
},
},
});
]);
});

test('should support rule overriding severity', async () => {
Expand Down Expand Up @@ -718,115 +716,6 @@ responses:: !!foo
});
});

describe('functional tests for the when statement', () => {
let fakeLintingFunction: any;

beforeEach(() => {
fakeLintingFunction = jest.fn();
spectral.setFunctions({
[fnName]: fakeLintingFunction,
});
spectral.setRules(rules);
});

describe('given no when', () => {
test('should simply lint anything it matches in the given', async () => {
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
expect(fakeLintingFunction.mock.calls[0][0]).toEqual(target.responses);
});
});

describe('given when with no pattern and regular field', () => {
test('should call linter if when field exists', async () => {
spectral.mergeRules({
example: {
when: {
field: '200.description',
},
},
});
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
'200': { description: 'a' },
'201': { description: 'b' },
'300': { description: 'c' },
});
});

test('should not call linter if when field not exist', async () => {
spectral.mergeRules({
example: {
when: {
field: '302.description',
},
},
});
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(0);
});
});

describe('given "when" with a pattern and regular field', () => {
test('should NOT lint if pattern does not match', async () => {
spectral.mergeRules({
example: {
when: {
field: '200.description',
pattern: 'X',
},
},
});
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(0);
});

test('should lint if pattern does match', async () => {
spectral.mergeRules({
example: {
when: {
field: '200.description',
pattern: 'a',
},
},
});
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(1);
expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
'200': { description: 'a' },
'201': { description: 'b' },
'300': { description: 'c' },
});
});
});

describe('given "when" with a pattern and @key field', () => {
test('should lint ONLY part of object that matches pattern', async () => {
spectral.mergeRules({
example: {
given: '$.responses[*]',
when: {
field: '@key',
pattern: '2..',
},
},
});
await spectral.run(target);

expect(fakeLintingFunction).toHaveBeenCalledTimes(2);
expect(fakeLintingFunction.mock.calls[0][0]).toEqual({
description: 'a',
});
});
});
});

describe('functional tests for the then statement', () => {
let fakeLintingFunction: any;
let fakeLintingFunction2: any;
Expand Down
85 changes: 2 additions & 83 deletions src/linter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,7 @@ export const lintNode = (
resolved: Resolved,
): IRuleResult[] => {
const givenPath = node.path[0] === '$' ? node.path.slice(1) : node.path;
const conditioning = whatShouldBeLinted(givenPath, node.value, rule);

// If the 'when' condition is not satisfied, simply don't run the linter
if (!conditioning.lint) {
return [];
}

const targetValue = conditioning.value;
const targetValue = node.value;

const targets: any[] = [];
if (then && then.field) {
Expand Down Expand Up @@ -138,83 +131,9 @@ export const lintNode = (
return results;
};

// TODO(SO-23): unit test idividually
export const whatShouldBeLinted = (
path: JsonPath,
originalValue: any,
rule: IRunRule,
): { lint: boolean; value: any } => {
const leaf = path[path.length - 1];

const when = rule.when;
if (!when) {
return {
lint: true,
value: originalValue,
};
}

const pattern = when.pattern;
const field = when.field;

// TODO: what if someone's field is called '@key'? should we use @@key?
const isKey = field === '@key';

if (!pattern) {
// isKey doesn't make sense without pattern
if (isKey) {
return {
lint: false,
value: originalValue,
};
}

return {
lint: has(originalValue, field),
value: originalValue,
};
}

if (isKey && pattern) {
return keyAndOptionalPattern(leaf, pattern, originalValue);
}

const fieldValue = String(get(originalValue, when.field));

return {
lint: fieldValue.match(pattern) !== null,
value: originalValue,
};
};

function keyAndOptionalPattern(key: string | number, pattern: string, value: any) {
/** arrays, look at the keys on the array object. note, this number check on id prop is not foolproof... */
if (typeof key === 'number' && typeof value === 'object') {
for (const k of Object.keys(value)) {
if (String(k).match(pattern)) {
return {
lint: true,
value,
};
}
}
} else if (String(key).match(pattern)) {
// objects
return {
lint: true,
value,
};
}

return {
lint: false,
value,
};
}

// todo: revisit -> https://github.com/stoplightio/spectral/issues/608
function getClosestJsonPath(data: unknown, path: JsonPath) {
if (data === null || typeof data !== 'object') return [];
if (!isObject(data)) return [];

while (path.length > 0 && !has(data, path)) {
path.pop();
Expand Down
14 changes: 0 additions & 14 deletions src/meta/rule.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,6 @@
"validation"
],
"type": "string"
},
"when": {
"properties": {
"field": {
"type": "string"
},
"pattern": {
"type": "string"
}
},
"required": [
"field"
],
"type": "object"
}
},
"required": [
Expand Down
9 changes: 0 additions & 9 deletions src/types/rule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ export interface IRule<T = string, O = any> {
// If undefined or true, resolved data will be supplied
resolved?: boolean;

when?: {
// the `path.to.prop` to field, or special `@key` value to target keys for matched `given` object
// EXAMPLE: if the target object is an oas object and given = `$..responses[*]`, then `@key` would be the response code (200, 400, etc)
field: string;

// a regex pattern
pattern?: string;
};

then: IThen<T, O> | Array<IThen<T, O>>;
}

Expand Down

0 comments on commit 8c1aedd

Please sign in to comment.