Skip to content

Commit

Permalink
feat!: move AST traversal into SourceCode (#18167)
Browse files Browse the repository at this point in the history 
* feat: Implement SourceCode#traverse()

Refs #16999

* Add more debugging

* Clean up sourcecode

* Try to fix code paths

* Fix rules and update migration guide

* Cache traversal steps

* Fix lint error

* Fix fuzz test

* Simplify TraversalStep constructor

* Remove unused parent arg

* Fix constructor-super
  • Loading branch information
@nzakas
nzakas committed Mar 21, 2024
1 parent b91f9dc commit 09bd7fe
Show file tree
Hide file tree
Showing 9 changed files with 334 additions and 65 deletions.
14 changes: 14 additions & 0 deletions docs/src/use/migrate-to-9.0.0.md
View file
Expand Up @@ -43,6 +43,7 @@ The lists below are ordered roughly by the number of users each change is expect
* [Removed multiple `context` methods](#removed-context-methods)
* [Removed `sourceCode.getComments()`](#removed-sourcecode-getcomments)
* [Removed `CodePath#currentSegments`](#removed-codepath-currentsegments)
* [Code paths are now precalculated](#codepath-precalc)
* [Function-style rules are no longer supported](#drop-function-style-rules)
* [`meta.schema` is required for rules with options](#meta-schema-required)
* [`FlatRuleTester` is now `RuleTester`](#flat-rule-tester)
Expand Down Expand Up @@ -490,6 +491,19 @@ ESLint v9.0.0 removes the deprecated `CodePath#currentSegments` property.

**Related Issues(s):** [#17457](https://github.com/eslint/eslint/issues/17457)

## <a name="codepath-precalc"></a> Code paths are now precalculated

Prior to ESLint v9.0.0, code paths were calculated during the same AST traversal used by rules, meaning that the information passed to methods like `onCodePathStart` and `onCodePathSegmentStart` was incomplete. Specifically, array properties like `CodePath#childCodePaths` and `CodePathSegment#nextSegments` began empty and then were filled with the appropriate information as the traversal completed, meaning that those arrays could have different elements depending on when you checked their values.

ESLint v9.0.0 now precalculates code path information before the traversal used by rules. As a result, the code path information is now complete regardless of where it is accessed inside of a rule.

**To address:** If you are accessing any array properties on `CodePath` or `CodePathSegment`, you'll need to update your code. Specifically:

* If you are checking the `length` of any array properties, ensure you are using relative comparison operators like `<`, `>`, `<=`, and `>=` instead of equals.
* If you are accessing the `nextSegments`, `prevSegments`, `allNextSegments`, or `allPrevSegments` properties on a `CodePathSegment`, or `CodePath#childCodePaths`, verify that your code will still work as expected. To be backwards compatible, consider moving the logic that checks these properties into `onCodePathEnd`.

**Related Issues(s):** [#16999](https://github.com/eslint/eslint/issues/16999)

## <a name="drop-function-style-rules"></a> Function-style rules are no longer supported

ESLint v9.0.0 drops support for function-style rules. Function-style rules are rules created by exporting a function rather than an object with a `create()` method. This rule format was deprecated in 2016.
Expand Down
1 change: 0 additions & 1 deletion lib/linter/code-path-analysis/code-path-analyzer.js
View file
Expand Up @@ -222,7 +222,6 @@ function forwardCurrentToHead(analyzer, node) {
: "onUnreachableCodePathSegmentStart";

debug.dump(`${eventName} ${headSegment.id}`);

CodePathSegment.markUsed(headSegment);
analyzer.emitter.emit(
eventName,
Expand Down
59 changes: 30 additions & 29 deletions lib/linter/linter.js
View file
Expand Up @@ -30,7 +30,6 @@ const
} = require("@eslint/eslintrc/universal"),
Traverser = require("../shared/traverser"),
{ SourceCode } = require("../source-code"),
CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer"),
applyDisableDirectives = require("./apply-disable-directives"),
ConfigCommentParser = require("./config-comment-parser"),
NodeEventGenerator = require("./node-event-generator"),
Expand All @@ -53,6 +52,8 @@ const commentParser = new ConfigCommentParser();
const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, column: 1 } };
const parserSymbol = Symbol.for("eslint.RuleTester.parser");
const { LATEST_ECMA_VERSION } = require("../../conf/ecma-version");
const STEP_KIND_VISIT = 1;
const STEP_KIND_CALL = 2;

//------------------------------------------------------------------------------
// Typedefs
Expand Down Expand Up @@ -986,22 +987,13 @@ function createRuleListeners(rule, ruleContext) {
* @param {string} physicalFilename The full path of the file on disk without any code block information
* @param {Function} ruleFilter A predicate function to filter which rules should be executed.
* @returns {LintMessage[]} An array of reported problems
* @throws {Error} If traversal into a node fails.
*/
function runRules(sourceCode, configuredRules, ruleMapper, parserName, languageOptions, settings, filename, disableFixes, cwd, physicalFilename, ruleFilter) {
const emitter = createEmitter();
const nodeQueue = [];
let currentNode = sourceCode.ast;

Traverser.traverse(sourceCode.ast, {
enter(node, parent) {
node.parent = parent;
nodeQueue.push({ isEntering: true, node });
},
leave(node) {
nodeQueue.push({ isEntering: false, node });
},
visitorKeys: sourceCode.visitorKeys
});

// must happen first to assign all node.parent properties
const eventQueue = sourceCode.traverse();

/*
* Create a frozen object with the ruleContext properties and methods that are shared by all rules.
Expand Down Expand Up @@ -1131,25 +1123,34 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserName, languageO
});
});

// 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, { visitorKeys: sourceCode.visitorKeys, fallback: Traverser.getKeys }))
: new NodeEventGenerator(emitter, { visitorKeys: sourceCode.visitorKeys, fallback: Traverser.getKeys });
const eventGenerator = new NodeEventGenerator(emitter, { visitorKeys: sourceCode.visitorKeys, fallback: Traverser.getKeys });

nodeQueue.forEach(traversalInfo => {
currentNode = traversalInfo.node;
for (const step of eventQueue) {
switch (step.kind) {
case STEP_KIND_VISIT: {
try {
if (step.phase === 1) {
eventGenerator.enterNode(step.target);
} else {
eventGenerator.leaveNode(step.target);
}
} catch (err) {
err.currentNode = step.target;
throw err;
}
break;
}

try {
if (traversalInfo.isEntering) {
eventGenerator.enterNode(currentNode);
} else {
eventGenerator.leaveNode(currentNode);
case STEP_KIND_CALL: {
emitter.emit(step.target, ...step.args);
break;
}
} catch (err) {
err.currentNode = currentNode;
throw err;

default:
throw new Error(`Invalid traversal step found: "${step.type}".`);
}
});

}

return lintingProblems;
}
Expand Down
76 changes: 53 additions & 23 deletions lib/rules/constructor-super.js
View file
Expand Up @@ -119,6 +119,30 @@ function isPossibleConstructor(node) {
}
}

/**
* A class to store information about a code path segment.
*/
class SegmentInfo {

/**
* Indicates if super() is called in all code paths.
* @type {boolean}
*/
calledInEveryPaths = false;

/**
* Indicates if super() is called in any code paths.
* @type {boolean}
*/
calledInSomePaths = false;

/**
* The nodes which have been validated and don't need to be reconsidered.
* @type {ASTNode[]}
*/
validNodes = [];
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -159,12 +183,8 @@ module.exports = {
*/
let funcInfo = null;

/*
* {Map<string, {calledInSomePaths: boolean, calledInEveryPaths: boolean}>}
* Information for each code path segment.
* - calledInSomePaths: A flag of be called `super()` in some code paths.
* - calledInEveryPaths: A flag of be called `super()` in all code paths.
* - validNodes:
/**
* @type {Record<string, SegmentInfo>}
*/
let segInfoMap = Object.create(null);

Expand All @@ -174,7 +194,16 @@ module.exports = {
* @returns {boolean} The flag which shows `super()` is called in some paths
*/
function isCalledInSomePath(segment) {
return segment.reachable && segInfoMap[segment.id].calledInSomePaths;
return segment.reachable && segInfoMap[segment.id]?.calledInSomePaths;
}

/**
* Determines if a segment has been seen in the traversal.
* @param {CodePathSegment} segment A code path segment to check.
* @returns {boolean} `true` if the segment has been seen.
*/
function hasSegmentBeenSeen(segment) {
return !!segInfoMap[segment.id];
}

/**
Expand All @@ -190,10 +219,10 @@ module.exports = {
* If not skipped, this never becomes true after a loop.
*/
if (segment.nextSegments.length === 1 &&
segment.nextSegments[0].isLoopedPrevSegment(segment)
) {
segment.nextSegments[0]?.isLoopedPrevSegment(segment)) {
return true;
}

return segment.reachable && segInfoMap[segment.id].calledInEveryPaths;
}

Expand Down Expand Up @@ -250,9 +279,9 @@ module.exports = {
}

// Reports if `super()` lacked.
const segments = codePath.returnedSegments;
const calledInEveryPaths = segments.every(isCalledInEveryPath);
const calledInSomePaths = segments.some(isCalledInSomePath);
const seenSegments = codePath.returnedSegments.filter(hasSegmentBeenSeen);
const calledInEveryPaths = seenSegments.every(isCalledInEveryPath);
const calledInSomePaths = seenSegments.some(isCalledInSomePath);

if (!calledInEveryPaths) {
context.report({
Expand All @@ -278,18 +307,16 @@ module.exports = {
}

// Initialize info.
const info = segInfoMap[segment.id] = {
calledInSomePaths: false,
calledInEveryPaths: false,
validNodes: []
};
const info = segInfoMap[segment.id] = new SegmentInfo();

// When there are previous segments, aggregates these.
const prevSegments = segment.prevSegments;

if (prevSegments.length > 0) {
info.calledInSomePaths = prevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = prevSegments.every(isCalledInEveryPath);
const seenPrevSegments = prevSegments.filter(hasSegmentBeenSeen);

info.calledInSomePaths = seenPrevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = seenPrevSegments.every(isCalledInEveryPath);
}
},

Expand Down Expand Up @@ -326,12 +353,12 @@ module.exports = {
funcInfo.codePath.traverseSegments(
{ first: toSegment, last: fromSegment },
segment => {
const info = segInfoMap[segment.id];
const prevSegments = segment.prevSegments;
const info = segInfoMap[segment.id] ?? new SegmentInfo();
const seenPrevSegments = segment.prevSegments.filter(hasSegmentBeenSeen);

// Updates flags.
info.calledInSomePaths = prevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = prevSegments.every(isCalledInEveryPath);
info.calledInSomePaths = seenPrevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = seenPrevSegments.every(isCalledInEveryPath);

// If flags become true anew, reports the valid nodes.
if (info.calledInSomePaths || isRealLoop) {
Expand All @@ -348,6 +375,9 @@ module.exports = {
});
}
}

// save just in case we created a new SegmentInfo object
segInfoMap[segment.id] = info;
}
);
},
Expand Down
37 changes: 28 additions & 9 deletions lib/rules/no-this-before-super.js
View file
Expand Up @@ -30,6 +30,29 @@ function isConstructorFunction(node) {
);
}

/*
* Information for each code path segment.
* - superCalled: The flag which shows `super()` called in all code paths.
* - invalidNodes: The array of invalid ThisExpression and Super nodes.
*/
/**
*
*/
class SegmentInfo {

/**
* Indicates whether `super()` is called in all code paths.
* @type {boolean}
*/
superCalled = false;

/**
* The array of invalid ThisExpression and Super nodes.
* @type {ASTNode[]}
*/
invalidNodes = [];
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -64,13 +87,7 @@ module.exports = {
*/
let funcInfo = null;

/*
* Information for each code path segment.
* Each key is the id of a code path segment.
* Each value is an object:
* - superCalled: The flag which shows `super()` called in all code paths.
* - invalidNodes: The array of invalid ThisExpression and Super nodes.
*/
/** @type {Record<string, SegmentInfo>} */
let segInfoMap = Object.create(null);

/**
Expand All @@ -79,7 +96,7 @@ module.exports = {
* @returns {boolean} `true` if `super()` is called.
*/
function isCalled(segment) {
return !segment.reachable || segInfoMap[segment.id].superCalled;
return !segment.reachable || segInfoMap[segment.id]?.superCalled;
}

/**
Expand Down Expand Up @@ -285,7 +302,7 @@ module.exports = {
funcInfo.codePath.traverseSegments(
{ first: toSegment, last: fromSegment },
(segment, controller) => {
const info = segInfoMap[segment.id];
const info = segInfoMap[segment.id] ?? new SegmentInfo();

if (info.superCalled) {
controller.skip();
Expand All @@ -295,6 +312,8 @@ module.exports = {
) {
info.superCalled = true;
}

segInfoMap[segment.id] = info;
}
);
},
Expand Down
9 changes: 7 additions & 2 deletions lib/rules/no-useless-return.js
View file
Expand Up @@ -146,7 +146,9 @@ module.exports = {
continue;
}

uselessReturns.push(...segmentInfoMap.get(segment).uselessReturns);
if (segmentInfoMap.has(segment)) {
uselessReturns.push(...segmentInfoMap.get(segment).uselessReturns);
}
}

return uselessReturns;
Expand Down Expand Up @@ -182,6 +184,10 @@ module.exports = {

const info = segmentInfoMap.get(segment);

if (!info) {
return;
}

info.uselessReturns = info.uselessReturns.filter(node => {
if (scopeInfo.traversedTryBlockStatements && scopeInfo.traversedTryBlockStatements.length > 0) {
const returnInitialRange = node.range[0];
Expand Down Expand Up @@ -275,7 +281,6 @@ module.exports = {
* NOTE: This event is notified for only reachable segments.
*/
onCodePathSegmentStart(segment) {

scopeInfo.currentSegments.add(segment);

const info = {
Expand Down

0 comments on commit 09bd7fe

Please sign in to comment.