-
-
Notifications
You must be signed in to change notification settings - Fork 368
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
prefer-query-selector
rule (#198)
Fixes #171
- Loading branch information
1 parent
db6f62e
commit a44e16c
Showing
5 changed files
with
271 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Prefer `querySelector` over `getElementById`, `querySelectorAll` over `getElementsByClassName` and `getElementsByTagName` | ||
|
||
They are not faster than `querySelector` and it's better to be consistent. | ||
|
||
|
||
## Fail | ||
|
||
```js | ||
document.getElementById('foo'); | ||
document.getElementsByClassName('foo bar'); | ||
document.getElementsByTagName('main'); | ||
document.getElementsByClassName(fn()); | ||
``` | ||
|
||
|
||
## Pass | ||
|
||
```js | ||
document.querySelector('#foo'); | ||
document.querySelector('.bar'); | ||
document.querySelector('main #foo .bar'); | ||
document.querySelectorAll('.foo .bar'); | ||
document.querySelectorAll('li a'); | ||
document.querySelector('li').querySelectorAll('a'); | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
'use strict'; | ||
const getDocsUrl = require('./utils/get-docs-url'); | ||
|
||
const forbiddenIdentifierNames = new Map([ | ||
['getElementById', 'querySelector'], | ||
['getElementsByClassName', 'querySelectorAll'], | ||
['getElementsByTagName', 'querySelectorAll'] | ||
]); | ||
|
||
const getReplacementForId = value => `#${value}`; | ||
const getReplacementForClass = value => value.match(/\S+/g).map(e => `.${e}`).join(''); | ||
const getQuotedReplacement = (node, value) => { | ||
const leftQuote = node.raw.charAt(0); | ||
const rightQuote = node.raw.charAt(node.raw.length - 1); | ||
return `${leftQuote}${value}${rightQuote}`; | ||
}; | ||
|
||
const getLiteralFix = (fixer, node, identifierName) => { | ||
let replacement = node.raw; | ||
if (identifierName === 'getElementById') { | ||
replacement = getQuotedReplacement(node, getReplacementForId(node.value)); | ||
} | ||
|
||
if (identifierName === 'getElementsByClassName') { | ||
replacement = getQuotedReplacement(node, getReplacementForClass(node.value)); | ||
} | ||
|
||
return [fixer.replaceText(node, replacement)]; | ||
}; | ||
|
||
const getTemplateLiteralFix = (fixer, node, identifierName) => { | ||
const fix = [fixer.insertTextAfter(node, '`'), fixer.insertTextBefore(node, '`')]; | ||
|
||
node.quasis.forEach(templateElement => { | ||
if (identifierName === 'getElementById') { | ||
fix.push(fixer.replaceText(templateElement, getReplacementForId(templateElement.value.cooked))); | ||
} | ||
|
||
if (identifierName === 'getElementsByClassName') { | ||
fix.push(fixer.replaceText(templateElement, getReplacementForClass(templateElement.value.cooked))); | ||
} | ||
}); | ||
|
||
return fix; | ||
}; | ||
|
||
const canBeFixed = node => { | ||
if (node.type === 'Literal') { | ||
return node.value === null || Boolean(node.value.trim()); | ||
} | ||
|
||
if (node.type === 'TemplateLiteral') { | ||
return node.expressions.length === 0 && | ||
node.quasis.some(templateElement => templateElement.value.cooked.trim()); | ||
} | ||
|
||
return false; | ||
}; | ||
|
||
const hasValue = node => { | ||
if (node.type === 'Literal') { | ||
return node.value; | ||
} | ||
|
||
return true; | ||
}; | ||
|
||
const fix = (node, identifierName, preferedSelector) => { | ||
const nodeToBeFixed = node.arguments[0]; | ||
if (identifierName === 'getElementsByTagName' || !hasValue(nodeToBeFixed)) { | ||
return fixer => fixer.replaceText(node.callee.property, preferedSelector); | ||
} | ||
|
||
const getArgumentFix = nodeToBeFixed.type === 'Literal' ? getLiteralFix : getTemplateLiteralFix; | ||
return fixer => [ | ||
...getArgumentFix(fixer, nodeToBeFixed, identifierName), | ||
fixer.replaceText(node.callee.property, 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); | ||
if (!preferedSelector) { | ||
return; | ||
} | ||
|
||
const report = { | ||
node, | ||
message: `Prefer \`${preferedSelector}\` over \`${identifierName}\`.` | ||
}; | ||
|
||
if (canBeFixed(node.arguments[0])) { | ||
report.fix = fix(node, identifierName, preferedSelector); | ||
} | ||
|
||
context.report(report); | ||
} | ||
}; | ||
}; | ||
|
||
module.exports = { | ||
create, | ||
meta: { | ||
docs: { | ||
url: getDocsUrl(__filename) | ||
}, | ||
fixable: 'code' | ||
} | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
import test from 'ava'; | ||
import avaRuleTester from 'eslint-ava-rule-tester'; | ||
import rule from '../rules/prefer-query-selector'; | ||
|
||
const ruleTester = avaRuleTester(test, { | ||
env: { | ||
es6: true | ||
} | ||
}); | ||
|
||
ruleTester.run('prefer-query-selector', rule, { | ||
valid: [ | ||
'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: [{message: 'Prefer `querySelector` over `getElementById`.'}], | ||
output: 'document.querySelector("#foo");' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName("foo");', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(".foo");' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName("foo bar");', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(".foo.bar");' | ||
}, | ||
{ | ||
code: 'document.getElementsByTagName("foo");', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}], | ||
output: 'document.querySelectorAll("foo");' | ||
}, | ||
{ | ||
code: 'document.getElementById("");', | ||
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementById(\'foo\');', | ||
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}], | ||
output: 'document.querySelector(\'#foo\');' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(\'foo\');', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(\'.foo\');' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(\'foo bar\');', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(\'.foo.bar\');' | ||
}, | ||
{ | ||
code: 'document.getElementsByTagName(\'foo\');', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}], | ||
output: 'document.querySelectorAll(\'foo\');' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(\'\');', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementById(`foo`);', | ||
errors: [{message: 'Prefer `querySelector` over `getElementById`.'}], | ||
output: 'document.querySelector(`#foo`);' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(`foo`);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(`.foo`);' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(`foo bar`);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(`.foo.bar`);' | ||
}, | ||
{ | ||
code: 'document.getElementsByTagName(`foo`);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}], | ||
output: 'document.querySelectorAll(`foo`);' | ||
}, | ||
{ | ||
code: 'document.getElementsByTagName(``);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(`${fn()}`);', // eslint-disable-line no-template-curly-in-string | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(`foo ${undefined}`);', // eslint-disable-line no-template-curly-in-string | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(null);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}], | ||
output: 'document.querySelectorAll(null);' | ||
}, | ||
{ | ||
code: 'document.getElementsByTagName(null);', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByTagName`.'}], | ||
output: 'document.querySelectorAll(null);' | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(fn());', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName("foo" + fn());', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
}, | ||
{ | ||
code: 'document.getElementsByClassName(foo + "bar");', | ||
errors: [{message: 'Prefer `querySelectorAll` over `getElementsByClassName`.'}] | ||
} | ||
] | ||
}); |