fix: false positive for reference in finally block#20655
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
|
@Tanujkanti4441 You need to claim an issue before working on it. Please take a look at https://eslint.org/docs/latest/contribute/work-on-issue |
| const isRefInFinally = | ||
| targetAssignment.variable.references.some(ref => { | ||
| const id = ref.identifier; | ||
|
|
||
| const hasRefInFinally = | ||
| ref.isRead() && | ||
| target.tryStatementBlocks.some(tryBlock => { | ||
| const tryNode = tryBlock.parent; | ||
|
|
||
| return ( | ||
| tryNode?.handler && | ||
| tryNode?.finalizer?.range[0] <= | ||
| id.range[0] && | ||
| id.range[1] <= tryNode?.finalizer?.range[1] | ||
| ); | ||
| }); | ||
|
|
||
| return hasRefInFinally; | ||
| }); | ||
|
|
||
| if (isRefInFinally) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I think this will produce false negatives when the reference in the finally block is unreachable, for example:
function main() {
let outcome = 'unknown';
try {
helper1();
} catch (err) {
helper2();
outcome = 'exception';
} finally {
return;
console.log(outcome);
}
}Currently, two problems are being reported, but after the change there will be none.
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
|
Closing this in favor of #20665 which fixes the problem in the code path analyzer. |
|
Hey @DMartens, I think we still have the issue that is reported in #20637, I closed that one as I thought we could solve both issues with this PR. So, is it okay if I reopen and continue to work on this PR to resolve the problems, such as: /* eslint no-useless-assignment: "error" */
function main() {
// False positive: no-useless-assignment.
// If `helper2()` throws an exception, then the 'unknown' value will get used.
let outcome = 'unknown';
try {
helper1();
outcome = 'success';
} catch (err) {
helper2();
outcome = 'exception';
} finally {
console.log(outcome);
}
}
function helper1() {
throw Error('helper1');
}
function helper2() {
throw Error('helper2');
}
main(); |
|
Oh I am sorry for prematurely closing. |
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
|
This isn't stale. It needs to be reviewed. |
Co-authored-by: Francesco Trotta <github@fasttime.org>
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Added a condition inno-useless-assignmentrule to allow the read reference infinallyblock. now it will not report following case:Added a method
makeFirstThrowablePathInCatchBlockincode-path-stateto handle the possibility of throwing an error by aCallExpression,ImportExpression,NewExpression,MemberExpressionandidentifier referecesin catch block with a preceding finally block. such asIs there anything you'd like reviewers to focus on?
Fixes #20583Fixes #20637