Skip to content

Commit

Permalink
fix: proper source for errors involving empty $ref (#1531)
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip committed Mar 8, 2021
1 parent af99014 commit 77128eb
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 118 deletions.
8 changes: 8 additions & 0 deletions src/__tests__/__fixtures__/document-with-external-refs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"todo": {
"$ref": "./models/todo-full.v1.json"
},
"empty": {
"$ref": ""
}
}
37 changes: 0 additions & 37 deletions src/__tests__/__fixtures__/document-with-external-refs.oas2.json

This file was deleted.

121 changes: 42 additions & 79 deletions src/__tests__/spectral.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as nock from 'nock';
import * as path from 'path';

import { Document } from '../document';
import { isOpenApiv2 } from '../formats';
import { pattern } from '../functions/pattern';
import * as Parsers from '../parsers';
import { httpAndFileResolver } from '../resolvers/http-and-file';
Expand Down Expand Up @@ -97,93 +96,57 @@ describe('Spectral', () => {
});

test('should report issues for correct files with correct ranges and paths', async () => {
const documentUri = normalize(path.join(__dirname, './__fixtures__/document-with-external-refs.oas2.json'));
const documentUri = normalize(path.join(__dirname, './__fixtures__/document-with-external-refs.json'));
const spectral = new Spectral({ resolver: httpAndFileResolver });
await spectral.loadRuleset('spectral:oas');
spectral.registerFormat('oas2', isOpenApiv2);
spectral.setRules({
'requires-type': {
given: ['$..allOf', '$.empty'],
then: {
field: 'type',
function: 'truthy',
},
},
});
const document = new Document(fs.readFileSync(documentUri, 'utf8'), Parsers.Json, documentUri);

const results = await spectral.run(document);

expect(results).toEqual(
expect.arrayContaining([
expect.objectContaining({
code: 'oas2-schema',
path: ['paths', '/todos/{todoId}', 'get', 'responses', '200', 'description'],
range: {
end: {
character: 29,
line: 17,
},
start: {
character: 27,
line: 17,
},
},
source: documentUri,
}),
expect.objectContaining({
code: 'oas2-schema',
path: [],
range: {
end: {
character: 1,
line: 37,
},
start: {
character: 0,
line: 0,
},
expect(results).toEqual([
{
code: 'requires-type',
message: '`empty.type` property is not truthy',
path: ['empty'],
range: {
end: {
character: 3,
line: 6,
},
source: expect.stringContaining('__fixtures__/models/todo-full.v1.json'),
}),
expect.objectContaining({
code: 'path-params',
path: ['paths', '/todos/{todoId}', 'get'],
range: {
end: {
character: 7,
line: 33,
},
start: {
character: 13,
line: 11,
},
start: {
character: 11,
line: 4,
},
source: documentUri,
}),
expect.objectContaining({
code: 'info-contact',
path: ['info'],
range: {
end: {
character: 3,
line: 6,
},
start: {
character: 10,
line: 2,
},
},
severity: DiagnosticSeverity.Warning,
source: documentUri,
},
{
code: 'requires-type',
message: '`allOf.type` property is not truthy',
path: ['allOf'],
range: {
end: {
character: 3,
line: 33,
},
source: documentUri,
}),
expect.objectContaining({
code: 'operation-description',
path: ['paths', '/todos/{todoId}', 'get'],
range: {
end: {
character: 7,
line: 33,
},
start: {
character: 13,
line: 11,
},
start: {
character: 11,
line: 3,
},
source: documentUri,
}),
]),
);
},
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('__fixtures__/models/todo-full.v1.json'),
},
]);
});

test('properly decorates results with metadata pertaining to the document being linted', async () => {
Expand Down
13 changes: 11 additions & 2 deletions src/documentInventory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,21 @@ export class DocumentInventory {
if ($ref === null) return null;

const scopedPath = [...safePointerToPath($ref), ...newPath];
let resolvedDoc;
let resolvedDoc = this.document;

if (isLocalRef($ref)) {
resolvedDoc = source === this.document.source ? this.document : this.referencedDocuments[source];
} else {
const extractedSource = extractSourceFromRef($ref)!;
const extractedSource = extractSourceFromRef($ref);

if (extractedSource === null) {
return {
document: resolvedDoc,
path: getClosestJsonPath(resolvedDoc.data, path),
missingPropertyPath: path,
};
}

source = isAbsoluteRef(extractedSource) ? extractedSource : resolve(source, '..', extractedSource);

resolvedDoc = source === this.document.source ? this.document : this.referencedDocuments[source];
Expand Down

0 comments on commit 77128eb

Please sign in to comment.