feat: correct no-useless-return behaviour in try statements

[snitin315]

Prerequisites checklist

• I have read the contributing guidelines.

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)

Fixes #7481

Implemented the solution as suggested in #16593 (comment)

Is there anything you'd like reviewers to focus on?

No

[netlify]

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	80c7367
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/646ad65c3ced690008eb59e7
😎 Deploy Preview	https://deploy-preview-16996--docs-eslint.netlify.app/rules
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[mdjermanovic]

I think we should check all try block statements in the list instead of just the first one. Otherwise, this would still be a false negative:

function foo() {
    try {
        bar();
    } catch {
        try {
            baz();
            return; // false negative
        } catch {
            qux();
        }
    }
}

[mdjermanovic]

Also, I believe this is a new false positive:

function foo() {
    try {
        bar();
        return; // false positive
    } finally {
        baz();
    }
    qux();
}

I'm not sure what exactly is causing it.

[snitin315]

I'm not sure what exactly is causing it.

I'm looking into it. I suspect the TryStatement:exit() query is not working correctly if there is finally instead of catch.

[github-actions]

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.

[snitin315]

I was away on vacation. I'll take a look this week. Currently stuck at a false positive #16996 (comment)

[snitin315]

I spent some time on this but couldn't figure out why we have this new false positive #16996 (comment). @mdjermanovic Do you have any idea?

[fasttime]

I spent some time on this but couldn't figure out why we have this new false positive #16996 (comment).

The problem is the overly eager caching of usedUnreachableSegments in markReturnStatementsOnSegmentAsUsed(), which is preventing a segment from being reprocessed even when the list of traversed try blocks changes.

Also, we should not discard all useless returns associated to a segment if only some of them are actually being marked as used.

I've added a commit to show a possible fix. Please, just edit my changes as required to move on with this PR.

[snitin315]

@fasttime Thanks for the update 🙌🏻.

@mdjermanovic Can you review it once again?

[mdjermanovic]

LGTM, thanks!

[mdjermanovic]

There's a merge conflict.