Issue 4864 - Start using ESLint for adblockpluschrome

lib/devtools.js

Index: lib/devtools.js
diff --git a/lib/devtools.js b/lib/devtools.js
index d4a6e7b9039d4fde52a158c91bb992f8decc0a9c..2fc2010e975db6140f913a14afcb74cd63cf5606 100644
--- a/lib/devtools.js
+++ b/lib/devtools.js
@@ -44,7 +44,7 @@ function hasPanels()
 function getActivePanel(page)
 {
   let panel = panels[page.id];
-  if(panel && !panel.reload && !panel.reloading)
+  if (panel && !panel.reload && !panel.reloading)
     return panel;
   return null;
 }
@@ -71,7 +71,7 @@ function getFilterInfo(filter)
   return {
     text: filter.text,
     whitelisted: filter instanceof WhitelistFilter,
-    userDefined: userDefined,
+    userDefined,
     subscription: subscriptionTitle
   };
 }
@@ -79,13 +79,16 @@ function getFilterInfo(filter)
 function hasRecord(panel, request, filter)
 {
   return panel.records.some(record =>
-    record.request.url       == request.url       &&
+    record.request.url == request.url &&
     record.request.docDomain == request.docDomain &&
 
     // Ignore partial (e.g. ELEMHIDE) whitelisting if there is already
     // a DOCUMENT exception which disables all means of blocking.
-    (record.request.type == "DOCUMENT" ? nonRequestTypes.indexOf(request.type) != -1
-                                       : record.request.type == request.type) &&
+    (
+      record.request.type == "DOCUMENT" ?

[Sebastian Noack, 2017/03/30 18:57:58]

I found this not quite as readable as in the previous form. But you could argue
that these were way to many chained conditions in the firs place. And now where
we have for..of, Array.some() which caused this mess doesn't provide much of a
benefit anymore. Perhaps the following would be easier to read, while also
compliant with our ESLint configuration:


  for (let record of panel.records)
  {
    if (record.request.url != request.url)
      continue;
    if (record.request.docDomain != request.docDomain)
      continue;
    if (record.request.type == "DOCUMENT")
    {
      if (nonRequestTypes.indexOf(request.type) == -1)
        continue;
    }
    else
    {
      if (record.request.type != request.type)
        continue;
    }

    return true;
  }
  return false;

[kzar, 2017/03/31 03:42:18]

I've had a go, what do you think?

[Sebastian Noack, 2017/03/31 07:59:35]

Hmm, in the end it doesn't seem to be any more readable, and my plan to at least
get rid of wrapped lines didn't work out either. After all, your initial changes
don't look too bad. Perhaps just add indentation, and keep the change to
Array.includes(), sorry for that:

  (record.request.type == "DOCUMENT" ?
     nonRequestTypes.includes(request.type) :
     record.request.type == request.type) &&

[kzar, 2017/03/31 08:27:35]

Done.

+      nonRequestTypes.indexOf(request.type) != -1 :
+      record.request.type == request.type
+    ) &&
 
     // Matched element hiding filters don't relate to a particular request,
     // so we also have to match the CSS selector in order to distinguish them.
@@ -99,14 +102,11 @@ function addRecord(panel, request, filter)
   {
     panel.port.postMessage({
       type: "add-record",
-      request: request,
+      request,
       filter: getFilterInfo(filter)
     });
 
-    panel.records.push({
-      request: request,
-      filter: filter
-    });
+    panel.records.push({request, filter});
   }
 }
 
@@ -129,10 +129,12 @@ function matchRequest(request)
  * @param {string}   url           The URL of the request
  * @param {string}   type          The request type
  * @param {string}   docDomain     The IDN-decoded hostname of the document
- * @param {boolean}  thirdParty    Whether the origin of the request and document differs
+ * @param {boolean}  thirdParty    Whether the origin of the request and
+ *                                 document differs
  * @param {?string}  sitekey       The active sitekey if there is any
  * @param {?boolean} specificOnly  Whether generic filters should be ignored
- * @param {?BlockingFilter} filter The matched filter or null if there is no match
+ * @param {?BlockingFilter} filter The matched filter or null if there is no
+ *                                 match
  */
 exports.logRequest = function(page, url, type, docDomain,
                               thirdParty, sitekey,
@@ -141,15 +143,7 @@ exports.logRequest = function(page, url, type, docDomain,
   let panel = getActivePanel(page);
   if (panel)
   {
-    let request = {
-      url: url,
-      type: type,
-      docDomain: docDomain,
-      thirdParty: thirdParty,
-      sitekey: sitekey,
-      specificOnly: specificOnly
-    };
-
+    let request = {url, type, docDomain, thirdParty, sitekey, specificOnly};
     addRecord(panel, request, filter);
   }
 };
@@ -181,11 +175,11 @@ function logHiddenElements(page, selectors, docDomain)
         if (!filter.isActiveOnDomain(docDomain))
           continue;
 
-        addRecord(panel, {type: "ELEMHIDE", docDomain: docDomain}, filter);
+        addRecord(panel, {type: "ELEMHIDE", docDomain}, filter);
       }
     }
   }
-};
+}
 
 /**
  * Logs a whitelisting filter, that disables (some kind of)
@@ -193,11 +187,14 @@ function logHiddenElements(page, selectors, docDomain)
  *
  * @param {Page}         page      The page the whitelisting is active on
  * @param {string}       url       The url of the whitelisted document
- * @param {number}       typeMask  The bit mask of whitelisting types checked for
- * @param {string}       docDomain The IDN-decoded hostname of the parent document
+ * @param {number}       typeMask  The bit mask of whitelisting types checked
+ *                                 for
+ * @param {string}       docDomain The IDN-decoded hostname of the parent
+ *                                 document
  * @param {WhitelistFilter} filter The matched whitelisting filter
  */
-exports.logWhitelistedDocument = function(page, url, typeMask, docDomain, filter)
+exports.logWhitelistedDocument = function(page, url, typeMask, docDomain,
+                                          filter)
 {
   let panel = getActivePanel(page);
   if (panel)
@@ -205,7 +202,7 @@ exports.logWhitelistedDocument = function(page, url, typeMask, docDomain, filter
     for (let type of nonRequestTypes)
     {
       if (typeMask & filter.contentType & RegExpFilter.typeMap[type])
-        addRecord(panel, {url: url, type: type, docDomain: docDomain}, filter);
+        addRecord(panel, {url, type, docDomain}, filter);
     }
   }
 };
@@ -339,9 +336,9 @@ function onSubscriptionAdded(subscription)
     updateFilters(subscription.filters, true);
 }
 
-chrome.runtime.onConnect.addListener(port =>
+chrome.runtime.onConnect.addListener(newPort =>
 {
-  let match = port.name.match(/^devtools-(\d+)$/);
+  let match = newPort.name.match(/^devtools-(\d+)$/);
   if (!match)
     return;
 
@@ -351,7 +348,7 @@ chrome.runtime.onConnect.addListener(port =>
   chrome.webRequest.onBeforeRequest.addListener(
     localOnBeforeRequest,
     {
-      urls:  ["<all_urls>"],
+      urls: ["<all_urls>"],
       types: ["main_frame"],
       tabId: inspectedTabId
     }
@@ -365,7 +362,7 @@ chrome.runtime.onConnect.addListener(port =>
     FilterNotifier.on("subscription.added", onSubscriptionAdded);
   }
 
-  port.onDisconnect.addListener(() =>
+  newPort.onDisconnect.addListener(() =>
   {
     delete panels[inspectedTabId];
     chrome.webRequest.onBeforeRequest.removeListener(localOnBeforeRequest);
@@ -379,7 +376,7 @@ chrome.runtime.onConnect.addListener(port =>
     }
   });
 
-  panels[inspectedTabId] = {port: port, records: []};
+  panels[inspectedTabId] = {port: newPort, records: []};
 });
 
 port.on("devtools.traceElemHide", (message, sender) =>