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.

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(core): fix 'resolved vs unresolved' json path mapping #2202

Merged
merged 4 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/cli/src/services/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,10 @@ describe('Linter service', () => {
{
code: 'info-matches-stoplight',
message: 'Info must contain Stoplight',
path: ['info', 'title'],
path: [],
range: expect.any(Object),
severity: DiagnosticSeverity.Warning,
source: expect.stringContaining('__fixtures__/resolver/document.json'),
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/Error"
"404":
$ref: './lib.yaml#/components/responses/NotFoundResponse'
"500":
description: No Bueno.
content:
Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/__tests__/__fixtures__/gh-658/lib.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,26 @@ components:
type: string
test:
$ref: ./URIError.yaml#/components/schemas/Foo
NotFound:
oneOf:
- $ref: '#/components/schemas/DoesntExist'
- $ref: '#/components/schemas/JustCantFindIt'
DoesntExist:
type: object
properties:
id:
type: string
JustCantFindIt:
type: object
properties:
name:
type: string

responses:
NotFoundResponse:
description: Not Found.
content:
application/json:
schema:
$ref: '#/components/schemas/NotFound'

36 changes: 33 additions & 3 deletions packages/core/src/__tests__/spectral.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('Spectral', () => {

const results = await s.run(new Document(fs.readFileSync(documentUri, 'utf8'), Parsers.Yaml, documentUri));

expect(results.length).toEqual(3);
expect(results.length).toEqual(5);

return expect(results).toEqual([
expect.objectContaining({
Expand All @@ -208,6 +208,36 @@ describe('Spectral', () => {
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'DoesntExist', 'properties', 'id'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 32,
},
start: {
character: 11,
line: 31,
},
},
}),

expect.objectContaining({
path: ['components', 'schemas', 'JustCantFindIt', 'properties', 'name'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
character: 22,
line: 37,
},
start: {
character: 13,
line: 36,
},
},
}),

expect.objectContaining({
path: ['paths', '/test', 'get', 'responses', '200', 'content', 'application/json', 'schema'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
Expand All @@ -229,11 +259,11 @@ describe('Spectral', () => {
range: {
end: {
character: 18,
line: 43,
line: 45,
},
start: {
character: 8,
line: 42,
line: 44,
},
},
}),
Expand Down
130 changes: 80 additions & 50 deletions packages/core/src/documentInventory.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import { extractSourceFromRef, hasRef, isLocalRef } from '@stoplight/json';
import { decodePointerFragment, encodePointerFragment, extractSourceFromRef, isLocalRef } from '@stoplight/json';
import { extname, resolve } from '@stoplight/path';
import { Dictionary, IParserResult, JsonPath } from '@stoplight/types';
import { get, isObjectLike } from 'lodash';
import { isObjectLike } from 'lodash';
import { Document, IDocument } from './document';
import { Resolver, ResolveResult } from '@stoplight/spectral-ref-resolver';

import { formatParserDiagnostics, formatResolverErrors } from './errorMessages';
import * as Parsers from '@stoplight/spectral-parsers';
import { IRuleResult } from './types';
import {
getClosestJsonPath,
getEndRef,
isAbsoluteRef,
safePointerToPath,
traverseObjUntilRef,
} from '@stoplight/spectral-runtime';
import { getClosestJsonPath, isAbsoluteRef, traverseObjUntilRef } from '@stoplight/spectral-runtime';
import { Format } from './ruleset/format';

export type DocumentInventoryItem = {
Expand Down Expand Up @@ -86,79 +80,115 @@ export class DocumentInventory implements IDocumentInventory {
public findAssociatedItemForPath(path: JsonPath, resolved: boolean): DocumentInventoryItem | null {
if (!resolved) {
const newPath: JsonPath = getClosestJsonPath(this.unresolved, path);

return {
const item: DocumentInventoryItem = {
document: this.document,
path: newPath,
missingPropertyPath: path,
};
return item;
}

try {
const newPath: JsonPath = getClosestJsonPath(this.resolved, path);
let $ref = traverseObjUntilRef(this.unresolved, newPath);
const $ref = traverseObjUntilRef(this.unresolved, newPath);

if ($ref === null) {
return {
const item: DocumentInventoryItem = {
document: this.document,
path: getClosestJsonPath(this.unresolved, path),
missingPropertyPath: path,
};
return item;
}

const missingPropertyPath =
newPath.length === 0 ? [] : path.slice(path.lastIndexOf(newPath[newPath.length - 1]) + 1);

let { source } = this;
if (source === null || this.graph === null) {
return null;
}

// eslint-disable-next-line no-constant-condition
while (true) {
if (source === null || this.graph === null) return null;

$ref = getEndRef(this.graph.getNodeData(source).refMap, $ref);

if ($ref === null) return null;

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

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

if (extractedSource === null) {
return {
document: resolvedDoc,
path: getClosestJsonPath(resolvedDoc.data, path),
missingPropertyPath: path,
};
}
let refMap = this.graph.getNodeData(source).refMap;
let resolvedDoc = this.document;

source = isAbsoluteRef(extractedSource) ? extractedSource : resolve(source, '..', extractedSource);
// Add '#' on the beginning of "path" to simplify the logic below.
const adjustedPath: string[] = ['#', ...path.map(String)];

resolvedDoc = source === this.document.source ? this.document : this.referencedDocuments[source];
const obj: unknown =
scopedPath.length === 0 || hasRef(resolvedDoc.data) ? resolvedDoc.data : get(resolvedDoc.data, scopedPath);
// Walk through the segments of 'path' one at a time, looking for
// json path locations containing a $ref.
let refMapKey = '';
for (const segment of adjustedPath) {
if (refMapKey.length > 0) {
refMapKey += '/';
}

if (hasRef(obj)) {
$ref = obj.$ref;
continue;
refMapKey += encodePointerFragment(segment);

// If our current refMapKey value is in fact a key in refMap,
// then we'll "reverse-resolve" it by replacing refMapKey with
// the actual value of that key within refMap.
// It's possible that we have a "ref to a ref", so we'll do this
// "reverse-resolve" step in a while loop.
while (refMapKey in refMap) {
const newRef = refMap[refMapKey];
if (isLocalRef(newRef)) {
refMapKey = newRef;
} else {
const extractedSource = extractSourceFromRef(newRef);
if (extractedSource === null) {
const item: DocumentInventoryItem = {
document: resolvedDoc,
path: getClosestJsonPath(resolvedDoc.data, path),
missingPropertyPath: path,
};
return item;
}

// Update 'source' to reflect the filename within the external ref.
source = isAbsoluteRef(extractedSource) ? extractedSource : resolve(source, '..', extractedSource);

// Update "resolvedDoc" to reflect the new "source" value and make sure we found an actual document.
const newResolvedDoc = source === this.document.source ? this.document : this.referencedDocuments[source];
if (newResolvedDoc === null || newResolvedDoc === void 0) {
const item: DocumentInventoryItem = {
document: resolvedDoc,
path: getClosestJsonPath(resolvedDoc.data, path),
missingPropertyPath: path,
};
return item;
}

resolvedDoc = newResolvedDoc;

// Update "refMap" to reflect the new "source" value.
refMap = this.graph.getNodeData(source).refMap;

refMapKey = newRef.indexOf('#') >= 0 ? newRef.slice(newRef.indexOf('#')) : '#';
}
}

const closestPath = getClosestJsonPath(resolvedDoc.data, scopedPath);
return {
document: resolvedDoc,
path: closestPath,
missingPropertyPath: [...closestPath, ...missingPropertyPath],
};
}

const closestPath = getClosestJsonPath(resolvedDoc.data, this.convertRefMapKeyToPath(refMapKey));
const item: DocumentInventoryItem = {
document: resolvedDoc,
path: closestPath,
missingPropertyPath: [...closestPath, ...missingPropertyPath],
};
return item;
} catch {
return null;
}
}

protected convertRefMapKeyToPath(refPath: string): JsonPath {
if (refPath.startsWith('#/')) {
refPath = refPath.slice(2);
}

return refPath.split('/').map(decodePointerFragment);
}

protected parseResolveResult: Resolver['parseResolveResult'] = resolveOpts => {
const source = resolveOpts.targetAuthority.href().replace(/\/$/, '');
const ext = extname(source);
Expand Down