Skip to content

Commit 4537737

Browse files
not-an-aardvarknot-an-aardvark
committed
New: no-useless-token-range rule
1 parent 643b884 commit 4537737

File tree

7 files changed

+314
-0
lines changed

7 files changed

+314
-0
lines changed

Diff for: ‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ Name | ✔️ | 🛠 | Description
5454
[no-missing-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-placeholders.md) | ✔️ | | Disallows missing placeholders in rule report messages
5555
[test-case-shorthand-strings](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/test-case-shorthand-strings.md) | | | Enforces consistent usage of shorthand strings for test cases with no options
5656
[prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | Disallows template literals as report messages
57+
[no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | | 🛠 | Disallows unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken
5758

5859
## Supported Presets
5960

Diff for: ‎docs/rules/no-useless-token-range.md

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken (no-useless-token-range)
2+
3+
AST nodes always start and end with tokens. As a result, the start index of the first token in a node is the same as the start index of the node itself, and the end index of the last token in a node is the same as the end index of the node itself. Using code like `sourceCode.getFirstToken(node).range[0]` unnecessarily hurts the performance of your rule, and makes your code less readable.
4+
5+
## Rule Details
6+
7+
This rule aims to avoid unnecessary calls to `sourceCode.getFirstToken` and `sourceCode.getLastToken`.
8+
9+
Examples of **incorrect** code for this rule:
10+
11+
```js
12+
module.exports = {
13+
create(context) {
14+
const sourceCode = context.getSourceCode();
15+
16+
const rangeStart = sourceCode.getFirstToken(node).range[0];
17+
const rangeEnd = sourceCode.getLastToken(node).range[1];
18+
}
19+
};
20+
```
21+
22+
Examples of **correct** code for this rule:
23+
24+
```js
25+
module.exports = {
26+
create(context) {
27+
const sourceCode = context.getSourceCode();
28+
29+
const rangeStart = node.range[0];
30+
const rangeEnd = node.range[1];
31+
}
32+
};
33+
```
34+
35+
## Known Limitations
36+
37+
* To ensure that your `SourceCode` instances can be detected, your rule must assign `context.getSourceCode()` to a variable somewhere.
38+
39+
## Further Reading
40+
41+
* [`SourceCode` API](http://eslint.org/docs/developer-guide/working-with-rules#contextgetsourcecode)

Diff for: ‎lib/rules/no-useless-token-range.js

+138
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/**
2+
* @fileoverview Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
const utils = require('../utils');
9+
10+
// ------------------------------------------------------------------------------
11+
// Rule Definition
12+
// ------------------------------------------------------------------------------
13+
14+
module.exports = {
15+
meta: {
16+
docs: {
17+
description: 'Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken',
18+
category: 'Rules',
19+
recommended: false,
20+
},
21+
fixable: 'code',
22+
schema: [],
23+
},
24+
25+
create (context) {
26+
const sourceCode = context.getSourceCode();
27+
28+
// ----------------------------------------------------------------------
29+
// Helpers
30+
// ----------------------------------------------------------------------
31+
32+
/**
33+
* Determines whether a second argument to getFirstToken or getLastToken changes the output of the function.
34+
* This occurs when the second argument exists and is not an object literal, or has keys other than `includeComments`.
35+
* @param {ASTNode} arg The second argument to `sourceCode.getFirstToken` or `sourceCode.getLastToken`
36+
* @returns {boolean} `true` if the argument affects the output of getFirstToken or getLastToken
37+
*/
38+
function affectsGetTokenOutput (arg) {
39+
if (!arg) {
40+
return false;
41+
}
42+
if (arg.type !== 'ObjectExpression') {
43+
return true;
44+
}
45+
return arg.properties.length >= 2 || (
46+
arg.properties.length === 1 && (
47+
utils.getKeyName(arg.properties[0]) !== 'includeComments' ||
48+
arg.properties[0].value.type !== 'Literal'
49+
));
50+
}
51+
52+
/**
53+
* Determines whether a node is a MemberExpression that accesses the `range` property
54+
* @param {ASTNode} node The node
55+
* @returns {boolean} `true` if the node is a MemberExpression that accesses the `range` property
56+
*/
57+
function isRangeAccess (node) {
58+
return node.type === 'MemberExpression' && node.property.type === 'Identifier' && node.property.name === 'range';
59+
}
60+
61+
/**
62+
* Determines whether a MemberExpression accesses the `start` property (either `.range[0]` or `.start`).
63+
* Note that this will also work correctly if the `.range` MemberExpression is passed.
64+
* @param {ASTNode} memberExpression The MemberExpression node to check
65+
* @returns {boolean} `true` if this node accesses either `.range[0]` or `.start`
66+
*/
67+
function isStartAccess (memberExpression) {
68+
if (isRangeAccess(memberExpression) && memberExpression.parent.type === 'MemberExpression') {
69+
return isStartAccess(memberExpression.parent);
70+
}
71+
return (
72+
(memberExpression.property.type === 'Identifier' && memberExpression.property.name === 'start') ||
73+
(
74+
memberExpression.computed && memberExpression.property.type === 'Literal' && memberExpression.property.value === 0 &&
75+
isRangeAccess(memberExpression.object)
76+
)
77+
);
78+
}
79+
80+
/**
81+
* Determines whether a MemberExpression accesses the `start` property (either `.range[1]` or `.end`).
82+
* Note that this will also work correctly if the `.range` MemberExpression is passed.
83+
* @param {ASTNode} memberExpression The MemberExpression node to check
84+
* @returns {boolean} `true` if this node accesses either `.range[1]` or `.end`
85+
*/
86+
function isEndAccess (memberExpression) {
87+
if (isRangeAccess(memberExpression) && memberExpression.parent.type === 'MemberExpression') {
88+
return isEndAccess(memberExpression.parent);
89+
}
90+
return (
91+
(memberExpression.property.type === 'Identifier' && memberExpression.property.name === 'end') ||
92+
(
93+
memberExpression.computed && memberExpression.property.type === 'Literal' && memberExpression.property.value === 1 &&
94+
isRangeAccess(memberExpression.object)
95+
)
96+
);
97+
}
98+
99+
// ----------------------------------------------------------------------
100+
// Public
101+
// ----------------------------------------------------------------------
102+
103+
return {
104+
'Program:exit' (ast) {
105+
Array.from(utils.getSourceCodeIdentifiers(context, ast))
106+
.filter(identifier => identifier.parent.type === 'MemberExpression' &&
107+
identifier.parent.object === identifier &&
108+
identifier.parent.property.type === 'Identifier' &&
109+
identifier.parent.parent.type === 'CallExpression' &&
110+
identifier.parent === identifier.parent.parent.callee &&
111+
identifier.parent.parent.arguments.length <= 2 &&
112+
!affectsGetTokenOutput(identifier.parent.parent.arguments[1]) &&
113+
identifier.parent.parent.parent.type === 'MemberExpression' &&
114+
identifier.parent.parent === identifier.parent.parent.parent.object && (
115+
(isStartAccess(identifier.parent.parent.parent) && identifier.parent.property.name === 'getFirstToken') ||
116+
(isEndAccess(identifier.parent.parent.parent) && identifier.parent.property.name === 'getLastToken')
117+
)
118+
)
119+
.forEach(identifier => {
120+
const fullRangeAccess = isRangeAccess(identifier.parent.parent.parent)
121+
? identifier.parent.parent.parent.parent
122+
: identifier.parent.parent.parent;
123+
const replacementText = sourceCode.text.slice(fullRangeAccess.range[0], identifier.parent.parent.range[0]) +
124+
sourceCode.getText(identifier.parent.parent.arguments[0]) +
125+
sourceCode.text.slice(identifier.parent.parent.range[1], fullRangeAccess.range[1]);
126+
context.report({
127+
node: identifier.parent.parent,
128+
message: "Use '{{replacementText}}' instead.",
129+
data: { replacementText },
130+
fix (fixer) {
131+
return fixer.replaceText(identifier.parent.parent, sourceCode.getText(identifier.parent.parent.arguments[0]));
132+
},
133+
});
134+
});
135+
},
136+
};
137+
},
138+
};

Diff for: ‎lib/utils.js

+26
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,30 @@ module.exports = {
235235
.slice(0, reportArgs.length)
236236
.reduce((reportInfo, key, index) => Object.assign(reportInfo, { [key]: reportArgs[index] }), {});
237237
},
238+
239+
/**
240+
* Gets a set of all `sourceCode` identifiers.
241+
* @param {RuleContext} context The context for the rule file
242+
* @param {ASTNode} ast The AST of the file. This must have `parent` properties.
243+
* @returns {Set<ASTNode>} A set of all identifiers referring to the `SourceCode` object.
244+
*/
245+
getSourceCodeIdentifiers (context, ast) {
246+
return new Set(Array.from(module.exports.getContextIdentifiers(context, ast))
247+
.filter(identifier => identifier.parent &&
248+
identifier.parent.type === 'MemberExpression' &&
249+
identifier === identifier.parent.object &&
250+
identifier.parent.property.type === 'Identifier' &&
251+
identifier.parent.property.name === 'getSourceCode' &&
252+
identifier.parent.parent.type === 'CallExpression' &&
253+
identifier.parent === identifier.parent.parent.callee &&
254+
identifier.parent.parent.parent.type === 'VariableDeclarator' &&
255+
identifier.parent.parent === identifier.parent.parent.parent.init &&
256+
identifier.parent.parent.parent.id.type === 'Identifier'
257+
)
258+
.map(identifier => context.getDeclaredVariables(identifier.parent.parent.parent))
259+
.reduce((allVariables, variablesForIdentifier) => allVariables.concat(variablesForIdentifier), [])
260+
.map(variable => variable.references)
261+
.reduce((allRefs, refsForVariable) => allRefs.concat(refsForVariable), [])
262+
.map(ref => ref.identifier));
263+
},
238264
};

Diff for: ‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"eslint-config-not-an-aardvark": "^2.0.0",
3636
"eslint-plugin-node": "^3.0.5",
3737
"espree": "^3.3.2",
38+
"estraverse": "^4.2.0",
3839
"lodash": "^4.17.2",
3940
"mocha": "^2.4.5"
4041
},

Diff for: ‎tests/lib/rules/no-useless-token-range.js

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @fileoverview Disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
// ------------------------------------------------------------------------------
9+
// Requirements
10+
// ------------------------------------------------------------------------------
11+
12+
const rule = require('../../../lib/rules/no-useless-token-range');
13+
const RuleTester = require('eslint').RuleTester;
14+
15+
/**
16+
* Wraps a code sample as an eslint rule
17+
* @param {string} code source text given a `sourceCode` variable
18+
* @returns {string} rule code containing that source text
19+
*/
20+
function wrapRule (code) {
21+
return `
22+
module.exports = {
23+
create(context) {
24+
const sourceCode = context.getSourceCode();
25+
${code}
26+
}
27+
};
28+
`;
29+
}
30+
31+
// ------------------------------------------------------------------------------
32+
// Tests
33+
// ------------------------------------------------------------------------------
34+
35+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
36+
ruleTester.run('no-useless-token-range', rule, {
37+
38+
valid: [
39+
'sourceCode.getLastToken(foo).range[0]',
40+
'sourceCode.getFirstToken(foo).range[1]',
41+
'sourceCode.getLastToken(foo).start',
42+
'sourceCode.getFirstToken(foo).end',
43+
'sourceCode.getSomethingElse(foo).range[0]',
44+
'notSourceCode.getFirstToken(foo).range[0]',
45+
'sourceCode.getFirstToken(foo, bar).range[0]',
46+
'sourceCode.getFirstToken(foo, { skip: 1 }).start',
47+
'sourceCode.getLastToken(foo, bar).range[1]',
48+
'sourceCode.getLastToken(foo, { skip: 1 }).end',
49+
].map(wrapRule),
50+
51+
invalid: [
52+
{
53+
code: 'sourceCode.getFirstToken(foo).range[0]',
54+
output: 'foo.range[0]',
55+
errors: [{ message: "Use 'foo.range[0]' instead.", type: 'CallExpression' }],
56+
},
57+
{
58+
code: 'sourceCode.getFirstToken(foo).start',
59+
output: 'foo.start',
60+
errors: [{ message: "Use 'foo.start' instead.", type: 'CallExpression' }],
61+
},
62+
{
63+
code: 'sourceCode.getLastToken(foo).range[1]',
64+
output: 'foo.range[1]',
65+
errors: [{ message: "Use 'foo.range[1]' instead.", type: 'CallExpression' }],
66+
},
67+
{
68+
code: 'sourceCode.getLastToken(foo).end',
69+
output: 'foo.end',
70+
errors: [{ message: "Use 'foo.end' instead.", type: 'CallExpression' }],
71+
},
72+
{
73+
code: 'sourceCode.getFirstToken(foo, { includeComments: true }).range[0]',
74+
output: 'foo.range[0]',
75+
errors: [{ message: "Use 'foo.range[0]' instead.", type: 'CallExpression' }],
76+
},
77+
{
78+
code: 'sourceCode.getLastToken(foo, { includeComments: true }).range[1]',
79+
output: 'foo.range[1]',
80+
errors: [{ message: "Use 'foo.range[1]' instead.", type: 'CallExpression' }],
81+
},
82+
].map(invalidCase => Object.assign(invalidCase, { code: wrapRule(invalidCase.code), output: wrapRule(invalidCase.output) })),
83+
});

Diff for: ‎tests/lib/utils.js

+24
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const util = require('util');
44
const lodash = require('lodash');
55
const espree = require('espree');
66
const escope = require('escope');
7+
const estraverse = require('estraverse');
78
const assert = require('chai').assert;
89
const utils = require('../../lib/utils');
910

@@ -320,4 +321,27 @@ describe('utils', () => {
320321
});
321322
}
322323
});
324+
325+
describe('getSourceCodeIdentifiers', () => {
326+
const CASES = {
327+
'module.exports = context => { const sourceCode = context.getSourceCode(); sourceCode; foo; }': 2,
328+
'module.exports = context => { const x = 1, sc = context.getSourceCode(); sc; sc; sc; sourceCode; }': 4,
329+
'module.exports = context => { const sourceCode = context.getNotSourceCode(); }': 0,
330+
};
331+
332+
Object.keys(CASES).forEach(testSource => {
333+
it(testSource, () => {
334+
const ast = espree.parse(testSource, { ecmaVersion: 6 });
335+
const scope = escope.analyze(ast, { ignoreEval: true, ecmaVersion: 6, sourceType: 'script', nodejsScope: true });
336+
337+
estraverse.traverse(ast, {
338+
enter (node, parent) {
339+
node.parent = parent;
340+
},
341+
});
342+
343+
assert.strictEqual(utils.getSourceCodeIdentifiers(scope, ast).size, CASES[testSource]);
344+
});
345+
});
346+
});
323347
});

0 commit comments

Comments
 (0)