prefer-query-selector: Add support for getElementsByName (#2398)

Co-authored-by: fisker Cheung <lionkay@gmail.com>
sindresorhus · Jul 25, 2024 · e511ffd · e511ffd
1 parent 4db75c4
commit e511ffd
Show file tree

Hide file tree

Showing 6 changed files with 196 additions and 4 deletions.
diff --git a/docs/rules/prefer-query-selector.md b/docs/rules/prefer-query-selector.md
@@ -1,4 +1,4 @@
-# Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`
+# Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()` and `.getElementsByName()`
 
 💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
 

diff --git a/readme.md b/readme.md
@@ -204,7 +204,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
 | [prefer-object-from-entries](docs/rules/prefer-object-from-entries.md)                           | Prefer using `Object.fromEntries(…)` to transform a list of key-value pairs into an object.                                                                                                                       | ✅  | 🔧 |    |
 | [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md)                     | Prefer omitting the `catch` binding parameter.                                                                                                                                                                    | ✅  | 🔧 |    |
 | [prefer-prototype-methods](docs/rules/prefer-prototype-methods.md)                               | Prefer borrowing methods from the prototype instead of the instance.                                                                                                                                              | ✅  | 🔧 |    |
-| [prefer-query-selector](docs/rules/prefer-query-selector.md)                                     | Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`.                                                                         | ✅  | 🔧 |    |
+| [prefer-query-selector](docs/rules/prefer-query-selector.md)                                     | Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()` and `.getElementsByName()`.                                              | ✅  | 🔧 |    |
 | [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md)                                       | Prefer `Reflect.apply()` over `Function#apply()`.                                                                                                                                                                 | ✅  | 🔧 |    |
 | [prefer-regexp-test](docs/rules/prefer-regexp-test.md)                                           | Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.                                                                                                                                                 | ✅  | 🔧 | 💡 |
 | [prefer-set-has](docs/rules/prefer-set-has.md)                                                   | Prefer `Set#has()` over `Array#includes()` when checking for existence or non-existence.                                                                                                                          | ✅  | 🔧 | 💡 |

diff --git a/rules/prefer-query-selector.js b/rules/prefer-query-selector.js
@@ -11,17 +11,37 @@ const disallowedIdentifierNames = new Map([
 	['getElementById', 'querySelector'],
 	['getElementsByClassName', 'querySelectorAll'],
 	['getElementsByTagName', 'querySelectorAll'],
+	['getElementsByName', 'querySelectorAll'],
 ]);
 
 const getReplacementForId = value => `#${value}`;
 const getReplacementForClass = value => value.match(/\S+/g).map(className => `.${className}`).join('');
+const getReplacementForName = (value, originQuote) => `[name=${wrapQuoted(value, originQuote)}]`;
 
 const getQuotedReplacement = (node, value) => {
 	const leftQuote = node.raw.charAt(0);
 	const rightQuote = node.raw.at(-1);
 	return `${leftQuote}${value}${rightQuote}`;
 };
 
+const wrapQuoted = (value, originalQuote) => {
+	switch (originalQuote) {
+		case '\'': {
+			return `"${value}"`;
+		}
+
+		case '"': {
+			return `'${value}'`;
+		}
+
+		case '`': {
+			return `'${value}'`;
+		}
+
+		// No default
+	}
+};
+
 function * getLiteralFix(fixer, node, identifierName) {
 	let replacement = node.raw;
 	if (identifierName === 'getElementById') {
@@ -32,6 +52,11 @@ function * getLiteralFix(fixer, node, identifierName) {
 		replacement = getQuotedReplacement(node, getReplacementForClass(node.value));
 	}
 
+	if (identifierName === 'getElementsByName') {
+		const quoted = node.raw.charAt(0);
+		replacement = getQuotedReplacement(node, getReplacementForName(node.value, quoted));
+	}
+
 	yield fixer.replaceText(node, replacement);
 }
 
@@ -53,6 +78,14 @@ function * getTemplateLiteralFix(fixer, node, identifierName) {
 				getReplacementForClass(templateElement.value.cooked),
 			);
 		}
+
+		if (identifierName === 'getElementsByName') {
+			const quoted = node.raw ? node.raw.charAt(0) : '"';
+			yield fixer.replaceText(
+				templateElement,
+				getReplacementForName(templateElement.value.cooked, quoted),
+			);
+		}
 	}
 }
 
@@ -91,7 +124,7 @@ const create = () => ({
 	CallExpression(node) {
 		if (
 			!isMethodCall(node, {
-				methods: ['getElementById', 'getElementsByClassName', 'getElementsByTagName'],
+				methods: ['getElementById', 'getElementsByClassName', 'getElementsByTagName', 'getElementsByName'],
 				argumentsLength: 1,
 				optionalCall: false,
 				optionalMember: false,
@@ -127,7 +160,7 @@ module.exports = {
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`.',
+			description: 'Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()` and `.getElementsByName()`.',
 			recommended: true,
 		},
 		fixable: 'code',

diff --git a/test/prefer-query-selector.mjs b/test/prefer-query-selector.mjs
@@ -30,6 +30,7 @@ test.snapshot({
 		'document.querySelectorAll(".foo .bar");',
 		'document.querySelectorAll("li a");',
 		'document.querySelector("li").querySelectorAll("a");',
+		'document.getElementsByName();',
 	],
 	invalid: [
 		'document.getElementById("foo");',
@@ -61,5 +62,13 @@ test.snapshot({
 		`,
 		// #1030
 		'e.getElementById(3)',
+		'document.getElementsByName("foo");',
+		'document.getElementsByName(\'foo\');',
+		'document.getElementsByName(`foo`);',
+		'document.getElementsByName(`${\'foo\'}`);', // eslint-disable-line no-template-curly-in-string
+		'document.getElementsByName(null);',
+		'document.getElementsByName("");',
+		'document.getElementsByName(foo + "bar");',
+		'document.getElementsByName("multiple name should be fixable");',
 	],
 });
diff --git a/test/snapshots/prefer-query-selector.mjs.md b/test/snapshots/prefer-query-selector.mjs.md
@@ -477,3 +477,153 @@ Generated by [AVA](https://avajs.dev).
     > 1 | e.getElementById(3)␊
         |   ^^^^^^^^^^^^^^ Prefer \`.querySelector()\` over \`.getElementById()\`.␊
     `
+
+## invalid(25): document.getElementsByName("foo");
+
+> Input
+
+    `␊
+      1 | document.getElementsByName("foo");␊
+    `
+
+> Output
+
+    `␊
+      1 | document.querySelectorAll("[name='foo']");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName("foo");␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(26): document.getElementsByName('foo');
+
+> Input
+
+    `␊
+      1 | document.getElementsByName('foo');␊
+    `
+
+> Output
+
+    `␊
+      1 | document.querySelectorAll('[name="foo"]');␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName('foo');␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(27): document.getElementsByName(`foo`);
+
+> Input
+
+    `␊
+      1 | document.getElementsByName(\`foo\`);␊
+    `
+
+> Output
+
+    `␊
+      1 | document.querySelectorAll(\`[name='foo']\`);␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName(\`foo\`);␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(28): document.getElementsByName(`${'foo'}`);
+
+> Input
+
+    `␊
+      1 | document.getElementsByName(\`${'foo'}\`);␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName(\`${'foo'}\`);␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(29): document.getElementsByName(null);
+
+> Input
+
+    `␊
+      1 | document.getElementsByName(null);␊
+    `
+
+> Output
+
+    `␊
+      1 | document.querySelectorAll(null);␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName(null);␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(30): document.getElementsByName("");
+
+> Input
+
+    `␊
+      1 | document.getElementsByName("");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName("");␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(31): document.getElementsByName(foo + "bar");
+
+> Input
+
+    `␊
+      1 | document.getElementsByName(foo + "bar");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName(foo + "bar");␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
+
+## invalid(32): document.getElementsByName("multiple name should be fixable");
+
+> Input
+
+    `␊
+      1 | document.getElementsByName("multiple name should be fixable");␊
+    `
+
+> Output
+
+    `␊
+      1 | document.querySelectorAll("[name='multiple name should be fixable']");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | document.getElementsByName("multiple name should be fixable");␊
+        |          ^^^^^^^^^^^^^^^^^ Prefer \`.querySelectorAll()\` over \`.getElementsByName()\`.␊
+    `
diff --git a/test/snapshots/prefer-query-selector.mjs.snap b/test/snapshots/prefer-query-selector.mjs.snap