Skip to content

Commit

Permalink
fix: consume graph data in doesBelongToSourceDoc
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip committed Nov 12, 2019
1 parent 502bd1b commit 0a2519a
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 9 deletions.
36 changes: 36 additions & 0 deletions src/__tests__/__fixtures__/gh-658/URIError.yaml
@@ -0,0 +1,36 @@
%YAML 1.2
---
openapi: 3.0.0

info:
title: Hey Mum! I'm on GitHub!
description: Resource definition.
version: 1.0.0

servers:
- url: https://boom.com

paths:
"/test":
get:
summary: Dummy endpoint.
description: Cf. summary
responses:
"200":
description: All is good.
content:
application/json:
schema:
type: string
"400":
description: Bad Request.
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
components:
schemas:
Error:
$ref: "#/components/schemas/Baz"
Baz:
$ref: ./lib.yaml#/components/schemas/Error
20 changes: 20 additions & 0 deletions src/__tests__/__fixtures__/gh-658/lib.yaml
@@ -0,0 +1,20 @@
%YAML 1.2
---
openapi: 3.0.0

info:
title: Library
description: Collection of reusable standard definitions.
version: 2.0.0
paths: {}
components:
schemas:
Error:
type: object
properties:
error:
type: string
error_description:
type: string
status_code:
type: string
89 changes: 88 additions & 1 deletion src/__tests__/spectral.jest.test.ts
Expand Up @@ -78,7 +78,7 @@ describe('Spectral', () => {
});
});

test('reports issues for correct files with correct ranges and paths', async () => {
it('reports issues for correct files with correct ranges and paths', async () => {
const documentUri = path.join(__dirname, './__fixtures__/document-with-external-refs.oas2.json');
const spectral = new Spectral({ resolver: httpAndFileResolver });
await spectral.loadRuleset('spectral:oas');
Expand Down Expand Up @@ -174,4 +174,91 @@ describe('Spectral', () => {
]),
);
});

it('recognizes the source of remote $refs', () => {
const s = new Spectral({ resolver: httpAndFileResolver });
const documentUri = path.join(__dirname, './__fixtures__/gh-658/URIError.yaml');

s.setRules({
'schema-strings-maxLength': {
severity: DiagnosticSeverity.Warning,
recommended: true,
message: "String typed properties MUST be further described using 'maxLength'. Error: {{error}}",
given: "$..*[?(@.type === 'string')]",
then: {
field: 'maxLength',
function: 'truthy',
},
},
});

return expect(s.run(fs.readFileSync(documentUri, 'utf8'), { resolve: { documentUri } })).resolves.toEqual(
expect.arrayContaining([
expect.objectContaining({
code: 'schema-strings-maxLength',
path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'],
range: {
end: {
character: 28,
line: 23,
},
start: {
character: 21,
line: 22,
},
},
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
}),
expect.objectContaining({
code: 'schema-strings-maxLength',
path: ['components', 'schemas', 'Baz', 'properties', 'error'],
range: {
end: {
character: 22,
line: 15,
},
start: {
character: 14,
line: 14,
},
},
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
}),
expect.objectContaining({
code: 'schema-strings-maxLength',
path: ['components', 'schemas', 'Baz', 'properties', 'error_description'],
range: {
end: {
character: 22,
line: 17,
},
start: {
character: 26,
line: 16,
},
},
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
}),
expect.objectContaining({
code: 'schema-strings-maxLength',
path: ['components', 'schemas', 'Baz', 'properties', 'status_code'],
range: {
end: {
character: 22,
line: 19,
},
start: {
character: 20,
line: 18,
},
},
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
}),
]),
);
});
});
2 changes: 1 addition & 1 deletion src/__tests__/spectral.test.ts
Expand Up @@ -262,7 +262,7 @@ describe('spectral', () => {
},
});

return expect(s.run(parsedResult)).resolves.toEqual([
return expect(s.run(parsedResult, { resolve: { documentUri: source } })).resolves.toEqual([
{
code: 'pagination-responses-have-x-next-token',
message: 'All collection endpoints have the X-Next-Token parameter in responses',
Expand Down
33 changes: 26 additions & 7 deletions src/resolved.ts
@@ -1,4 +1,4 @@
import { decodePointerFragment, pointerToPath } from '@stoplight/json';
import { decodePointerFragment } from '@stoplight/json';
import { IGraphNodeData, IResolveError } from '@stoplight/json-ref-resolver/types';
import { Dictionary, ILocation, IRange, JsonPath, Segment } from '@stoplight/types';
import { DepGraph } from 'dependency-graph';
Expand Down Expand Up @@ -36,7 +36,7 @@ export class Resolved {
this.errors = resolveResult.errors;
}

public doesBelongToDoc(path: JsonPath): boolean {
public doesBelongToSourceDoc(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
Expand All @@ -45,13 +45,32 @@ export class Resolved {

let piece = this.unresolved;

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

if (path[i] in piece) {
piece = piece[path[i]];
if (segment in piece) {
piece = piece[segment];
} else if (hasRef(piece)) {
return this.doesBelongToDoc([...pointerToPath(piece.$ref), ...path.slice(i)]);
if (this.spec.source === void 0) return false;
let nodeData;
try {
nodeData = this.graph.getNodeData(this.spec.source);
} catch {
return false;
}

const { refMap } = nodeData;
let { $ref } = piece;

while ($ref in refMap) {
$ref = refMap[$ref];
}

return (
$ref.length === 0 ||
$ref[0] === '#' ||
$ref.slice(0, Math.max(0, Math.min($ref.length, $ref.indexOf('#')))) === this.spec.source
);
}
}

Expand Down Expand Up @@ -80,7 +99,7 @@ export class Resolved {
};
}

if (!this.doesBelongToDoc(path)) {
if (!this.doesBelongToSourceDoc(path)) {
return null;
}

Expand Down
1 change: 1 addition & 0 deletions src/spectral.ts
Expand Up @@ -90,6 +90,7 @@ export class Spectral {
mergeKeys: true,
}),
getLocationForJsonPath: getLocationForJsonPathYAML,
source: opts.resolve && opts.resolve.documentUri,
};
} else {
parsedResult = target;
Expand Down

0 comments on commit 0a2519a

Please sign in to comment.