From abeb7203e4c979b2446bf0cca0155427f0c46c18 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 11 Jul 2024 22:36:45 +0000 Subject: [PATCH 1/5] Add regression test for issue 127545 --- .../transforming-option-ref-issue-127545.rs | 8 ++++ ...ransforming-option-ref-issue-127545.stderr | 40 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs index 5ba58e74275..9125ffeeb06 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs @@ -4,3 +4,11 @@ pub fn foo(arg: Option<&Vec>) -> Option<&[i32]> { arg //~ ERROR 5:5: 5:8: mismatched types [E0308] } + +pub fn bar(arg: Option<&Vec>) -> &[i32] { + arg.unwrap_or(&[]) //~ ERROR 9:19: 9:22: mismatched types [E0308] +} + +pub fn barzz<'a>(arg: Option<&'a Vec>, v: &'a [i32]) -> &'a [i32] { + arg.unwrap_or(v) //~ ERROR 13:19: 13:20: mismatched types [E0308] +} diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr index b7c7202113a..2d4f26c388d 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr @@ -14,5 +14,45 @@ LL | arg.map(|v| &**v) | ++++++++++++++ error: aborting due to 1 previous error + --> $DIR/transforming-option-ref-issue-127545.rs:9:19 + | +LL | arg.unwrap_or(&[]) + | --------- ^^^ expected `&Vec`, found `&[_; 0]` + | | + | arguments to this method are incorrect + | + = note: expected reference `&Vec` + found reference `&[_; 0]` +help: the return type of this call is `&[_; 0]` due to the type of the argument passed + --> $DIR/transforming-option-ref-issue-127545.rs:9:5 + | +LL | arg.unwrap_or(&[]) + | ^^^^^^^^^^^^^^---^ + | | + | this argument influences the return type of `unwrap_or` +note: method defined here + --> $SRC_DIR/core/src/option.rs:LL:COL + +error[E0308]: mismatched types + --> $DIR/transforming-option-ref-issue-127545.rs:13:19 + | +LL | arg.unwrap_or(v) + | --------- ^ expected `&Vec`, found `&[i32]` + | | + | arguments to this method are incorrect + | + = note: expected reference `&Vec` + found reference `&'a [i32]` +help: the return type of this call is `&'a [i32]` due to the type of the argument passed + --> $DIR/transforming-option-ref-issue-127545.rs:13:5 + | +LL | arg.unwrap_or(v) + | ^^^^^^^^^^^^^^-^ + | | + | this argument influences the return type of `unwrap_or` +note: method defined here + --> $SRC_DIR/core/src/option.rs:LL:COL + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. From 5e1cabe98271fc79cd8ebf8e9722df781cad7aa8 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 12 Jul 2024 08:43:28 +0000 Subject: [PATCH 2/5] Add suggestion to use Option::map_or over erroneous Option::unwrap_or --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 11 ++++ .../src/fn_ctxt/suggestions.rs | 65 +++++++++++++++++++ ...ransforming-option-ref-issue-127545.stderr | 10 ++- 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index ab0f356ce91..6fdf616f50e 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -948,6 +948,17 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { &mut err, ); + self.suggest_deref_unwrap_or( + &mut err, + error_span, + callee_ty, + call_ident, + expected_ty, + provided_ty, + provided_args[*provided_idx], + is_method, + ); + // Call out where the function is defined self.label_fn_like( &mut err, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 5975c52ca46..15a1a18daa7 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1429,6 +1429,71 @@ pub(crate) fn suggest_option_to_bool( true } + // Suggest to change `Option<&Vec>::unwrap_or(&[])` to `Option::map_or(&[], |v| v)`. + #[instrument(level = "trace", skip(self, err, provided_expr))] + pub(crate) fn suggest_deref_unwrap_or( + &self, + err: &mut Diag<'_>, + error_span: Span, + callee_ty: Option>, + call_ident: Option, + expected_ty: Ty<'tcx>, + provided_ty: Ty<'tcx>, + provided_expr: &Expr<'tcx>, + is_method: bool, + ) { + if !is_method { + return; + } + let Some(callee_ty) = callee_ty else { + return; + }; + let ty::Adt(callee_adt, _) = callee_ty.peel_refs().kind() else { + return; + }; + if !self.tcx.is_diagnostic_item(sym::Option, callee_adt.did()) { + return; + } + + if call_ident.map_or(true, |ident| ident.name != sym::unwrap_or) { + return; + } + + let ty::Ref(_, peeled, _mutability) = provided_ty.kind() else { + return; + }; + + // NOTE: Can we reuse `suggest_deref_or_ref`? + + // Create an dummy type `&[_]` so that both &[] and `&Vec` can coerce to it. + let dummy_ty = if let ty::Array(elem_ty, size) = peeled.kind() + && let ty::Infer(_) = elem_ty.kind() + && size.try_eval_target_usize(self.tcx, self.param_env) == Some(0) + { + let slice = Ty::new_slice(self.tcx, *elem_ty); + Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_static, slice) + } else { + provided_ty + }; + + if !self.can_coerce(expected_ty, dummy_ty) { + return; + } + let (provided_snip, applicability) = + match self.tcx.sess.source_map().span_to_snippet(provided_expr.span) { + Ok(snip) => (snip, Applicability::MachineApplicable), + Err(_) => ("/* _ */".to_owned(), Applicability::MaybeIncorrect), + }; + let sugg = &format!("map_or({provided_snip}, |v| v)"); + err.span_suggestion_verbose( + error_span, + "use `Option::map_or` to deref inner value of `Option`", + sugg, + applicability, + ); + return; + } + /// Suggest wrapping the block in square brackets instead of curly braces /// in case the block was mistaken array syntax, e.g. `{ 1 }` -> `[ 1 ]`. pub(crate) fn suggest_block_to_brackets( diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr index 2d4f26c388d..90a7cfdb449 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr @@ -13,7 +13,7 @@ help: try using `.map(|v| &**v)` to convert `Option<&Vec>` to `Option<&[i32 LL | arg.map(|v| &**v) | ++++++++++++++ -error: aborting due to 1 previous error +error[E0308]: mismatched types --> $DIR/transforming-option-ref-issue-127545.rs:9:19 | LL | arg.unwrap_or(&[]) @@ -32,6 +32,10 @@ LL | arg.unwrap_or(&[]) | this argument influences the return type of `unwrap_or` note: method defined here --> $SRC_DIR/core/src/option.rs:LL:COL +help: use `Option::map_or` to deref inner value of `Option` + | +LL | arg.map_or(&[], |v| v) + | ~~~~~~~~~~~~~~~~~~ error[E0308]: mismatched types --> $DIR/transforming-option-ref-issue-127545.rs:13:19 @@ -52,6 +56,10 @@ LL | arg.unwrap_or(v) | this argument influences the return type of `unwrap_or` note: method defined here --> $SRC_DIR/core/src/option.rs:LL:COL +help: use `Option::map_or` to deref inner value of `Option` + | +LL | arg.map_or(v, |v| v) + | ~~~~~~~~~~~~~~~~ error: aborting due to 3 previous errors From 9c1a9e03d5e001543a21eaff5d616de8cb4e2220 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 13 Jul 2024 00:54:00 +0000 Subject: [PATCH 3/5] add test for Result<&T, _> where T; Deref --- .../transforming-option-ref-issue-127545.rs | 4 ++++ ...ransforming-option-ref-issue-127545.stderr | 22 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs index 9125ffeeb06..f589e88f68e 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.rs @@ -12,3 +12,7 @@ pub fn bar(arg: Option<&Vec>) -> &[i32] { pub fn barzz<'a>(arg: Option<&'a Vec>, v: &'a [i32]) -> &'a [i32] { arg.unwrap_or(v) //~ ERROR 13:19: 13:20: mismatched types [E0308] } + +pub fn convert_result(arg: Result<&Vec, ()>) -> &[i32] { + arg.unwrap_or(&[]) //~ ERROR 17:19: 17:22: mismatched types [E0308] +} diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr index 90a7cfdb449..0a6d47339d8 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr @@ -61,6 +61,26 @@ help: use `Option::map_or` to deref inner value of `Option` LL | arg.map_or(v, |v| v) | ~~~~~~~~~~~~~~~~ -error: aborting due to 3 previous errors +error[E0308]: mismatched types + --> $DIR/transforming-option-ref-issue-127545.rs:17:19 + | +LL | arg.unwrap_or(&[]) + | --------- ^^^ expected `&Vec`, found `&[_; 0]` + | | + | arguments to this method are incorrect + | + = note: expected reference `&Vec` + found reference `&[_; 0]` +help: the return type of this call is `&[_; 0]` due to the type of the argument passed + --> $DIR/transforming-option-ref-issue-127545.rs:17:5 + | +LL | arg.unwrap_or(&[]) + | ^^^^^^^^^^^^^^---^ + | | + | this argument influences the return type of `unwrap_or` +note: method defined here + --> $SRC_DIR/core/src/result.rs:LL:COL + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`. From 9c3c278b545d9e03939e3bfcbebb256758b9a8b9 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 13 Jul 2024 01:06:40 +0000 Subject: [PATCH 4/5] Add support for `Result<&T, _>' --- .../src/fn_ctxt/suggestions.rs | 19 +++++++++---------- ...ransforming-option-ref-issue-127545.stderr | 4 ++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 15a1a18daa7..faa6d06ccaf 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1451,9 +1451,13 @@ pub(crate) fn suggest_deref_unwrap_or( let ty::Adt(callee_adt, _) = callee_ty.peel_refs().kind() else { return; }; - if !self.tcx.is_diagnostic_item(sym::Option, callee_adt.did()) { + let adt_name = if self.tcx.is_diagnostic_item(sym::Option, callee_adt.did()) { + "Option" + } else if self.tcx.is_diagnostic_item(sym::Result, callee_adt.did()) { + "Result" + } else { return; - } + }; if call_ident.map_or(true, |ident| ident.name != sym::unwrap_or) { return; @@ -1484,14 +1488,9 @@ pub(crate) fn suggest_deref_unwrap_or( Ok(snip) => (snip, Applicability::MachineApplicable), Err(_) => ("/* _ */".to_owned(), Applicability::MaybeIncorrect), }; - let sugg = &format!("map_or({provided_snip}, |v| v)"); - err.span_suggestion_verbose( - error_span, - "use `Option::map_or` to deref inner value of `Option`", - sugg, - applicability, - ); - return; + let sugg = format!("map_or({provided_snip}, |v| v)"); + let msg = format!("use `{adt_name}::map_or` to deref inner value of `{adt_name}`"); + err.span_suggestion_verbose(error_span, msg, sugg, applicability); } /// Suggest wrapping the block in square brackets instead of curly braces diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr index 0a6d47339d8..1790fc1249a 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr @@ -80,6 +80,10 @@ LL | arg.unwrap_or(&[]) | this argument influences the return type of `unwrap_or` note: method defined here --> $SRC_DIR/core/src/result.rs:LL:COL +help: use `Result::map_or` to deref inner value of `Result` + | +LL | arg.map_or(&[], |v| v) + | ~~~~~~~~~~~~~~~~~~ error: aborting due to 4 previous errors From bdc9df247841e259c24e95fd81754fb61e332c65 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 13 Jul 2024 01:18:21 +0000 Subject: [PATCH 5/5] Use multipart_suggestion to avoid place holder in span_to_snippet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CO-AUTHORED-BY: Esteban Küber --- .../src/fn_ctxt/suggestions.rs | 20 +++++++++++-------- ...ransforming-option-ref-issue-127545.stderr | 6 +++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index faa6d06ccaf..b3b4c5a56fb 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1459,7 +1459,10 @@ pub(crate) fn suggest_deref_unwrap_or( return; }; - if call_ident.map_or(true, |ident| ident.name != sym::unwrap_or) { + let Some(call_ident) = call_ident else { + return; + }; + if call_ident.name != sym::unwrap_or { return; } @@ -1483,14 +1486,15 @@ pub(crate) fn suggest_deref_unwrap_or( if !self.can_coerce(expected_ty, dummy_ty) { return; } - let (provided_snip, applicability) = - match self.tcx.sess.source_map().span_to_snippet(provided_expr.span) { - Ok(snip) => (snip, Applicability::MachineApplicable), - Err(_) => ("/* _ */".to_owned(), Applicability::MaybeIncorrect), - }; - let sugg = format!("map_or({provided_snip}, |v| v)"); let msg = format!("use `{adt_name}::map_or` to deref inner value of `{adt_name}`"); - err.span_suggestion_verbose(error_span, msg, sugg, applicability); + err.multipart_suggestion_verbose( + msg, + vec![ + (call_ident.span, "map_or".to_owned()), + (provided_expr.span.shrink_to_hi(), ", |v| v".to_owned()), + ], + Applicability::MachineApplicable, + ); } /// Suggest wrapping the block in square brackets instead of curly braces diff --git a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr index 1790fc1249a..ad423f86ef9 100644 --- a/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr +++ b/tests/ui/mismatched_types/transforming-option-ref-issue-127545.stderr @@ -35,7 +35,7 @@ note: method defined here help: use `Option::map_or` to deref inner value of `Option` | LL | arg.map_or(&[], |v| v) - | ~~~~~~~~~~~~~~~~~~ + | ~~~~~~ +++++++ error[E0308]: mismatched types --> $DIR/transforming-option-ref-issue-127545.rs:13:19 @@ -59,7 +59,7 @@ note: method defined here help: use `Option::map_or` to deref inner value of `Option` | LL | arg.map_or(v, |v| v) - | ~~~~~~~~~~~~~~~~ + | ~~~~~~ +++++++ error[E0308]: mismatched types --> $DIR/transforming-option-ref-issue-127545.rs:17:19 @@ -83,7 +83,7 @@ note: method defined here help: use `Result::map_or` to deref inner value of `Result` | LL | arg.map_or(&[], |v| v) - | ~~~~~~~~~~~~~~~~~~ + | ~~~~~~ +++++++ error: aborting due to 4 previous errors