Bug 1385820 - Enable the ESLint no-new-wrappers rule across mozilla-central; r?standard8, markh draft
authordagasatvik <dagasatvik10@gmail.com>
Mon, 07 Aug 2017 23:57:51 +0530
changeset 642153 676d75cdd710179ef1099a20a2aafc94e0f7360e
parent 618977 51ffb9283f0c7c00e08eb8c39b33fbee218c370d
child 724915 5c573c43ee1dd8a072e7a865fbe9c642c7cfe1f9
push id72660
push userbmo:dagasatvik10@gmail.com
push dateMon, 07 Aug 2017 18:29:45 +0000
reviewersstandard8, markh
bugs1385820
milestone56.0a1
Bug 1385820 - Enable the ESLint no-new-wrappers rule across mozilla-central; r?standard8, markh MozReview-Commit-ID: FvJO3ibeiwg
accessible/tests/browser/.eslintrc.js
browser/base/content/browser.js
browser/extensions/formautofill/.eslintrc.js
security/.eslintrc.js
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/resource.js
testing/marionette/components/marionette.js
toolkit/components/contentprefs/tests/unit/test_stringGroups.js
toolkit/components/ctypes/tests/unit/test_jsctypes.js
toolkit/components/extensions/.eslintrc.js
toolkit/components/jsdownloads/test/unit/test_Downloads.js
toolkit/components/payments/.eslintrc.js
toolkit/components/printing/content/printPreviewBindings.xml
toolkit/components/satchel/.eslintrc.js
toolkit/components/telemetry/TelemetryEnvironment.jsm
toolkit/components/telemetry/TelemetryStorage.jsm
toolkit/content/aboutTelemetry.js
toolkit/modules/PropertyListUtils.jsm
toolkit/modules/tests/xpcshell/test_Log.js
tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
--- a/accessible/tests/browser/.eslintrc.js
+++ b/accessible/tests/browser/.eslintrc.js
@@ -100,17 +100,16 @@ module.exports = {
     "no-iterator": "off",
     "no-label-var": "off",
     "no-lone-blocks": "off",
     "no-loop-func": "off",
     "no-negated-in-lhs": "off",
     "no-new": "off",
     "no-new-func": "off",
     "no-new-object": "off",
-    "no-new-wrappers": "off",
     "no-obj-calls": "off",
     "no-octal-escape": "off",
     "no-undef-init": "error",
     "object-curly-spacing": "off",
     "no-unused-expressions": "off",
     "no-void": "off",
     "no-wrap-func": "off",
     "operator-assignment": "off",
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4190,17 +4190,16 @@ function OpenBrowserWindow(options) {
     }
   }
 
   // Make sure to remove the 'document-shown' observer in case the window
   // is being closed right after it was opened to avoid leaking.
   Services.obs.addObserver(newDocumentShown, "document-shown");
   Services.obs.addObserver(windowClosed, "domwindowclosed");
 
-  var charsetArg = new String();
   var handler = Components.classes["@mozilla.org/browser/clh;1"]
                           .getService(Components.interfaces.nsIBrowserHandler);
   var defaultArgs = handler.defaultArgs;
   var wintype = document.documentElement.getAttribute("windowtype");
 
   var extraFeatures = "";
   if (options && options.private) {
     extraFeatures = ",private";
@@ -4226,17 +4225,17 @@ function OpenBrowserWindow(options) {
   }
 
   // if and only if the current window is a browser window and it has a document with a character
   // set, then extract the current charset menu setting from the current document and use it to
   // initialize the new browser window...
   var win;
   if (window && (wintype == "navigator:browser") && window.content && window.content.document) {
     var DocCharset = window.content.document.characterSet;
-    charsetArg = "charset=" + DocCharset;
+    let charsetArg = "charset=" + DocCharset;
 
     // we should "inherit" the charset menu setting in a new window
     win = window.openDialog("chrome://browser/content/", "_blank", "chrome,all,dialog=no" + extraFeatures, defaultArgs, charsetArg);
   } else {
     // forget about the charset information.
     win = window.openDialog("chrome://browser/content/", "_blank", "chrome,all,dialog=no" + extraFeatures, defaultArgs);
   }
 
--- a/browser/extensions/formautofill/.eslintrc.js
+++ b/browser/extensions/formautofill/.eslintrc.js
@@ -119,13 +119,10 @@ module.exports = {
     // Require "use strict" to be defined globally in the script.
     "strict": ["error", "global"],
 
     // Disallow Yoda conditions (where literal value comes first).
     "yoda": "error",
 
     // Disallow function or variable declarations in nested blocks
     "no-inner-declarations": "error",
-
-    // Disallow creating new instances of String, Number, and Boolean
-    "no-new-wrappers": "error",
   },
 };
--- a/security/.eslintrc.js
+++ b/security/.eslintrc.js
@@ -47,20 +47,16 @@ module.exports = {
 
     // Disallow case statement fallthrough without explicit `// falls through`
     // annotation.
     "no-fallthrough": "error",
 
     // No reassigning native JS objects or read only globals.
     "no-global-assign": "error",
 
-    // Disallow primitive wrapper instances like `new Boolean(false)`, which
-    // seem like they should act like primitives but don't.
-    "no-new-wrappers": "error",
-
     // Disallow use of assignment in return statement.
     "no-return-assign": ["error", "always"],
 
     // Disallow use of the comma operator.
     "no-sequences": "error",
 
     // Disallow template literal placeholder syntax in regular strings.
     "no-template-curly-in-string": "error",
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -1004,16 +1004,19 @@ SyncEngine.prototype = {
       engineData.syncID = this.syncID;
 
       // Put the new data back into meta/global and mark for upload
       engines[this.name] = engineData;
       metaGlobal.payload.engines = engines;
       metaGlobal.changed = true;
     } else if (engineData.version > this.version) {
       // Don't sync this engine if the server has newer data
+
+      // Changes below need to be processed in bug 1295510 that's why eslint is ignored
+      // eslint-disable-next-line no-new-wrappers
       let error = new String("New data: " + [engineData.version, this.version]);
       error.failureCode = VERSION_OUT_OF_DATE;
       throw error;
     } else if (engineData.syncID != this.syncID) {
       // Changes to syncID mean we'll need to upload everything
       this._log.debug("Engine syncIDs: " + [engineData.syncID, this.syncID]);
       this.syncID = engineData.syncID;
       await this._resetClient();
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -352,16 +352,19 @@ BookmarksEngine.prototype = {
           continue;
       }
 
       let parentName = parent.title || "";
       if (guidMap[parentName] == null)
         guidMap[parentName] = {};
 
       // If the entry already exists, remember that there are explicit dupes.
+
+      // Changes below need to be processed in bug 1295510 that's why eslint is ignored
+      // eslint-disable-next-line no-new-wrappers
       let entry = new String(guid);
       entry.hasDupe = guidMap[parentName][key] != null;
 
       // Remember this item's GUID for its parent-name/key pair.
       guidMap[parentName][key] = entry;
       this._log.trace("Mapped: " + [parentName, key, entry, entry.hasDupe]);
     }
 
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -296,16 +296,18 @@ AsyncResource.prototype = {
         this._log.warn("The response body's length of: " + data.length +
                        " doesn't match the header's content-length of: " +
                        contentLength + ".");
       }
     } catch (ex) {
       this._log.debug("Caught exception visiting headers in _onComplete", ex);
     }
 
+    // Changes below need to be processed in bug 1295510 that's why eslint is ignored
+    // eslint-disable-next-line no-new-wrappers
     let ret     = new String(data);
     ret.url     = channel.URI.spec;
     ret.status  = status;
     ret.success = success;
     ret.headers = headers;
 
     if (!success) {
       this._log.warn(`${action} request to ${ret.url} failed with status ${status}`);
--- a/testing/marionette/components/marionette.js
+++ b/testing/marionette/components/marionette.js
@@ -34,17 +34,17 @@ const LOG_LEVELS = new class extends Map
       ["info", Log.Level.Info],
       ["config", Log.Level.Config],
       ["debug", Log.Level.Debug],
       ["trace", Log.Level.Trace],
     ]);
   }
 
   get(level) {
-    let s = new String(level).toLowerCase();
+    let s = String(level).toLowerCase();
     if (!this.has(s)) {
       return DEFAULT_LOG_LEVEL;
     }
     return super.get(s);
   }
 };
 
 // Complements -marionette flag for starting the Marionette server.
--- a/toolkit/components/contentprefs/tests/unit/test_stringGroups.js
+++ b/toolkit/components/contentprefs/tests/unit/test_stringGroups.js
@@ -10,16 +10,19 @@ function run_test() {
   var statement = cps.DBConnection.createStatement("PRAGMA synchronous");
   statement.executeStep();
   do_check_eq(0, statement.getInt32(0));
 
   // These are the different types of aGroup arguments we'll test.
   var anObject = {"foo": "bar"};                               // a simple object
   var uri = ContentPrefTest.getURI("http://www.example.com/"); // nsIURI
   var stringURI = "www.example.com";                           // typeof = "string"
+
+  // Test wants to check for a String object
+  // eslint-disable-next-line no-new-wrappers
   var stringObjectURI = new String("www.example.com");         // typeof = "object"
 
   // First check that all the methods work or don't work.
   function simple_test_methods(aGroup, shouldThrow) {
     var prefName = "test.pref.0";
     var prefValue = Math.floor(Math.random() * 100);
 
     if (shouldThrow) {
--- a/toolkit/components/ctypes/tests/unit/test_jsctypes.js
+++ b/toolkit/components/ctypes/tests/unit/test_jsctypes.js
@@ -1,13 +1,15 @@
 /* -*-  indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/* eslint-disable no-new-wrappers */
+
 try {
   // We might be running without privileges, in which case it's up to the
   // harness to give us the 'ctypes' object.
   Components.utils.import("resource://gre/modules/ctypes.jsm");
 } catch (e) {
 }
 
 const CTYPES_TEST_LIB = ctypes.libraryName("jsctypes-test");
--- a/toolkit/components/extensions/.eslintrc.js
+++ b/toolkit/components/extensions/.eslintrc.js
@@ -299,13 +299,10 @@ module.exports = {
     // Disallow Yoda conditions (where literal value comes first).
     "yoda": "error",
 
     // Disallow function or variable declarations in nested blocks
     "no-inner-declarations": "error",
 
     // Disallow labels that share a name with a variable
     "no-label-var": "error",
-
-    // Disallow creating new instances of String, Number, and Boolean
-    "no-new-wrappers": "error",
   },
 };
--- a/toolkit/components/jsdownloads/test/unit/test_Downloads.js
+++ b/toolkit/components/jsdownloads/test/unit/test_Downloads.js
@@ -106,18 +106,18 @@ add_task(async function test_fetch_objec
  * Tests "fetch" with string arguments.
  */
 add_task(async function test_fetch_string_arguments() {
   let targetPath = getTempFile(TEST_TARGET_FILE_NAME).path;
   await Downloads.fetch(httpUrl("source.txt"), targetPath);
   await promiseVerifyContents(targetPath, TEST_DATA_SHORT);
 
   targetPath = getTempFile(TEST_TARGET_FILE_NAME).path;
-  await Downloads.fetch(new String(httpUrl("source.txt")),
-                        new String(targetPath));
+  await Downloads.fetch(httpUrl("source.txt"),
+                        targetPath);
   await promiseVerifyContents(targetPath, TEST_DATA_SHORT);
 });
 
 /**
  * Tests that the getList function returns the same list when called multiple
  * times with the same argument, but returns different lists when called with
  * different arguments.  More detailed tests are implemented separately for the
  * DownloadList module.
--- a/toolkit/components/payments/.eslintrc.js
+++ b/toolkit/components/payments/.eslintrc.js
@@ -36,17 +36,16 @@ module.exports = {
     "new-parens": "error",
     "no-caller": "error",
     "no-console": "error",
     "no-fallthrough": "error",
     "no-multi-str": "error",
     "no-multiple-empty-lines": ["error", {
       max: 2,
     }],
-    "no-new-wrappers": "error",
     "no-proto": "error",
     "no-throw-literal": "error",
     "no-unused-expressions": "error",
     "no-unused-vars": ["error", {
       args: "none",
       varsIgnorePattern: "^(Cc|Ci|Cr|Cu|EXPORTED_SYMBOLS)$",
     }],
     "no-use-before-define": ["error", {
--- a/toolkit/components/printing/content/printPreviewBindings.xml
+++ b/toolkit/components/printing/content/printPreviewBindings.xml
@@ -306,17 +306,17 @@
       </method>
 
       <method name="setScaleCombobox">
         <parameter name="aValue"/>
         <body>
         <![CDATA[
           var scaleValues = [0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, 1.25, 1.5, 1.75, 2];
 
-          aValue = new Number(aValue);
+          aValue = Number(aValue);
 
           for (var i = 0; i < scaleValues.length; i++) {
             if (aValue == scaleValues[i]) {
               this.mScaleCombobox.selectedIndex = i;
               return;
             }
           }
           this.mScaleCombobox.value = "Custom";
--- a/toolkit/components/satchel/.eslintrc.js
+++ b/toolkit/components/satchel/.eslintrc.js
@@ -36,17 +36,16 @@ module.exports = {
     "new-parens": "error",
     "no-caller": "error",
     "no-console": "error",
     "no-fallthrough": "error",
     "no-multi-str": "error",
     "no-multiple-empty-lines": ["error", {
       max: 2,
     }],
-    "no-new-wrappers": "error",
     "no-proto": "error",
     "no-throw-literal": "error",
     "no-unused-expressions": "error",
     "no-unused-vars": ["error", {
       args: "none",
       varsIgnorePattern: "^(Cc|Ci|Cr|Cu|EXPORTED_SYMBOLS)$",
     }],
     "no-use-before-define": ["error", {
--- a/toolkit/components/telemetry/TelemetryEnvironment.jsm
+++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -264,17 +264,17 @@ const PREF_CHANGED_TOPIC = "nsPref:chang
  * @param aValue The input value.
  * @return {Boolean|Object} If aValue is a boolean or a number, returns its truthfulness
  *         value. Otherwise, return null.
  */
 function enforceBoolean(aValue) {
   if (typeof(aValue) !== "number" && typeof(aValue) !== "boolean") {
     return null;
   }
-  return (new Boolean(aValue)).valueOf();
+  return Boolean(aValue);
 }
 
 /**
  * Get the current browser locale.
  * @return a string with the locale or null on failure.
  */
 function getBrowserLocale() {
   try {
--- a/toolkit/components/telemetry/TelemetryStorage.jsm
+++ b/toolkit/components/telemetry/TelemetryStorage.jsm
@@ -1884,17 +1884,17 @@ function getPingDirectory() {
  * @param {String} aPingId The ping id.
  * @param {Object} aDate The ping creation date.
  * @param {String} aType The ping type.
  * @return {String} The full path to the archived ping.
  */
 function getArchivedPingPath(aPingId, aDate, aType) {
   // Get the ping creation date and generate the archive directory to hold it. Note
   // that getMonth returns a 0-based month, so we need to add an offset.
-  let month = new String(aDate.getMonth() + 1);
+  let month = String(aDate.getMonth() + 1);
   let archivedPingDir = OS.Path.join(gPingsArchivePath,
     aDate.getFullYear() + "-" + month.padStart(2, "0"));
   // Generate the archived ping file path as YYYY-MM/<TIMESTAMP>.UUID.type.json
   let fileName = [aDate.getTime(), aPingId, aType, "json"].join(".");
   return OS.Path.join(archivedPingDir, fileName);
 }
 
 /**
--- a/toolkit/content/aboutTelemetry.js
+++ b/toolkit/content/aboutTelemetry.js
@@ -165,17 +165,17 @@ function removeAllChildNodes(node) {
     node.removeChild(node.lastChild);
   }
 }
 
 /**
  * Pad a number to two digits with leading "0".
  */
 function padToTwoDigits(n) {
-  return new String(n).padStart(2, "0");
+  return String(n).padStart(2, "0");
 }
 
 /**
  * Return yesterdays date with the same time.
  */
 function yesterday(date) {
   let d = new Date(date);
   d.setDate(d.getDate() - 1);
--- a/toolkit/modules/PropertyListUtils.jsm
+++ b/toolkit/modules/PropertyListUtils.jsm
@@ -198,16 +198,19 @@ this.PropertyListUtils = Object.freeze({
    *        a number in the form of either a primitive string or a primitive number.
    * @return a String wrapper around aNumberStr that can later be identified
    * as holding 64-bit number using getObjectType.
    */
   wrapInt64: function PLU_wrapInt64(aPrimitive) {
     if (typeof(aPrimitive) != "string" && typeof(aPrimitive) != "number")
       throw new Error("aPrimitive should be a string primitive");
 
+    // The function converts string or number to object
+    // So eslint rule is disabled
+    // eslint-disable-next-line no-new-wrappers
     let wrapped = new String(aPrimitive);
     Object.defineProperty(wrapped, "__INT_64_WRAPPER__", { value: true });
     return wrapped;
   }
 });
 
 /**
  * Here's the base structure of binary-format property lists.
--- a/toolkit/modules/tests/xpcshell/test_Log.js
+++ b/toolkit/modules/tests/xpcshell/test_Log.js
@@ -411,19 +411,19 @@ add_task(async function log_message_with
   do_check_eq(formatMessage("All params internal", {_level: 20, _message: "froo",
                                                     _time: 123456, _namespace: "here.there"}),
               "All params internal");
 
   // Format params with null and undefined values.
   do_check_eq(formatMessage("Null ${n} undefined ${u}", {n: null, u: undefined}),
               "Null null undefined undefined");
 
-  // Format params with number, bool, and Object/String type.
+  // Format params with number, bool, and String type.
   do_check_eq(formatMessage("number ${n} boolean ${b} boxed Boolean ${bx} String ${s}",
-                            {n: 45, b: false, bx: new Boolean(true), s: new String("whatevs")}),
+                            {n: 45, b: false, bx: Boolean(true), s: String("whatevs")}),
               "number 45 boolean false boxed Boolean true String whatevs");
 
   /*
    * Check that errors get special formatting if they're formatted directly as
    * a named param or they're the only param, but not if they're a field in a
    * larger structure.
    */
   let err = Components.Exception("test exception", Components.results.NS_ERROR_FAILURE);
@@ -436,18 +436,18 @@ add_task(async function log_message_with
   do_check_true(str.includes('Exception is [Exception... "test exception"'));
   str = formatMessage("Exception is", {_error: err});
   do_print(str);
   // Exceptions buried inside objects are formatted badly.
   do_check_true(str.includes('Exception is: {"_error":{}'));
   // If the message text is null, the message contains only the formatted params object.
   str = formatMessage(null, err);
   do_check_true(str.startsWith('[Exception... "test exception"'));
-  // If the text is null and 'params' is a String object, the message is exactly that string.
-  str = formatMessage(null, new String("String in place of params"));
+  // If the text is null and 'params' is a string, the message is exactly that string.
+  str = formatMessage(null, "String in place of params");
   do_check_eq(str, "String in place of params");
 
   // We use object.valueOf() internally; make sure a broken valueOf() method
   // doesn't cause the logger to fail.
   /* eslint-disable object-shorthand */
   let vOf = {a: 1, valueOf: function() {throw "oh noes valueOf"}};
   do_check_eq(formatMessage("Broken valueOf ${}", vOf),
               'Broken valueOf ({a:1, valueOf:(function() {throw "oh noes valueOf"})})');
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -247,16 +247,19 @@ module.exports = {
     "no-native-reassign": "error",
 
     // Nested ternary statements are confusing
     "no-nested-ternary": "error",
 
     // Use {} instead of new Object()
     "no-new-object": "error",
 
+    // Dissallow use of new wrappers
+    "no-new-wrappers": "error",
+
     // No Math() or JSON()
     "no-obj-calls": "error",
 
     // No octal literals
     "no-octal": "error",
 
     // No redeclaring variables
     "no-redeclare": "error",