Issue 4864 - Start using ESLint for adblockpluschrome

lib/devtools.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 26 matching lines @@
 let panels = Object.create(null);
 
 function hasPanels()
 {
   return Object.keys(panels).length > 0;
 }
 
 function getActivePanel(page)
 {
   let panel = panels[page.id];
-  if(panel && !panel.reload && !panel.reloading)
+  if (panel && !panel.reload && !panel.reloading)
     return panel;
   return null;
 }
 
 function getFilterInfo(filter)
 {
   if (!filter)
     return null;
 
   let userDefined = false;
   let subscriptionTitle = null;
 
   for (let subscription of filter.subscriptions)
   {
     if (!subscription.disabled)
     {
       if (subscription instanceof SpecialSubscription)
         userDefined = true;
       else
         subscriptionTitle = subscription.title;
     }
   }
 
   return {
     text: filter.text,
     whitelisted: filter instanceof WhitelistFilter,
-    userDefined: userDefined,
+    userDefined,
     subscription: subscriptionTitle
   };
 }
 
 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.
     (record.filter && record.filter.selector) == (filter && filter.selector)
   );
 }
 
 function addRecord(panel, request, filter)
 {
   if (!hasRecord(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});
   }
 }
 
 function matchRequest(request)
 {
   return defaultMatcher.matchesAny(
     request.url,
     RegExpFilter.typeMap[request.type],
     request.docDomain,
     request.thirdParty,
     request.sitekey,
     request.specificOnly
   );
 }
 
 /**
  * Logs a request to the devtools panel.
  *
  * @param {Page}     page          The page the request occured on
  * @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,
                               specificOnly, filter)
 {
   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);
   }
 };
 
 /**
  * Logs active element hiding filters to the devtools panel.
  *
  * @param {Page}     page       The page the elements were hidden on
  * @param {string[]} selectors  The CSS selectors of active elemhide filters
  * @param {string}   docDomain  The IDN-decoded hostname of the document
@@ skipping 11 matching lines @@
       for (let filter of subscription.filters)
       {
         if (!(filter instanceof ElemHideFilter) &&
             !(filter instanceof ElemHideEmulationFilter))
           continue;
         if (selectors.indexOf(filter.selector) == -1)
           continue;
         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)
  * blocking for a particular document, to the devtools panel.
  *
  * @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)
   {
     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);
     }
   }
 };
 
 /**
  * Checks whether a page is inspected by the devtools panel.
  *
  * @param {Page} page
  * @return {boolean}
  */
@@ skipping 113 matching lines @@
 {
   updateFilters([filter], false);
 }
 
 function onSubscriptionAdded(subscription)
 {
   if (subscription instanceof SpecialSubscription)
     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;
 
   let inspectedTabId = parseInt(match[1], 10);
   let localOnBeforeRequest = onBeforeRequest.bind();
 
   chrome.webRequest.onBeforeRequest.addListener(
     localOnBeforeRequest,
     {
-      urls:  ["<all_urls>"],
+      urls: ["<all_urls>"],
       types: ["main_frame"],
       tabId: inspectedTabId
     }
   );
 
   if (!hasPanels())
   {
     ext.pages.onLoading.addListener(onLoading);
     FilterNotifier.on("filter.added", onFilterAdded);
     FilterNotifier.on("filter.removed", onFilterRemoved);
     FilterNotifier.on("subscription.added", onSubscriptionAdded);
   }
 
-  port.onDisconnect.addListener(() =>
+  newPort.onDisconnect.addListener(() =>
   {
     delete panels[inspectedTabId];
     chrome.webRequest.onBeforeRequest.removeListener(localOnBeforeRequest);
 
     if (!hasPanels())
     {
       ext.pages.onLoading.removeListener(onLoading);
       FilterNotifier.off("filter.added", onFilterAdded);
       FilterNotifier.off("filter.removed", onFilterRemoved);
       FilterNotifier.off("subscription.added", onSubscriptionAdded);
     }
   });
 
-  panels[inspectedTabId] = {port: port, records: []};
+  panels[inspectedTabId] = {port: newPort, records: []};
 });
 
 port.on("devtools.traceElemHide", (message, sender) =>
 {
   logHiddenElements(
     sender.page, message.selectors,
     extractHostFromFrame(sender.frame)
   );
 });