Skip to content

Commit

Permalink
fix(resolving): detect the source of local references properly
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip committed Sep 30, 2019
1 parent d019111 commit b968957
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 7 deletions.
62 changes: 61 additions & 1 deletion src/__tests__/spectral.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,71 @@ describe('spectral', () => {
line: 0,
},
},
severity: 0,
severity: DiagnosticSeverity.Error,
source: void 0,
},
]);
});

test('should recognize the source of local $refs', () => {
const s = new Spectral();
const source = 'foo.yaml';

const parsedResult: IParsedResult = {
getLocationForJsonPath,
source,
parsed: parseWithPointers(
JSON.stringify(
{
paths: {
'/agreements': {
get: {
description: 'Get some Agreements',
responses: {
'200': {
$ref: '#/responses/GetAgreementsOk',
},
default: {},
},
summary: 'List agreements',
tags: ['agreements', 'pagination'],
},
},
},
responses: {
GetAgreementsOk: {
description: 'Successful operation',
headers: {},
},
},
},
null,
2,
),
),
};

s.setRules({
'pagination-responses-have-x-next-token': {
description: 'All collection endpoints have the X-Next-Token parameter in responses',
given: "$.paths..get.responses['200'].headers",
severity: 'error',
recommended: true,
then: { field: 'X-Next-Token', function: 'truthy' },
},
});

return expect(s.run(parsedResult)).resolves.toEqual([
{
code: 'pagination-responses-have-x-next-token',
message: 'All collection endpoints have the X-Next-Token parameter in responses',
path: ['paths', '/agreements', 'get', 'responses', '200', 'headers'],
range: expect.any(Object),
severity: DiagnosticSeverity.Error,
source,
},
]);
});
});
});

Expand Down
6 changes: 4 additions & 2 deletions src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,11 @@ export const lintNode = (
results = results.concat(
targetResults.map<IRuleResult>(result => {
const escapedJsonPath = (result.path || targetPath).map(segment => decodePointerFragment(String(segment)));
const path = getRealJsonPath(
const path = getClosestJsonPath(
rule.resolved === false ? resolved.unresolved : resolved.resolved,
escapedJsonPath,
);
// todo: https://github.com/stoplightio/spectral/issues/608
const location = resolved.getLocationForJsonPath(path, true);

return {
Expand Down Expand Up @@ -199,7 +200,8 @@ function keyAndOptionalPattern(key: string | number, pattern: string, value: any
};
}

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

while (path.length > 0 && !has(data, path)) {
Expand Down
28 changes: 26 additions & 2 deletions src/resolved.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { pointerToPath } from '@stoplight/json';
import { IResolveError } from '@stoplight/json-ref-resolver/types';
import { Dictionary, ILocation, IRange, JsonPath, Segment } from '@stoplight/types';
import { get, has } from 'lodash';
import { get } from 'lodash';
import { IParseMap, REF_METADATA, ResolveResult } from './spectral';
import { IParsedResult } from './types';
import { hasRef, isObject } from './utils';

const getDefaultRange = (): IRange => ({
start: {
Expand Down Expand Up @@ -31,6 +33,28 @@ export class Resolved {
this.errors = resolveResult.errors;
}

public doesBelongToDoc(path: JsonPath): boolean {
if (path.length === 0) {
// todo: each rule and their function should be context-aware, meaning they should aware of the fact they operate on resolved content
// let's assume the error was reported correctly by any custom rule /shrug
return true;
}

let piece = this.unresolved;

for (let i = 0; i < path.length; i++) {
if (!isObject(piece)) return false;

if (path[i] in piece) {
piece = piece[path[i]];
} else if (hasRef(piece)) {
return this.doesBelongToDoc([...pointerToPath(piece.$ref), ...path.slice(i)]);
}
}

return true;
}

public getParsedForJsonPath(path: JsonPath) {
let target: object = this.parsedMap.refs;
const newPath = [...path];
Expand All @@ -53,7 +77,7 @@ export class Resolved {
};
}

if (path.length > 0 && !has(this.spec.parsed.data, path)) {
if (!this.doesBelongToDoc(path)) {
return null;
}

Expand Down
4 changes: 2 additions & 2 deletions src/rulesets/oas/functions/refSiblings.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { JsonPath } from '@stoplight/types';
import { IFunction, IFunctionResult } from '../../../types';
import { hasRef } from '../../../utils/hasRef';

// function is needed because `$..$ref` or `$..[?(@.$ref)]` are not parsed correctly
// and therefore lead to infinite recursion due to the dollar sign ('$' in '$ref')
function* siblingIterator(obj: object, path: JsonPath): IterableIterator<JsonPath> {
const hasRef = '$ref' in obj;
for (const key in obj) {
if (!Object.hasOwnProperty.call(obj, key)) continue;
const scopedPath = [...path, key];
if (hasRef && key !== '$ref') {
if (hasRef(obj) && key !== '$ref') {
yield scopedPath;
}

Expand Down
2 changes: 2 additions & 0 deletions src/utils/hasRef.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const hasRef = (obj: object): obj is object & { $ref: string } =>
'$ref' in obj && typeof (obj as Partial<{ $ref: unknown }>).$ref === 'string';
1 change: 1 addition & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './hasRef';
export * from './hasIntersectingElement';
export * from './isObject';

0 comments on commit b968957

Please sign in to comment.