Fix issue #12034: add autofixes for unnecessary_fallible_conversions #12070
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Alexendoo (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
the
S-waiting-on-review
roife
changed the title
| let qpath_spans = qpath.and_then(|qpath| match qpath { | ||
| QPath::Resolved(_, path) => { | ||
| let segments = path.segments.iter().map(|seg| seg.ident).collect::<Vec<_>>(); | ||
| (segments.len() == 2).then(|| vec![segments[0].span, segments[1].span]) | ||
| }, | ||
| QPath::TypeRelative(_, seg) => Some(vec![seg.ident.span]), | ||
| QPath::LangItem(_, _) => unreachable!("`TryFrom` and `TryInto` are not lang items"), | ||
| }); |
There was a problem hiding this comment.
This is a bit of a nit, and may be unnecessary, but if you return Option<[&'_ span]>, I think the matching below could be cleaner as you will be matching something like
(FunctionKind::TryFromFunction, [from_method_span], Some(unwrap_span)) => todo!(),
(FunctionKind::TryFromFunction, [from_cls_span, from_method_span], Some(unwrap_span)) => todo!(),
_ => unreachable!()which I think would make this a bit cleaner, other than that, I think it's very good and a great addition!
There was a problem hiding this comment.
Sorry it took me so long to see this comment. I tried this but it introduced too many arms (multiple FunctionKind::TryFromFunctions and FunctionKind::TryIntoFunctions) and made the code more complicated
There was a problem hiding this comment.
I see. I am good then, but I think we still need another reviewer with r perms. Maybe zulip?
Thanks for your suggestions! I have refactored the code according to your advice: I removed the large match statement, split it into several parts, and separated them into various sub-functions, which made the logic of the code much clearer. |
|
@bors r+ ty for your work on this! |
Fix issue #12034: add autofixes for unnecessary_fallible_conversions fixes #12034 Currently, the `unnecessary_fallible_conversions` lint was capable of autofixing expressions like `0i32.try_into().unwrap()`. However, it couldn't autofix expressions in the form of `i64::try_from(0i32).unwrap()` or `<i64 as TryFrom<i32>>::try_from(0).unwrap()`. This pull request extends the functionality to correctly autofix these latter forms as well. changelog: [`unnecessary_fallible_conversions`]: Add autofixes for more forms
|
💔 Test failed - checks-action_test |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fixes #12034
Currently, the
unnecessary_fallible_conversionslint was capable of autofixing expressions like0i32.try_into().unwrap(). However, it couldn't autofix expressions in the form ofi64::try_from(0i32).unwrap()or<i64 as TryFrom<i32>>::try_from(0).unwrap().This pull request extends the functionality to correctly autofix these latter forms as well.
changelog: [
unnecessary_fallible_conversions]: Add autofixes for more forms