Replace the remaining occurences of instanceof Array with Array.isArray()

*Follow-up to PRs 8864 and 8813.* As explained in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray, `instanceof Array` can have inconsistent behavior. To ensure that only `Array.isArray` is used, an ESLint plugin/rule is added to enforce this.
2018-07-09 13:11:35 +02:00 · 2018-07-09 13:11:35 +02:00 · 61186698c3
commit 61186698c3
parent 200e3d6bd8
8 changed files with 93 additions and 13 deletions
--- a/.eslintrc
+++ b/.eslintrc
@ -5,7 +5,8 @@
  },

  "plugins": [
-    "mozilla"
+    "mozilla",
+    "unicorn",
  ],

  "env": {
@ -25,6 +26,7 @@
    // Plugins
    "mozilla/avoid-removeChild": "error",
    "mozilla/use-includes-instead-of-indexOf": "error",
+    "unicorn/no-array-instanceof": "error",

    // Possible errors
    "for-direction": "error",
--- a/package-lock.json
+++ b/package-lock.json
@ -1820,6 +1820,15 @@
        }
      }
    },
+    "clean-regexp": {
+      "version": "1.0.0",
+      "resolved": "https://registry.npmjs.org/clean-regexp/-/clean-regexp-1.0.0.tgz",
+      "integrity": "sha1-jffHquUf02h06PjQW5GAvBGj/tc=",
+      "dev": true,
+      "requires": {
+        "escape-string-regexp": "^1.0.5"
+      }
+    },
    "cli-cursor": {
      "version": "2.1.0",
      "resolved": "https://registry.npmjs.org/cli-cursor/-/cli-cursor-2.1.0.tgz",
@ -2752,6 +2761,16 @@
        }
      }
    },
+    "eslint-ast-utils": {
+      "version": "1.1.0",
+      "resolved": "https://registry.npmjs.org/eslint-ast-utils/-/eslint-ast-utils-1.1.0.tgz",
+      "integrity": "sha512-otzzTim2/1+lVrlH19EfQQJEhVJSu0zOb9ygb3iapN6UlyaDtyRq4b5U1FuW0v1lRa9Fp/GJyHkSwm6NqABgCA==",
+      "dev": true,
+      "requires": {
+        "lodash.get": "^4.4.2",
+        "lodash.zip": "^4.2.0"
+      }
+    },
    "eslint-plugin-mozilla": {
      "version": "0.14.0",
      "resolved": "https://registry.npmjs.org/eslint-plugin-mozilla/-/eslint-plugin-mozilla-0.14.0.tgz",
@ -2769,6 +2788,22 @@
      "integrity": "sha512-JnwpoH8Sv4QOjrTDutENBHzSnyYtspdjtglYtqUtAHe6f6LLKqykJle+UwFPg23GGwt5hI3amS9CRDezW8GAww==",
      "dev": true
    },
+    "eslint-plugin-unicorn": {
+      "version": "4.0.3",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-unicorn/-/eslint-plugin-unicorn-4.0.3.tgz",
+      "integrity": "sha512-F1JMyd42hx4qGhIaVdOSbDyhcxPgTy4BOzctTCkV+hqebPBUOAQn1f5AhMK2LTyiqCmKiTs8huAErbLBSWKoCQ==",
+      "dev": true,
+      "requires": {
+        "clean-regexp": "^1.0.0",
+        "eslint-ast-utils": "^1.0.0",
+        "import-modules": "^1.1.0",
+        "lodash.camelcase": "^4.1.1",
+        "lodash.kebabcase": "^4.0.1",
+        "lodash.snakecase": "^4.0.1",
+        "lodash.upperfirst": "^4.2.0",
+        "safe-regex": "^1.1.0"
+      }
+    },
    "eslint-scope": {
      "version": "3.7.1",
      "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-3.7.1.tgz",
@ -4615,6 +4650,12 @@
      "integrity": "sha512-Pgs951kaMm5GXP7MOvxERINe3gsaVjUWFm+UZPSq9xYriQAksyhg0csnS0KXSNRD5NmNdapXEpjxG49+AKh/ug==",
      "dev": true
    },
+    "import-modules": {
+      "version": "1.1.0",
+      "resolved": "https://registry.npmjs.org/import-modules/-/import-modules-1.1.0.tgz",
+      "integrity": "sha1-dI23nFzEK7lwHvq0JPiU5yYA6dw=",
+      "dev": true
+    },
    "imurmurhash": {
      "version": "0.1.4",
      "resolved": "https://registry.npmjs.org/imurmurhash/-/imurmurhash-0.1.4.tgz",
@ -5478,6 +5519,12 @@
      "integrity": "sha1-+6HEUkwZ7ppfgTa0YJ8BfPTe1pI=",
      "dev": true
    },
+    "lodash.camelcase": {
+      "version": "4.3.0",
+      "resolved": "https://registry.npmjs.org/lodash.camelcase/-/lodash.camelcase-4.3.0.tgz",
+      "integrity": "sha1-soqmKIorn8ZRA1x3EfZathkDMaY=",
+      "dev": true
+    },
    "lodash.clone": {
      "version": "4.5.0",
      "resolved": "https://registry.npmjs.org/lodash.clone/-/lodash.clone-4.5.0.tgz",
@ -5493,6 +5540,12 @@
        "lodash._root": "^3.0.0"
      }
    },
+    "lodash.get": {
+      "version": "4.4.2",
+      "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz",
+      "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=",
+      "dev": true
+    },
    "lodash.isarguments": {
      "version": "3.1.0",
      "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz",
@ -5505,6 +5558,12 @@
      "integrity": "sha1-eeTriMNqgSKvhvhEqpvNhRtfu1U=",
      "dev": true
    },
+    "lodash.kebabcase": {
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/lodash.kebabcase/-/lodash.kebabcase-4.1.1.tgz",
+      "integrity": "sha1-hImxyw0p/4gZXM7KRI/21swpXDY=",
+      "dev": true
+    },
    "lodash.keys": {
      "version": "3.1.2",
      "resolved": "https://registry.npmjs.org/lodash.keys/-/lodash.keys-3.1.2.tgz",
@ -5522,6 +5581,12 @@
      "integrity": "sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=",
      "dev": true
    },
+    "lodash.snakecase": {
+      "version": "4.1.1",
+      "resolved": "https://registry.npmjs.org/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz",
+      "integrity": "sha1-OdcUo1NXFHg3rv1ktdy7Fr7Nj40=",
+      "dev": true
+    },
    "lodash.some": {
      "version": "4.6.0",
      "resolved": "https://registry.npmjs.org/lodash.some/-/lodash.some-4.6.0.tgz",
@ -5555,6 +5620,18 @@
        "lodash.escape": "^3.0.0"
      }
    },
+    "lodash.upperfirst": {
+      "version": "4.3.1",
+      "resolved": "https://registry.npmjs.org/lodash.upperfirst/-/lodash.upperfirst-4.3.1.tgz",
+      "integrity": "sha1-E2Xt9DFIBIHvDRxolXpe2Z1J984=",
+      "dev": true
+    },
+    "lodash.zip": {
+      "version": "4.2.0",
+      "resolved": "https://registry.npmjs.org/lodash.zip/-/lodash.zip-4.2.0.tgz",
+      "integrity": "sha1-7GZi5IlkCO1KtsVCo5kLcswIACA=",
+      "dev": true
+    },
    "long": {
      "version": "3.2.0",
      "resolved": "https://registry.npmjs.org/long/-/long-3.2.0.tgz",
--- a/package.json
+++ b/package.json
@ -13,6 +13,7 @@
    "eslint": "^5.1.0",
    "eslint-plugin-mozilla": "^0.14.0",
    "eslint-plugin-no-unsanitized": "^3.0.2",
+    "eslint-plugin-unicorn": "^4.0.3",
    "fancy-log": "^1.3.2",
    "gulp": "^3.9.1",
    "gulp-postcss": "^7.0.1",
--- a/test/unit/api_spec.js
+++ b/test/unit/api_spec.js
@ -752,12 +752,12 @@ describe('api', function() {
      var promise = doc.getOutline();
      promise.then(function(outline) {
        // Two top level entries.
-        expect(outline instanceof Array).toEqual(true);
+        expect(Array.isArray(outline)).toEqual(true);
        expect(outline.length).toEqual(2);
        // Make sure some basic attributes are set.
        var outlineItem = outline[1];
        expect(outlineItem.title).toEqual('Chapter 1');
-        expect(outlineItem.dest instanceof Array).toEqual(true);
+        expect(Array.isArray(outlineItem.dest)).toEqual(true);
        expect(outlineItem.url).toEqual(null);
        expect(outlineItem.unsafeUrl).toBeUndefined();
        expect(outlineItem.newWindow).toBeUndefined();
@ -778,7 +778,7 @@ describe('api', function() {

      loadingTask.promise.then(function (pdfDocument) {
        pdfDocument.getOutline().then(function (outline) {
-          expect(outline instanceof Array).toEqual(true);
+          expect(Array.isArray(outline)).toEqual(true);
          expect(outline.length).toEqual(5);

          var outlineItemTwo = outline[2];
--- a/web/base_viewer.js
+++ b/web/base_viewer.js
@ -499,7 +499,7 @@ class BaseViewer {
    }
    if (!labels) {
      this._pageLabels = null;
-    } else if (!(labels instanceof Array &&
+    } else if (!(Array.isArray(labels) &&
                 this.pdfDocument.numPages === labels.length)) {
      this._pageLabels = null;
      console.error(`${this._name}.setPageLabels: Invalid page labels.`);
--- a/web/pdf_history.js
+++ b/web/pdf_history.js
@ -164,7 +164,7 @@ class PDFHistory {
      return;
    }
    if ((namedDest && typeof namedDest !== 'string') ||
-        !(explicitDest instanceof Array) ||
+        !Array.isArray(explicitDest) ||
        !(Number.isInteger(pageNumber) &&
          pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) {
      console.error('PDFHistory.push: Invalid parameters.');
@ -567,7 +567,7 @@ function isDestArraysEqual(firstDest, secondDest) {
    if (typeof first !== typeof second) {
      return false;
    }
-    if (first instanceof Array || second instanceof Array) {
+    if (Array.isArray(first) || Array.isArray(second)) {
      return false;
    }
    if (first !== null && typeof first === 'object' && second !== null) {
@ -584,7 +584,7 @@ function isDestArraysEqual(firstDest, secondDest) {
    return first === second || (Number.isNaN(first) && Number.isNaN(second));
  }

-  if (!(firstDest instanceof Array && secondDest instanceof Array)) {
+  if (!(Array.isArray(firstDest) && Array.isArray(secondDest))) {
    return false;
  }
  if (firstDest.length !== secondDest.length) {
--- a/web/pdf_link_service.js
+++ b/web/pdf_link_service.js
@ -162,7 +162,7 @@ class PDFLinkService {
        explicitDest: dest,
      });
    }).then((data) => {
-      if (!(data.explicitDest instanceof Array)) {
+      if (!Array.isArray(data.explicitDest)) {
        console.error(`PDFLinkService.navigateTo: "${data.explicitDest}" is` +
                      ` not a valid destination array, for dest="${dest}".`);
        return;
@ -179,7 +179,7 @@ class PDFLinkService {
    if (typeof dest === 'string') {
      return this.getAnchorUrl('#' + escape(dest));
    }
-    if (dest instanceof Array) {
+    if (Array.isArray(dest)) {
      let str = JSON.stringify(dest);
      return this.getAnchorUrl('#' + escape(str));
    }
@ -273,7 +273,7 @@ class PDFLinkService {
      try {
        dest = JSON.parse(dest);

-        if (!(dest instanceof Array)) {
+        if (!Array.isArray(dest)) {
          // Avoid incorrectly rejecting a valid named destination, such as
          // e.g. "4.3" or "true", because `JSON.parse` converted its type.
          dest = dest.toString();
@ -368,7 +368,7 @@ class PDFLinkService {
 }

 function isValidExplicitDestination(dest) {
-  if (!(dest instanceof Array)) {
+  if (!Array.isArray(dest)) {
    return false;
  }
  let destLength = dest.length, allowNull = true;
--- a/web/pdf_thumbnail_viewer.js
+++ b/web/pdf_thumbnail_viewer.js
@ -208,7 +208,7 @@ class PDFThumbnailViewer {
    }
    if (!labels) {
      this._pageLabels = null;
-    } else if (!(labels instanceof Array &&
+    } else if (!(Array.isArray(labels) &&
                 this.pdfDocument.numPages === labels.length)) {
      this._pageLabels = null;
      console.error('PDFThumbnailViewer_setPageLabels: Invalid page labels.');