fix(core): fix 'resolved vs unresolved' json path mapping (#2202)

* test(core): add test demonstrating path mapper issue

* fix(core): fix 'resolved vs unresolved' json path mapping

* refactor(core): use stoplight/json utils

Co-authored-by: Dustin Popp <dpopp07@gmail.com>
Co-authored-by: Jakub Rożek <jakub@stoplight.io>

packages/cli/src/services/__tests__/linter.test.ts

@@ -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'),
         },
       ]);
     });

packages/core/src/__tests__/__fixtures__/gh-658/URIError.yaml

@@ -28,6 +28,8 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/Error"
+        "404":
+          $ref: './lib.yaml#/components/responses/NotFoundResponse'
         "500":
           description: No Bueno.
           content:

packages/core/src/__tests__/__fixtures__/gh-658/lib.yaml

@@ -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'
+

packages/core/src/__tests__/spectral.jest.test.ts

@@ -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({
@@ -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'),
@@ -229,11 +259,11 @@ describe('Spectral', () => {
         range: {
           end: {
             character: 18,
-            line: 43,
+            line: 45,
           },
           start: {
             character: 8,
-            line: 42,
+            line: 44,
           },
         },
       }),

packages/core/src/documentInventory.ts

@@ -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 = {
@@ -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);