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 66 matching lines @@
 }
 
 function hasRecord(panel, request, filter)
 {
   return panel.records.some(record =>
     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" ?

[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
-    ) &&
+    (record.request.type == "DOCUMENT" ?
+       nonRequestTypes.includes(request.type) :
+       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.
+    // so we have to compare the selector in order to avoid duplicates.
     (record.filter && record.filter.selector) == (filter && filter.selector)
   );
 }
 
 function addRecord(panel, request, filter)
 {
   if (!hasRecord(panel, request, filter))
   {
     panel.port.postMessage({
       type: "add-record",
@@ skipping 274 matching lines @@
   panels[inspectedTabId] = {port: newPort, records: []};
 });
 
 port.on("devtools.traceElemHide", (message, sender) =>
 {
   logHiddenElements(
     sender.page, message.selectors,
     extractHostFromFrame(sender.frame)
   );
 });