Bug 1330074 - Enable no-self-assign, no-useless-call and no-useless-return in devtools/. r=jryans draft
authorTim Nguyen <ntim.bugs@gmail.com>
Tue, 10 Jan 2017 22:14:03 +0000
changeset 458711 c10d47aaf56a8227d4642a03fc2b10f3a459ec05
parent 458695 9532ebc3faf001a04f12b97cb9e95224898f22ff
child 458720 399622648910bc509865a3867e20a73b1e8b1494
child 458723 43948a5f55fd37d9201a77483505dfda5f830d3a
child 459364 fb108aaa63c514063c4b693c5141efa6517cdfb8
push id41024
push userbmo:ntim.bugs@gmail.com
push dateTue, 10 Jan 2017 22:14:32 +0000
reviewersjryans
bugs1330074
milestone53.0a1
Bug 1330074 - Enable no-self-assign, no-useless-call and no-useless-return in devtools/. r=jryans MozReview-Commit-ID: A345G1QOOpm
devtools/.eslintrc.js
devtools/client/inspector/inspector-search.js
devtools/client/inspector/rules/rules.js
devtools/client/memory/actions/snapshot.js
devtools/client/netmonitor/request-utils.js
devtools/client/shared/components/tree.js
devtools/client/shared/widgets/view-helpers.js
devtools/client/styleeditor/StyleEditorUtil.jsm
devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
devtools/shared/apps/app-actor-front.js
devtools/shared/event-emitter.js
devtools/shared/security/auth.js
devtools/shared/task.js
--- a/devtools/.eslintrc.js
+++ b/devtools/.eslintrc.js
@@ -278,16 +278,18 @@ module.exports = {
     "no-reserved-keys": "off",
     // Don't restrict usage of specified node modules (not a node environment).
     "no-restricted-modules": "off",
     // Disallow use of assignment in return statement. It is preferable for a
     // single line of code to have only one easily predictable effect.
     "no-return-assign": "error",
     // Allow use of javascript: urls.
     "no-script-url": "off",
+    // Disallow assignments like foo = foo
+    "no-self-assign": "error",
     // Disallow comparisons where both sides are exactly the same.
     "no-self-compare": "error",
     // Disallow use of comma operator.
     "no-sequences": "error",
     // Warn about declaration of variables already declared in the outer scope.
     // This isn't an error because it sometimes is useful to use the same name
     // in a small helper function rather than having to come up with another
     // random name.
@@ -327,16 +329,20 @@ module.exports = {
     "no-unreachable": "error",
     // Disallow global and local variables that aren't used, but allow unused
     // function arguments.
     "no-unused-vars": ["error", {"vars": "all", "args": "none"}],
     // Disallow flow control that escapes from "finally".
     "no-unsafe-finally": "error",
     // Allow using variables before they are defined.
     "no-use-before-define": "off",
+    // Disallow useless Function.prototype.{call/apply}
+    "no-useless-call": "error",
+    // Disallow useless return;
+    "no-useless-return": "error",
     // We use var-only-at-top-level instead of no-var as we allow top level
     // vars.
     "no-var": "off",
     // Allow using TODO/FIXME comments.
     "no-warning-comments": "off",
     // Disallow use of the with statement.
     "no-with": "error",
     // Don't require method and property shorthand syntax for object literals.
--- a/devtools/client/inspector/inspector-search.js
+++ b/devtools/client/inspector/inspector-search.js
@@ -538,12 +538,10 @@ SelectorAutocompleter.prototype = {
           result.suggestions[0][0] === firstPart) {
         result.suggestions = [];
       }
 
       // Wait for the autocomplete-popup to fire its popup-opened event, to make sure
       // the autoSelect item has been selected.
       return this._showPopup(result.suggestions, firstPart, state);
     });
-
-    return;
   }
 };
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -1602,17 +1602,16 @@ RuleViewTool.prototype = {
     location.then(({ source, href, line, column }) => {
       let target = this.inspector.target;
       if (Tools.styleEditor.isTargetSupported(target)) {
         gDevTools.showToolbox(target, "styleeditor").then(function (toolbox) {
           let url = source || href;
           toolbox.getCurrentPanel().selectStyleSheet(url, line, column);
         });
       }
-      return;
     });
   },
 
   onPropertyChanged: function () {
     this.inspector.markDirty();
   },
 
   onViewRefreshed: function () {
--- a/devtools/client/memory/actions/snapshot.js
+++ b/devtools/client/memory/actions/snapshot.js
@@ -639,17 +639,16 @@ exports.fetchImmediatelyDominated = Task
     removeFromCache();
     dispatch({
       type: actions.FETCH_IMMEDIATELY_DOMINATED_END,
       id,
       path: response.path,
       nodes: response.nodes,
       moreChildrenAvailable: response.moreChildrenAvailable,
     });
-    return;
   }
 });
 
 /**
  * Compute and then fetch the dominator tree of the snapshot with the given
  * `id`.
  *
  * @param {HeapAnalysesClient} heapWorker
@@ -695,27 +694,25 @@ exports.refreshSelectedDominatorTree = f
 
     if (snapshot.dominatorTree &&
         !(snapshot.dominatorTree.state === dominatorTreeState.COMPUTED ||
           snapshot.dominatorTree.state === dominatorTreeState.LOADED ||
           snapshot.dominatorTree.state === dominatorTreeState.INCREMENTAL_FETCHING)) {
       return;
     }
 
+    // We need to check for the snapshot state because if there was an error,
+    // we can't continue and if we are still saving or reading the snapshot,
+    // then takeSnapshotAndCensus will finish the job for us
     if (snapshot.state === states.READ) {
       if (snapshot.dominatorTree) {
         yield dispatch(fetchDominatorTree(heapWorker, snapshot.id));
       } else {
         yield dispatch(computeAndFetchDominatorTree(heapWorker, snapshot.id));
       }
-    } else {
-        // If there was an error, we can't continue. If we are still saving or
-        // reading the snapshot, then takeSnapshotAndCensus will finish the job
-        // for us.
-      return;
     }
   };
 };
 
 /**
  * Select the snapshot with the given id.
  *
  * @param {snapshotId} id
--- a/devtools/client/netmonitor/request-utils.js
+++ b/devtools/client/netmonitor/request-utils.js
@@ -26,17 +26,17 @@ const { Task } = require("devtools/share
 function getKeyWithEvent(callback, onlySpaceOrReturn) {
   return function (event) {
     let key = event.target.getAttribute("data-key");
     let filterKeyboardEvent = !onlySpaceOrReturn ||
                               event.keyCode === KeyCodes.DOM_VK_SPACE ||
                               event.keyCode === KeyCodes.DOM_VK_RETURN;
 
     if (key && filterKeyboardEvent) {
-      callback.call(null, key);
+      callback(key);
     }
   };
 }
 
 /**
  * Extracts any urlencoded form data sections (e.g. "?foo=bar&baz=42") from a
  * POST request.
  *
--- a/devtools/client/shared/components/tree.js
+++ b/devtools/client/shared/components/tree.js
@@ -440,38 +440,38 @@ module.exports = createClass({
       return;
     }
 
     this._preventArrowKeyScrolling(e);
 
     switch (e.key) {
       case "ArrowUp":
         this._focusPrevNode();
-        return;
+        break;
 
       case "ArrowDown":
         this._focusNextNode();
-        return;
+        break;
 
       case "ArrowLeft":
         if (this.props.isExpanded(this.props.focused)
             && this.props.getChildren(this.props.focused).length) {
           this._onCollapse(this.props.focused);
         } else {
           this._focusParentNode();
         }
-        return;
+        break;
 
       case "ArrowRight":
         if (!this.props.isExpanded(this.props.focused)) {
           this._onExpand(this.props.focused);
         } else {
           this._focusNextNode();
         }
-        return;
+        break;
     }
   },
 
   /**
    * Sets the previous node relative to the currently focused item, to focused.
    */
   _focusPrevNode: oncePerAnimationFrame(function () {
     // Start a depth first search and keep going until we reach the currently
--- a/devtools/client/shared/widgets/view-helpers.js
+++ b/devtools/client/shared/widgets/view-helpers.js
@@ -1519,35 +1519,35 @@ const WidgetMethods = exports.WidgetMeth
   _onWidgetKeyPress: function (name, event) {
     // Prevent scrolling when pressing navigation keys.
     ViewHelpers.preventScrolling(event);
 
     switch (event.keyCode) {
       case KeyCodes.DOM_VK_UP:
       case KeyCodes.DOM_VK_LEFT:
         this.focusPrevItem();
-        return;
+        break;
       case KeyCodes.DOM_VK_DOWN:
       case KeyCodes.DOM_VK_RIGHT:
         this.focusNextItem();
-        return;
+        break;
       case KeyCodes.DOM_VK_PAGE_UP:
         this.focusItemAtDelta(-(this.pageSize ||
                                (this.itemCount / PAGE_SIZE_ITEM_COUNT_RATIO)));
-        return;
+        break;
       case KeyCodes.DOM_VK_PAGE_DOWN:
         this.focusItemAtDelta(+(this.pageSize ||
                                (this.itemCount / PAGE_SIZE_ITEM_COUNT_RATIO)));
-        return;
+        break;
       case KeyCodes.DOM_VK_HOME:
         this.focusFirstVisibleItem();
-        return;
+        break;
       case KeyCodes.DOM_VK_END:
         this.focusLastVisibleItem();
-        return;
+        break;
     }
   },
 
   /**
    * The mousePress event listener for this container.
    * @param string name
    * @param MouseEvent event
    */
--- a/devtools/client/styleeditor/StyleEditorUtil.jsm
+++ b/devtools/client/styleeditor/StyleEditorUtil.jsm
@@ -225,10 +225,9 @@ function showFilePicker(path, toSave, pa
   if (toSave && suggestedFilename) {
     fp.defaultString = suggestedFilename;
   }
 
   fp.init(parentWindow, getString(key + ".title"), mode);
   fp.appendFilter(getString(key + ".filter"), "*.css");
   fp.appendFilters(fp.filterAll);
   fp.open(fpCallback);
-  return;
 }
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -44,31 +44,25 @@ NewConsoleOutputWrapper.prototype = {
         emitNewMessage: (node, messageId) => {
           this.jsterm.hud.emit("new-messages", new Set([{
             node,
             messageId,
           }]));
         },
         hudProxyClient: this.jsterm.hud.proxy.client,
         onViewSourceInDebugger: frame => {
-          this.toolbox.viewSourceInDebugger.call(
-            this.toolbox,
-            frame.url,
-            frame.line
-          ).then(() =>
+          this.toolbox.viewSourceInDebugger(frame.url, frame.line).then(() =>
             this.jsterm.hud.emit("source-in-debugger-opened")
           );
         },
-        onViewSourceInScratchpad: frame => this.toolbox.viewSourceInScratchpad.call(
-          this.toolbox,
+        onViewSourceInScratchpad: frame => this.toolbox.viewSourceInScratchpad(
           frame.url,
           frame.line
         ),
-        onViewSourceInStyleEditor: frame => this.toolbox.viewSourceInStyleEditor.call(
-          this.toolbox,
+        onViewSourceInStyleEditor: frame => this.toolbox.viewSourceInStyleEditor(
           frame.url,
           frame.line
         ),
         openContextMenu: (e, message) => {
           let { screenX, screenY, target } = e;
 
           let messageEl = target.closest(".message");
           let clipboardText = messageEl ? messageEl.textContent : null;
@@ -87,17 +81,17 @@ NewConsoleOutputWrapper.prototype = {
           return menu;
         },
         openNetworkPanel: (requestId) => {
           return this.toolbox.selectTool("netmonitor").then(panel => {
             return panel.panelWin.NetMonitorController.inspectRequest(requestId);
           });
         },
         sourceMapService: this.toolbox ? this.toolbox._sourceMapService : null,
-        openLink: url => this.jsterm.hud.owner.openLink.call(this.jsterm.hud.owner, url),
+        openLink: url => this.jsterm.hud.owner.openLink(url),
         createElement: nodename => {
           return this.document.createElementNS("http://www.w3.org/1999/xhtml", nodename);
         },
         highlightDomElement: (grip, options = {}) => {
           return this.toolbox && this.toolbox.highlighterUtils
             ? this.toolbox.highlighterUtils.highlightDomValueGrip(grip, options)
             : null;
         },
--- a/devtools/shared/apps/app-actor-front.js
+++ b/devtools/shared/apps/app-actor-front.js
@@ -714,18 +714,16 @@ AppActorFront.prototype = {
           // Fake a appClose event if we didn't got one before uninstall
           if (app.running) {
             app.running = false;
             this._notifyListeners("appClose", app);
           }
           this._apps.delete(manifestURL);
           this._notifyListeners("appUninstall", app);
           break;
-        default:
-          return;
       }
     });
   },
 
   _notifyListeners: function (type, app) {
     this._listeners.forEach(f => {
       f(type, app);
     });
--- a/devtools/shared/event-emitter.js
+++ b/devtools/shared/event-emitter.js
@@ -127,17 +127,17 @@
      *        that this is needed) then use listener
      */
     once(event, listener) {
       let deferred = defer();
 
       let handler = (_, first, ...rest) => {
         this.off(event, handler);
         if (listener) {
-          listener.apply(null, [event, first, ...rest]);
+          listener(event, first, ...rest);
         }
         deferred.resolve(first);
       };
 
       handler._originalListener = listener;
       this.on(event, handler);
 
       return deferred.promise;
--- a/devtools/shared/security/auth.js
+++ b/devtools/shared/security/auth.js
@@ -375,17 +375,17 @@ OOBCert.Client.prototype = {
             // Server previously persisted Client as allowed
             // Step C.5
             // Debugging begins
             transport.hooks = null;
             deferred.resolve(transport);
             break;
           default:
             transport.close(new Error("Invalid auth result: " + authResult));
-            return;
+            break;
         }
       }.bind(this)),
       onClosed(reason) {
         closeDialog();
         // Transport died before auth completed
         transport.hooks = null;
         deferred.reject(reason);
       }
--- a/devtools/shared/task.js
+++ b/devtools/shared/task.js
@@ -153,17 +153,17 @@ var Task = {
    *          returned promise.
    *        - If you specify anything else, you get a promise that is already
    *          resolved with the specified value.
    *
    * @return A promise object where you can register completion callbacks to be
    *         called when the task terminates.
    */
   spawn: function (task) {
-    return createAsyncFunction(task).call(undefined);
+    return createAsyncFunction(task)();
   },
 
   /**
    * Create and return an 'async function' that starts a new task.
    *
    * This is similar to 'spawn' except that it doesn't immediately start
    * the task, it binds the task to the async function's 'this' object and
    * arguments, and it requires the task to be a function.