diff --git a/docs/src/use/migrate-to-9.0.0.md b/docs/src/use/migrate-to-9.0.0.md index e6e0ed58e66..08523b19644 100644 --- a/docs/src/use/migrate-to-9.0.0.md +++ b/docs/src/use/migrate-to-9.0.0.md @@ -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) @@ -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) +## 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) + ## 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. diff --git a/lib/linter/code-path-analysis/code-path-analyzer.js b/lib/linter/code-path-analysis/code-path-analyzer.js index b60e55c16de..f5f26d0f470 100644 --- a/lib/linter/code-path-analysis/code-path-analyzer.js +++ b/lib/linter/code-path-analysis/code-path-analyzer.js @@ -222,7 +222,6 @@ function forwardCurrentToHead(analyzer, node) { : "onUnreachableCodePathSegmentStart"; debug.dump(`${eventName} ${headSegment.id}`); - CodePathSegment.markUsed(headSegment); analyzer.emitter.emit( eventName, diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 605c8604388..8ae8ad367a3 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -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"), @@ -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 @@ -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. @@ -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; } diff --git a/lib/rules/constructor-super.js b/lib/rules/constructor-super.js index bab709d4188..7ded20f6075 100644 --- a/lib/rules/constructor-super.js +++ b/lib/rules/constructor-super.js @@ -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 //------------------------------------------------------------------------------ @@ -159,12 +183,8 @@ module.exports = { */ let funcInfo = null; - /* - * {Map} - * 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} */ let segInfoMap = Object.create(null); @@ -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]; } /** @@ -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; } @@ -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({ @@ -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); } }, @@ -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) { @@ -348,6 +375,9 @@ module.exports = { }); } } + + // save just in case we created a new SegmentInfo object + segInfoMap[segment.id] = info; } ); }, diff --git a/lib/rules/no-this-before-super.js b/lib/rules/no-this-before-super.js index bcd00a404ba..01e355acbd1 100644 --- a/lib/rules/no-this-before-super.js +++ b/lib/rules/no-this-before-super.js @@ -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 //------------------------------------------------------------------------------ @@ -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} */ let segInfoMap = Object.create(null); /** @@ -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; } /** @@ -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(); @@ -295,6 +312,8 @@ module.exports = { ) { info.superCalled = true; } + + segInfoMap[segment.id] = info; } ); }, diff --git a/lib/rules/no-useless-return.js b/lib/rules/no-useless-return.js index 81d61051053..1f85cdb9aaf 100644 --- a/lib/rules/no-useless-return.js +++ b/lib/rules/no-useless-return.js @@ -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; @@ -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]; @@ -275,7 +281,6 @@ module.exports = { * NOTE: This event is notified for only reachable segments. */ onCodePathSegmentStart(segment) { - scopeInfo.currentSegments.add(segment); const info = { diff --git a/lib/source-code/source-code.js b/lib/source-code/source-code.js index 31dec21cf49..f3418e7e5b7 100644 --- a/lib/source-code/source-code.js +++ b/lib/source-code/source-code.js @@ -18,8 +18,12 @@ const directivesPattern } = require("../shared/directives"), - /* eslint-disable-next-line n/no-restricted-require -- Too messy to figure out right now. */ + /* eslint-disable n/no-restricted-require -- Should eventually be moved into SourceCode. */ + CodePathAnalyzer = require("../linter/code-path-analysis/code-path-analyzer"), + createEmitter = require("../linter/safe-emitter"), ConfigCommentParser = require("../linter/config-comment-parser"), + /* eslint-enable n/no-restricted-require -- Should eventually be moved into SourceCode. */ + eslintScope = require("eslint-scope"); //------------------------------------------------------------------------------ @@ -34,6 +38,16 @@ const const commentParser = new ConfigCommentParser(); +const CODE_PATH_EVENTS = [ + "onCodePathStart", + "onCodePathEnd", + "onCodePathSegmentStart", + "onCodePathSegmentEnd", + "onCodePathSegmentLoop", + "onUnreachableCodePathSegmentStart", + "onUnreachableCodePathSegmentEnd" +]; + /** * Validates that the given AST has the required information. * @param {ASTNode} ast The Program node of the AST to check. @@ -300,6 +314,65 @@ function markExportedVariables(globalScope, variables) { } +const STEP_KIND = { + visit: 1, + call: 2 +}; + +/** + * A class to represent a step in the traversal process. + */ +class TraversalStep { + + /** + * The type of the step. + * @type {string} + */ + type; + + /** + * The kind of the step. Represents the same data as the `type` property + * but it's a number for performance. + * @type {number} + */ + kind; + + /** + * The target of the step. + * @type {ASTNode|string} + */ + target; + + /** + * The phase of the step. + * @type {number|undefined} + */ + phase; + + /** + * The arguments of the step. + * @type {Array} + */ + args; + + /** + * Creates a new instance. + * @param {Object} options The options for the step. + * @param {string} options.type The type of the step. + * @param {ASTNode|string} options.target The target of the step. + * @param {number|undefined} [options.phase] The phase of the step. + * @param {Array} options.args The arguments of the step. + * @returns {void} + */ + constructor({ type, target, phase, args }) { + this.type = type; + this.kind = STEP_KIND[type]; + this.target = target; + this.phase = phase; + this.args = args; + } +} + //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -311,6 +384,12 @@ const caches = Symbol("caches"); */ class SourceCode extends TokenStore { + /** + * The cache of steps that were taken while traversing the source code. + * @type {Array} + */ + #steps; + /** * @param {string|Object} textOrConfig The source code text or config object. * @param {string} textOrConfig.text The source code text. @@ -972,6 +1051,91 @@ class SourceCode extends TokenStore { } + /** + * Traverse the source code and return the steps that were taken. + * @returns {Array} The steps that were taken while traversing the source code. + */ + traverse() { + + // Because the AST doesn't mutate, we can cache the steps + if (this.#steps) { + return this.#steps; + } + + const steps = this.#steps = []; + + /* + * This logic works for any AST, not just ESTree. Because ESLint has allowed + * custom parsers to return any AST, we need to ensure that the traversal + * logic works for any AST. + */ + const emitter = createEmitter(); + let analyzer = { + enterNode(node) { + steps.push(new TraversalStep({ + type: "visit", + target: node, + phase: 1, + args: [node, node.parent] + })); + }, + leaveNode(node) { + steps.push(new TraversalStep({ + type: "visit", + target: node, + phase: 2, + args: [node, node.parent] + })); + }, + emitter + }; + + /* + * We do code path analysis for ESTree only. Code path analysis is not + * necessary for other ASTs, and it's also not possible to do for other + * ASTs because the necessary information is not available. + * + * Generally speaking, we can tell that the AST is an ESTree if it has a + * Program node at the top level. This is not a perfect heuristic, but it + * is good enough for now. + */ + const isESTree = this.ast.type === "Program"; + + if (isESTree) { + analyzer = new CodePathAnalyzer(analyzer); + + CODE_PATH_EVENTS.forEach(eventName => { + emitter.on(eventName, (...args) => { + steps.push(new TraversalStep({ + type: "call", + target: eventName, + args + })); + }); + }); + } + + /* + * The actual AST traversal is done by the `Traverser` class. This class + * is responsible for walking the AST and calling the appropriate methods + * on the `analyzer` object, which is appropriate for the given AST. + */ + Traverser.traverse(this.ast, { + enter(node, parent) { + + // save the parent node on a property for backwards compatibility + node.parent = parent; + + analyzer.enterNode(node); + }, + leave(node) { + analyzer.leaveNode(node); + }, + visitorKeys: this.visitorKeys + }); + + return steps; + } } module.exports = SourceCode; diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 8e7c3c5070c..6e98b93f3a1 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -14337,6 +14337,30 @@ var a = "test2"; assert.strictEqual(suppressedMessages.length, 0); }); + it("reports no problems for no-fallthrough despite comment pattern match", () => { + const code = "switch (foo) { case 0: a(); \n// eslint-disable-next-line no-fallthrough\n case 1: }"; + const config = { + linterOptions: { + reportUnusedDisableDirectives: true + }, + rules: { + "no-fallthrough": 2 + } + }; + + const messages = linter.verify(code, config, { + filename, + allowInlineConfig: true + }); + const suppressedMessages = linter.getSuppressedMessages(); + + assert.strictEqual(messages.length, 0); + + assert.strictEqual(suppressedMessages.length, 1); + assert.strictEqual(suppressedMessages[0].ruleId, "no-fallthrough"); + }); + + describe("autofix", () => { const alwaysReportsRule = { create(context) { diff --git a/tests/lib/rules/constructor-super.js b/tests/lib/rules/constructor-super.js index a65999dfbdb..70fd14a8859 100644 --- a/tests/lib/rules/constructor-super.js +++ b/tests/lib/rules/constructor-super.js @@ -272,6 +272,19 @@ ruleTester.run("constructor-super", rule, { } }`, errors: [{ messageId: "missingAll", type: "MethodDefinition" }] + }, + + { + code: `class C extends D { + + constructor() { + do { + something(); + } while (foo); + } + + }`, + errors: [{ messageId: "missingAll", type: "MethodDefinition" }] } ] });