From 0fc411a4228f7e7055eb15b9fbaeb9cdbdc39e02 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Tue, 27 Oct 2015 11:21:46 +0100 Subject: [PATCH] Bug 1218409 - Eslint rule that checks for balanced listeners. r=miker --- devtools/.eslintrc | 1 + .../docs/balanced-listeners.rst | 11 ++ testing/eslint-plugin-mozilla/docs/index.rst | 5 + testing/eslint-plugin-mozilla/lib/index.js | 2 + .../lib/rules/balanced-listeners.js | 107 ++++++++++++++++++ 5 files changed, 126 insertions(+) create mode 100644 testing/eslint-plugin-mozilla/docs/balanced-listeners.rst create mode 100644 testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js diff --git a/devtools/.eslintrc b/devtools/.eslintrc index 90db6d13397..c88664c6972 100644 --- a/devtools/.eslintrc +++ b/devtools/.eslintrc @@ -26,6 +26,7 @@ // 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, diff --git a/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst b/testing/eslint-plugin-mozilla/docs/balanced-listeners.rst new file mode 100644 index 00000000000..d153edf94f5 --- /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. diff --git a/testing/eslint-plugin-mozilla/docs/index.rst b/testing/eslint-plugin-mozilla/docs/index.rst index 48eb62b1e12..af1f7a17b2c 100644 --- a/testing/eslint-plugin-mozilla/docs/index.rst +++ b/testing/eslint-plugin-mozilla/docs/index.rst @@ -4,6 +4,9 @@ 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. @@ -31,6 +34,7 @@ level invalid. Example configuration:: "rules": { + "mozilla/balanced-listeners": 2, "mozilla/components-imports": 1, "mozilla/import-headjs-globals": 1, "mozilla/mark-test-function-used": 1, @@ -40,6 +44,7 @@ Example configuration:: .. toctree:: :maxdepth: 1 + balanced-listeners components-imports import-headjs-globals mark-test-function-used diff --git a/testing/eslint-plugin-mozilla/lib/index.js b/testing/eslint-plugin-mozilla/lib/index.js index 072289a179b..169f72dcbe3 100644 --- a/testing/eslint-plugin-mozilla/lib/index.js +++ b/testing/eslint-plugin-mozilla/lib/index.js @@ -13,12 +13,14 @@ 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, diff --git a/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js b/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js new file mode 100644 index 00000000000..9ee2f9f8d3d --- /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 + }); + }); + } + }; +};