Repository: stoplightio/spectral (ts)

Issue:
Paths are not reported correctly after multiple recursions of resolution
**Describe the bug**

It appears that the path of an error is reported correctly if resolve goes through a single `$ref` resolution, but not if it goes through more than one.

**To Reproduce**

Given this set of schemas:

<details>
<summary>Example grandparent, parent, and child schemas</summary>

```yaml
openapi: 3.0.3
components:
  schemas:
    Grandparent:
      type: object
      description: Grandparent resource
      properties:
        child:
          $ref: '#/components/schemas/Parent'
    Parent:
      type: object
      description: Parent resource
      properties:
        child:
          $ref: '#/components/schemas/Child'
    Child:
      type: object
      properties:
        name:
          type: string
          description: Name for a child
```
</details>

...only the `Child` schema violates this rule:

```yaml
rules:
  object-schema-description-defined:
    description: Object schemas must have a description
    severity: error
    formats: [oas3]
    given: $.components.schemas..[?(@.type == 'object')]
    then:
      field: description
      function: truthy
```

...but Spectral reports a violation in redundant locations:

```bash
$ spectral lint dedup.yaml

/experiments/dedup.yaml
 14:15  error  object-schema-description-defined  Object schemas must have a description  components.schemas.Parent.properties.child
 16:11  error  object-schema-description-defined  Object schemas must have a description  components.schemas.Child

✖ 2 problems (2 errors, 0 warnings, 0 infos, 0 hints)
```

This doesn't happen if the `Grandparent` schema doesn't exist, so it seems to be an artifact of Spectral looking "through" the `Grandparent` schema. The result makes _sense_ to me in that if `components.schemas.Parent.properties.child` is "resolved" then it's the location of a second error.

But things get even stranger if I add a `GreatGrandparent`:

<details>
  <summary>Example schemas with <b>great grandparent</b></summary>

```yaml
openapi: 3.0.3
components:
  schemas:
    GreatGrandparent:
      type: object
      description: Great grandparent resource
      properties:
        child:
          $ref: '#/components/schemas/Grandparent'
    Grandparent:
      type: object
      description: Grandparent resource
      properties:
        child:
          $ref: '#/components/schemas/Parent'
    Parent:
      type: object
      description: Parent resource
      properties:
        child:
          $ref: '#/components/schemas/Child'
    Child:
      type: object
      properties:
        name:
          type: string
          description: Name for a child
```
</details>

Now the same Spectral rule reports three errors:

```bash
$ spectral lint dedup.yaml

/experiments/dedup.yaml
 14:15  error  object-schema-description-defined  Object schemas must have a description  components.schemas.Grandparent.properties.child
 20:15  error  object-schema-description-defined  Object schemas must have a description  components.schemas.Parent.properties.child
 22:11  error  object-schema-description-defined  Object schemas must have a description  components.schemas.Child

✖ 3 problems (3 errors, 0 warnings, 0 infos, 0 hints)
```

The error at `components.schemas.Grandparent.properties.child` seems to exist because of the presence of the `GreatGrandparent` schema, but it doesn't really make sense to me at all, because even if one fully "resolved" the schema, there wouldn't be an error at that location. Rather, there would be one at "`components.schemas.Grandparent.properties.child.properties.child`", I think.

In any case, it seems like Spectral isn't able to keep track of the "real" path of an error as it recurses through a resolved document.

**Expected behavior**

Any number of errors caused by the same problem in the same place should report the same path.

**Environment (remove any that are not applicable):**
 - Library version: `spectral --version` reports `6.2.0`
 - OS: macOS 12.1 Monterey

Problem in paths reported for rule violations involving schema references within oneOf
**Describe the bug**
The spectral-core code related to mapping "resolved" paths to "unresolved" paths appears to get tripped up when dealing with schema references within a oneOf list.   The testcase mentioned below contains a fairly simple OpenAPI document (api.yaml) along with a custom rule which verifies that each schema contains a valid description.
The custom rule's function logs the individual paths that are visited, along with the individual violations that
are found.
The API contains a handful of re-usable (named) schemas, with one (Soda) that lacks a description.
This Soda schema is referenced from within the Drink schema's oneOf list and is reachable in three
different ways:
1. the post requestBody
2. the post response
3. the get response

**To Reproduce**
A testcase that demonstrates this problem can be found here:
https://github.com/padamstx/spectral-dedup-issue
The README.md contains detailed instructions to reproduce, but a summary is:
- clone the repo mentioned above
- `spectral lint api.yaml`

**Expected behavior**
Based on the way in which the api.yaml file is defined, I'd expect a single error to be displayed
by spectral with the path `components.schemas.Soda`

**Screenshots**
sample debug output from the custom function:
```sh
$ spectral lint api.yaml
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
>> Violation at: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.name

/home/padams/work/test/spectral-dedup-issue/api.yaml
 62:11  error  schema-description  Schema must have a non-empty description.  components.schemas.Drink.oneOf[1]
 75:17  error  schema-description  Schema must have a non-empty description.  components.schemas.Drinks.properties.drinks.items

✖ 2 problems (2 errors, 0 warnings, 0 infos, 0 hints)
```

**Environment:**
This problem was found in spectral v6.2.1 running on Linux (Fedora 34)

**Additional context**
This problem was originally discovered during development of custom rules as part of the IBM openapi-validator project (https://github.com/IBM/openapi-validator), which is tightly integrated with spectral.


Pull request:
fix(core): fix 'resolved vs unresolved' json path mapping
This PR improves the DocumentInventory.findAssociatedItemForPath() function
by slightly changing the approach used to map a path from the resolved document
back to the corresponding path within the original unresolved document.
The result is a more accurate mapping due to the fact that the updated code
now more closely mimics the steps performed by the resolver when processing
the $refs, but just in reverse. 

Fixes #2059
Fixes #2043

**Checklist**

- [x] Tests added / updated
- [ ] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [x] No


Patch:
diff --git a/packages/core/src/documentInventory.ts b/packages/core/src/documentInventory.ts
index cec163be..f449e981 100644
--- a/packages/core/src/documentInventory.ts
+++ b/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);


Test patch:
diff --git a/packages/cli/src/services/__tests__/linter.test.ts b/packages/cli/src/services/__tests__/linter.test.ts
index edcda750..3b7324bb 100644
--- a/packages/cli/src/services/__tests__/linter.test.ts
+++ b/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'),
         },
       ]);
     });
diff --git a/packages/core/src/__tests__/__fixtures__/gh-658/URIError.yaml b/packages/core/src/__tests__/__fixtures__/gh-658/URIError.yaml
index 4d545fa3..062e6d78 100644
--- a/packages/core/src/__tests__/__fixtures__/gh-658/URIError.yaml
+++ b/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:
diff --git a/packages/core/src/__tests__/__fixtures__/gh-658/lib.yaml b/packages/core/src/__tests__/__fixtures__/gh-658/lib.yaml
index be265ae0..64b75aeb 100644
--- a/packages/core/src/__tests__/__fixtures__/gh-658/lib.yaml
+++ b/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'
+      
diff --git a/packages/core/src/__tests__/spectral.jest.test.ts b/packages/core/src/__tests__/spectral.jest.test.ts
index 38a8984a..40ebc4d4 100644
--- a/packages/core/src/__tests__/spectral.jest.test.ts
+++ b/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,
           },
         },
       }),