Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/languages/js/source-code/source-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const { isCommentToken } = require("@eslint-community/eslint-utils"),
astUtils = require("../../../shared/ast-utils"),
Traverser = require("../../../shared/traverser"),
globals = require("../../../../conf/globals"),
EventEmitter = require("node:events"),
{ directivesPattern } = require("../../../shared/directives"),
CodePathAnalyzer = require("../../../linter/code-path-analysis/code-path-analyzer"),
createEmitter = require("../../../linter/safe-emitter"),
{
ConfigCommentParser,
VisitNodeStep,
Expand Down Expand Up @@ -1209,7 +1209,7 @@ class SourceCode extends TokenStore {
* custom parsers to return any AST, we need to ensure that the traversal
* logic works for any AST.
*/
const emitter = createEmitter();
const emitter = new EventEmitter();
let analyzer = {
enterNode(node) {
steps.push(
Expand Down
11 changes: 7 additions & 4 deletions lib/linter/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ const path = require("node:path"),
{ ConfigCommentParser } = require("@eslint/plugin-kit"),
createReportTranslator = require("./report-translator"),
Rules = require("./rules"),
createEmitter = require("./safe-emitter"),
SourceCodeFixer = require("./source-code-fixer"),
{ SourceCodeVisitor } = require("./source-code-visitor"),
timing = require("./timing"),
ruleReplacements = require("../../conf/replacements.json");
const { FlatConfigArray } = require("../config/flat-config-array");
Expand Down Expand Up @@ -1175,7 +1175,7 @@ function runRules(
stats,
slots,
) {
const emitter = createEmitter();
const visitor = new SourceCodeVisitor();

/*
* Create a frozen object with the ruleContext properties and methods that are shared by all rules.
Expand All @@ -1198,6 +1198,9 @@ function runRules(
const lintingProblems = [];
const steps = sourceCode.traverse();

// TODO: Necessary to initialize the .parent properties of each node. Need to find a better way...
sourceCode.traverse();

Object.keys(configuredRules).forEach(ruleId => {
const severity = Config.getRuleNumericSeverity(configuredRules[ruleId]);

Expand Down Expand Up @@ -1339,13 +1342,13 @@ function runRules(
? timing.time(ruleId, ruleListeners[selector], stats)
: ruleListeners[selector];

emitter.on(selector, addRuleErrorHandler(ruleListener));
visitor.add(selector, addRuleErrorHandler(ruleListener));
});
});

const traverser = SourceCodeTraverser.getInstance(language);

traverser.traverseSync(sourceCode, emitter, { steps });
traverser.traverseSync(sourceCode, visitor, { steps });

return lintingProblems;
}
Expand Down
52 changes: 0 additions & 52 deletions lib/linter/safe-emitter.js

This file was deleted.

107 changes: 61 additions & 46 deletions lib/linter/source-code-traverser.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const vk = require("eslint-visitor-keys");
/**
* @import { ESQueryParsedSelector } from "./esquery.js";
* @import { Language, SourceCode } from "@eslint/core";
* @import { SourceCodeVisitor } from "./source-code-visitor.js";
*/

//-----------------------------------------------------------------------------
Expand All @@ -43,19 +44,17 @@ function compareSpecificity(a, b) {
*/
class ESQueryHelper {
/**
* @param {SafeEmitter} emitter
* 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`.)
* Creates a new instance.
* @param {SourceCodeVisitor} visitor The visitor containing the functions to call.
* @param {ESQueryOptions} esqueryOptions `esquery` options for traversing custom nodes.
* @returns {NodeEventGenerator} new instance
*/
constructor(emitter, esqueryOptions) {
constructor(visitor, esqueryOptions) {
/**
* The emitter to use during traversal.
* @type {SafeEmitter}
* @type {SourceCodeVisitor}
*/
this.emitter = emitter;
this.visitor = visitor;

/**
* The options for `esquery` to use during matching.
Expand Down Expand Up @@ -91,7 +90,7 @@ class ESQueryHelper {
*/
this.anyTypeExitSelectors = [];

emitter.eventNames().forEach(rawSelector => {
visitor.forEachName(rawSelector => {
const selector = parse(rawSelector);

/*
Expand Down Expand Up @@ -141,12 +140,10 @@ class ESQueryHelper {
* @param {ASTNode} node The node to check
* @param {ASTNode[]} ancestry The ancestry of the node being checked.
* @param {ESQueryParsedSelector} selector An AST selector descriptor
* @returns {void}
* @returns {boolean} `true` if the selector matches the node, `false` otherwise
*/
#applySelector(node, ancestry, selector) {
if (matches(node, selector.root, ancestry, this.esqueryOptions)) {
this.emitter.emit(selector.source, node);
}
matches(node, ancestry, selector) {
return matches(node, selector.root, ancestry, this.esqueryOptions);
}

/**
Expand All @@ -156,8 +153,9 @@ class ESQueryHelper {
* @param {boolean} isExit `false` if the node is currently being entered, `true` if it's currently being exited
* @returns {void}
*/
applySelectors(node, ancestry, isExit) {
calculateSelectors(node, ancestry, isExit) {
const nodeTypeKey = this.esqueryOptions?.nodeTypeKey || "type";
const selectors = [];

/*
* Get the selectors that may match this node. First, check
Expand Down Expand Up @@ -189,27 +187,36 @@ class ESQueryHelper {
* or if the next any type selector is more specific than the
* next selector for this node type, apply the any type selector.
*/
if (
selectorsByNodeTypeIndex >= selectorsByNodeType.length ||
(anyTypeSelectorsIndex < anyTypeSelectors.length &&
anyTypeSelectors[anyTypeSelectorsIndex].compare(
selectorsByNodeType[selectorsByNodeTypeIndex],
) < 0)
) {
this.#applySelector(
node,
ancestry,
anyTypeSelectors[anyTypeSelectorsIndex++],
);
const hasMoreNodeTypeSelectors =
selectorsByNodeTypeIndex < selectorsByNodeType.length;
const hasMoreAnyTypeSelectors =
anyTypeSelectorsIndex < anyTypeSelectors.length;
const anyTypeSelector = anyTypeSelectors[anyTypeSelectorsIndex];
const nodeTypeSelector =
selectorsByNodeType[selectorsByNodeTypeIndex];

// Only compare specificity if both selectors exist
const isAnyTypeSelectorMoreSpecific =
hasMoreAnyTypeSelectors &&
hasMoreNodeTypeSelectors &&
anyTypeSelector.compare(nodeTypeSelector) < 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be isAnyTypeSelectorLessSpecific, not More.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I've found this specificity concept difficult to wrap my brain around. In CSS, higher specificity takes priority but here lower specificity does.


if (!hasMoreNodeTypeSelectors || isAnyTypeSelectorMoreSpecific) {
anyTypeSelectorsIndex++;

if (this.matches(node, ancestry, anyTypeSelector)) {
selectors.push(anyTypeSelector.source);
}
} else {
// otherwise apply the node type selector
this.#applySelector(
node,
ancestry,
selectorsByNodeType[selectorsByNodeTypeIndex++],
);
selectorsByNodeTypeIndex++;

if (this.matches(node, ancestry, nodeTypeSelector)) {
selectors.push(nodeTypeSelector.source);
}
}
}

return selectors;
}
}

Expand Down Expand Up @@ -253,14 +260,14 @@ class SourceCodeTraverser {
/**
* Traverses the given source code synchronously.
* @param {SourceCode} sourceCode The source code to traverse.
* @param {SafeEmitter} emitter The emitter to use for events.
* @param {SourceCodeVisitor} visitor The emitter to use for events.
* @param {Object} options Options for traversal.
* @param {ReturnType<SourceCode["traverse"]>} options.steps The steps to take during traversal.
* @returns {void}
* @throws {Error} If an error occurs during traversal.
*/
traverseSync(sourceCode, emitter, { steps } = {}) {
const esquery = new ESQueryHelper(emitter, {
traverseSync(sourceCode, visitor, { steps } = {}) {
const esquery = new ESQueryHelper(visitor, {
visitorKeys: sourceCode.visitorKeys ?? this.#language.visitorKeys,
fallback: vk.getKeys,
matchClass: this.#language.matchesSelectorClass ?? (() => false),
Expand All @@ -274,19 +281,27 @@ class SourceCodeTraverser {
case STEP_KIND_VISIT: {
try {
if (step.phase === 1) {
esquery.applySelectors(
step.target,
currentAncestry,
false,
);
esquery
.calculateSelectors(
step.target,
currentAncestry,
false,
)
.forEach(selector => {
visitor.callSync(selector, step.target);
});
currentAncestry.unshift(step.target);
} else {
currentAncestry.shift();
esquery.applySelectors(
step.target,
currentAncestry,
true,
);
esquery
.calculateSelectors(
step.target,
currentAncestry,
true,
)
.forEach(selector => {
visitor.callSync(selector, step.target);
});
}
} catch (err) {
err.currentNode = step.target;
Expand All @@ -296,7 +311,7 @@ class SourceCodeTraverser {
}

case STEP_KIND_CALL: {
emitter.emit(step.target, ...step.args);
visitor.callSync(step.target, ...step.args);
break;
}

Expand Down
81 changes: 81 additions & 0 deletions lib/linter/source-code-visitor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* @fileoverview SourceCodeVisitor class
* @author Nicholas C. Zakas
*/

"use strict";

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

const emptyArray = Object.freeze([]);

//------------------------------------------------------------------------------
// Exports
//------------------------------------------------------------------------------

/**
* A structure to hold a list of functions to call for a given name.
* This is used to allow multiple rules to register functions for a given name
* without having to know about each other.
*/
class SourceCodeVisitor {
/**
* The functions to call for a given name.
* @type {Map<string, Function[]>}
*/
#functions = new Map();

/**
* Adds a function to the list of functions to call for a given name.
* @param {string} name The name of the function to call.
* @param {Function} func The function to call.
* @returns {void}
*/
add(name, func) {
if (this.#functions.has(name)) {
this.#functions.get(name).push(func);
} else {
this.#functions.set(name, [func]);
}
}

/**
* Gets the list of functions to call for a given name.
* @param {string} name The name of the function to call.
* @returns {Function[]} The list of functions to call.
*/
get(name) {
if (this.#functions.has(name)) {
return this.#functions.get(name);
}

return emptyArray;
}

/**
* Iterates over all names and calls the callback with the name.
* @param {(name:string) => void} callback The callback to call for each name.
* @returns {void}
*/
forEachName(callback) {
this.#functions.forEach((funcs, name) => {
callback(name);
});
}

/**
* Calls the functions for a given name with the given arguments.
* @param {string} name The name of the function to call.
* @param {any[]} args The arguments to pass to the function.
* @returns {void}
*/
callSync(name, ...args) {
if (this.#functions.has(name)) {
this.#functions.get(name).forEach(func => func(...args));
}
}
}

module.exports = { SourceCodeVisitor };
Loading
Loading