Issue 4878 - Start using ESLint for adblockpluscore

lib/filterStorage.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";
 
 /**
  * @fileOverview FilterStorage class responsible for managing user's
  *               subscriptions and filters.
  */
 
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/FileUtils.jsm");
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+const {FileUtils} = Cu.import("resource://gre/modules/FileUtils.jsm", {});
+const {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 
 const {IO} = require("io");
 const {Prefs} = require("prefs");
 const {Filter, ActiveFilter} = require("filterClasses");
 const {Subscription, SpecialSubscription,
        ExternalSubscription} = require("subscriptionClasses");
 const {FilterNotifier} = require("filterNotifier");
 const {Utils} = require("utils");
 
 /**
@@ skipping 36 matching lines @@
       // Place the file in the data dir
       file = IO.resolveFilePath(Prefs.data_directory);
       if (file)
         file.append("patterns.ini");
     }
     if (!file)
     {
       // Data directory pref misconfigured? Try the default value
       try
       {
-        file = IO.resolveFilePath(
-          Services.prefs.getDefaultBranch("extensions.adblockplus."
-        ).getCharPref("data_directory"));

[Wladimir Palant, 2017/03/02 14:06:57]

Now you have the closing parenthesis from getDefaultBranch call matched up with
the opening parenthesis from resolveFilePath.

  let dir = Services.prefs.getDefaultBranch("extensions.adblockplus.")
                          .getCharPref("data_directory");
  file = IO.resolveFilePath(dir);

[kzar, 2017/03/08 12:33:44]

Whoops, done.

+        let dir = Services.prefs.getDefaultBranch("extensions.adblockplus.")
+                          .getCharPref("data_directory");
+        file = IO.resolveFilePath(dir);
         if (file)
           file.append("patterns.ini");
       }
       catch (e) {}
     }
 
     if (!file)
     {
       Cu.reportError("Adblock Plus: Failed to resolve filter file location " +
                      "from extensions.adblockplus.patternsfile preference");
@@ skipping 26 matching lines @@
   /**
    * Map of subscriptions already on the list, by their URL/identifier
    * @type {Object}
    */
   knownSubscriptions: Object.create(null),
 
   /**
    * Finds the filter group that a filter should be added to by default. Will
    * return null if this group doesn't exist yet.
    * @param {Filter} filter
-   * @return {SpecialSubscription|null}
+   * @return {?SpecialSubscription}
    */
   getGroupForFilter(filter)
   {
     let generalSubscription = null;
     for (let subscription of FilterStorage.subscriptions)
     {
       if (subscription instanceof SpecialSubscription && !subscription.disabled)
       {
         // Always prefer specialized subscriptions
         if (subscription.isDefaultFor(filter))
           return subscription;
 
         // If this is a general subscription - store it as fallback
         if (!generalSubscription &&
             (!subscription.defaults || !subscription.defaults.length))
+        {
           generalSubscription = subscription;
+        }
       }
     }
     return generalSubscription;
   },
 
   /**
    * Adds a filter subscription to the list
    * @param {Subscription} subscription filter subscription to be added
    * @param {boolean} silent  if true, no listeners will be triggered
    *                          (to be used when filter list is reloaded)
@@ skipping 74 matching lines @@
     subscription.oldFilters = subscription.filters;
     subscription.filters = filters;
     addSubscriptionFilters(subscription);
     FilterNotifier.triggerListeners("subscription.updated", subscription);
     delete subscription.oldFilters;
   },
 
   /**
    * Adds a user-defined filter to the list
    * @param {Filter} filter
-   * @param {SpecialSubscription} [subscription] particular group that the
-   *                                             filter should be added to
-   * @param {number} [position] position within the subscription at which
-   *                             the filter should be added
-   * @param {boolean} silent  if true, no listeners will be triggered (to
-   *                          be used when filter list is reloaded)

[Wladimir Palant, 2017/03/02 14:06:54]

Messy indentation here.

[kzar, 2017/03/08 12:33:39]

Done.

+   * @param {SpecialSubscription} [subscription]
+   *   particular group that the filter should be added to
+   * @param {number} [position]
+   *   position within the subscription at which the filter should be added
+   * @param {boolean} silent
+   *   if true, no listeners will be triggered (to be used when filter list is
+   *   reloaded)
    */
   addFilter(filter, subscription, position, silent)
   {
     if (!subscription)
     {
       if (filter.subscriptions.some(s => s instanceof SpecialSubscription &&
-                                    !s.disabled))

[Wladimir Palant, 2017/03/02 14:06:57]

Wrong indentation, this should be aligned with "s instanceof"

[kzar, 2017/03/08 12:33:44]

Done.

+                                         !s.disabled))
+      {
         return;   // No need to add
+      }
       subscription = FilterStorage.getGroupForFilter(filter);
     }
     if (!subscription)
     {
       // No group for this filter exists, create one
       subscription = SpecialSubscription.createForFilter(filter);
       this.addSubscription(subscription);
       return;
     }
 
@@ skipping 69 matching lines @@
    * @param {Filter} filter
    * @param {SpecialSubscription} subscription filter group where the filter is
    *                                           located
    * @param {number} oldPosition current position of the filter
    * @param {number} newPosition new position of the filter
    */
   moveFilter(filter, subscription, oldPosition, newPosition)
   {
     if (!(subscription instanceof SpecialSubscription) ||
         subscription.filters[oldPosition] != filter)
-      return;
+    {
+      return;
+    }
 
     newPosition = Math.min(Math.max(newPosition, 0),
                            subscription.filters.length - 1);
     if (oldPosition == newPosition)
       return;
 
     subscription.filters.splice(oldPosition, 1);
     subscription.filters.splice(newPosition, 0, filter);
     FilterNotifier.triggerListeners("filter.moved", filter, subscription,
                                     oldPosition, newPosition);
@@ skipping 38 matching lines @@
    * Loads all subscriptions from the disk
    * @param {nsIFile} [sourceFile] File to read from
    */
   loadFromDisk(sourceFile)
   {
     if (this._loading)
       return;
 
     this._loading = true;
 
-    let readFile = function(currentSourceFile, backupIndex)

[Wladimir Palant, 2017/03/02 14:06:56]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:43]

Done.

+    let readFile = (currentSourceFile, backupIndex) =>
     {
       let parser = new INIParser();
       IO.readFromFile(currentSourceFile, parser, readFromFileException =>

[Wladimir Palant, 2017/03/02 14:06:58]

Ok, having to name every exception is ridiculous...

[kzar, 2017/03/08 12:33:42]

Acknowledged.

       {
         if (!readFromFileException && parser.subscriptions.length == 0)
         {
           // No filter subscriptions in the file, this isn't right.
           readFromFileException = new Error("No data in the file");
         }
 
         if (readFromFileException)
           Cu.reportError(readFromFileException);
 
@@ skipping 17 matching lines @@
               if (!statFileException && statData.exists)
                 readFile(currentSourceFile, backupIndex);
               else
                 doneReading(parser);
             });
             return;
           }
         }
         doneReading(parser);
       });
-    }.bind(this);
-
-    let doneReading = function(parser)

[Wladimir Palant, 2017/03/02 14:06:56]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:42]

Done.

+    };
+
+    let doneReading = parser =>
     {
       // Old special groups might have been converted, remove them if
       // they are empty
       let specialMap = {"~il~": true, "~wl~": true, "~fl~": true, "~eh~": true};
       let knownSubscriptions = Object.create(null);
       for (let i = 0; i < parser.subscriptions.length; i++)
       {
         let subscription = parser.subscriptions[i];
         if (subscription instanceof SpecialSubscription &&
             subscription.filters.length == 0 && subscription.url in specialMap)
+        {
           parser.subscriptions.splice(i--, 1);
+        }
         else
           knownSubscriptions[subscription.url] = subscription;
       }
 
       this.fileProperties = parser.fileProperties;
       this.subscriptions = parser.subscriptions;
       this.knownSubscriptions = knownSubscriptions;
       Filter.knownFilters = parser.knownFilters;
       Subscription.knownSubscriptions = parser.knownSubscriptions;
 
       if (parser.userFilters)
       {
         for (let i = 0; i < parser.userFilters.length; i++)
         {
           let filter = Filter.fromText(parser.userFilters[i]);
           this.addFilter(filter, null, undefined, true);
         }
       }
 
       this._loading = false;
       FilterNotifier.triggerListeners("load");
 
       if (sourceFile != this.sourceFile)
         this.saveToDisk();
-    }.bind(this);
+    };
 
     let explicitFile;
     if (sourceFile)
     {
       explicitFile = true;
       readFile(sourceFile, 0);
     }
     else
     {
       explicitFile = false;
@@ skipping 98 matching lines @@
     }
 
     // Make sure the file's parent directory exists
     try
     {
       targetFile.parent.create(Ci.nsIFile.DIRECTORY_TYPE,
                                FileUtils.PERMS_DIRECTORY);
     }
     catch (e) {}
 
-    let writeFilters = function()

[Wladimir Palant, 2017/03/02 14:06:54]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:41]

Done.

+    let writeFilters = () =>
     {
       IO.writeToFile(targetFile, this._generateFilterData(subscriptions), e =>
       {
         if (!explicitFile)
           this._saving = false;
 
         if (e)
           Cu.reportError(e);
 
         if (!explicitFile && this._needsSave)
         {
           this._needsSave = false;
           this.saveToDisk();
         }
         else
           FilterNotifier.triggerListeners("save");
       });
-    }.bind(this);
-
-    let checkBackupRequired = function(callbackNotRequired, callbackRequired)

[Wladimir Palant, 2017/03/02 14:06:57]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:40]

Done.

+    };
+
+    let checkBackupRequired = (callbackNotRequired, callbackRequired) =>
     {
       if (explicitFile || Prefs.patternsbackups <= 0)
         callbackNotRequired();
       else
       {
         IO.statFile(targetFile, (statFileException, statData) =>
         {
           if (statFileException || !statData.exists)
             callbackNotRequired();
           else
           {
             let [, part1, part2] = /^(.*)(\.\w+)$/.exec(targetFile.leafName) ||
               [null, targetFile.leafName, ""];
             let newestBackup = targetFile.clone();
             newestBackup.leafName = part1 + "-backup1" + part2;
             IO.statFile(
               newestBackup,
               (statBackupFileException, statBackupData) =>
               {
                 if (!statBackupFileException && (!statBackupData.exists ||
                            (Date.now() - statBackupData.lastModified) /
                              3600000 >= Prefs.patternsbackupinterval))
+                {
                   callbackRequired(part1, part2);
+                }
                 else
                   callbackNotRequired();
               }
             );
           }
         });
       }
     };
 
-    let removeLastBackup = function(part1, part2)

[Wladimir Palant, 2017/03/02 14:06:55]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:38]

Done.

+    let removeLastBackup = (part1, part2) =>
     {
       let file = targetFile.clone();
       file.leafName = part1 + "-backup" + Prefs.patternsbackups + part2;
       IO.removeFile(
         file, e => renameBackup(part1, part2, Prefs.patternsbackups - 1)
       );
     };
 
-    let renameBackup = function(part1, part2, index)

[Wladimir Palant, 2017/03/02 14:06:55]

Why hasn't this been converted to an arrow function?

[kzar, 2017/03/08 12:33:43]

Done.

+    let renameBackup = (part1, part2, index) =>
     {
       if (index > 0)
       {
         let fromFile = targetFile.clone();
         fromFile.leafName = part1 + "-backup" + index + part2;
 
         let toName = part1 + "-backup" + (index + 1) + part2;
 
         IO.renameFile(fromFile, toName, e => renameBackup(part1, part2,
                                                           index - 1));
@@ skipping 36 matching lines @@
         result.push(file);
       else
         break;
     }
     return result;
   }
 };
 
 /**
  * Joins subscription's filters to the subscription without any notifications.
- * @param {Subscription} subscription filter subscription that should be
- *                                    connected to its filters

[Wladimir Palant, 2017/03/02 14:06:54]

Messy indentation here.

[kzar, 2017/03/08 12:33:41]

Done.

+ * @param {Subscription} subscription
+ *   filter subscription that should be connected to its filters
  */
 function addSubscriptionFilters(subscription)
 {
   if (!(subscription.url in FilterStorage.knownSubscriptions))
     return;
 
   for (let filter of subscription.filters)
     filter.subscriptions.push(subscription);
 }
 
@@ skipping 120 matching lines @@
       Subscription.knownSubscriptions = origKnownSubscriptions;
     }
 
     // Allow events to be processed every now and then.
     // Note: IO.readFromFile() will deal with the potential reentrance here.
     this.linesProcessed++;
     if (this.linesProcessed % 1000 == 0)
       return Utils.yield();
   }
 };