Fix invalid suggestions on unit range bindings #97505
Conversation
rustbot
added
the
T-compiler
|
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| let suggestion = if exp_found.found.is_unit() { | ||
| let exp_str = format!("{:?}", expected_def); | ||
|
|
||
| // `RangeInclusive`'s fields are private, we have to call methods instead. | ||
| if exp_str.contains("core::ops::RangeInclusive") | ||
| || exp_str.contains("std::ops::RangeInclusive") | ||
| { |
There was a problem hiding this comment.
I don't think this is the right approach. I would instead add a &hir::Expr (or a HirId to get the expression back) to Pattern so that we can then call .precedence() and compare it to ExprPrecedence::Field. This will work regardless of the types. Also, with access to the expression itself, it'd be nice to see if notice these kind of range literals to suggest changing the range to the start or end, instead of constructing the range and then field accessing one of those.
There was a problem hiding this comment.
Thanks, I've tried to implement your suggestion but it would be a bigger change than I thought. I'm going to ask some questions on Zulip once I get some time to summarize them!
There was a problem hiding this comment.
Switching to waiting on author then :) Feel free to request a review when ready, thanks!
@rustbot author
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #105486) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@JohnTitor any updates on this pr? |
|
Totally forgot about this PR, sorry! Since the branch is quite old and I'm not sure when I have time to work on this, I'm going to close for now. |
|
No worries. Thanks for the update |
Fixes #93122