-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Suggest unwrap_or_else when a closure is given #102441
Conversation
May need more work to expand to suggest |
fcc5a23
to
5c3a8e2
Compare
☔ The latest upstream changes (presumably #102545) made this pull request unmergeable. Please resolve the merge conflicts. |
5c3a8e2
to
aebc5e1
Compare
This comment has been minimized.
This comment has been minimized.
aebc5e1
to
5d69f3a
Compare
… to unwrap_or instead of suggesting calling it
5d69f3a
to
f712c41
Compare
err: &mut Diagnostic, | ||
expr: &hir::Expr<'_>, | ||
found: Ty<'tcx>, | ||
can_satisfy: impl FnOnce(Ty<'tcx>) -> bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be an argument. Can you inline the can_coerce
call here by passing in expected: Ty<'tcx>
?
.., | ||
) = call_expr.kind | ||
{ | ||
let self_ty = self.typeck_results.borrow().expr_ty(self_expr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is an expr_ty_opt
version of this that doesn't ICE if the node is missing a type If so, then please use that, just in case.
let fn_ty = self.tcx.type_of(pick.item.def_id) && | ||
let fn_sig = fn_ty.fn_sig(self.tcx) && | ||
let fn_args = fn_sig.skip_binder().inputs() && | ||
fn_args.len() == args.len() +1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fn_args.len() == args.len() +1 { | |
fn_args.len() == args.len() + 1 { |
let fn_ty = self.tcx.type_of(pick.item.def_id) && | ||
let fn_sig = fn_ty.fn_sig(self.tcx) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let fn_ty = self.tcx.type_of(pick.item.def_id) && | |
let fn_sig = fn_ty.fn_sig(self.tcx) && | |
let fn_sig = self.tcx.fn_sig(pick.item.def_id) && |
Few more nits |
f712c41
to
0188273
Compare
Fixed, thanks! |
Hm, this suggestion might have some false positives still, but this is probably ok... @bors r+ |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#102441 (Suggest unwrap_or_else when a closure is given) - rust-lang#102547 (Migrate CSS theme for search results) - rust-lang#102567 (Delay evaluating lint primary message until after it would be suppressed) - rust-lang#102624 (rustdoc: remove font family CSS on `.rustdoc-toggle summary::before`) - rust-lang#102628 (Change the parameter name of From::from to `value`) - rust-lang#102637 (Ignore fuchsia on two compiler tests) - rust-lang#102639 (Improve spans when splitting multi-char operator tokens for proc macros.) Failed merges: - rust-lang#102496 (Suggest `.into()` when all other coercion suggestions fail) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #102320
r? @compiler-errors