Improve prefer-query-selector (#908)

sindresorhus · Nov 24, 2020 · 635601d · 635601d
1 parent 9e4dcb4
commit 635601d
Show file tree

Hide file tree

Showing 4 changed files with 432 additions and 131 deletions.
diff --git a/rules/prefer-query-selector.js b/rules/prefer-query-selector.js
@@ -1,11 +1,21 @@
 'use strict';
 const getDocumentationUrl = require('./utils/get-documentation-url');
+const methodSelector = require('./utils/method-selector');
+const {notDomNodeSelector} = require('./utils/not-dom-node');
 
 const MESSAGE_ID = 'prefer-query-selector';
 const messages = {
 	[MESSAGE_ID]: 'Prefer `.{{replacement}}()` over `.{{method}}()`.'
 };
 
+const selector = [
+	methodSelector({
+		names: ['getElementById', 'getElementsByClassName', 'getElementsByTagName'],
+		length: 1
+	}),
+	notDomNodeSelector('callee.object')
+].join('');
+
 const forbiddenIdentifierNames = new Map([
 	['getElementById', 'querySelector'],
 	['getElementsByClassName', 'querySelectorAll'],
@@ -93,37 +103,27 @@ const fix = (node, identifierName, preferedSelector) => {
 
 const create = context => {
 	return {
-		CallExpression(node) {
-			const {callee: {property, type}} = node;
-			if (!property || type !== 'MemberExpression') {
-				return;
-			}
-
-			const identifierName = property.name;
-			const preferedSelector = forbiddenIdentifierNames.get(identifierName);
+		[selector](node) {
+			const method = node.callee.property.name;
+			const preferedSelector = forbiddenIdentifierNames.get(method);
 			if (!preferedSelector) {
 				return;
 			}
 
-			const [firstArgument] = node.arguments;
-			if (node.arguments.length !== 1 || firstArgument.type === 'SpreadElement') {
-				return;
-			}
-
-			const report = {
-				node,
+			const problem = {
+				node: node.callee.property,
 				messageId: MESSAGE_ID,
 				data: {
 					replacement: preferedSelector,
-					method: identifierName
+					method
 				}
 			};
 
 			if (canBeFixed(node.arguments[0])) {
-				report.fix = fix(node, identifierName, preferedSelector);
+				problem.fix = fix(node, method, preferedSelector);
 			}
 
-			context.report(report);
+			context.report(problem);
 		}
 	};
 };

diff --git a/test/prefer-query-selector.js b/test/prefer-query-selector.js
@@ -1,130 +1,63 @@
 import {test} from './utils/test';
-
-const createError = (method, replacement) => ({
-	messageId: 'prefer-query-selector',
-	data: {method, replacement}
-});
+import notDomNodeTypes from './utils/not-dom-node-types';
+import {outdent} from 'outdent';
 
 test({
 	valid: [
-		// More or less arguments
+		// Not `CallExpression`
+		'new document.getElementById(foo);',
+		// Not `MemberExpression`
+		'getElementById(foo);',
+		// `callee.property` is not a `Identifier`
+		'document[\'getElementById\'](bar);',
+		// Computed
+		'document[getElementById](bar);',
+		// Not listed method
+		'document.foo(bar);',
+		// More or less argument(s)
 		'document.getElementById();',
 		'document.getElementsByClassName("foo", "bar");',
 		'document.getElementById(...["id"]);',
 
+		// `callee.object` is not a DOM Node,
+		...notDomNodeTypes.map(data => `(${data}).getElementById(foo)`),
+
 		'document.querySelector("#foo");',
 		'document.querySelector(".bar");',
 		'document.querySelector("main #foo .bar");',
 		'document.querySelectorAll(".foo .bar");',
 		'document.querySelectorAll("li a");',
 		'document.querySelector("li").querySelectorAll("a");'
 	],
-	invalid: [
-		{
-			code: 'document.getElementById("foo");',
-			errors: [createError('getElementById', 'querySelector')],
-			output: 'document.querySelector("#foo");'
-		},
-		{
-			code: 'document.getElementsByClassName("foo");',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(".foo");'
-		},
-		{
-			code: 'document.getElementsByClassName("foo bar");',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(".foo.bar");'
-		},
-		{
-			code: 'document.getElementsByTagName("foo");',
-			errors: [createError('getElementsByTagName', 'querySelectorAll')],
-			output: 'document.querySelectorAll("foo");'
-		},
-		{
-			code: 'document.getElementById("");',
-			errors: [createError('getElementById', 'querySelector')]
-		},
-		{
-			code: 'document.getElementById(\'foo\');',
-			errors: [createError('getElementById', 'querySelector')],
-			output: 'document.querySelector(\'#foo\');'
-		},
-		{
-			code: 'document.getElementsByClassName(\'foo\');',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(\'.foo\');'
-		},
-		{
-			code: 'document.getElementsByClassName(\'foo bar\');',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(\'.foo.bar\');'
-		},
-		{
-			code: 'document.getElementsByTagName(\'foo\');',
-			errors: [createError('getElementsByTagName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(\'foo\');'
-		},
-		{
-			code: 'document.getElementsByClassName(\'\');',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementById(`foo`);',
-			errors: [createError('getElementById', 'querySelector')],
-			output: 'document.querySelector(`#foo`);'
-		},
-		{
-			code: 'document.getElementsByClassName(`foo`);',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(`.foo`);'
-		},
-		{
-			code: 'document.getElementsByClassName(`foo bar`);',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(`.foo.bar`);'
-		},
-		{
-			code: 'document.getElementsByTagName(`foo`);',
-			errors: [createError('getElementsByTagName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(`foo`);'
-		},
-		{
-			code: 'document.getElementsByTagName(``);',
-			errors: [createError('getElementsByTagName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementsByClassName(`${fn()}`);', // eslint-disable-line no-template-curly-in-string
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementsByClassName(`foo ${undefined}`);', // eslint-disable-line no-template-curly-in-string
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementsByClassName(null);',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(null);'
-		},
-		{
-			code: 'document.getElementsByTagName(null);',
-			errors: [createError('getElementsByTagName', 'querySelectorAll')],
-			output: 'document.querySelectorAll(null);'
-		},
-		{
-			code: 'document.getElementsByClassName(fn());',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementsByClassName("foo" + fn());',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		},
-		{
-			code: 'document.getElementsByClassName(foo + "bar");',
-			errors: [createError('getElementsByClassName', 'querySelectorAll')]
-		}
-	]
+	invalid: []
 });
 
 test.visualize([
-	'document.getElementById("foo");'
+	'document.getElementById("foo");',
+	'document.getElementsByClassName("foo");',
+	'document.getElementsByClassName("foo bar");',
+	'document.getElementsByTagName("foo");',
+	'document.getElementById("");',
+	'document.getElementById(\'foo\');',
+	'document.getElementsByClassName(\'foo\');',
+	'document.getElementsByClassName(\'foo bar\');',
+	'document.getElementsByTagName(\'foo\');',
+	'document.getElementsByClassName(\'\');',
+	'document.getElementById(`foo`);',
+	'document.getElementsByClassName(`foo`);',
+	'document.getElementsByClassName(`foo bar`);',
+	'document.getElementsByTagName(`foo`);',
+	'document.getElementsByTagName(``);',
+	'document.getElementsByClassName(`${fn()}`);', // eslint-disable-line no-template-curly-in-string
+	'document.getElementsByClassName(`foo ${undefined}`);', // eslint-disable-line no-template-curly-in-string
+	'document.getElementsByClassName(null);',
+	'document.getElementsByTagName(null);',
+	'document.getElementsByClassName(fn());',
+	'document.getElementsByClassName("foo" + fn());',
+	'document.getElementsByClassName(foo + "bar");',
+	outdent`
+		for (const div of document.body.getElementById("id").getElementsByClassName("class")) {
+			console.log(div.getElementsByTagName("div"));
+		}
+	`
 ]);