Change Request: Deprecate and remove CodePath#currentSegments

[nzakas]

ESLint version

HEAD

What problem do you want to solve?

Currently, the CodePath objects produced via onCodePathStart and onCodePathEnd contain a mix of data and state. The most glaring example is CodePath#currentSegments, which is actually a live array that is updated as CodePathAnalyzer traverses the AST. (CodePath#currentSegments actually is a passthrough to an underlying CodePathState#currentSegments property.)

This is problematic with respect to the language plugins SourceCode#traverse(), which allows for a traversal to happen at any point and returns an array of steps to complete the traversal. The data available at each step should be the same as when there's an active traversal, but that doesn't happen with code path analysis due to the CodePath objects exposing state information.

So, exposing state information on CodePath objects is a blocker for the language plugins proposal.

What do you think is the correct solution?

As best I can tell, the only problematic property is CodePath#currentSegments, which I think we should deprecate in the v8 timeframe and see if we can remove it in either v9 or v10. From a search through the codebase it seems that this is the primary property we are using off of CodePath objects, and it doesn't appear that other state information that changes during traversal is being exposed.

Further, I doubt that there are many people using CodePath#currentSegments in the wild, so I think we can be pretty aggressive with removing it. I searched through some of the popular plugins and found zero instances of code path usage.

The currentSegments property is easily duplicated inside of rules using this code:

const codePathSegments = [];
let currentCodePathSegments = [];

return {
            onCodePathStart() {
                codePathSegments.push(currentCodePathSegments);
                currentCodePathSegments = [];
            },
            onCodePathSegmentStart(segment) {
                currentCodePathSegments.push(segment);
            },
            onCodePathSegmentEnd() {
                currentCodePathSegments.pop();
            },
            onCodePathEnd() {
                currentCodePathSegments = codePathSegments.pop();
            },
};

I'd argue that this is the most appropriate way to track current traversal state (from within a rule rather than inside of the core).

Participation

• I am willing to submit a pull request for this change.

Additional comments

No response

[mdjermanovic]

This is problematic with respect to the language plugins SourceCode#traverse(), which allows for a traversal to happen at any point and returns an array of steps to complete the traversal. The data available at each step should be the same as when there's an active traversal, but that doesn't happen with code path analysis due to the CodePath objects exposing state information.

I don't fully understand the problem. Do you have an implementation of SourceCode#traverse that doesn't work as expected because of CodePath#currentSegments? That would help a lot in clarifying the problem.

const codePathSegments = [];
let currentCodePathSegments = [];

return {
            onCodePathStart() {
                codePathSegments.push(currentCodePathSegments);
                currentCodePathSegments = [];
            },
            onCodePathSegmentStart(segment) {
                currentCodePathSegments.push(segment);
            },
            onCodePathSegmentEnd() {
                currentCodePathSegments.pop();
            },
            onCodePathEnd() {
                currentCodePathSegments = codePathSegments.pop();
            },
};

I believe this won't work for rules that need to know whether the currently traversed code is reachable, because onCodePathSegmentStart/onCodePathSegmentEnd doesn't fire for unreachable segments.

[nzakas]

I just pushed the updated code.

I believe this won't work for rules that need to know whether the currently traversed code is reachable, because onCodePathSegmentStart/onCodePathSegmentEnd doesn't fire for unreachable segments.

Good point. That seems easily solvable, though. We could introduce onUnreachableCodePathSegmentStart/End to give access to that information?

[mdjermanovic]

I just pushed the updated code.

Thanks! I believe I understand the problem now, and I support the proposal to deprecate and remove CodePath#currentSegments.

I believe this won't work for rules that need to know whether the currently traversed code is reachable, because onCodePathSegmentStart/onCodePathSegmentEnd doesn't fire for unreachable segments.

Good point. That seems easily solvable, though. We could introduce onUnreachableCodePathSegmentStart/End to give access to that information?

Sounds good to me. We could introduce onUnreachableCodePathSegmentStart/End and remove all uses of CodePath#currentSegments in core rules to see if everything still works well, then deprecate CodePath#currentSegments.

[nzakas]

Working on this.

[nzakas]

Here's an initial draft PR:
#17511

To start, I just updated no-unreachable to use the new events. If we decide this is the way forward, I'll go through and update the other rules that use code paths as well.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.