Add no-invalid-remove-event-listener rule (#1216)

Co-authored-by: fisker Cheung <lionkay@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

docs/rules/no-invalid-remove-event-listener.md

@@ -0,0 +1,52 @@
+# Prevent calling `EventTarget#removeEventListener()` with the result of an expression
+
+The [`removeEventListener`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener) function must be called with a reference to the same function that was passed to [`addEventListener`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener). Calling `removeEventListener` with an inline function or the result of an inline `.bind()` call is indicative of an error, and won't actually remove the listener.
+
+## Fail
+
+```js
+window.removeEventListener('click', fn.bind(window));
+```
+
+```js
+window.removeEventListener('click', () => {});
+```
+
+```js
+window.removeEventListener('click', function () {});
+```
+
+```js
+class MyElement extends HTMLElement {
+	handler() {}
+
+	disconnectedCallback() {
+		this.removeEventListener('click', this.handler.bind(this));
+	}
+}
+```
+
+## Pass
+
+```js
+window.removeEventListener('click', listener);
+```
+
+```js
+window.removeEventListener('click', getListener());
+```
+
+```js
+class MyElement extends HTMLElement {
+	constructor() {
+		super();
+		this.handler = this.handler.bind(this);
+	}
+
+	handler() {}
+
+	disconnectedCallback() {
+		this.removeEventListener('click', this.handler);
+	}
+}
+```

index.js

@@ -63,6 +63,7 @@ module.exports = {
 				'unicorn/no-for-loop': 'error',
 				'unicorn/no-hex-escape': 'error',
 				'unicorn/no-instanceof-array': 'error',
+				'unicorn/no-invalid-remove-event-listener': 'error',
 				'unicorn/no-keyword-prefix': 'off',
 				'unicorn/no-lonely-if': 'error',
 				'no-nested-ternary': 'off',

readme.md

@@ -60,6 +60,7 @@ Configure it in `package.json`.
 			"unicorn/no-for-loop": "error",
 			"unicorn/no-hex-escape": "error",
 			"unicorn/no-instanceof-array": "error",
+			"unicorn/no-invalid-remove-event-listener": "error",
 			"unicorn/no-keyword-prefix": "off",
 			"unicorn/no-lonely-if": "error",
 			"no-nested-ternary": "off",
@@ -179,6 +180,7 @@ Each rule has emojis denoting:
 | [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. | ✅ | 🔧 |  |
 | [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. | ✅ | 🔧 |  |
 | [no-instanceof-array](docs/rules/no-instanceof-array.md) | Require `Array.isArray()` instead of `instanceof Array`. | ✅ | 🔧 |  |
+| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. | ✅ |  |  |
 | [no-keyword-prefix](docs/rules/no-keyword-prefix.md) | Disallow identifiers starting with `new` or `class`. |  |  |  |
 | [no-lonely-if](docs/rules/no-lonely-if.md) | Disallow `if` statements as the only statement in `if` blocks without `else`. | ✅ | 🔧 |  |
 | [no-nested-ternary](docs/rules/no-nested-ternary.md) | Disallow nested ternary expressions. | ✅ | 🔧 |  |

rules/no-invalid-remove-event-listener.js

@@ -0,0 +1,56 @@
+'use strict';
+const {getFunctionHeadLocation} = require('eslint-utils');
+const getDocumentationUrl = require('./utils/get-documentation-url.js');
+const {methodCallSelector, matches} = require('./selectors/index.js');
+
+const MESSAGE_ID = 'no-invalid-remove-event-listener';
+const messages = {
+	[MESSAGE_ID]: 'The listener argument should be a function reference.',
+};
+
+const removeEventListenerSelector = [
+	methodCallSelector({
+		method: 'removeEventListener',
+		minimumArguments: 2,
+	}),
+	'[arguments.0.type!="SpreadElement"]',
+	matches([
+		'[arguments.1.type="FunctionExpression"]',
+		'[arguments.1.type="ArrowFunctionExpression"]',
+		methodCallSelector({method: 'bind', path: 'arguments.1'}),
+	]),
+].join('');
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => {
+	return {
+		[removeEventListenerSelector]: node => {
+			const listener = node.arguments[1];
+			if (['ArrowFunctionExpression', 'FunctionExpression'].includes(listener.type)) {
+				return {
+					node: listener,
+					loc: getFunctionHeadLocation(listener, context.getSourceCode()),
+					messageId: MESSAGE_ID,
+				};
+			}
+
+			return {
+				node: listener.callee.property,
+				messageId: MESSAGE_ID,
+			};
+		},
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'problem',
+		docs: {
+			description: 'Prevent calling `EventTarget#removeEventListener()` with the result of an expression.',
+			url: getDocumentationUrl(__filename),
+		},
+		schema: [],
+		messages,
+	},
+};

test/no-invalid-remove-event-listener.mjs

@@ -0,0 +1,55 @@
+import outdent from 'outdent';
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+test.snapshot({
+	valid: [
+		// CallExpression
+		'new el.removeEventListener("click", () => {})',
+		'el?.removeEventListener("click", () => {})',
+		'el.removeEventListener?.("click", () => {})',
+		'el.notRemoveEventListener("click", () => {})',
+		'el[removeEventListener]("click", () => {})',
+
+		// Arguments
+		'el.removeEventListener("click")',
+		'el.removeEventListener()',
+		'el.removeEventListener(() => {})',
+		'el.removeEventListener(...["click", () => {}], () => {})',
+		'el.removeEventListener(() => {}, "click")',
+		'window.removeEventListener("click", bind())',
+		'window.removeEventListener("click", handler.notBind())',
+		'window.removeEventListener("click", handler[bind]())',
+		'window.removeEventListener("click", handler.bind?.())',
+		'window.removeEventListener("click", handler?.bind())',
+
+		'window.removeEventListener(handler)',
+		outdent`
+			class MyComponent {
+				handler() {}
+				disconnectedCallback() {
+					this.removeEventListener('click', this.handler);
+				}
+			}
+		`,
+		'this.removeEventListener("click", getListener())',
+		'el.removeEventListener("scroll", handler)',
+		'el.removeEventListener("keydown", obj.listener)',
+		'removeEventListener("keyup", () => {})',
+		'removeEventListener("keydown", function () {})',
+
+	],
+	invalid: [
+		'window.removeEventListener("scroll", handler.bind(abc))',
+		'window.removeEventListener("scroll", this.handler.bind(abc))',
+		'window.removeEventListener("click", () => {})',
+		'window.removeEventListener("keydown", function () {})',
+		'el.removeEventListener("click", (e) => { e.preventDefault(); })',
+		'el.removeEventListener("mouseover", fn.bind(abc))',
+		'el.removeEventListener("mouseout", function (e) {})',
+		'el.removeEventListener("mouseout", function (e) {}, true)',
+		'el.removeEventListener("click", function (e) {}, ...moreArguments)',
+		'el.removeEventListener(() => {}, () => {}, () => {})',
+	],
+});

test/snapshots/no-invalid-remove-event-listener.mjs.md

@@ -0,0 +1,105 @@
+# Snapshot report for `test/no-invalid-remove-event-listener.mjs`
+
+The actual snapshot is saved in `no-invalid-remove-event-listener.mjs.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## Invalid #1
+      1 | window.removeEventListener("scroll", handler.bind(abc))
+
+> Error 1/1
+
+    `␊
+    > 1 | window.removeEventListener("scroll", handler.bind(abc))␊
+        |                                              ^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #2
+      1 | window.removeEventListener("scroll", this.handler.bind(abc))
+
+> Error 1/1
+
+    `␊
+    > 1 | window.removeEventListener("scroll", this.handler.bind(abc))␊
+        |                                                   ^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #3
+      1 | window.removeEventListener("click", () => {})
+
+> Error 1/1
+
+    `␊
+    > 1 | window.removeEventListener("click", () => {})␊
+        |                                        ^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #4
+      1 | window.removeEventListener("keydown", function () {})
+
+> Error 1/1
+
+    `␊
+    > 1 | window.removeEventListener("keydown", function () {})␊
+        |                                       ^^^^^^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #5
+      1 | el.removeEventListener("click", (e) => { e.preventDefault(); })
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener("click", (e) => { e.preventDefault(); })␊
+        |                                     ^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #6
+      1 | el.removeEventListener("mouseover", fn.bind(abc))
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener("mouseover", fn.bind(abc))␊
+        |                                        ^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #7
+      1 | el.removeEventListener("mouseout", function (e) {})
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener("mouseout", function (e) {})␊
+        |                                    ^^^^^^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #8
+      1 | el.removeEventListener("mouseout", function (e) {}, true)
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener("mouseout", function (e) {}, true)␊
+        |                                    ^^^^^^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #9
+      1 | el.removeEventListener("click", function (e) {}, ...moreArguments)
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener("click", function (e) {}, ...moreArguments)␊
+        |                                 ^^^^^^^^^ The listener argument should be a function reference.␊
+    `
+
+## Invalid #10
+      1 | el.removeEventListener(() => {}, () => {}, () => {})
+
+> Error 1/1
+
+    `␊
+    > 1 | el.removeEventListener(() => {}, () => {}, () => {})␊
+        |                                     ^^ The listener argument should be a function reference.␊
+    `