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

fix: improve error source detection #685

Merged
merged 8 commits into from
Dec 10, 2019
Merged

fix: improve error source detection #685

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
59daebd
fix: consume graph data in doesBelongToSourceDoc
P0lip Oct 15, 2019
b61fefa
fix: write detection from scratch
P0lip Dec 3, 2019
7030d17
refactor: flatten parsedMap structure
P0lip Dec 3, 2019
a1cf6c2
fix: normalize source
P0lip Dec 3, 2019
2d098cf
Merge remote-tracking branch 'origin/develop' into fix/uri-error
P0lip Dec 6, 2019
e7ba421
chore: bump @stoplight/json and use utils
P0lip Dec 9, 2019
3444f50
build: rollup config
P0lip Dec 9, 2019
b315a87
Merge branch 'develop' into fix/uri-error
P0lip Dec 10, 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
44 changes: 44 additions & 0 deletions src/__tests__/__fixtures__/gh-658/URIError.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
%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"
"500":
description: No Bueno.
content:
application/json:
schema:
$ref: "./lib.yaml#/components/schemas/Error"
components:
schemas:
Error:
$ref: "#/components/schemas/Baz"
Baz:
$ref: ./lib.yaml#/components/schemas/Error
Foo:
type: string
24 changes: 24 additions & 0 deletions src/__tests__/__fixtures__/gh-658/lib.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
%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
maxLength: 1
error_description:
type: string
maxLength: 1
status_code:
type: string
test:
$ref: ./URIError.yaml#/components/schemas/Foo
223 changes: 218 additions & 5 deletions src/__tests__/spectral.jest.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,15 @@ describe('Spectral', () => {
);
});

test('reports issues for correct files with correct ranges and paths', async () => {
test('should report 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');
spectral.registerFormat('oas2', isOpenApiv2);
const parsed = {
parsed: parseWithPointers(fs.readFileSync(documentUri, 'utf8')),
getLocationForJsonPath,
source: documentUri,
};

const results = await spectral.run(parsed, {
Expand All @@ -128,7 +129,7 @@ describe('Spectral', () => {
line: 16,
},
},
source: undefined,
source: documentUri,
}),
expect.objectContaining({
code: 'oas2-schema',
Expand Down Expand Up @@ -158,7 +159,7 @@ describe('Spectral', () => {
line: 10,
},
},
source: undefined,
source: documentUri,
}),
expect.objectContaining({
code: 'info-contact',
Expand All @@ -173,7 +174,7 @@ describe('Spectral', () => {
line: 2,
},
},
source: undefined,
source: documentUri,
}),
expect.objectContaining({
code: 'operation-description',
Expand All @@ -188,9 +189,221 @@ describe('Spectral', () => {
line: 11,
},
},
source: undefined,
source: documentUri,
}),
]),
);
});

test('should recognize the source of remote $refs', async () => {
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',
},
},
});

const results = await s.run(fs.readFileSync(documentUri, 'utf8'), { resolve: { documentUri } });

return expect(results).toEqual([
expect.objectContaining({
path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 28,
line: 23,
},
start: {
character: 21,
line: 22,
},
},
}),

expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'400',
'content',
'application/json',
'schema',
'properties',
'status_code',
],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'400',
'content',
'application/json',
'schema',
'properties',
'test',
],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'500',
'content',
'application/json',
'schema',
'properties',
'status_code',
],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'500',
'content',
'application/json',
'schema',
'properties',
'test',
],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'test'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'Baz', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 21,
},
start: {
character: 20,
line: 20,
},
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Baz', 'properties', 'test'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
character: 18,
line: 43,
},
start: {
character: 8,
line: 42,
},
},
}),
]);
});
});
2 changes: 1 addition & 1 deletion src/__tests__/spectral.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,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
29 changes: 21 additions & 8 deletions src/cli/services/__tests__/linter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('Linter service', () => {
expect.arrayContaining([
expect.objectContaining({
code: 'oas3-schema',
path: ['info', 'contact', 'name'],
range: {
end: {
character: 14,
Expand Down Expand Up @@ -708,14 +709,26 @@ describe('Linter service', () => {

describe('--resolver', () => {
it('uses provided resolver for $ref resolving', async () => {
expect(await run(`lint --resolver ${fooResolver} ${fooDocument}`)).toEqual([
expect.objectContaining({
code: 'oas3-api-servers',
}),
expect.objectContaining({
code: 'openapi-tags',
}),
]);
expect(await run(`lint --resolver ${fooResolver} ${fooDocument}`)).toEqual(
expect.arrayContaining([
expect.objectContaining({
code: 'info-contact',
source: 'foo://openapi-3.0-no-contact.yaml',
}),
expect.objectContaining({
code: 'info-description',
source: 'foo://openapi-3.0-no-contact.yaml',
}),
expect.objectContaining({
code: 'oas3-api-servers',
source: 'foo://openapi-3.0-no-contact.yaml',
}),
expect.objectContaining({
code: 'openapi-tags',
source: 'foo://openapi-3.0-no-contact.yaml',
}),
]),
);
});
});
});
2 changes: 1 addition & 1 deletion src/error-messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const formatResolverErrors = (resolved: Resolved): IRuleResult[] => {
message: prettyPrintResolverErrorMessage(error.message),
severity: DiagnosticSeverity.Error,
range: location.range,
source: resolved.spec.source,
source: resolved.source,
});
}

Expand Down
Loading