Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add a diagnostic with a fix to remove return at the end of functions #11020

Closed

Conversation

izik1
Copy link
Contributor

@izik1 izik1 commented Dec 15, 2021

#10970, probably not closes, because of all the fixmes. But I feel comfortable at least asking for review.
Should I have marked the diagnostic as experimental?

I feel like this should be named along the lines of unwrap expression but I can't think of a good name there

@@ -141,6 +145,30 @@ impl ExprValidator {
});
}

fn check_for_trailing_return(&mut self, db: &dyn HirDatabase) {
// todo: handle inner functions and lambdas.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inner functions I believe should already be covered by this as they are their own DefWithBodyId again.

For lambdas you will just have to go iterate through all expressions of the body and search for a lambda expression, then do the logic you already do here for it's body expression again


#[test]
fn remove_trailing_return() {
// fixme: this should include the semi.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm ye, this one is tricky as diagnostics_display_range doesn't really offer you a nice way to go to the parent as is for checking the semi, leaving the FIXME here for the time being is fine I think, since its not a severe problem.

@Veykril
Copy link
Member

Veykril commented Dec 16, 2021

The name seems fine to me, though we could also give with clippy's name choice here which is needless_return. Not that it matters too much as it can be changed any time.

@izik1
Copy link
Contributor Author

izik1 commented Dec 19, 2021

Note to maintainers: still working on this, but I'm kinda stalled for the moment, major life event happened (not a bad one) and I haven't been able to get back to this yet (don't have wifi rn, typing this from my phone, my computer isn't even so much as plugged in)

@Veykril Veykril marked this pull request as draft March 22, 2022 17:05
@Veykril
Copy link
Member

Veykril commented Jul 18, 2022

I'll close this for now, if you get back to this feel free to either re-open the PR or open a new one.

@Veykril Veykril closed this Jul 18, 2022
bors added a commit that referenced this pull request Feb 8, 2024
…ykril

feat: Add diagnostic with fix to replace trailing `return <val>;` with `<val>`

Works for functions and closures.
Ignores desugared return expressions (e.g. from desugared try operators).

Fixes: #10970
Completes: #11020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants