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.

Sign up for GitHub

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(rule): remove 'when' #614

Merged
merged 2 commits into from
Oct 24, 2019
Merged

feat(rule): remove 'when' #614

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70fc34d
feat(rule): remove 'when'
P0lip Sep 28, 2019
92f06c4
Merge branch 'develop' into feat/remove-when
Oct 24, 2019
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
147 changes: 18 additions & 129 deletions src/__tests__/linter.test.ts
View file
Open in desktop
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 @@ -689,115 +687,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
Open in desktop
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 @@ -137,83 +130,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
Open in desktop
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
Open in desktop
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