Merge pull request #171 from stoplightio/fix/nullish-resolved

Resolved might be null

Author: Phil Sturgeon

Diff for: lib/index.d.ts

@@ -409,6 +409,8 @@ declare namespace $RefParser {
   export type JSONParserErrorType = "EUNKNOWN" | "EPARSER" | "EUNMATCHEDPARSER" | "ERESOLVER" | "EUNMATCHEDRESOLVER" | "EMISSINGPOINTER" | "EINVALIDPOINTER";
 
   export class JSONParserError extends Error {
+    public constructor(message: string, source: string);
+
     public readonly name: string;
     public readonly message: string;
     public readonly source: string;
@@ -439,28 +441,40 @@ declare namespace $RefParser {
   }
 
   export class ParserError extends JSONParserError {
+    public constructor(message: string, source: string);
+
     public readonly name = "ParserError";
     public readonly code = "EPARSER";
   }
   export class UnmatchedParserError extends JSONParserError {
+    public constructor(source: string);
+
     public readonly name = "UnmatchedParserError";
-    public readonly code ="EUNMATCHEDPARSER";
+    public readonly code = "EUNMATCHEDPARSER";
   }
   export class ResolverError extends JSONParserError {
+    public constructor(ex: Error | NodeJS.ErrnoException, source: string);
+
     public readonly name = "ResolverError";
-    public readonly code ="ERESOLVER";
+    public readonly code = "ERESOLVER";
     public readonly ioErrorCode?: string;
   }
   export class UnmatchedResolverError extends JSONParserError {
+    public constructor(source: string);
+
     public readonly name = "UnmatchedResolverError";
-    public readonly code ="EUNMATCHEDRESOLVER";
+    public readonly code = "EUNMATCHEDRESOLVER";
   }
   export class MissingPointerError extends JSONParserError {
+    public constructor(token: string | number, source: string);
+
     public readonly name = "MissingPointerError";
-    public readonly code ="EMISSINGPOINTER";
+    public readonly code = "EMISSINGPOINTER";
   }
   export class InvalidPointerError extends JSONParserError {
+    public constructor(pointer: string, source: string);
+
     public readonly name = "InvalidPointerError";
-    public readonly code ="EINVALIDPOINTER";
+    public readonly code = "EINVALIDPOINTER";
   }
 }

Diff for: lib/pointer.js

@@ -235,6 +235,10 @@ function resolveIf$Ref (pointer, options) {
     }
     else {
       let resolved = pointer.$ref.$refs._resolve($refPath, pointer.path, options);
+      if (resolved === null) {
+        return false;
+      }
+
       pointer.indirections += resolved.indirections + 1;
 
       if ($Ref.isExtended$Ref(pointer.value)) {

Diff for: lib/ref.js

@@ -9,7 +9,7 @@ const { safePointerToPath, stripHash, getHash } = require("./util/url");
 /**
  * This class represents a single JSON reference and its resolved value.
  *
- * @constructor
+ * @class
  */
 function $Ref () {
   /**
@@ -27,24 +27,28 @@ function $Ref () {
   /**
    * The resolved value of the JSON reference.
    * Can be any JSON type, not just objects. Unknown file types are represented as Buffers (byte arrays).
+   *
    * @type {?*}
    */
   this.value = undefined;
 
   /**
    * The {@link $Refs} object that contains this {@link $Ref} object.
+   *
    * @type {$Refs}
    */
   this.$refs = undefined;
 
   /**
    * Indicates the type of {@link $Ref#path} (e.g. "file", "http", etc.)
+   *
    * @type {?string}
    */
   this.pathType = undefined;
 
   /**
    * List of all errors. Undefined if no errors.
+   *
    * @type {Array<JSONParserError | ResolverError | ParserError | MissingPointerError>}
    */
   this.errors = undefined;
@@ -53,25 +57,30 @@ function $Ref () {
 /**
  * Pushes an error to errors array.
  *
- * @param {Array<JSONParserError | JSONParserErrorGroup>} error - The error to be pushed
+ * @param {Array<JSONParserError | JSONParserErrorGroup>} err - The error to be pushed
  * @returns {void}
  */
 $Ref.prototype.addError = function (err) {
   if (this.errors === undefined) {
     this.errors = [];
   }
 
+  const existingErrors = this.errors.map(({ footprint }) => footprint);
+
   // the path has been almost certainly set at this point,
-  // but just in case something went wrong, let's inject path if necessary
+  // but just in case something went wrong, normalizeError injects path if necessary
+  // moreover, certain errors might point at the same spot, so filter them out to reduce noise
   if (Array.isArray(err.errors)) {
-    this.errors.push(...err.errors.map(normalizeError));
+    this.errors.push(...err.errors
+      .map(normalizeError)
+      .filter(({ footprint }) => !existingErrors.includes(footprint)),
+    );
   }
-  else {
+  else if (!existingErrors.includes(err.footprint)) {
     this.errors.push(normalizeError(err));
   }
 };
 
-
 /**
  * Determines whether the given JSON reference exists within this {@link $Ref#value}.
  *
@@ -106,8 +115,8 @@ $Ref.prototype.get = function (path, options) {
  * @param {string} path - The full path being resolved, optionally with a JSON pointer in the hash
  * @param {$RefParserOptions} options
  * @param {string} friendlyPath - The original user-specified path (used for error messages)
-*  @param {string} pathFromRoot - The path of `obj` from the schema root
- * @returns {Pointer}
+ *  @param {string} pathFromRoot - The path of `obj` from the schema root
+ * @returns {Pointer | null}
  */
 $Ref.prototype.resolve = function (path, options, friendlyPath, pathFromRoot) {
   let pointer = new Pointer(this, path, friendlyPath);

Diff for: lib/util/errors.js

@@ -15,6 +15,10 @@ const JSONParserError = exports.JSONParserError = class JSONParserError extends
 
     Ono.extend(this);
   }
+
+  get footprint () {
+    return `${this.path}+${this.source}+${this.code}+${this.message}`;
+  }
 };
 
 setErrorName(JSONParserError);

Diff for: test/specs/missing-pointers/external-from-internal.yaml

@@ -0,0 +1,5 @@
+internal1:
+  $ref: '#/internal2'
+internal2:
+  $ref: '#/external'
+

Diff for: test/specs/missing-pointers/missing-pointers.spec.js

@@ -7,6 +7,7 @@ const { expect } = chai;
 const $RefParser = require("../../../lib");
 const { JSONParserErrorGroup, MissingPointerError } = require("../../../lib/util/errors");
 const helper = require("../../utils/helper");
+const path = require("../../utils/path");
 
 describe("Schema with missing pointers", () => {
   it("should throw an error for missing pointer", async () => {
@@ -20,25 +21,65 @@ describe("Schema with missing pointers", () => {
     }
   });
 
-  it("should throw a grouped error for missing pointer if continueOnError is true", async () => {
-    const parser = new $RefParser();
+  it("should throw an error for missing pointer in external file", async () => {
     try {
-      await parser.dereference({ foo: { $ref: "#/baz" }}, { continueOnError: true });
+      await $RefParser.dereference({ foo: { $ref: path.abs("specs/missing-pointers/external-from-internal.yaml") }});
       helper.shouldNotGetCalled();
     }
     catch (err) {
-      expect(err).to.be.instanceof(JSONParserErrorGroup);
-      expect(err.files).to.equal(parser);
-      expect(err.files.$refs._root$Ref.value).to.deep.equal({ foo: null });
-      expect(err.message).to.have.string("1 error occurred while reading '");
-      expect(err.errors).to.containSubset([
-        {
-          name: MissingPointerError.name,
-          message: "Token \"baz\" does not exist.",
-          path: ["foo"],
-          source: message => message.endsWith("/test/") || message.startsWith("http://localhost"),
-        }
-      ]);
+      expect(err).to.be.an.instanceOf(MissingPointerError);
+      expect(err.message).to.contain("Token \"external\" does not exist.");
     }
   });
+
+  context("when continueOnError is true", () => {
+    it("should throw a grouped error for missing pointer", async () => {
+      const parser = new $RefParser();
+      try {
+        await parser.dereference({ foo: { $ref: "#/baz" }}, { continueOnError: true });
+        helper.shouldNotGetCalled();
+      }
+      catch (err) {
+        expect(err).to.be.instanceof(JSONParserErrorGroup);
+        expect(err.files).to.equal(parser);
+        expect(err.files.$refs._root$Ref.value).to.deep.equal({ foo: null });
+        expect(err.message).to.have.string("1 error occurred while reading '");
+        expect(err.errors).to.containSubset([
+          {
+            name: MissingPointerError.name,
+            message: "Token \"baz\" does not exist.",
+            path: ["foo"],
+            source: message => message.endsWith("/test/") || message.startsWith("http://localhost"),
+          }
+        ]);
+      }
+    });
+
+    it("should throw an error for missing pointer in external file", async () => {
+      const parser = new $RefParser();
+      try {
+        await parser.dereference({ foo: { $ref: path.abs("specs/missing-pointers/external-from-internal.yaml") }}, { continueOnError: true });
+        helper.shouldNotGetCalled();
+      }
+      catch (err) {
+        expect(err).to.be.instanceof(JSONParserErrorGroup);
+        expect(err.files).to.equal(parser);
+        expect(err.files.$refs._root$Ref.value).to.deep.equal({
+          foo: {
+            internal1: null,
+            internal2: null,
+          }
+        });
+        expect(err.message).to.have.string("1 error occurred while reading '");
+        expect(err.errors).to.containSubset([
+          {
+            name: MissingPointerError.name,
+            message: "Token \"external\" does not exist.",
+            path: ["internal2"],
+            source: message => message.endsWith("missing-pointers/external-from-internal.yaml") || message.startsWith("http://localhost"),
+          }
+        ]);
+      }
+    });
+  });
 });