Repository: stoplightio/spectral
Pull request #615 (2019-09-30 22:08:21)

fix(resolving): detect the source of local references properly
Fixes https://github.com/stoplightio/spectral/issues/572#issuecomment-536019623 and a potential issue (see https://github.com/stoplightio/spectral/commit/28f8b65ce3504abed49f38727b921c01ab0cf496)

**Checklist**

- [x] Tests added / updated
- [x] Docs added / updated

**Does this PR introduce a breaking change?**

- [ ] Yes
- [x] No




P0lip: Hm, some weird issue with bundling :thinking: 
On it.

Patch:
diff --git a/src/linter.ts b/src/linter.ts
index 77edc123..9fe2281e 100644
--- a/src/linter.ts
+++ b/src/linter.ts
@@ -96,10 +96,11 @@ export const lintNode = (
     results = results.concat(
       targetResults.map<IRuleResult>(result => {
         const escapedJsonPath = (result.path || targetPath).map(segment => decodePointerFragment(String(segment)));
-        const path = getRealJsonPath(
+        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);
 
         return {
@@ -199,7 +200,8 @@ function keyAndOptionalPattern(key: string | number, pattern: string, value: any
   };
 }
 
-function getRealJsonPath(data: unknown, path: JsonPath) {
+// todo: revisit -> https://github.com/stoplightio/spectral/issues/608
+function getClosestJsonPath(data: unknown, path: JsonPath) {
   if (data === null || typeof data !== 'object') return [];
 
   while (path.length > 0 && !has(data, path)) {
diff --git a/src/resolved.ts b/src/resolved.ts
index e3106a83..202f5b3e 100644
--- a/src/resolved.ts
+++ b/src/resolved.ts
@@ -1,8 +1,10 @@
+import { decodePointerFragment, pointerToPath } from '@stoplight/json';
 import { IResolveError } from '@stoplight/json-ref-resolver/types';
 import { Dictionary, ILocation, IRange, JsonPath, Segment } from '@stoplight/types';
-import { get, has } from 'lodash';
+import { get } from 'lodash';
 import { IParseMap, REF_METADATA, ResolveResult } from './spectral';
 import { IParsedResult } from './types';
+import { hasRef, isObject } from './utils';
 
 const getDefaultRange = (): IRange => ({
   start: {
@@ -31,6 +33,28 @@ export class Resolved {
     this.errors = resolveResult.errors;
   }
 
+  public doesBelongToDoc(path: JsonPath): boolean {
+    if (path.length === 0) {
+      // todo: each rule and their function should be context-aware, meaning they should aware of the fact they operate on resolved content
+      // let's assume the error was reported correctly by any custom rule /shrug
+      return true;
+    }
+
+    let piece = this.unresolved;
+
+    for (let i = 0; i < path.length; i++) {
+      if (!isObject(piece)) return false;
+
+      if (path[i] in piece) {
+        piece = piece[path[i]];
+      } else if (hasRef(piece)) {
+        return this.doesBelongToDoc([...pointerToPath(piece.$ref), ...path.slice(i)]);
+      }
+    }
+
+    return true;
+  }
+
   public getParsedForJsonPath(path: JsonPath) {
     let target: object = this.parsedMap.refs;
     const newPath = [...path];
@@ -48,12 +72,12 @@ export class Resolved {
 
     if (target && target[REF_METADATA]) {
       return {
-        path: [...get(target, [REF_METADATA, 'root'], []), ...newPath],
+        path: [...get(target, [REF_METADATA, 'root'], []).map(decodePointerFragment), ...newPath],
         doc: get(this.parsedMap.parsed, get(target, [REF_METADATA, 'ref']), this.spec),
       };
     }
 
-    if (path.length > 0 && !has(this.spec.parsed.data, path)) {
+    if (!this.doesBelongToDoc(path)) {
       return null;
     }
 
diff --git a/src/rulesets/oas/functions/refSiblings.ts b/src/rulesets/oas/functions/refSiblings.ts
index 7527629f..1b7e991a 100644
--- a/src/rulesets/oas/functions/refSiblings.ts
+++ b/src/rulesets/oas/functions/refSiblings.ts
@@ -1,14 +1,14 @@
 import { JsonPath } from '@stoplight/types';
 import { IFunction, IFunctionResult } from '../../../types';
+import { hasRef } from '../../../utils/hasRef';
 
 // function is needed because `$..$ref` or `$..[?(@.$ref)]` are not parsed correctly
 // and therefore lead to infinite recursion due to the dollar sign ('$' in '$ref')
 function* siblingIterator(obj: object, path: JsonPath): IterableIterator<JsonPath> {
-  const hasRef = '$ref' in obj;
   for (const key in obj) {
     if (!Object.hasOwnProperty.call(obj, key)) continue;
     const scopedPath = [...path, key];
-    if (hasRef && key !== '$ref') {
+    if (hasRef(obj) && key !== '$ref') {
       yield scopedPath;
     }
 
diff --git a/src/utils/hasRef.ts b/src/utils/hasRef.ts
new file mode 100644
index 00000000..3d42e5f2
--- /dev/null
+++ b/src/utils/hasRef.ts
@@ -0,0 +1,2 @@
+export const hasRef = (obj: object): obj is object & { $ref: string } =>
+  '$ref' in obj && typeof (obj as Partial<{ $ref: unknown }>).$ref === 'string';
diff --git a/src/utils/index.ts b/src/utils/index.ts
index e235cac3..85f1d7b0 100644
--- a/src/utils/index.ts
+++ b/src/utils/index.ts
@@ -1,2 +1,3 @@
+export * from './hasRef';
 export * from './hasIntersectingElement';
 export * from './isObject';


Test patch:
diff --git a/src/__tests__/spectral.test.ts b/src/__tests__/spectral.test.ts
index e7958d84..1b29053d 100644
--- a/src/__tests__/spectral.test.ts
+++ b/src/__tests__/spectral.test.ts
@@ -201,11 +201,71 @@ describe('spectral', () => {
                 line: 0,
               },
             },
-            severity: 0,
+            severity: DiagnosticSeverity.Error,
             source: void 0,
           },
         ]);
       });
+
+      test('should recognize the source of local $refs', () => {
+        const s = new Spectral();
+        const source = 'foo.yaml';
+
+        const parsedResult: IParsedResult = {
+          getLocationForJsonPath,
+          source,
+          parsed: parseWithPointers(
+            JSON.stringify(
+              {
+                paths: {
+                  '/agreements': {
+                    get: {
+                      description: 'Get some Agreements',
+                      responses: {
+                        '200': {
+                          $ref: '#/responses/GetAgreementsOk',
+                        },
+                        default: {},
+                      },
+                      summary: 'List agreements',
+                      tags: ['agreements', 'pagination'],
+                    },
+                  },
+                },
+                responses: {
+                  GetAgreementsOk: {
+                    description: 'Successful operation',
+                    headers: {},
+                  },
+                },
+              },
+              null,
+              2,
+            ),
+          ),
+        };
+
+        s.setRules({
+          'pagination-responses-have-x-next-token': {
+            description: 'All collection endpoints have the X-Next-Token parameter in responses',
+            given: "$.paths..get.responses['200'].headers",
+            severity: 'error',
+            recommended: true,
+            then: { field: 'X-Next-Token', function: 'truthy' },
+          },
+        });
+
+        return expect(s.run(parsedResult)).resolves.toEqual([
+          {
+            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'],
+            range: expect.any(Object),
+            severity: DiagnosticSeverity.Error,
+            source,
+          },
+        ]);
+      });
     });
   });
 
diff --git a/src/cli/services/__tests__/__fixtures__/draft-nested-ref.oas2.json b/src/cli/services/__tests__/__fixtures__/draft-nested-ref.oas2.json
index 441e923e..da6d298d 100644
--- a/src/cli/services/__tests__/__fixtures__/draft-nested-ref.oas2.json
+++ b/src/cli/services/__tests__/__fixtures__/draft-nested-ref.oas2.json
@@ -3,5 +3,10 @@
   "host": "d",
   "info": {
     "$ref": "./refs/info-ref.json"
+  },
+  "paths": {
+    "/test": {
+      "$ref": "./refs/paths.json#/paths/~1test"
+    }
   }
 }
diff --git a/src/cli/services/__tests__/__fixtures__/refs/paths.json b/src/cli/services/__tests__/__fixtures__/refs/paths.json
new file mode 100644
index 00000000..fceaaf76
--- /dev/null
+++ b/src/cli/services/__tests__/__fixtures__/refs/paths.json
@@ -0,0 +1,9 @@
+{
+  "paths": {
+    "/test": {
+      "get": {
+        "response": "bar"
+      }
+    }
+  }
+}
diff --git a/src/cli/services/__tests__/linter.test.ts b/src/cli/services/__tests__/linter.test.ts
index 1b518bb9..b7602bd1 100644
--- a/src/cli/services/__tests__/linter.test.ts
+++ b/src/cli/services/__tests__/linter.test.ts
@@ -380,6 +380,54 @@ describe('Linter service', () => {
           },
           source: expect.stringContaining('__tests__/__fixtures__/refs/contact.json'),
         }),
+        expect.objectContaining({
+          code: 'operation-description',
+          message: 'Operation `description` must be present and non-empty string.',
+          path: ['paths', '/test', 'get'],
+          range: {
+            end: {
+              character: 7,
+              line: 5,
+            },
+            start: {
+              character: 13,
+              line: 3,
+            },
+          },
+          source: expect.stringContaining('__tests__/__fixtures__/refs/paths.json'),
+        }),
+        expect.objectContaining({
+          code: 'operation-operationId',
+          message: 'Operation should have an `operationId`.',
+          path: ['paths', '/test', 'get'],
+          range: {
+            end: {
+              character: 7,
+              line: 5,
+            },
+            start: {
+              character: 13,
+              line: 3,
+            },
+          },
+          source: expect.stringContaining('__tests__/__fixtures__/refs/paths.json'),
+        }),
+        expect.objectContaining({
+          code: 'operation-tags',
+          message: 'Operation should have non-empty `tags` array.',
+          path: ['paths', '/test', 'get'],
+          range: {
+            end: {
+              character: 7,
+              line: 5,
+            },
+            start: {
+              character: 13,
+              line: 3,
+            },
+          },
+          source: expect.stringContaining('__tests__/__fixtures__/refs/paths.json'),
+        }),
       ]);
     });
   });