Skip to content

Commit a44e16c

Browse files
janowsianysindresorhus
janowsiany
authored and
sindresorhus
committed
Add prefer-query-selector rule (#198)
Fixes #171
1 parent db6f62e commit a44e16c

File tree

5 files changed

+271
-2
lines changed

5 files changed

+271
-2
lines changed

‎docs/rules/prefer-query-selector.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Prefer `querySelector` over `getElementById`, `querySelectorAll` over `getElementsByClassName` and `getElementsByTagName`
2+
3+
They are not faster than `querySelector` and it's better to be consistent.
4+
5+
6+
## Fail
7+
8+
```js
9+
document.getElementById('foo');
10+
document.getElementsByClassName('foo bar');
11+
document.getElementsByTagName('main');
12+
document.getElementsByClassName(fn());
13+
```
14+
15+
16+
## Pass
17+
18+
```js
19+
document.querySelector('#foo');
20+
document.querySelector('.bar');
21+
document.querySelector('main #foo .bar');
22+
document.querySelectorAll('.foo .bar');
23+
document.querySelectorAll('li a');
24+
document.querySelector('li').querySelectorAll('a');
25+
```

‎index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ module.exports = {
5353
'unicorn/no-console-spaces': 'error',
5454
'unicorn/no-unreadable-array-destructuring': 'error',
5555
'unicorn/no-unused-properties': 'off',
56-
'unicorn/prefer-node-append': 'error'
56+
'unicorn/prefer-node-append': 'error',
57+
'unicorn/prefer-query-selector': 'error'
5758
}
5859
}
5960
}

‎readme.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ Configure it in `package.json`.
7070
"unicorn/no-console-spaces": "error",
7171
"unicorn/no-unreadable-array-destructuring": "error",
7272
"unicorn/no-unused-properties": "off",
73-
"unicorn/prefer-node-append": "error"
73+
"unicorn/prefer-node-append": "error",
74+
"unicorn/prefer-query-selector": "error"
7475
}
7576
}
7677
}
@@ -106,6 +107,7 @@ Configure it in `package.json`.
106107
- [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) - Disallow unreadable array destructuring.
107108
- [no-unused-properties](docs/rules/no-unused-properties.md) - Disallow unused object properties.
108109
- [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `append` over `appendChild`. *(fixable)*
110+
- [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `querySelector` over `getElementById`, `querySelectorAll` over `getElementsByClassName` and `getElementsByTagName`. *(partly fixable)*
109111

110112

111113
## Recommended config

‎rules/prefer-query-selector.js‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
'use strict';
2+
const getDocsUrl = require('./utils/get-docs-url');
3+
4+
const forbiddenIdentifierNames = new Map([
5+
['getElementById', 'querySelector'],
6+
['getElementsByClassName', 'querySelectorAll'],
7+
['getElementsByTagName', 'querySelectorAll']
8+
]);
9+
10+
const getReplacementForId = value => `#${value}`;
11+
const getReplacementForClass = value => value.match(/\S+/g).map(e => `.${e}`).join('');
12+
const getQuotedReplacement = (node, value) => {
13+
const leftQuote = node.raw.charAt(0);
14+
const rightQuote = node.raw.charAt(node.raw.length - 1);
15+
return `${leftQuote}${value}${rightQuote}`;
16+
};
17+
18+
const getLiteralFix = (fixer, node, identifierName) => {
19+
let replacement = node.raw;
20+
if (identifierName === 'getElementById') {
21+
replacement = getQuotedReplacement(node, getReplacementForId(node.value));
22+
}
23+
24+
if (identifierName === 'getElementsByClassName') {
25+
replacement = getQuotedReplacement(node, getReplacementForClass(node.value));
26+
}
27+
28+
return [fixer.replaceText(node, replacement)];
29+
};
30+
31+
const getTemplateLiteralFix = (fixer, node, identifierName) => {
32+
const fix = [fixer.insertTextAfter(node, '`'), fixer.insertTextBefore(node, '`')];
33+
34+
node.quasis.forEach(templateElement => {
35+
if (identifierName === 'getElementById') {
36+
fix.push(fixer.replaceText(templateElement, getReplacementForId(templateElement.value.cooked)));
37+
}
38+
39+
if (identifierName === 'getElementsByClassName') {
40+
fix.push(fixer.replaceText(templateElement, getReplacementForClass(templateElement.value.cooked)));
41+
}
42+
});
43+
44+
return fix;
45+
};
46+
47+
const canBeFixed = node => {
48+
if (node.type === 'Literal') {
49+
return node.value === null || Boolean(node.value.trim());
50+
}
51+
52+
if (node.type === 'TemplateLiteral') {
53+
return node.expressions.length === 0 &&
54+
node.quasis.some(templateElement => templateElement.value.cooked.trim());
55+
}
56+
57+
return false;
58+
};
59+
60+
const hasValue = node => {
61+
if (node.type === 'Literal') {
62+
return node.value;
63+
}
64+
65+
return true;
66+
};
67+
68+
const fix = (node, identifierName, preferedSelector) => {
69+
const nodeToBeFixed = node.arguments[0];
70+
if (identifierName === 'getElementsByTagName' || !hasValue(nodeToBeFixed)) {
71+
return fixer => fixer.replaceText(node.callee.property, preferedSelector);
72+
}
73+
74+
const getArgumentFix = nodeToBeFixed.type === 'Literal' ? getLiteralFix : getTemplateLiteralFix;
75+
return fixer => [
76+
...getArgumentFix(fixer, nodeToBeFixed, identifierName),
77+
fixer.replaceText(node.callee.property, preferedSelector)
78+
];
79+
};
80+
81+
const create = context => {
82+
return {
83+
CallExpression(node) {
84+
const {callee: {property, type}} = node;
85+
if (!property || type !== 'MemberExpression') {
86+
return;
87+
}
88+
89+
const identifierName = property.name;
90+
const preferedSelector = forbiddenIdentifierNames.get(identifierName);
91+
if (!preferedSelector) {
92+
return;
93+
}
94+
95+
const report = {
96+
node,
97+
message: `Prefer \`${preferedSelector}\` over \`${identifierName}\`.`
98+
};
99+
100+
if (canBeFixed(node.arguments[0])) {
101+
report.fix = fix(node, identifierName, preferedSelector);
102+
}
103+
104+
context.report(report);
105+
}
106+
};
107+
};
108+
109+
module.exports = {
110+
create,
111+
meta: {
112+
docs: {
113+
url: getDocsUrl(__filename)
114+
},
115+
fixable: 'code'
116+
}
117+
};

‎test/prefer-query-selector.js‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import test from 'ava';
2+
import avaRuleTester from 'eslint-ava-rule-tester';
3+
import rule from '../rules/prefer-query-selector';
4+
5+
const ruleTester = avaRuleTester(test, {
6+
env: {
7+
es6: true
8+
}
9+
});
10+
11+
ruleTester.run('prefer-query-selector', rule, {
12+
valid: [
13+
'document.querySelector("#foo");',
14+
'document.querySelector(".bar");',
15+
'document.querySelector("main #foo .bar");',
16+
'document.querySelectorAll(".foo .bar");',
17+
'document.querySelectorAll("li a");',
18+
'document.querySelector("li").querySelectorAll("a");'
19+
],
20+
invalid: [
21+
{
22+
code: 'document.getElementById("foo");',
23+
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}],
24+
output: 'document.querySelector("#foo");'
25+
},
26+
{
27+
code: 'document.getElementsByClassName("foo");',
28+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
29+
output: 'document.querySelectorAll(".foo");'
30+
},
31+
{
32+
code: 'document.getElementsByClassName("foo bar");',
33+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
34+
output: 'document.querySelectorAll(".foo.bar");'
35+
},
36+
{
37+
code: 'document.getElementsByTagName("foo");',
38+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}],
39+
output: 'document.querySelectorAll("foo");'
40+
},
41+
{
42+
code: 'document.getElementById("");',
43+
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}]
44+
},
45+
{
46+
code: 'document.getElementById(\'foo\');',
47+
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}],
48+
output: 'document.querySelector(\'#foo\');'
49+
},
50+
{
51+
code: 'document.getElementsByClassName(\'foo\');',
52+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
53+
output: 'document.querySelectorAll(\'.foo\');'
54+
},
55+
{
56+
code: 'document.getElementsByClassName(\'foo bar\');',
57+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
58+
output: 'document.querySelectorAll(\'.foo.bar\');'
59+
},
60+
{
61+
code: 'document.getElementsByTagName(\'foo\');',
62+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}],
63+
output: 'document.querySelectorAll(\'foo\');'
64+
},
65+
{
66+
code: 'document.getElementsByClassName(\'\');',
67+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
68+
},
69+
{
70+
code: 'document.getElementById(`foo`);',
71+
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}],
72+
output: 'document.querySelector(`#foo`);'
73+
},
74+
{
75+
code: 'document.getElementsByClassName(`foo`);',
76+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
77+
output: 'document.querySelectorAll(`.foo`);'
78+
},
79+
{
80+
code: 'document.getElementsByClassName(`foo bar`);',
81+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
82+
output: 'document.querySelectorAll(`.foo.bar`);'
83+
},
84+
{
85+
code: 'document.getElementsByTagName(`foo`);',
86+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}],
87+
output: 'document.querySelectorAll(`foo`);'
88+
},
89+
{
90+
code: 'document.getElementsByTagName(``);',
91+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}]
92+
},
93+
{
94+
code: 'document.getElementsByClassName(`${fn()}`);', // eslint-disable-line no-template-curly-in-string
95+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
96+
},
97+
{
98+
code: 'document.getElementsByClassName(`foo ${undefined}`);', // eslint-disable-line no-template-curly-in-string
99+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
100+
},
101+
{
102+
code: 'document.getElementsByClassName(null);',
103+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}],
104+
output: 'document.querySelectorAll(null);'
105+
},
106+
{
107+
code: 'document.getElementsByTagName(null);',
108+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}],
109+
output: 'document.querySelectorAll(null);'
110+
},
111+
{
112+
code: 'document.getElementsByClassName(fn());',
113+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
114+
},
115+
{
116+
code: 'document.getElementsByClassName("foo" + fn());',
117+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
118+
},
119+
{
120+
code: 'document.getElementsByClassName(foo + "bar");',
121+
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}]
122+
}
123+
]
124+
});

0 commit comments

Comments
 (0)