avajs / eslint-plugin-ava

ESLint rules for AVA
https://avajs.dev
MIT License
230 stars 49 forks source link

Fix crash, false-positive in no-statement-after-end #316

Closed ninevra closed 3 years ago

ninevra commented 3 years ago

Fixes #315 by changing the way no-statement-after-end records CodePathSegments. The stack is used only to hold the in-progress segment in the outer CodePaths. When CodePaths begin or end, the current segment is pushed onto or popped from the stack. When CodePathSegments begin or end, the current segment is set or unset.

When in unreachable code at the end of a path, the current segment is therefore undefined, rather than erroneously taken from the next-outermost path.

Checks whether the current segment is set before operating on it.

(Interestingly, this rule doesn't catch statements after t.end() in unreachable code, e.g.:

throw new Error();
t.end();
1; // <-- does not trigger a report

This behavior predates this PR.)

While I believe this fixes the issue, I don't fully understand how this rule ought to handle its edge-cases nor why the codepath analysis system works this way. Please review carefully?

novemberborn commented 3 years ago

@sindresorhus could you take a look at this when you get some time? 😄

sindresorhus commented 3 years ago

I don't have a lot of experience with code path analysis, but the changes look ok from what I can tell.