Skip to content

Commit 1b678a6

Browse files
not-an-aardvarkilyavolodin
authored andcommitted
New: allow rules to listen for AST selectors (fixes #5407) (#7833)
* New: allow rules to listen for AST selectors (fixes #5407) * Optimize by predicting matched node types * Allow no-restricted-syntax to handle query selectors as options * Clean up node-event-generator * Memoize calls to esquery.parse * pass a Set to NodeEventGenerator instead of an array * Don't special-case '*' * Make the queries argument mandatory * add selector tests * Use CSS specificity to sort selectors * Use eventNames() if available * Fix comment typos * Add documentation for selectors * Use stronger assertions in node-event-generator tests * Handle selector parsing errors more gracefully * Use node types in selector syntax examples * Put the no-restricted-syntax section at the bottom of the selector docs * Upgrade esquery to v1.0.0 * Use a JSDoc typedef for selector descriptors * Make the assertion more specific for invalid-syntax error checking * Use a consistent example in the no-restricted-syntax docs * Add a test for AST node classes
1 parent 63ca0c5 commit 1b678a6

15 files changed

+765
-98
lines changed

docs/developer-guide/selectors.md

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# Selectors
2+
3+
Some rules and APIs allow the use of selectors to query an AST. This page is intended to:
4+
5+
1. Explain what selectors are
6+
1. Describe the syntax for creating selectors
7+
1. Describe what selectors can be used for
8+
9+
## What is a selector?
10+
11+
A selector is a string that can be used to match nodes in an Abstract Syntax Tree (AST). This is useful for describing a particular syntax pattern in your code.
12+
13+
The syntax for AST selectors is similar to the syntax for [CSS selectors](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors). If you've used CSS selectors before, the syntax for AST selectors should be easy to understand.
14+
15+
The simplest selector is just a node type. A node type selector will match all nodes with the given type. For example, consider the following program:
16+
17+
```js
18+
var foo = 1;
19+
bar.baz();
20+
```
21+
22+
The selector "`Identifier`" will match all `Identifier` nodes in the program. In this case, the selector will match the nodes for `foo`, `bar`, and `baz`.
23+
24+
Selectors are not limited to matching against single node types. For example, the selector `VariableDeclarator > Identifier` will match all `Identifier` nodes that have a `VariableDeclarator` as a direct parent. In the program above, this will match the node for `foo`, but not the nodes for `bar` and `baz`.
25+
26+
## What syntax can selectors have?
27+
28+
The following selectors are supported:
29+
30+
* AST node type: `ForStatement`
31+
* wildcard (matches all nodes): `*`
32+
* attribute existence: `[attr]`
33+
* attribute value: `[attr="foo"]` or `[attr=123]`
34+
* attribute regex: `[attr=/foo.*/]`
35+
* attribute conditons: `[attr!="foo"]`, `[attr>2]`, `[attr<3]`, `[attr>=2]`, or `[attr<=3]`
36+
* nested attribute: `[attr.level2="foo"]`
37+
* field: `FunctionDeclaration > Identifier.id`
38+
* First or last child: `:first-child` or `:last-child`
39+
* nth-child (no ax+b support): `:nth-child(2)`
40+
* nth-last-child (no ax+b support): `:nth-last-child(1)`
41+
* descendant: `FunctionExpression ReturnStatement`
42+
* child: `UnaryExpression > Literal`
43+
* following sibling: `ArrayExpression > Literal + SpreadElement`
44+
* adjacent sibling: `VariableDeclaration ~ VariableDeclaration`
45+
* negation: `:not(ForStatement)`
46+
* matches-any: `:matches([attr] > :first-child, :last-child)`
47+
* class of AST node: `:statement`, `:expression`, `:declaration`, `:function`, or `:pattern`
48+
49+
This syntax is very powerful, and can be used to precisely select many syntactic patterns in your code.
50+
51+
<sup>The examples in this section were adapted from the [esquery](https://github.com/estools/esquery) documentation.</sup>
52+
53+
## What can selectors be used for?
54+
55+
If you're writing custom ESLint rules, you might be interested in using selectors to examine specific parts of the AST. If you're configuring ESLint for your codebase, you might be interested in restricting particular syntax patterns with selectors.
56+
57+
### Listening for selectors in rules
58+
59+
When writing a custom ESLint rule, you can listen for nodes that match a particular selector as the AST is traversed.
60+
61+
```js
62+
module.exports = {
63+
create(context) {
64+
// ...
65+
66+
return {
67+
68+
// This listener will be called for all IfStatement nodes with blocks.
69+
"IfStatement > BlockStatement": function(blockStatementNode) {
70+
// ...your logic here
71+
},
72+
73+
// This listener will be called for all function declarations with more than 3 parameters.
74+
"FunctionDeclaration[params.length>3]": function(functionDeclarationNode) {
75+
// ...your logic here
76+
}
77+
};
78+
}
79+
};
80+
```
81+
82+
Adding `:exit` to the end of a selector will cause the listener to be called when the matching nodes are exited during traversal, rather than when they are entered.
83+
84+
If two or more selectors match the same node, their listeners will be called in order of increasing specificity. The specificity of an AST selector is similar to the specificity of a CSS selector:
85+
86+
* When comparing two selectors, the selector that contains more class selectors, attribute selectors, and pseudo-class selectors (excluding `:not()`) has higher specificity.
87+
* If the class/attribute/pseudo-class count is tied, the selector that contains more node type selectors has higher specificity.
88+
89+
If multiple selectors have equal specificity, their listeners will be called in alphabetical order for that node.
90+
91+
### Restricting syntax with selectors
92+
93+
With the [no-restricted-syntax](/docs/rules/no-restricted-syntax) rule, you can restrict the usage of particular syntax in your code. For example, you can use the following configuration to disallow using `if` statements that do not have block statements as their body:
94+
95+
```json
96+
{
97+
"rules": {
98+
"no-restricted-syntax": ["error", "IfStatement > :not(BlockStatement).body"]
99+
}
100+
}
101+
```
102+
103+
...or equivalently, you can use this configuration:
104+
105+
```json
106+
{
107+
"rules": {
108+
"no-restricted-syntax": ["error", "IfStatement[body.type!='BlockStatement']"]
109+
}
110+
}
111+
```
112+
113+
As another example, you can disallow calls to `require()`:
114+
115+
```json
116+
{
117+
"rules": {
118+
"no-restricted-syntax": ["error", "CallExpression[callee.name='require']"]
119+
}
120+
}
121+
```
122+
123+
Or you can enforce that calls to `setTimeout` always have two arguments:
124+
125+
```json
126+
{
127+
"rules": {
128+
"no-restricted-syntax": ["error", "CallExpression[callee.name='setTimeout'][arguments.length!=2]"]
129+
}
130+
}
131+
```
132+
133+
Using selectors in the `no-restricted-syntax` rule can give you a lot of control over problematic patterns in your codebase, without needing to write custom rules to detect each pattern.

docs/developer-guide/working-with-rules.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ The source file for a rule exports an object with the following properties.
6565

6666
`create` (function) returns an object with methods that ESLint calls to "visit" nodes while traversing the abstract syntax tree (AST as defined by [ESTree](https://github.com/estree/estree)) of JavaScript code:
6767

68-
* if a key is a node type, ESLint calls that **visitor** function while going **down** the tree
69-
* if a key is a node type plus `:exit`, ESLint calls that **visitor** function while going **up** the tree
68+
* if a key is a node type or a [selector](./selectors), ESLint calls that **visitor** function while going **down** the tree
69+
* if a key is a node type or a [selector](./selectors) plus `:exit`, ESLint calls that **visitor** function while going **up** the tree
7070
* if a key is an event name, ESLint calls that **handler** function for [code path analysis](./code-path-analysis.md)
7171

7272
A rule can use the current node and its surrounding tree to report or fix problems.

docs/rules/no-restricted-syntax.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
# disallow specified syntax (no-restricted-syntax)
22

3-
JavaScript has a lot of language features, and not everyone likes all of them. As a result, some projects choose to disallow the use of certain language features altogether. For instance, you might decide to disallow the use of `try-catch` or `class`.
3+
JavaScript has a lot of language features, and not everyone likes all of them. As a result, some projects choose to disallow the use of certain language features altogether. For instance, you might decide to disallow the use of `try-catch` or `class`, or you might decide to disallow the use of the `in` operator.
44

55
Rather than creating separate rules for every language feature you want to turn off, this rule allows you to configure the syntax elements you want to restrict use of. These elements are represented by their [ESTree](https://github.com/estree/estree) node types. For example, a function declaration is represented by `FunctionDeclaration` and the `with` statement is represented by `WithStatement`. You may find the full list of AST node names you can use [on GitHub](https://github.com/eslint/espree/blob/master/lib/ast-node-types.js) and use the [online parser](http://eslint.org/parser/) to see what type of nodes your code consists of.
66

7+
You can also specify [AST selectors](../developer-guide/selectors) to restrict, allowing much more precise control over syntax patterns.
8+
79
## Rule Details
810

911
This rule disallows specified (that is, user-defined) syntax.
@@ -15,31 +17,35 @@ This rule takes a list of strings:
1517
```json
1618
{
1719
"rules": {
18-
"no-restricted-syntax": ["error", "FunctionExpression", "WithStatement"]
20+
"no-restricted-syntax": ["error", "FunctionExpression", "WithStatement", "BinaryExpression[operator='in']"]
1921
}
2022
}
2123
```
2224

23-
Examples of **incorrect** code for this rule with the `"FunctionExpression", "WithStatement"` options:
25+
Examples of **incorrect** code for this rule with the `"FunctionExpression", "WithStatement", BinaryExpression[operator='in']` options:
2426

2527
```js
26-
/* eslint no-restricted-syntax: ["error", "FunctionExpression", "WithStatement"] */
28+
/* eslint no-restricted-syntax: ["error", "FunctionExpression", "WithStatement", "BinaryExpression[operator='in']"] */
2729

2830
with (me) {
2931
dontMess();
3032
}
3133

3234
var doSomething = function () {};
35+
36+
foo in bar;
3337
```
3438

35-
Examples of **correct** code for this rule with the `"FunctionExpression", "WithStatement"` options:
39+
Examples of **correct** code for this rule with the `"FunctionExpression", "WithStatement", BinaryExpression[operator='in']` options:
3640

3741
```js
38-
/* eslint no-restricted-syntax: ["error", "FunctionExpression", "WithStatement"] */
42+
/* eslint no-restricted-syntax: ["error", "FunctionExpression", "WithStatement", "BinaryExpression[operator='in']"] */
3943

4044
me.dontMess();
4145

4246
function doSomething() {};
47+
48+
foo instanceof bar;
4349
```
4450

4551
## When Not To Use It

lib/eslint.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -867,11 +867,11 @@ module.exports = (function() {
867867
const rule = ruleCreator.create ? ruleCreator.create(ruleContext)
868868
: ruleCreator(ruleContext);
869869

870-
// add all the node types as listeners
871-
Object.keys(rule).forEach(nodeType => {
872-
api.on(nodeType, timing.enabled
873-
? timing.time(key, rule[nodeType])
874-
: rule[nodeType]
870+
// add all the selectors from the rule as listeners
871+
Object.keys(rule).forEach(selector => {
872+
api.on(selector, timing.enabled
873+
? timing.time(key, rule[selector])
874+
: rule[selector]
875875
);
876876
});
877877
} catch (ex) {

lib/rules/no-new-func.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,18 @@ module.exports = {
2727
//--------------------------------------------------------------------------
2828

2929
/**
30-
* Checks if the callee is the Function constructor, and if so, reports an issue.
31-
* @param {ASTNode} node The node to check and report on
30+
* Reports a node.
31+
* @param {ASTNode} node The node to report
3232
* @returns {void}
3333
* @private
3434
*/
35-
function validateCallee(node) {
36-
if (node.callee.name === "Function") {
37-
context.report({ node, message: "The Function constructor is eval." });
38-
}
35+
function report(node) {
36+
context.report({ node, message: "The Function constructor is eval." });
3937
}
4038

4139
return {
42-
NewExpression: validateCallee,
43-
CallExpression: validateCallee
40+
"NewExpression[callee.name = 'Function']": report,
41+
"CallExpression[callee.name = 'Function']": report
4442
};
4543

4644
}

lib/rules/no-new.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,8 @@ module.exports = {
2424
create(context) {
2525

2626
return {
27-
28-
ExpressionStatement(node) {
29-
30-
if (node.expression.type === "NewExpression") {
31-
context.report({ node, message: "Do not use 'new' for side effects." });
32-
}
27+
"ExpressionStatement > NewExpression"(node) {
28+
context.report({ node: node.parent, message: "Do not use 'new' for side effects." });
3329
}
3430
};
3531

lib/rules/no-process-exit.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,9 @@ module.exports = {
2626
//--------------------------------------------------------------------------
2727

2828
return {
29-
30-
CallExpression(node) {
31-
const callee = node.callee;
32-
33-
if (callee.type === "MemberExpression" && callee.object.name === "process" &&
34-
callee.property.name === "exit"
35-
) {
36-
context.report({ node, message: "Don't use process.exit(); throw an error instead." });
37-
}
29+
"CallExpression > MemberExpression.callee[object.name = 'process'][property.name = 'exit']"(node) {
30+
context.report({ node: node.parent, message: "Don't use process.exit(); throw an error instead." });
3831
}
39-
4032
};
4133

4234
}

lib/rules/no-restricted-syntax.js

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
// Rule Definition
99
//------------------------------------------------------------------------------
1010

11-
const nodeTypes = require("espree").Syntax;
12-
1311
module.exports = {
1412
meta: {
1513
docs: {
@@ -20,32 +18,18 @@ module.exports = {
2018

2119
schema: {
2220
type: "array",
23-
items: [
24-
{
25-
enum: Object.keys(nodeTypes).map(k => nodeTypes[k])
26-
}
27-
],
21+
items: [{ type: "string" }],
2822
uniqueItems: true,
2923
minItems: 0
3024
}
3125
},
3226

3327
create(context) {
34-
35-
/**
36-
* Generates a warning from the provided node, saying that node type is not allowed.
37-
* @param {ASTNode} node The node to warn on
38-
* @returns {void}
39-
*/
40-
function warn(node) {
41-
context.report({ node, message: "Using '{{type}}' is not allowed.", data: node });
42-
}
43-
44-
return context.options.reduce((result, nodeType) => {
45-
result[nodeType] = warn;
46-
47-
return result;
48-
}, {});
28+
return context.options.reduce((result, selector) => Object.assign(result, {
29+
[selector](node) {
30+
context.report({ node, message: "Using '{{selector}}' is not allowed.", data: { selector } });
31+
}
32+
}), {});
4933

5034
}
5135
};

lib/rules/no-sync.js

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,14 @@ module.exports = {
2626

2727
return {
2828

29-
MemberExpression(node) {
30-
const propertyName = node.property.name,
31-
syncRegex = /.*Sync$/;
32-
33-
if (syncRegex.exec(propertyName) !== null) {
34-
context.report({
35-
node,
36-
message: "Unexpected sync method: '{{propertyName}}'.",
37-
data: {
38-
propertyName
39-
}
40-
});
41-
}
29+
"MemberExpression[property.name=/.*Sync$/]"(node) {
30+
context.report({
31+
node,
32+
message: "Unexpected sync method: '{{propertyName}}'.",
33+
data: {
34+
propertyName: node.property.name
35+
}
36+
});
4237
}
4338
};
4439

lib/testers/rule-tester.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,13 @@ RuleTester.prototype = {
343343
* running the rule under test.
344344
*/
345345
eslint.reset();
346+
346347
eslint.on("Program", node => {
347348
beforeAST = cloneDeeplyExcludesParent(node);
349+
});
348350

349-
eslint.on("Program:exit", node => {
350-
afterAST = cloneDeeplyExcludesParent(node);
351-
});
351+
eslint.on("Program:exit", node => {
352+
afterAST = node;
352353
});
353354

354355
// Freezes rule-context properties.
@@ -385,7 +386,7 @@ RuleTester.prototype = {
385386
return {
386387
messages: eslint.verify(code, config, filename, true),
387388
beforeAST,
388-
afterAST
389+
afterAST: cloneDeeplyExcludesParent(afterAST)
389390
};
390391
} finally {
391392
rules.get = originalGet;

0 commit comments

Comments
 (0)