Issue 4878 - Start using ESLint for adblockpluscore

lib/downloader.js

Index: lib/downloader.js
diff --git a/lib/downloader.js b/lib/downloader.js
index 74d1ae3d404cb8aa44e36a407d066c8a312868ba..de45dc01260cbf00c911df69d9746e423364afd1 100644
--- a/lib/downloader.js
+++ b/lib/downloader.js
@@ -15,40 +15,45 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+"use strict";
+
 /**
  * @fileOverview Downloads a set of URLs in regular time intervals.
  */
 
-let {Utils} = require("utils");
+const {Utils} = require("utils");

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

ESLint doesn't seem to require this change. But if you are doing it, you should
also change the declarations below - these are actual constants.

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

Done.

 
 let MILLIS_IN_SECOND = exports.MILLIS_IN_SECOND = 1000;
 let MILLIS_IN_MINUTE = exports.MILLIS_IN_MINUTE = 60 * MILLIS_IN_SECOND;
 let MILLIS_IN_HOUR = exports.MILLIS_IN_HOUR = 60 * MILLIS_IN_MINUTE;
 let MILLIS_IN_DAY = exports.MILLIS_IN_DAY = 24 * MILLIS_IN_HOUR;
 
+let Downloader =
 /**
  * Creates a new downloader instance.
- * @param {Function} dataSource  Function that will yield downloadable objects on each check
- * @param {Integer} initialDelay  Number of milliseconds to wait before the first check
- * @param {Integer} checkInterval  Interval between the checks
+ * @param {Function} dataSource  Function that will yield downloadable objects
+ *                               on each check
+ * @param {number} initialDelay  Number of milliseconds to wait before the
+ *                                 first check
+ * @param {number} checkInterval  Interval between the checks

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

Nit: Indentation is messy here. I suggest avoiding such issues by putting
comments on the next line:

 * @param {Function} dataSource
 *   Function that will yield downloadable objects on each check
 * @param {number} initialDelay
 *   Number of milliseconds to wait before the first check
 * @param {number} checkInterval
 *   Interval between the checks

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

Done.

  * @constructor
  */
-let Downloader = exports.Downloader = function Downloader(dataSource, initialDelay, checkInterval)
+exports.Downloader = function(dataSource, initialDelay, checkInterval)
 {
   this.dataSource = dataSource;
   this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-  this._timer.initWithCallback(function()
+  this._timer.initWithCallback(() =>
   {
     this._timer.delay = checkInterval;
     this._doCheck();
-  }.bind(this), initialDelay, Ci.nsITimer.TYPE_REPEATING_SLACK);
+  }, initialDelay, Ci.nsITimer.TYPE_REPEATING_SLACK);
   this._downloading = Object.create(null);
-}
+};
 Downloader.prototype =
 {
   /**
    * Timer triggering the downloads.
-   * @type nsITimer
+   * @type {nsITimer}
    */
   _timer: null,
 
@@ -59,74 +64,75 @@ Downloader.prototype =
 
   /**
    * Function that will yield downloadable objects on each check.
-   * @type Function
+   * @type {Function}
    */
   dataSource: null,
 
   /**
    * Maximal time interval that the checks can be left out until the soft
    * expiration interval increases.
-   * @type Integer
+   * @type {number}
    */
   maxAbsenceInterval: 1 * MILLIS_IN_DAY,
 
   /**
    * Minimal time interval before retrying a download after an error.
-   * @type Integer
+   * @type {number}
    */
   minRetryInterval: 1 * MILLIS_IN_DAY,
 
   /**
    * Maximal allowed expiration interval, larger expiration intervals will be
    * corrected.
-   * @type Integer
+   * @type {number}
    */
   maxExpirationInterval: 14 * MILLIS_IN_DAY,
 
   /**
    * Maximal number of redirects before the download is considered as failed.
-   * @type Integer
+   * @type {number}
    */
   maxRedirects: 5,
 
   /**
    * Called whenever expiration intervals for an object need to be adapted.
-   * @type Function
+   * @type {Function}
    */
   onExpirationChange: null,
 
   /**
    * Callback to be triggered whenever a download starts.
-   * @type Function
+   * @type {Function}
    */
   onDownloadStarted: null,
 
   /**
    * Callback to be triggered whenever a download finishes successfully. The
    * callback can return an error code to indicate that the data is wrong.
-   * @type Function
+   * @type {Function}
    */
   onDownloadSuccess: null,
 
   /**
    * Callback to be triggered whenever a download fails.
-   * @type Function
+   * @type {Function}
    */
   onDownloadError: null,
 
   /**
    * Checks whether anything needs downloading.
    */
-  _doCheck: function()
+  _doCheck()
   {
     let now = Date.now();
     for (let downloadable of this.dataSource())
     {
-      if (downloadable.lastCheck && now - downloadable.lastCheck > this.maxAbsenceInterval)
+      if (downloadable.lastCheck &&
+          now - downloadable.lastCheck > this.maxAbsenceInterval)
       {
-        // No checks for a long time interval - user must have been offline, e.g.
-        // during a weekend. Increase soft expiration to prevent load peaks on the
-        // server.
+        // No checks for a long time interval - user must have been
+        // offline, e.g.  during a weekend. Increase soft expiration

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

Nit: "offline" should go on the previous line.

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

Done.

+        // to prevent load peaks on the server.
         downloadable.softExpiration += now - downloadable.lastCheck;
       }
       downloadable.lastCheck = now;
@@ -143,11 +149,13 @@ Downloader.prototype =
         this.onExpirationChange(downloadable);
 
       // Does that object need downloading?
-      if (downloadable.softExpiration > now && downloadable.hardExpiration > now)
+      if (downloadable.softExpiration > now &&
+          downloadable.hardExpiration > now)
         continue;

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

Nit: For multi-line conditions I suggest always adding brackets around the body,
makes it more obvious where the if body actually starts. Same below.

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

Done.

 
       // Do not retry downloads too often
-      if (downloadable.lastError && now - downloadable.lastError < this.minRetryInterval)
+      if (downloadable.lastError &&
+          now - downloadable.lastError < this.minRetryInterval)
         continue;
 
       this._download(downloadable, 0);
@@ -157,23 +165,26 @@ Downloader.prototype =
   /**
    * Stops the periodic checks.
    */
-  cancel: function()
+  cancel()
   {
     this._timer.cancel();
   },
 
   /**
    * Checks whether an address is currently being downloaded.
+   * @param {string} url
+   * @return {boolean}
    */
-  isDownloading: function(/**String*/ url) /**Boolean*/
+  isDownloading(url)
   {
     return url in this._downloading;
   },
 
   /**
    * Starts downloading for an object.
+   * @param {Downloadable} downloadable
    */
-  download: function(/**Downloadable*/ downloadable)
+  download(downloadable)
   {
     // Make sure to detach download from the current execution context
     Utils.runAsync(this._download.bind(this, downloadable, 0));
@@ -182,17 +193,20 @@ Downloader.prototype =
   /**
    * Generates the real download URL for an object by appending various
    * parameters.
+   * @param {Downloadable} downloadable
+   * @return {string}
    */
-  getDownloadUrl: function(/**Downloadable*/ downloadable)  /** String*/
+  getDownloadUrl(downloadable)
   {
-    let {addonName, addonVersion, application, applicationVersion, platform, platformVersion} = require("info");
+    const {addonName, addonVersion, application, applicationVersion,
+           platform, platformVersion} = require("info");
     let url = downloadable.redirectURL || downloadable.url;
     if (url.indexOf("?") >= 0)
       url += "&";
     else
       url += "?";
     // We limit the download count to 4+ to keep the request anonymized
-    let downloadCount = downloadable.downloadCount;
+    let {downloadCount} = downloadable;
     if (downloadCount > 4)
       downloadCount = "4+";
     url += "addonName=" + encodeURIComponent(addonName) +
@@ -206,7 +220,7 @@ Downloader.prototype =
     return url;
   },
 
-  _download: function(downloadable, redirects)
+  _download(downloadable, redirects)
   {
     if (this.isDownloading(downloadable.url))
       return;
@@ -220,11 +234,13 @@ Downloader.prototype =
       try
       {
         channelStatus = request.channel.status;
-      } catch (e) {}
+      }
+      catch (e) {}
 
       let responseStatus = request.status;
 
-      Cu.reportError("Adblock Plus: Downloading URL " + downloadable.url + " failed (" + error + ")\n" +
+      Cu.reportError("Adblock Plus: Downloading URL " + downloadable.url +
+                     " failed (" + error + ")\n" +
                      "Download address: " + downloadUrl + "\n" +
                      "Channel status: " + channelStatus + "\n" +
                      "Server response: " + responseStatus);
@@ -235,14 +251,15 @@ Downloader.prototype =
         let redirectCallback = null;
         if (redirects <= this.maxRedirects)
         {
-          redirectCallback = function redirectCallback(url)
+          redirectCallback = (url) =>

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

Nit: Parentheses around parameter are unnecessary here.

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

Done. (Also added the "as-needed" arrow-parens rule, since that's the convention
here.)

           {
             downloadable.redirectURL = url;
             this._download(downloadable, redirects + 1);
-          }.bind(this);
+          };
         }
 
-        this.onDownloadError(downloadable, downloadUrl, error, channelStatus, responseStatus, redirectCallback);
+        this.onDownloadError(downloadable, downloadUrl, error, channelStatus,
+                             responseStatus, redirectCallback);
       }
     }.bind(this);
 
@@ -258,7 +275,8 @@ Downloader.prototype =
       return;
     }
 
-    try {
+    try
+    {
       request.overrideMimeType("text/plain");
       request.channel.loadFlags = request.channel.loadFlags |
                                   request.channel.INHIBIT_CACHING |
@@ -270,19 +288,19 @@ Downloader.prototype =
     }
     catch (e)
     {
-      Cu.reportError(e)
+      Cu.reportError(e);
     }
 
-    request.addEventListener("error", function(event)
+    request.addEventListener("error", event =>
     {
       if (onShutdown.done)
         return;
 
       delete this._downloading[downloadable.url];
       errorCallback("synchronize_connection_error");
-    }.bind(this), false);
+    }, false);
 
-    request.addEventListener("load", function(event)
+    request.addEventListener("load", event =>
     {
       if (onShutdown.done)
         return;
@@ -298,17 +316,20 @@ Downloader.prototype =
 
       downloadable.downloadCount++;
 
-      this.onDownloadSuccess(downloadable, request.responseText, errorCallback, function redirectCallback(url)
-      {
-        if (redirects >= this.maxRedirects)
-          errorCallback("synchronize_connection_error");
-        else
+      this.onDownloadSuccess(
+        downloadable, request.responseText, errorCallback,
+        url =>
         {
-          downloadable.redirectURL = url;
-          this._download(downloadable, redirects + 1);
+          if (redirects >= this.maxRedirects)
+            errorCallback("synchronize_connection_error");
+          else
+          {
+            downloadable.redirectURL = url;
+            this._download(downloadable, redirects + 1);
+          }
         }
-      }.bind(this));
-    }.bind(this), false);
+      );
+    });
 
     request.send(null);
 
@@ -320,9 +341,10 @@ Downloader.prototype =
   /**
    * Produces a soft and a hard expiration interval for a given supplied
    * expiration interval.
+   * @param {number} interval
    * @return {Array} soft and hard expiration interval
    */
-  processExpirationInterval: function(/**Integer*/ interval)
+  processExpirationInterval(interval)
   {
     interval = Math.min(Math.max(interval, 0), this.maxExpirationInterval);
     let soft = Math.round(interval * (Math.random() * 0.4 + 0.8));
@@ -334,61 +356,61 @@ Downloader.prototype =
 
 /**
  * An object that can be downloaded by the downloadable
- * @param {String} url  URL that has to be requested for the object
+ * @param {string} url  URL that has to be requested for the object
  * @constructor
  */
 let Downloadable = exports.Downloadable = function Downloadable(url)
 {
   this.url = url;
-}
+};
 Downloadable.prototype =
 {
   /**
    * URL that has to be requested for the object.
-   * @type String
+   * @type {string}
    */
   url: null,
 
   /**
    * URL that the download was redirected to if any.
-   * @type String
+   * @type {string}
    */
   redirectURL: null,
 
   /**
    * Time of last download error or 0 if the last download was successful.
-   * @type Integer
+   * @type {number}
    */
   lastError: 0,
 
   /**
    * Time of last check whether the object needs downloading.
-   * @type Integer
+   * @type {number}
    */
   lastCheck: 0,
 
   /**
    * Object version corresponding to the last successful download.
-   * @type Integer
+   * @type {number}
    */
   lastVersion: 0,
 
   /**
    * Soft expiration interval, will increase if no checks are performed for a
    * while.
-   * @type Integer
+   * @type {number}
    */
   softExpiration: 0,
 
   /**
    * Hard expiration interval, this is fixed.
-   * @type Integer
+   * @type {number}
    */
   hardExpiration: 0,
-  
+
   /**
    * Number indicating how often the object was downloaded.
-   * @type Integer
+   * @type {number}
    */
-  downloadCount: 0,
+  downloadCount: 0
 };