author | Florian Quèze <florian@queze.net> |
Wed, 25 Jan 2017 07:03:50 +0100 | |
changeset 330994 | 337f6e703a5ba9b08fee4fca422d243c1e71c5a5 |
parent 330993 | 37161ea3d91bcf16576bf72e6604d7dc1e369ded |
child 330995 | 1604679f84e0a169f8cccece471905811f3d40f5 |
push id | 31256 |
push user | cbook@mozilla.com |
push date | Wed, 25 Jan 2017 12:47:57 +0000 |
treeherder | mozilla-central@c989c7b35227 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | jaws |
bugs | 1331599 |
milestone | 54.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
|
--- a/.eslintrc.js +++ b/.eslintrc.js @@ -4,16 +4,17 @@ module.exports = { // When adding items to this file please check for effects on sub-directories. "plugins": [ "mozilla" ], "rules": { "mozilla/import-globals": "warn", "mozilla/no-import-into-var-and-global": "error", "mozilla/no-useless-parameters": "error", + "mozilla/no-useless-removeEventListener": "error", // No (!foo in bar) or (!object instanceof Class) "no-unsafe-negation": "error", }, "env": { "es6": true }, "parserOptions": {
--- a/tools/lint/docs/linters/eslint-plugin-mozilla.rst +++ b/tools/lint/docs/linters/eslint-plugin-mozilla.rst @@ -127,16 +127,21 @@ imported. no-useless-parameters --------------------- Reject common XPCOM methods called with useless optional parameters (eg. ``Services.io.newURI(url, null, null)``, or non-existent parameters (eg. ``Services.obs.removeObserver(name, observer, false)``). +no-useless-removeEventListener +------------------------------ + +Reject calls to removeEventListener where {once: true} could be used instead. + reject-importGlobalProperties ----------------------------- Rejects calls to ``Cu.importGlobalProperties``. Use of this function is undesirable in some parts of the tree. reject-some-requires
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js +++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/index.js @@ -23,29 +23,31 @@ module.exports = { "import-headjs-globals": require("../lib/rules/import-headjs-globals"), "import-test-globals": require("../lib/rules/import-test-globals"), "mark-test-function-used": require("../lib/rules/mark-test-function-used"), "no-aArgs": require("../lib/rules/no-aArgs"), "no-cpows-in-tests": require("../lib/rules/no-cpows-in-tests"), "no-single-arg-cu-import": require("../lib/rules/no-single-arg-cu-import"), "no-import-into-var-and-global": require("../lib/rules/no-import-into-var-and-global.js"), "no-useless-parameters": require("../lib/rules/no-useless-parameters"), + "no-useless-removeEventListener": require("../lib/rules/no-useless-removeEventListener"), "reject-importGlobalProperties": require("../lib/rules/reject-importGlobalProperties"), "reject-some-requires": require("../lib/rules/reject-some-requires"), "var-only-at-top-level": require("../lib/rules/var-only-at-top-level") }, rulesConfig: { "balanced-listeners": 0, "import-browserjs-globals": 0, "import-globals": 0, "import-headjs-globals": 0, "import-test-globals": 0, "mark-test-function-used": 0, "no-aArgs": 0, "no-cpows-in-tests": 0, "no-single-arg-cu-import": 0, "no-import-into-var-and-global": 0, "no-useless-parameters": 0, + "no-useless-removeEventListener": 0, "reject-importGlobalProperties": 0, "reject-some-requires": 0, "var-only-at-top-level": 0 } };
new file mode 100644 --- /dev/null +++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-useless-removeEventListener.js @@ -0,0 +1,55 @@ +/** + * @fileoverview Reject calls to removeEventListenter where {once: true} could + * be used instead. + * + * 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 +// ----------------------------------------------------------------------------- + +var helpers = require("../helpers"); + +module.exports = function(context) { + + // --------------------------------------------------------------------------- + // Public + // -------------------------------------------------------------------------- + + return { + "CallExpression": function(node) { + let callee = node.callee; + if (callee.type !== "MemberExpression" || + callee.property.type !== "Identifier" || + callee.property.name !== "addEventListener" || + node.arguments.length == 4) { + return; + } + + let listener = node.arguments[1]; + if (listener.type != "FunctionExpression" || !listener.body || + listener.body.type != "BlockStatement" || + !listener.body.body.length || + listener.body.body[0].type != "ExpressionStatement" || + listener.body.body[0].expression.type != "CallExpression") { + return; + } + + let call = listener.body.body[0].expression; + if (call.callee.type == "MemberExpression" && + call.callee.property.type == "Identifier" && + call.callee.property.name == "removeEventListener" && + call.arguments[0].type == "Literal" && + call.arguments[0].value == node.arguments[0].value) { + context.report(call, + "use {once: true} instead of removeEventListener as " + + "the first instruction of the listener"); + } + } + }; +};
--- a/tools/lint/eslint/eslint-plugin-mozilla/package.json +++ b/tools/lint/eslint/eslint-plugin-mozilla/package.json @@ -1,11 +1,11 @@ { "name": "eslint-plugin-mozilla", - "version": "0.2.12", + "version": "0.2.13", "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.", "keywords": [ "eslint", "eslintplugin", "eslint-plugin", "mozilla", "firefox" ],
new file mode 100644 --- /dev/null +++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-useless-removeEventListener.js @@ -0,0 +1,69 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../lib/rules/no-useless-removeEventListener"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +function invalidCode(code) { + let message = "use {once: true} instead of removeEventListener " + + "as the first instruction of the listener"; + return {code: code, errors: [{message: message, type: "CallExpression"}]}; +} + +exports.runTest = function(ruleTester) { + ruleTester.run("no-useless-removeEventListener", rule, { + valid: [ + // Listeners that aren't a function are always valid. + "elt.addEventListener('click', handler);", + "elt.addEventListener('click', handler, true);", + "elt.addEventListener('click', handler, {once: true});", + + // Should not fail on empty functions. + "elt.addEventListener('click', function() {});", + + // Should not reject when removing a listener for another event. + "elt.addEventListener('click', function listener() {" + + " elt.removeEventListener('keypress', listener);" + + "});", + + // Should not reject when there's another instruction before + // removeEventListener. + "elt.addEventListener('click', function listener() {" + + " elt.focus();" + + " elt.removeEventListener('click', listener);" + + "});", + + // Should not reject when wantsUntrusted is true. + "elt.addEventListener('click', function listener() {" + + " elt.removeEventListener('click', listener);" + + "}, false, true);", + ], + invalid: [ + invalidCode("elt.addEventListener('click', function listener() {" + + " elt.removeEventListener('click', listener);" + + "});"), + invalidCode("elt.addEventListener('click', function listener() {" + + " elt.removeEventListener('click', listener, true);" + + "}, true);"), + invalidCode("elt.addEventListener('click', function listener() {" + + " elt.removeEventListener('click', listener);" + + "}, {once: true});"), + invalidCode("elt.addEventListener('click', function listener() {" + + " /* Comment */" + + " elt.removeEventListener('click', listener);" + + "});"), + invalidCode("elt.addEventListener('click', function() {" + + " elt.removeEventListener('click', arguments.callee);" + + "});"), + ] + }); +};