Issue 4871 - Have ESLint pass for the messageResponder

messageResponder.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 {
-  var ext = ext || require("ext_background");
+  if (typeof ext == "undefined")

[kzar, 2017/02/08 08:39:24]

(This change is required since we'll be using `let` instead of `var` to declare
`ext` in adblockpluschrome when the related ESLint changes land there.)

+    window.ext = require("ext_background");
 
   const {port} = require("messaging");
   const {Prefs} = require("prefs");
   const {Utils} = require("utils");
   const {FilterStorage} = require("filterStorage");
   const {FilterNotifier} = require("filterNotifier");
   const {defaultMatcher} = require("matcher");
   const {ElemHideEmulation} = require("elemHideEmulation");
   const {Notification: NotificationStorage} = require("notification");
 
@@ skipping 39 matching lines @@
     obj.isDownloading = Synchronizer.isExecuting(subscription.url);
     return obj;
   }
 
   let convertFilter = convertObject.bind(null, ["text"]);
 
   let changeListeners = new ext.PageMap();
   let listenedPreferences = Object.create(null);
   let listenedFilterChanges = Object.create(null);
   let messageTypes = {
-    "app": "app.respond",
-    "filter": "filters.respond",
-    "pref": "prefs.respond",
-    "subscription": "subscriptions.respond"
+    app: "app.respond",
+    filter: "filters.respond",
+    pref: "prefs.respond",
+    subscription: "subscriptions.respond"
   };
 
-  function sendMessage(type, action)
+  function sendMessage(type, action, ...args)
   {
     let pages = changeListeners.keys();
     if (pages.length == 0)
       return;
 
-    let args = [];
-    for (let i = 2; i < arguments.length; i++)
+    for (let i = 0; i < args.length; i++)

[Thomas Greiner, 2017/02/08 10:11:09]

Why not use a for-of loop instead to simplify the code?

[kzar, 2017/02/08 10:27:24]

Since we need to replace some arguments in place.

[Thomas Greiner, 2017/02/08 10:37:26]

Ah, don't know how I overlooked that. :p You're of course absolutely right.

Usually we'd use `Array.map()` for that instead of a for-loop so not sure why it
wasn't used here. Anyway, no need to make this change if you consider it to be
out of scope for this review.

     {
-      let arg = arguments[i];
+      let arg = args[i];
       if (arg instanceof Subscription)
-        args.push(convertSubscription(arg));
+        args[i] = convertSubscription(arg);
       else if (arg instanceof Filter)
-        args.push(convertFilter(arg));
-      else
-        args.push(arg);
+        args[i] = convertFilter(arg);
     }
 
     for (let page of pages)
     {
       let filters = changeListeners.get(page);
       let actions = filters[type];
       if (actions && actions.indexOf(action) != -1)
       {
         page.sendMessage({
           type: messageTypes[type],
-          action: action,
-          args: args
+          action,
+          args
         });
       }
     }
   }
 
-  function addFilterListeners(type, actions)
+  function addFilterListeners(type, actions, ...args)
   {
     for (let action of actions)
     {
       let name;
       if (type == "filter" && action == "loaded")
         name = "load";
       else
         name = type + "." + action;
 
       if (!(name in listenedFilterChanges))
       {
         listenedFilterChanges[name] = null;
-        FilterNotifier.on(name, function()
+        FilterNotifier.on(name, () =>
         {
-          let args = [type, action];
-          for (let arg of arguments)
-            args.push(arg);
-          sendMessage.apply(null, args);
+          sendMessage(...[type, action].concat(args));

[Thomas Greiner, 2017/02/08 10:11:09]

Looks like a more complicated way of writing `sendMessage(type, action,
...args)`.

[kzar, 2017/02/08 10:27:24]

Good point, Done.

         });
       }
     }
   }
 
   function getListenerFilters(page)
   {
     let listenerFilters = changeListeners.get(page);
     if (!listenerFilters)
     {
       listenerFilters = Object.create(null);
       changeListeners.set(page, listenerFilters);
     }
     return listenerFilters;
   }
 
   port.on("app.get", (message, sender) =>
   {
     if (message.what == "issues")
     {
       let subscriptionInit = tryRequire("subscriptionInit");
       return {
-        filterlistsReinitialized: subscriptionInit ? subscriptionInit.reinitialized : false
+        filterlistsReinitialized: subscriptionInit
+          ? subscriptionInit.reinitialized : false
       };
     }
 
     if (message.what == "doclink")
       return Utils.getDocLink(message.link);
 
     if (message.what == "localeInfo")
     {
       let bidiDir;
       if ("chromeRegistry" in Utils)
-        bidiDir = Utils.chromeRegistry.isLocaleRTL("adblockplus") ? "rtl" : "ltr";
+      {
+        let rtl = Utils.chromeRegistry.isLocaleRTL("adblockplus");

[Thomas Greiner, 2017/02/08 10:11:09]

Detail: It'd be great if the variable name would reflect that it's a boolean by
calling it `isRtl` or `isRTL`.

[kzar, 2017/02/08 10:27:24]

Done.

+        bidiDir = rtl ? "rtl" : "ltr";
+      }
       else
+      {
         bidiDir = ext.i18n.getMessage("@@bidi_dir");
+      }
 
-      return {locale: Utils.appLocale, bidiDir: bidiDir};
+      return {locale: Utils.appLocale, bidiDir};
     }
 
     if (message.what == "features")
     {
       return {
         devToolsPanel: info.platform == "chromium"
       };
     }
 
     return info[message.what];
@@ skipping 35 matching lines @@
   {
     if (message.what == "elemhideemulation")
     {
       let filters = [];
       const {checkWhitelisted} = require("whitelisting");
 
       if (Prefs.enabled && !checkWhitelisted(sender.page, sender.frame,
                             RegExpFilter.typeMap.DOCUMENT |
                             RegExpFilter.typeMap.ELEMHIDE))
       {
-        let hostname = sender.frame.url.hostname;
+        let {hostname} = sender.frame.url;
         filters = ElemHideEmulation.getRulesForDomain(hostname);
         filters = filters.map(filter =>
         {
           return {
             selector: filter.selector,
             text: filter.text
           };
         });
       }
       return filters;
@@ skipping 30 matching lines @@
       return errors;
 
     for (let subscription of FilterStorage.subscriptions)
     {
       if (!(subscription instanceof SpecialSubscription))
         continue;
 
       for (let j = subscription.filters.length - 1; j >= 0; j--)
       {
         let filter = subscription.filters[j];
-        if (/^@@\|\|([^\/:]+)\^\$document$/.test(filter.text))
+        if (/^@@\|\|([^/:]+)\^\$document$/.test(filter.text))
           continue;
 
         if (!(filter.text in seenFilter))
           FilterStorage.removeFilter(filter);
       }
     }
 
     return errors;
   });
 
@@ skipping 58 matching lines @@
       ext.showOptions(() =>
       {
         sendMessage("app", "addSubscription", subscription);
       });
     }
     else
     {
       subscription.disabled = false;
       FilterStorage.addSubscription(subscription);
 
-      if (subscription instanceof DownloadableSubscription && !subscription.lastDownload)
+      if (subscription instanceof DownloadableSubscription &&
+          !subscription.lastDownload)
         Synchronizer.execute(subscription);
     }
   });
 
   port.on("subscriptions.get", (message, sender) =>
   {
     let subscriptions = FilterStorage.subscriptions.filter(s =>
     {
       if (message.ignoreDisabled && s.disabled)
         return false;
@@ skipping 36 matching lines @@
       subscription.title = message.title;
       subscription.homepage = message.homepage;
       FilterStorage.addSubscription(subscription);
       if (!subscription.lastDownload)
         Synchronizer.execute(subscription);
     }
   });
 
   port.on("subscriptions.update", (message, sender) =>
   {
-    let subscriptions = message.url ? [Subscription.fromURL(message.url)] :
-                        FilterStorage.subscriptions;
+    let subscriptions = message.url ? [Subscription.fromURL(message.url)]
+                        : FilterStorage.subscriptions;
     for (let subscription of subscriptions)
     {
       if (subscription instanceof DownloadableSubscription)
         Synchronizer.execute(subscription, true);
     }
   });
 }