Skip to content

Commit

Permalink
Merge pull request #839 from stoplightio/fix/path-result
Browse files Browse the repository at this point in the history 
fix: absolute paths
  • Loading branch information
@github-actions
github-actions[bot] committed Dec 19, 2019
2 parents a4f0759 + 8f14d4c commit 22301f4
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 130 deletions.
6 changes: 3 additions & 3 deletions src/__tests__/linter.jest.test.ts
View file
Expand Up @@ -120,17 +120,17 @@ describe('Linter', () => {
expect.objectContaining({
code: 'empty-is-falsy',
message: 'Value "https://example.com" should be falsy',
path: ['empty'],
path: ['info', 'contact', 'url'],
}),
expect.objectContaining({
code: 'empty-is-falsy',
message: 'Value Array[] should be falsy',
path: ['bar', 'empty'],
path: ['servers'],
}),
expect.objectContaining({
code: 'empty-is-falsy',
message: 'Value Object{} should be falsy',
path: ['foo', 'empty'],
path: ['info'],
}),
]),
);
Expand Down
60 changes: 8 additions & 52 deletions src/__tests__/spectral.jest.test.ts
View file
Expand Up @@ -133,7 +133,7 @@ describe('Spectral', () => {
}),
expect.objectContaining({
code: 'oas2-schema',
path: ['paths', '/todos/{todoId}', 'get', 'responses', '200', 'schema'],
path: [],
range: {
end: {
character: 1,
Expand Down Expand Up @@ -231,18 +231,7 @@ describe('Spectral', () => {
}),

expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'400',
'content',
'application/json',
'schema',
'properties',
'status_code',
],
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
Expand All @@ -256,18 +245,7 @@ describe('Spectral', () => {
},
}),
expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'400',
'content',
'application/json',
'schema',
'properties',
'test',
],
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
Expand All @@ -282,18 +260,7 @@ describe('Spectral', () => {
}),

expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'500',
'content',
'application/json',
'schema',
'properties',
'status_code',
],
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
Expand All @@ -307,18 +274,7 @@ describe('Spectral', () => {
},
}),
expect.objectContaining({
path: [
'paths',
'/test',
'get',
'responses',
'500',
'content',
'application/json',
'schema',
'properties',
'test',
],
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
Expand Down Expand Up @@ -362,7 +318,7 @@ describe('Spectral', () => {
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Error', 'properties', 'test'],
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
Expand All @@ -377,7 +333,7 @@ describe('Spectral', () => {
}),

expect.objectContaining({
path: ['components', 'schemas', 'Baz', 'properties', 'status_code'],
path: ['components', 'schemas', 'Error', 'properties', 'status_code'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/lib.yaml'),
range: {
end: {
Expand All @@ -391,7 +347,7 @@ describe('Spectral', () => {
},
}),
expect.objectContaining({
path: ['components', 'schemas', 'Baz', 'properties', 'test'],
path: ['components', 'schemas', 'Foo'],
source: expect.stringContaining('/src/__tests__/__fixtures__/gh-658/URIError.yaml'),
range: {
end: {
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/spectral.test.ts
View file
Expand Up @@ -188,11 +188,11 @@ describe('spectral', () => {
path: ['foo', 'bar', 'baz'],
range: {
end: {
character: 0,
character: 12,
line: 0,
},
start: {
character: 0,
character: 7,
line: 0,
},
},
Expand Down Expand Up @@ -254,7 +254,7 @@ describe('spectral', () => {
{
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'],
path: ['responses', 'GetAgreementsOk', 'headers'],
range: expect.any(Object),
severity: DiagnosticSeverity.Error,
source,
Expand Down
8 changes: 4 additions & 4 deletions src/cli/services/__tests__/linter.test.ts
View file
Expand Up @@ -575,7 +575,7 @@ describe('Linter service', () => {
expect.objectContaining({
code: 'oas2-schema',
message: '/info Property foo is not expected to be here',
path: ['info'],
path: ['definitions', 'info'],
range: {
end: {
character: 5,
Expand All @@ -591,7 +591,7 @@ describe('Linter service', () => {
expect.objectContaining({
code: 'info-description',
message: 'OpenAPI object info `description` must be present and non-empty string.',
path: ['info', 'description'], // todo: relative path or absolute path? there is no such path in linted file, but there is such in spec when working on resolved file
path: ['definitions', 'info', 'description'],
range: {
end: {
character: 22,
Expand Down Expand Up @@ -626,7 +626,7 @@ describe('Linter service', () => {
expect.objectContaining({
code: 'oas2-schema',
message: "/info should have required property 'title'",
path: ['info'],
path: [],
range: {
end: {
character: 1,
Expand All @@ -642,7 +642,7 @@ describe('Linter service', () => {
expect.objectContaining({
code: 'info-description',
message: 'OpenAPI object info `description` must be present and non-empty string.',
path: ['info', 'description'],
path: ['description'],
range: {
end: {
character: 18,
Expand Down
73 changes: 30 additions & 43 deletions src/linter.ts
View file
@@ -1,14 +1,13 @@
import { JsonPath } from '@stoplight/types';
import { get, has } from 'lodash';
import { get, isObject } from 'lodash';

const { JSONPath } = require('jsonpath-plus');

import { decodePointerFragment, pathToPointer } from '@stoplight/json';
import { Resolved } from './resolved';
import { message } from './rulesets/message';
import { getDefaultRange, Resolved } from './resolved';
import { IMessageVars, message } from './rulesets/message';
import { getDiagnosticSeverity } from './rulesets/severity';
import { IFunction, IGivenNode, IRuleResult, IRunRule, IThen } from './types';
import { isObject } from './utils';
import { getClosestJsonPath } from './utils';

// TODO(SO-23): unit test but mock whatShouldBeLinted
export const lintNode = (
Expand Down Expand Up @@ -91,53 +90,41 @@ export const lintNode = (
results.push(
...targetResults.map<IRuleResult>(result => {
const escapedJsonPath = (result.path || targetPath).map(segment => decodePointerFragment(String(segment)));
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);
const parsed = resolved.getParsedForJsonPath(escapedJsonPath);
const path = parsed?.path || getClosestJsonPath(resolved.resolved, escapedJsonPath);
const doc = parsed?.doc || resolved.parsed;
const range = doc.getLocationForJsonPath(doc.parsed, path, true)?.range || getDefaultRange();

const vars: IMessageVars = {
property: path.length > 0 ? path[path.length - 1] : '',
path: pathToPointer(path),
error: result.message,
description: rule.description,
get value() {
// let's make `value` lazy
const value = get(parsed?.doc.parsed.data, path);
if (isObject(value)) {
return Array.isArray(value) ? 'Array[]' : 'Object{}';
}

return JSON.stringify(value);
},
};

const resultMessage = message(result.message, vars);
vars.error = resultMessage;

return {
code: rule.name,

message:
rule.message === undefined
? rule.description || result.message
: message(rule.message, {
error: result.message,
property: path.length > 0 ? path[path.length - 1] : '',
path: pathToPointer(path),
description: rule.description,
get value() {
// let's make it `value` lazy
const value = resolved.getValueForJsonPath(path);
if (isObject(value)) {
return Array.isArray(value) ? 'Array[]' : 'Object{}';
}

return JSON.stringify(value);
},
}),
message: rule.message === void 0 ? rule.description || resultMessage : message(rule.message, vars),
path,
severity: getDiagnosticSeverity(rule.severity),
source: location.uri,
range: location.range,
source: parsed?.doc.source,
range,
};
}),
);
}

return results;
};

// todo: revisit -> https://github.com/stoplightio/spectral/issues/608
function getClosestJsonPath(data: unknown, path: JsonPath) {
if (!isObject(data)) return [];

while (path.length > 0 && !has(data, path)) {
path.pop();
}

return path;
}
56 changes: 31 additions & 25 deletions src/resolved.ts
View file
@@ -1,13 +1,13 @@
import { extractSourceFromRef, hasRef, isLocalRef, pointerToPath } from '@stoplight/json';
import { extractSourceFromRef, hasRef, isLocalRef } from '@stoplight/json';
import { IGraphNodeData, IResolveError } from '@stoplight/json-ref-resolver/types';
import { normalize, resolve } from '@stoplight/path';
import { Dictionary, ILocation, IRange, JsonPath } from '@stoplight/types';
import { DepGraph } from 'dependency-graph';
import { get } from 'lodash';
import { IParsedResult, ResolveResult } from './types';
import { getEndRef, isAbsoluteRef, safePointerToPath, traverseObjUntilRef } from './utils';
import { getClosestJsonPath, getEndRef, isAbsoluteRef, safePointerToPath, traverseObjUntilRef } from './utils';

const getDefaultRange = (): IRange => ({
export const getDefaultRange = (): IRange => ({
start: {
line: 0,
character: 0,
Expand All @@ -31,7 +31,7 @@ export class Resolved {
}

constructor(
protected parsed: IParsedResult,
public readonly parsed: IParsedResult,
resolveResult: ResolveResult,
public parsedRefs: Dictionary<IParsedResult>,
) {
Expand All @@ -46,16 +46,20 @@ export class Resolved {

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

if ($ref === null) {
return {
path,
path: getClosestJsonPath(this.unresolved, path),
doc: this.parsed,
missingPropertyPath: path,
};
}

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

let { source } = this;

while (true) {
Expand All @@ -65,30 +69,32 @@ export class Resolved {

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

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

if (isLocalRef($ref)) {
return {
path: pointerToPath($ref),
doc: source === this.parsed.source ? this.parsed : this.parsedRefs[source],
};
}
resolvedDoc = source === this.parsed.source ? this.parsed : this.parsedRefs[source];
} else {
const extractedSource = extractSourceFromRef($ref)!;
source = isAbsoluteRef(extractedSource) ? extractedSource : resolve(source, '..', extractedSource);

const extractedSource = extractSourceFromRef($ref)!;
source = isAbsoluteRef(extractedSource) ? extractedSource : resolve(source, '..', extractedSource);
resolvedDoc = source === this.parsed.source ? this.parsed : this.parsedRefs[source];
const { parsed } = resolvedDoc;

const doc = source === this.parsed.source ? this.parsed : this.parsedRefs[source];
const { parsed } = doc;
const scopedPath = [...safePointerToPath($ref), ...newPath];
const obj = scopedPath.length === 0 || hasRef(parsed.data) ? parsed.data : get(parsed.data, scopedPath);

const obj = scopedPath.length === 0 || hasRef(parsed.data) ? parsed.data : get(parsed.data, scopedPath);

if (hasRef(obj)) {
$ref = obj.$ref;
} else {
return {
doc,
path: scopedPath,
};
if (hasRef(obj)) {
$ref = obj.$ref;
continue;
}
}

const closestPath = getClosestJsonPath(resolvedDoc.parsed.data, scopedPath);
return {
doc: resolvedDoc,
path: closestPath,
missingPropertyPath: [...closestPath, ...missingPropertyPath],
};
}
} catch {
return null;
Expand Down

0 comments on commit 22301f4

Please sign in to comment.