diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index 06f9f8f5a..c320b3471 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -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'], }), ]), ); diff --git a/src/__tests__/spectral.jest.test.ts b/src/__tests__/spectral.jest.test.ts index c37d922d3..847840c89 100644 --- a/src/__tests__/spectral.jest.test.ts +++ b/src/__tests__/spectral.jest.test.ts @@ -133,7 +133,7 @@ describe('Spectral', () => { }), expect.objectContaining({ code: 'oas2-schema', - path: ['paths', '/todos/{todoId}', 'get', 'responses', '200', 'schema'], + path: [], range: { end: { character: 1, @@ -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: { @@ -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: { @@ -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: { @@ -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: { @@ -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: { @@ -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: { @@ -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: { diff --git a/src/__tests__/spectral.test.ts b/src/__tests__/spectral.test.ts index 9c512f7a4..c96f31909 100644 --- a/src/__tests__/spectral.test.ts +++ b/src/__tests__/spectral.test.ts @@ -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, }, }, @@ -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, diff --git a/src/cli/services/__tests__/linter.test.ts b/src/cli/services/__tests__/linter.test.ts index ece847976..332f55c63 100644 --- a/src/cli/services/__tests__/linter.test.ts +++ b/src/cli/services/__tests__/linter.test.ts @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/src/linter.ts b/src/linter.ts index b07244451..8a5e8708c 100644 --- a/src/linter.ts +++ b/src/linter.ts @@ -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 = ( @@ -91,38 +90,37 @@ export const lintNode = ( results.push( ...targetResults.map(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, }; }), ); @@ -130,14 +128,3 @@ export const lintNode = ( 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; -} diff --git a/src/resolved.ts b/src/resolved.ts index 8e1ac8207..8aeaaae9a 100644 --- a/src/resolved.ts +++ b/src/resolved.ts @@ -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, @@ -31,7 +31,7 @@ export class Resolved { } constructor( - protected parsed: IParsedResult, + public readonly parsed: IParsedResult, resolveResult: ResolveResult, public parsedRefs: Dictionary, ) { @@ -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) { @@ -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; diff --git a/src/rulesets/__tests__/message.spec.ts b/src/rulesets/__tests__/message.spec.ts index 0b5f4c079..feee949ab 100644 --- a/src/rulesets/__tests__/message.spec.ts +++ b/src/rulesets/__tests__/message.spec.ts @@ -8,6 +8,7 @@ describe('message util', () => { property: 'description', error: 'expected property to be truthy', path: '', + fullPath: '', value: void 0, }), ).toEqual('oops... "description" is missing;error: expected property to be truthy'); @@ -20,6 +21,7 @@ describe('message util', () => { property: 'description', error: 'expected property to be truthy', path: '', + fullPath: '', value, }), ).toEqual(`Value must not equal ${value}`); @@ -32,6 +34,7 @@ describe('message util', () => { property: 'baz', error: 'foo', path: '', + fullPath: '', value: void 0, }), ).toEqual('foofoobazfoofoo'); @@ -44,6 +47,7 @@ describe('message util', () => { property: 'description', error: 'expected property to be truthy', path: '', + fullPath: '', value: void 0, }), ).toEqual('missing :('); diff --git a/src/rulesets/message.ts b/src/rulesets/message.ts index cece55962..6475bf774 100644 --- a/src/rulesets/message.ts +++ b/src/rulesets/message.ts @@ -6,6 +6,7 @@ export interface IMessageVars { description?: string; value: unknown; path: string; + fullPath?: string; } export type MessageInterpolator = (str: string, values: IMessageVars) => string; diff --git a/src/utils/refs.ts b/src/utils/refs.ts index 03935be6f..fbc0f023b 100644 --- a/src/utils/refs.ts +++ b/src/utils/refs.ts @@ -43,3 +43,20 @@ export const safePointerToPath = (pointer: string): JsonPath => { const rawPointer = extractPointerFromRef(pointer); return rawPointer ? pointerToPath(rawPointer) : []; }; + +export const getClosestJsonPath = (data: unknown, path: JsonPath) => { + const closestPath: JsonPath = []; + + if (!isObject(data)) return closestPath; + + let piece = data; + + for (const segment of path) { + if (!(segment in piece)) break; + closestPath.push(segment); + if (!isObject(piece[segment])) break; + piece = piece[segment]; + } + + return closestPath; +};