Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 27 Oct 2015 11:21:46 +0100
changeset 305098 604e693761bdc5c28f11afad54674f3856e7f518
parent 305097 167cd2f19d9368498359f528df23acde21fc5b16
child 305099 f1cef30cb767723c80ffbe186fd17adfca12cef6
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1218409
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker
devtools/.eslintrc
testing/eslint-plugin-mozilla/docs/balanced-listeners.rst
testing/eslint-plugin-mozilla/docs/index.rst
testing/eslint-plugin-mozilla/lib/index.js
testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js
--- a/devtools/.eslintrc
+++ b/devtools/.eslintrc
@@ -21,16 +21,17 @@
     "XPCNativeWrapper": true,
     "XPCOMUtils": true,
   },
   "rules": {
     // These are the rules that have been configured so far to match the
     // devtools coding style.
 
     // Rules from the mozilla plugin
+    "mozilla/balanced-listeners": 2,
     "mozilla/components-imports": 1,
     "mozilla/import-headjs-globals": 1,
     "mozilla/mark-test-function-used": 1,
     "mozilla/var-only-at-top-level": 1,
 
     // Disallow using variables outside the blocks they are defined (especially
     // since only let and const are used, see "no-var").
     "block-scoped-var": 2,
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst
@@ -0,0 +1,11 @@
+.. _balanced-listeners:
+
+==================
+balanced-listeners
+==================
+
+Rule Details
+------------
+
+Checks that for every occurences of 'addEventListener' or 'on' there is an
+occurence of 'removeEventListener' or 'off' with the same event name.
--- a/testing/eslint-plugin-mozilla/docs/index.rst
+++ b/testing/eslint-plugin-mozilla/docs/index.rst
@@ -1,14 +1,17 @@
 .. _index:
 
 =====================
 Mozilla ESLint Plugin
 =====================
 
+``balanced-listeners`` checks that every addEventListener has a
+removeEventListener (and does the same for on/off).
+
 ``components-imports`` adds the filename of imported files e.g.
 ``Cu.import("some/path/Blah.jsm")`` adds Blah to the global scope.
 
 ``import-headjs-globals`` imports globals from head.js and from any files that
 should be imported by head.js (as far as we can correctly resolve the path).
 
 ``mark-test-function-used`` simply marks test (the test method) as used. This
 avoids ESLint telling us that the function is never called.
@@ -26,21 +29,23 @@ level invalid.
 | 1     | Warning               |
 +-------+-----------------------+
 | 2     | Error                 |
 +-------+-----------------------+
 
 Example configuration::
 
    "rules": {
+     "mozilla/balanced-listeners": 2,
      "mozilla/components-imports": 1,
      "mozilla/import-headjs-globals": 1,
      "mozilla/mark-test-function-used": 1,
      "mozilla/var-only-at-top-level": 1,
    }
 
 .. toctree::
    :maxdepth: 1
 
+   balanced-listeners
    components-imports
    import-headjs-globals
    mark-test-function-used
    var-only-at-top-level
--- a/testing/eslint-plugin-mozilla/lib/index.js
+++ b/testing/eslint-plugin-mozilla/lib/index.js
@@ -8,20 +8,22 @@
 "use strict";
 
 //------------------------------------------------------------------------------
 // Plugin Definition
 //------------------------------------------------------------------------------
 
 module.exports = {
   rules: {
+    "balanced-listeners": require("../lib/rules/balanced-listeners"),
     "components-imports": require("../lib/rules/components-imports"),
     "import-headjs-globals": require("../lib/rules/import-headjs-globals"),
     "mark-test-function-used": require("../lib/rules/mark-test-function-used"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level")
   },
   rulesConfig: {
+    "balanced-listeners": 0,
     "components-imports": 0,
     "import-headjs-globals": 0,
     "mark-test-function-used": 0,
     "var-only-at-top-level": 0
   }
 };
new file mode 100644
--- /dev/null
+++ b/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js
@@ -0,0 +1,107 @@
+/**
+ * @fileoverview Check that there's a removeEventListener for each
+ * addEventListener and an off for each on.
+ * Note that for now, this rule is rather simple in that it only checks that
+ * for each event name there is both an add and remove listener. It doesn't
+ * check that these are called on the right objects or with the same callback.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function(context) {
+  //--------------------------------------------------------------------------
+  // Helpers
+  //--------------------------------------------------------------------------
+
+  var DICTIONARY = {
+    "addEventListener": "removeEventListener",
+    "on": "off"
+  };
+  // Invert this dictionary to make it easy later.
+  var INVERTED_DICTIONARY = {};
+  for (var i in DICTIONARY) {
+    INVERTED_DICTIONARY[DICTIONARY[i]] = i;
+  }
+
+  // Collect the add/remove listeners in these 2 arrays.
+  var addedListeners = [];
+  var removedListeners = [];
+
+  function addAddedListener(node) {
+    addedListeners.push({
+      functionName: node.callee.property.name,
+      type: node.arguments[0].value,
+      node: node.callee.property,
+      useCapture: node.arguments[2] ? node.arguments[2].value : null
+    });
+  }
+
+  function addRemovedListener(node) {
+    removedListeners.push({
+      functionName: node.callee.property.name,
+      type: node.arguments[0].value,
+      useCapture: node.arguments[2] ? node.arguments[2].value : null
+    });
+  }
+
+  function getUnbalancedListeners() {
+    var unbalanced = [];
+
+    for (var i = 0; i < addedListeners.length; i ++) {
+      if (!hasRemovedListener(addedListeners[i])) {
+        unbalanced.push(addedListeners[i]);
+      }
+    }
+    addedListeners = removedListeners = [];
+
+    return unbalanced;
+  }
+
+  function hasRemovedListener(addedListener) {
+    for (var i = 0; i < removedListeners.length; i ++) {
+      var listener = removedListeners[i];
+      if (DICTIONARY[addedListener.functionName] === listener.functionName &&
+          addedListener.type === listener.type &&
+          addedListener.useCapture === listener.useCapture) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  //--------------------------------------------------------------------------
+  // Public
+  //--------------------------------------------------------------------------
+
+  return {
+    CallExpression: function(node) {
+      if (node.callee.type === "MemberExpression") {
+        var listenerMethodName = node.callee.property.name;
+
+        if (DICTIONARY.hasOwnProperty(listenerMethodName)) {
+          addAddedListener(node);
+        } else if (INVERTED_DICTIONARY.hasOwnProperty(listenerMethodName)) {
+          addRemovedListener(node);
+        }
+      }
+    },
+
+    "Program:exit": function() {
+      getUnbalancedListeners().forEach(function(listener) {
+        context.report(listener.node,
+          "No corresponding '{{functionName}}({{type}})' was found.", {
+          functionName: DICTIONARY[listener.functionName],
+          type: listener.type
+        });
+      });
+    }
+  };
+};