Skip to content

Commit

Permalink
Fix: Crash with esquery when using JSX (fixes #13639) (#14072)
Browse files Browse the repository at this point in the history 
* Fix: Crash with esquery when using JSX (fixes #13639)

* Chore: Add and change test cases related to esquery options

* Chore: Add test cases related to esquery options
  • Loading branch information
@ota-meshi
ota-meshi committed Feb 12, 2021
1 parent a0871f1 commit 9d6063a
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 23 deletions.
4 changes: 3 additions & 1 deletion lib/linter/linter.js
View file
Expand Up @@ -942,7 +942,9 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserOptions, parser
});

// only run code path analyzer if the top level node is "Program", skip otherwise
const eventGenerator = nodeQueue[0].node.type === "Program" ? new CodePathAnalyzer(new NodeEventGenerator(emitter)) : new NodeEventGenerator(emitter);
const eventGenerator = nodeQueue[0].node.type === "Program"
? new CodePathAnalyzer(new NodeEventGenerator(emitter, { visitorKeys: sourceCode.visitorKeys, fallback: Traverser.getKeys }))
: new NodeEventGenerator(emitter, { visitorKeys: sourceCode.visitorKeys, fallback: Traverser.getKeys });

nodeQueue.forEach(traversalInfo => {
currentNode = traversalInfo.node;
Expand Down
6 changes: 4 additions & 2 deletions lib/linter/node-event-generator.js
View file
Expand Up @@ -208,10 +208,12 @@ class NodeEventGenerator {
* An SafeEmitter which is the destination of events. This emitter must already
* have registered listeners for all of the events that it needs to listen for.
* (See lib/linter/safe-emitter.js for more details on `SafeEmitter`.)
* @param {ESQueryOptions} esqueryOptions `esquery` options for traversing custom nodes.
* @returns {NodeEventGenerator} new instance
*/
constructor(emitter) {
constructor(emitter, esqueryOptions) {
this.emitter = emitter;
this.esqueryOptions = esqueryOptions;
this.currentAncestry = [];
this.enterSelectorsByNodeType = new Map();
this.exitSelectorsByNodeType = new Map();
Expand Down Expand Up @@ -250,7 +252,7 @@ class NodeEventGenerator {
* @returns {void}
*/
applySelector(node, selector) {
if (esquery.matches(node, selector.parsedSelector, this.currentAncestry)) {
if (esquery.matches(node, selector.parsedSelector, this.currentAncestry, this.esqueryOptions)) {
this.emitter.emit(selector.rawSelector, node);
}
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -58,7 +58,7 @@
"eslint-utils": "^2.1.0",
"eslint-visitor-keys": "^2.0.0",
"espree": "^7.3.1",
"esquery": "^1.2.0",
"esquery": "^1.4.0",
"esutils": "^2.0.2",
"file-entry-cache": "^6.0.0",
"functional-red-black-tree": "^1.0.1",
Expand Down
8 changes: 6 additions & 2 deletions tests/lib/linter/code-path-analysis/code-path-analyzer.js
View file
Expand Up @@ -12,19 +12,23 @@
const assert = require("assert"),
fs = require("fs"),
path = require("path"),
vk = require("eslint-visitor-keys"),
{ Linter } = require("../../../../lib/linter"),
EventGeneratorTester = require("../../../../tools/internal-testers/event-generator-tester"),
createEmitter = require("../../../../lib/linter/safe-emitter"),
debug = require("../../../../lib/linter/code-path-analysis/debug-helpers"),
CodePath = require("../../../../lib/linter/code-path-analysis/code-path"),
CodePathAnalyzer = require("../../../../lib/linter/code-path-analysis/code-path-analyzer"),
CodePathSegment = require("../../../../lib/linter/code-path-analysis/code-path-segment"),
NodeEventGenerator = require("../../../../lib/linter/node-event-generator");
NodeEventGenerator = require("../../../../lib/linter/node-event-generator"),
Traverser = require("../../../lib/shared/traverser");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const STANDARD_ESQUERY_OPTION = { visitorKeys: vk.KEYS, fallback: Traverser.getKeys };

const expectedPattern = /\/\*expected\s+((?:.|[\r\n])+?)\s*\*\//gu;
const lineEndingPattern = /\r?\n/gu;
const linter = new Linter();
Expand Down Expand Up @@ -54,7 +58,7 @@ function getExpectedDotArrows(source) {

describe("CodePathAnalyzer", () => {
EventGeneratorTester.testEventGeneratorInterface(
new CodePathAnalyzer(new NodeEventGenerator(createEmitter()))
new CodePathAnalyzer(new NodeEventGenerator(createEmitter(), STANDARD_ESQUERY_OPTION))
);

describe("interface of code paths", () => {
Expand Down
17 changes: 16 additions & 1 deletion tests/lib/linter/linter.js
View file
Expand Up @@ -5293,9 +5293,11 @@ var a = "test2";
let types = [];
let sourceCode;
let scopeManager;
let firstChildNodes = [];

beforeEach(() => {
types = [];
firstChildNodes = [];
linter.defineRule("collect-node-types", () => ({
"*"(node) {
types.push(node.type);
Expand All @@ -5306,12 +5308,18 @@ var a = "test2";

return {};
});
linter.defineRule("esquery-option", () => ({
":first-child"(node) {
firstChildNodes.push(node);
}
}));
linter.defineParser("enhanced-parser2", testParsers.enhancedParser2);
linter.verify("@foo class A {}", {
parser: "enhanced-parser2",
rules: {
"collect-node-types": "error",
"save-scope-manager": "error"
"save-scope-manager": "error",
"esquery-option": "error"
}
});

Expand Down Expand Up @@ -5351,6 +5359,13 @@ var a = "test2";
["Program", "ClassDeclaration", "Decorator", "Identifier", "Identifier", "ClassBody"]
);
});

it("esquery should use the visitorKeys (so 'visitorKeys.ClassDeclaration' includes 'experimentalDecorators')", () => {
assert.deepStrictEqual(
firstChildNodes,
[sourceCode.ast.body[0], sourceCode.ast.body[0].experimentalDecorators[0]]
);
});
});

describe("if a parser provides 'scope'", () => {
Expand Down
129 changes: 125 additions & 4 deletions tests/lib/linter/node-event-generator.js
View file
Expand Up @@ -11,6 +11,7 @@
const assert = require("assert"),
sinon = require("sinon"),
espree = require("espree"),
vk = require("eslint-visitor-keys"),
Traverser = require("../../../lib/shared/traverser"),
EventGeneratorTester = require("../../../tools/internal-testers/event-generator-tester"),
createEmitter = require("../../../lib/linter/safe-emitter"),
Expand All @@ -28,9 +29,11 @@ const ESPREE_CONFIG = {
loc: true
};

const STANDARD_ESQUERY_OPTION = { visitorKeys: vk.KEYS, fallback: Traverser.getKeys };

describe("NodeEventGenerator", () => {
EventGeneratorTester.testEventGeneratorInterface(
new NodeEventGenerator(createEmitter())
new NodeEventGenerator(createEmitter(), STANDARD_ESQUERY_OPTION)
);

describe("entering a single AST node", () => {
Expand All @@ -40,7 +43,7 @@ describe("NodeEventGenerator", () => {
emitter = Object.create(createEmitter(), { emit: { value: sinon.spy() } });

["Foo", "Bar", "Foo > Bar", "Foo:exit"].forEach(selector => emitter.on(selector, () => {}));
generator = new NodeEventGenerator(emitter);
generator = new NodeEventGenerator(emitter, STANDARD_ESQUERY_OPTION);
});

it("should generate events for entering AST node.", () => {
Expand Down Expand Up @@ -89,7 +92,7 @@ describe("NodeEventGenerator", () => {
});

possibleQueries.forEach(query => emitter.on(query, () => {}));
const generator = new NodeEventGenerator(emitter);
const generator = new NodeEventGenerator(emitter, STANDARD_ESQUERY_OPTION);

Traverser.traverse(ast, {
enter(node, parent) {
Expand Down Expand Up @@ -308,13 +311,131 @@ describe("NodeEventGenerator", () => {
);
});

describe("traversing the entire non-standard AST", () => {

/**
* Gets a list of emitted types/selectors from the generator, in emission order
* @param {ASTNode} ast The AST to traverse
* @param {Record<string, string[]>} visitorKeys The custom visitor keys.
* @param {Array<string>|Set<string>} possibleQueries Selectors to detect
* @returns {Array[]} A list of emissions, in the order that they were emitted. Each emission is a two-element
* array where the first element is a string, and the second element is the emitted AST node.
*/
function getEmissions(ast, visitorKeys, possibleQueries) {
const emissions = [];
const emitter = Object.create(createEmitter(), {
emit: {
value: (selector, node) => emissions.push([selector, node])
}
});

possibleQueries.forEach(query => emitter.on(query, () => {}));
const generator = new NodeEventGenerator(emitter, { visitorKeys, fallback: Traverser.getKeys });

Traverser.traverse(ast, {
visitorKeys,
enter(node, parent) {
node.parent = parent;
generator.enterNode(node);
},
leave(node) {
generator.leaveNode(node);
}
});

return emissions;
}

/**
* Creates a test case that asserts a particular sequence of generator emissions
* @param {ASTNode} ast The AST to traverse
* @param {Record<string, string[]>} visitorKeys The custom visitor keys.
* @param {string[]} possibleQueries A collection of selectors that rules are listening for
* @param {Array[]} expectedEmissions A function that accepts the AST and returns a list of the emissions that the
* generator is expected to produce, in order.
* Each element of this list is an array where the first element is a selector (string), and the second is an AST node
* This should only include emissions that appear in possibleQueries.
* @returns {void}
*/
function assertEmissions(ast, visitorKeys, possibleQueries, expectedEmissions) {
it(possibleQueries.join("; "), () => {
const emissions = getEmissions(ast, visitorKeys, possibleQueries)
.filter(emission => possibleQueries.indexOf(emission[0]) !== -1);

assert.deepStrictEqual(emissions, expectedEmissions(ast));
});
}

assertEmissions(
espree.parse("const foo = [<div/>, <div/>]", { ...ESPREE_CONFIG, ecmaFeatures: { jsx: true } }),
vk.KEYS,
["* ~ *"],
ast => [
["* ~ *", ast.body[0].declarations[0].init.elements[1]] // entering second JSXElement
]
);

assertEmissions(
{

// Parse `class A implements B {}` with typescript-eslint.
type: "Program",
errors: [],
comments: [],
sourceType: "module",
body: [
{
type: "ClassDeclaration",
id: {
type: "Identifier",
name: "A"
},
superClass: null,
implements: [
{
type: "ClassImplements",
id: {
type: "Identifier",
name: "B"
},
typeParameters: null
}
],
body: {
type: "ClassBody",
body: []
}
}
]
},
vk.unionWith({

// see https://github.com/typescript-eslint/typescript-eslint/blob/e4d737b47574ff2c53cabab22853035dfe48c1ed/packages/visitor-keys/src/visitor-keys.ts#L27
ClassDeclaration: [
"decorators",
"id",
"typeParameters",
"superClass",
"superTypeParameters",
"implements",
"body"
]
}),
[":first-child"],
ast => [
[":first-child", ast.body[0]], // entering first ClassDeclaration
[":first-child", ast.body[0].implements[0]] // entering first ClassImplements
]
);
});

describe("parsing an invalid selector", () => {
it("throws a useful error", () => {
const emitter = createEmitter();

emitter.on("Foo >", () => {});
assert.throws(
() => new NodeEventGenerator(emitter),
() => new NodeEventGenerator(emitter, STANDARD_ESQUERY_OPTION),
/Syntax error in selector "Foo >" at position 5: Expected " ", "!", .*/u
);
});
Expand Down
32 changes: 20 additions & 12 deletions tests/lib/rules/no-restricted-syntax.js
View file
Expand Up @@ -145,18 +145,26 @@ ruleTester.run("no-restricted-syntax", rule, {
{ messageId: "restrictedSyntax", data: { message: "Using '[optional=true]' is not allowed." }, type: "CallExpression" },
{ messageId: "restrictedSyntax", data: { message: "Using '[optional=true]' is not allowed." }, type: "MemberExpression" }
]
}
},

// fix https://github.com/estools/esquery/issues/110
{
code: "a?.b",
options: [":nth-child(1)"],
parserOptions: { ecmaVersion: 2020 },
errors: [
{ messageId: "restrictedSyntax", data: { message: "Using ':nth-child(1)' is not allowed." }, type: "ExpressionStatement" }
]
},

/*
* TODO(mysticatea): fix https://github.com/estools/esquery/issues/110
* {
* code: "a?.b",
* options: [":nth-child(1)"],
* parserOptions: { ecmaVersion: 2020 },
* errors: [
* { messageId: "restrictedSyntax", data: { message: "Using ':nth-child(1)' is not allowed." }, type: "ExpressionStatement" }
* ]
* }
*/
// https://github.com/eslint/eslint/issues/13639#issuecomment-683976062
{
code: "const foo = [<div/>, <div/>]",
options: ["* ~ *"],
parserOptions: { ecmaVersion: 2020, ecmaFeatures: { jsx: true } },
errors: [
{ messageId: "restrictedSyntax", data: { message: "Using '* ~ *' is not allowed." }, type: "JSXElement" }
]
}
]
});

0 comments on commit 9d6063a

Please sign in to comment.