Bug 1766533 - Enable ESLint rule no-constant-binary-expression. r?Gijs draft
authorMark Banner <standard8@mozilla.com>
Thu, 05 May 2022 16:36:30 +0100 (2022-05-05)
changeset 4355861 0fe5678fb8b71f4eb26f0a153c52d0be45fc5ac1
parent 4355860 83b185696656959af2d63450820950501de764b7
child 4355862 0c210a986cf2e6d3fce8548d58df8686cafaed94
push id807950
push usermbanner@mozilla.com
push dateThu, 05 May 2022 15:54:06 +0000 (2022-05-05)
treeherdertry@0c210a986cf2 [default view] [failures only]
reviewersGijs
bugs1766533
milestone102.0a1
Bug 1766533 - Enable ESLint rule no-constant-binary-expression. r?Gijs
browser/components/newtab/test/unit/content-src/components/HelpText.test.jsx
toolkit/components/places/Bookmarks.jsm
toolkit/content/aboutTelemetry.js
toolkit/content/tests/chrome/test_props.xhtml
tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
--- a/browser/components/newtab/test/unit/content-src/components/HelpText.test.jsx
+++ b/browser/components/newtab/test/unit/content-src/components/HelpText.test.jsx
@@ -17,17 +17,17 @@ describe("<HelpText>", () => {
           src:
             "chrome://activity-stream/content/data/content/assets/cfr_fb_container.png",
         }}
       />
     );
     assert.ok(
       shallowWrapper
         .find(Localized)
-        .findWhere(n => n.text === { string_id: "test_id" })
+        .findWhere(n => n.text.string_id === "test_id")
     );
     assert.lengthOf(shallowWrapper.find("p.helptext"), 1);
     assert.lengthOf(shallowWrapper.find("img[data-l10n-name='help-img']"), 1);
   });
   it("should render the img if there is an img and plain text", () => {
     const shallowWrapper = shallow(
       <HelpText
         text={"Sample help text"}
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -1430,17 +1430,17 @@ var Bookmarks = Object.freeze({
    * @return {Promise} resolved when the listing is complete.
    * @resolves to an array of recent bookmark-items.
    * @rejects if an error happens while querying.
    */
   getRecent(numberOfItems) {
     if (numberOfItems === undefined) {
       throw new Error("numberOfItems argument is required");
     }
-    if (!typeof numberOfItems === "number" || numberOfItems % 1 !== 0) {
+    if (typeof numberOfItems !== "number" || numberOfItems % 1 !== 0) {
       throw new Error("numberOfItems argument must be an integer");
     }
     if (numberOfItems <= 0) {
       throw new Error("numberOfItems argument must be greater than zero");
     }
 
     return fetchRecentBookmarks(numberOfItems);
   },
--- a/toolkit/content/aboutTelemetry.js
+++ b/toolkit/content/aboutTelemetry.js
@@ -26,16 +26,21 @@ ChromeUtils.defineModuleGetter(
   "AppConstants",
   "resource://gre/modules/AppConstants.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this,
   "Preferences",
   "resource://gre/modules/Preferences.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "ObjectUtils",
+  "resource://gre/modules/ObjectUtils.jsm"
+);
 
 const Telemetry = Services.telemetry;
 const bundle = Services.strings.createBundle(
   "chrome://global/locale/aboutTelemetry.properties"
 );
 const brandBundle = Services.strings.createBundle(
   "chrome://branding/locale/brand.properties"
 );
@@ -1798,17 +1803,17 @@ class Section {
       ? Object.keys(payload[selectedStore]).sort(this.processesComparator)
       : Object.keys(aPayload.processes).sort(this.processesComparator);
 
     // Render content by process
     for (const process of sortedProcesses) {
       data = isCurrentPayload
         ? this.dataFiltering(payload, selectedStore, process)
         : this.archivePingDataFiltering(aPayload, process);
-      hasData = hasData || data !== {};
+      hasData = hasData || !ObjectUtils.isEmpty(data);
       this.renderContent(data, process, div, section, this.renderData);
     }
     setHasData(section, hasData);
   }
 }
 
 class Scalars extends Section {
   /**
@@ -1914,18 +1919,18 @@ var Events = {
       "about-telemetry-values-header",
       "about-telemetry-extra-header",
     ];
     let payload = aPayload.processes;
     let hasData = false;
     if (payload) {
       for (const process of Object.keys(aPayload.processes)) {
         let data = aPayload.processes[process].events;
-        hasData = hasData || data !== {};
         if (data && Object.keys(data).length) {
+          hasData = true;
           let s = GenericSubsection.renderSubsectionHeader(
             process,
             true,
             "events-section"
           );
           let heading = document.createElement("h2");
           heading.textContent = process;
           s.appendChild(heading);
@@ -1936,18 +1941,18 @@ var Events = {
           separator.classList.add("clearfix");
           eventsDiv.appendChild(separator);
         }
       }
     } else {
       // handle archived ping
       for (const process of Object.keys(aPayload.events)) {
         let data = process;
-        hasData = hasData || data !== {};
         if (data && Object.keys(data).length) {
+          hasData = true;
           let s = GenericSubsection.renderSubsectionHeader(
             process,
             true,
             "events-section"
           );
           let heading = document.createElement("h2");
           heading.textContent = process;
           s.appendChild(heading);
--- a/toolkit/content/tests/chrome/test_props.xhtml
+++ b/toolkit/content/tests/chrome/test_props.xhtml
@@ -4,18 +4,18 @@
 <!--
   XUL Widget Test for basic properties - this test checks that the basic
   properties defined in general.js and inherited by a number of elements
   work properly.
   -->
 <window title="Basic Properties Test"
         onload="setTimeout(test_props, 0);"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>      
-  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>      
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
 
 <command id="cmd_nothing"/>
 <command id="cmd_action"/>
 
 <button id="button" label="Button" accesskey="B"
         crop="end" image="happy.png" command="cmd_nothing"/>
 <checkbox id="checkbox" label="Checkbox" accesskey="B"
           crop="end" image="happy.png" command="cmd_nothing"/>
@@ -68,16 +68,17 @@ function test_props_forelement(element, 
     is(element.value, "lb", "element value after set");
   is(element.accessKey, "L", "element accessKey after set");
   is(element.image, "sad.png", "element image after set");
   is(element.command, "cmd_action", "element command after set");
 
   synthesizeMouseExpectEvent(element, 4, 4, { }, $("cmd_action"), "command", "element");
 
   // check that clicks on disabled items don't fire a command event
+  // eslint-disable-next-line no-constant-binary-expression
   ok((element.disabled = true) === true, "element disabled setter return");
   ok(element.disabled === true, "element disabled after set");
   synthesizeMouseExpectEvent(element, 4, 4, { }, $("cmd_action"), "!command", "element");
 
   element.disabled = false;
 }
 
 ]]>
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
@@ -172,16 +172,19 @@ module.exports = {
     // XXX Bug 1487642 - decide if we want to enable this or not.
     // Disallow lexical declarations in case clauses
     "no-case-declarations": "off",
 
     // XXX Bug 1487642 - decide if we want to enable this or not.
     // Disallow the use of console
     "no-console": "off",
 
+    // Disallows expressions where the operation doesn't affect the value.
+    "no-constant-binary-expression": "error",
+
     // XXX Bug 1487642 - decide if we want to enable this or not.
     // Disallow constant expressions in conditions
     "no-constant-condition": "off",
 
     // No duplicate keys in object declarations
     "no-dupe-keys": "error",
 
     // If an if block ends with a return no need for an else block