Rollup merge of #116841 - chenyukang:yukang-suggest-unwrap-expect, r=b-naber

Suggest unwrap/expect for let binding type mismatch

Found it when investigating #116738
I'm not sure whether it's a good style to suggest `unwrap`, seems it's may helpful for newcomers.

#116738 needs another fix to improve it.

Author: matthiaskrgr

compiler/rustc_hir_typeck/src/demand.rs

@@ -58,7 +58,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             || self.suggest_into(err, expr, expr_ty, expected)
             || self.suggest_floating_point_literal(err, expr, expected)
             || self.suggest_null_ptr_for_literal_zero_given_to_ptr_arg(err, expr, expected)
-            || self.suggest_coercing_result_via_try_operator(err, expr, expected, expr_ty);
+            || self.suggest_coercing_result_via_try_operator(err, expr, expected, expr_ty)
+            || self.suggest_missing_unwrap_expect(err, expr, expected, expr_ty);
 
         if !suggested {
             self.note_source_of_type_mismatch_constraint(

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -6,6 +6,7 @@ use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
 use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
 use rustc_errors::{Applicability, Diagnostic, MultiSpan};
 use rustc_hir as hir;
+use rustc_hir::def::Res;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind};
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{
@@ -1738,4 +1739,83 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // If the field is hygienic it must come from the same syntax context.
             && self.tcx.def_ident_span(field.did).unwrap().normalize_to_macros_2_0().eq_ctxt(span)
     }
+
+    pub(crate) fn suggest_missing_unwrap_expect(
+        &self,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'tcx>,
+        expected: Ty<'tcx>,
+        found: Ty<'tcx>,
+    ) -> bool {
+        let ty::Adt(adt, args) = found.kind() else { return false };
+        let ret_ty_matches = |diagnostic_item| {
+            if let Some(ret_ty) = self
+                    .ret_coercion
+                    .as_ref()
+                    .map(|c| self.resolve_vars_if_possible(c.borrow().expected_ty()))
+                    && let ty::Adt(kind, _) = ret_ty.kind()
+                    && self.tcx.get_diagnostic_item(diagnostic_item) == Some(kind.did())
+                {
+                    true
+                } else {
+                    false
+                }
+        };
+
+        // don't suggest anything like `Ok(ok_val).unwrap()` , `Some(some_val).unwrap()`,
+        // `None.unwrap()` etc.
+        let is_ctor = matches!(
+            expr.kind,
+            hir::ExprKind::Call(
+                hir::Expr {
+                    kind: hir::ExprKind::Path(hir::QPath::Resolved(
+                        None,
+                        hir::Path { res: Res::Def(hir::def::DefKind::Ctor(_, _), _), .. },
+                    )),
+                    ..
+                },
+                ..,
+            ) | hir::ExprKind::Path(hir::QPath::Resolved(
+                None,
+                hir::Path { res: Res::Def(hir::def::DefKind::Ctor(_, _), _), .. },
+            )),
+        );
+
+        let (article, kind, variant, sugg_operator) =
+            if self.tcx.is_diagnostic_item(sym::Result, adt.did()) {
+                ("a", "Result", "Err", ret_ty_matches(sym::Result))
+            } else if self.tcx.is_diagnostic_item(sym::Option, adt.did()) {
+                ("an", "Option", "None", ret_ty_matches(sym::Option))
+            } else {
+                return false;
+            };
+        if is_ctor || !self.can_coerce(args.type_at(0), expected) {
+            return false;
+        }
+
+        let (msg, sugg) = if sugg_operator {
+            (
+                format!(
+                    "use the `?` operator to extract the `{found}` value, propagating \
+                            {article} `{kind}::{variant}` value to the caller"
+                ),
+                "?",
+            )
+        } else {
+            (
+                format!(
+                    "consider using `{kind}::expect` to unwrap the `{found}` value, \
+                                panicking if the value is {article} `{kind}::{variant}`"
+                ),
+                ".expect(\"REASON\")",
+            )
+        };
+        err.span_suggestion_verbose(
+            expr.span.shrink_to_hi(),
+            msg,
+            sugg,
+            Applicability::HasPlaceholders,
+        );
+        return true;
+    }
 }

tests/ui/lifetimes/issue-26638.stderr

@@ -44,6 +44,10 @@ LL | fn parse_type(iter: Box<dyn Iterator<Item=&str>+'static>) -> &str { iter.ne
    |
    = note: expected reference `&str`
                    found enum `Option<&str>`
+help: consider using `Option::expect` to unwrap the `Option<&str>` value, panicking if the value is an `Option::None`
+   |
+LL | fn parse_type(iter: Box<dyn Iterator<Item=&str>+'static>) -> &str { iter.next().expect("REASON") }
+   |                                                                                +++++++++++++++++
 
 error[E0061]: this function takes 1 argument but 0 arguments were supplied
   --> $DIR/issue-26638.rs:5:47

tests/ui/mismatched_types/mismatch-ty-dont-suggest.rs

@@ -0,0 +1,23 @@
+#![allow(unused, dead_code)]
+
+fn test_unwrap() -> Option<i32> {
+    let b: Result<i32, ()> = Ok(1);
+    let v: i32 = b; // return type is not `Result`, we don't suggest ? here
+    //~^ ERROR mismatched types
+    Some(v)
+}
+
+fn test_unwrap_option() -> Result<i32, ()> {
+    let b = Some(1);
+    let v: i32 = b; // return type is not `Option`, we don't suggest ? here
+    //~^ ERROR mismatched types
+    Ok(v)
+}
+
+fn main() {
+    let v: i32 = Some(0); //~ ERROR mismatched types
+
+    let c = Ok(false);
+    let v: i32 = c; //~ ERROR mismatched types
+
+}

tests/ui/mismatched_types/mismatch-ty-dont-suggest.stderr

@@ -0,0 +1,55 @@
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-dont-suggest.rs:5:18
+   |
+LL |     let v: i32 = b; // return type is not `Result`, we don't suggest ? here
+   |            ---   ^ expected `i32`, found `Result<i32, ()>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Result<i32, ()>`
+help: consider using `Result::expect` to unwrap the `Result<i32, ()>` value, panicking if the value is a `Result::Err`
+   |
+LL |     let v: i32 = b.expect("REASON"); // return type is not `Result`, we don't suggest ? here
+   |                   +++++++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-dont-suggest.rs:12:18
+   |
+LL |     let v: i32 = b; // return type is not `Option`, we don't suggest ? here
+   |            ---   ^ expected `i32`, found `Option<{integer}>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<{integer}>`
+help: consider using `Option::expect` to unwrap the `Option<{integer}>` value, panicking if the value is an `Option::None`
+   |
+LL |     let v: i32 = b.expect("REASON"); // return type is not `Option`, we don't suggest ? here
+   |                   +++++++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-dont-suggest.rs:18:18
+   |
+LL |     let v: i32 = Some(0);
+   |            ---   ^^^^^^^ expected `i32`, found `Option<{integer}>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<{integer}>`
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-dont-suggest.rs:21:18
+   |
+LL |     let v: i32 = c;
+   |            ---   ^ expected `i32`, found `Result<bool, _>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Result<bool, _>`
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0308`.

tests/ui/mismatched_types/mismatch-ty-unwrap-expect.fixed

@@ -0,0 +1,31 @@
+// run-rustfix
+#![allow(unused, dead_code)]
+
+fn func() -> Option<i32> {
+    Some(1)
+}
+
+fn test_unwrap() -> Result<i32, ()> {
+    let b: Result<i32, ()> = Ok(1);
+    let v: i32 = b?; //~ ERROR mismatched types
+    Ok(v)
+}
+
+fn test_unwrap_option() -> Option<i32> {
+    let b = Some(1);
+    let v: i32 = b?; //~ ERROR mismatched types
+    Some(v)
+}
+
+fn main() {
+    let a = Some(1);
+    let v: i32 = a.expect("REASON"); //~ ERROR mismatched types
+
+    let b: Result<i32, ()> = Ok(1);
+    let v: i32 = b.expect("REASON"); //~ ERROR mismatched types
+
+    let v: i32 = func().expect("REASON"); //~ ERROR mismatched types
+
+    let a = None;
+    let v: i32 = a.expect("REASON"); //~ ERROR mismatched types
+}

tests/ui/mismatched_types/mismatch-ty-unwrap-expect.rs

@@ -0,0 +1,31 @@
+// run-rustfix
+#![allow(unused, dead_code)]
+
+fn func() -> Option<i32> {
+    Some(1)
+}
+
+fn test_unwrap() -> Result<i32, ()> {
+    let b: Result<i32, ()> = Ok(1);
+    let v: i32 = b; //~ ERROR mismatched types
+    Ok(v)
+}
+
+fn test_unwrap_option() -> Option<i32> {
+    let b = Some(1);
+    let v: i32 = b; //~ ERROR mismatched types
+    Some(v)
+}
+
+fn main() {
+    let a = Some(1);
+    let v: i32 = a; //~ ERROR mismatched types
+
+    let b: Result<i32, ()> = Ok(1);
+    let v: i32 = b; //~ ERROR mismatched types
+
+    let v: i32 = func(); //~ ERROR mismatched types
+
+    let a = None;
+    let v: i32 = a; //~ ERROR mismatched types
+}

tests/ui/mismatched_types/mismatch-ty-unwrap-expect.stderr

@@ -0,0 +1,93 @@
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:10:18
+   |
+LL |     let v: i32 = b;
+   |            ---   ^ expected `i32`, found `Result<i32, ()>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Result<i32, ()>`
+help: use the `?` operator to extract the `Result<i32, ()>` value, propagating a `Result::Err` value to the caller
+   |
+LL |     let v: i32 = b?;
+   |                   +
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:16:18
+   |
+LL |     let v: i32 = b;
+   |            ---   ^ expected `i32`, found `Option<{integer}>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<{integer}>`
+help: use the `?` operator to extract the `Option<{integer}>` value, propagating an `Option::None` value to the caller
+   |
+LL |     let v: i32 = b?;
+   |                   +
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:22:18
+   |
+LL |     let v: i32 = a;
+   |            ---   ^ expected `i32`, found `Option<{integer}>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<{integer}>`
+help: consider using `Option::expect` to unwrap the `Option<{integer}>` value, panicking if the value is an `Option::None`
+   |
+LL |     let v: i32 = a.expect("REASON");
+   |                   +++++++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:25:18
+   |
+LL |     let v: i32 = b;
+   |            ---   ^ expected `i32`, found `Result<i32, ()>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Result<i32, ()>`
+help: consider using `Result::expect` to unwrap the `Result<i32, ()>` value, panicking if the value is a `Result::Err`
+   |
+LL |     let v: i32 = b.expect("REASON");
+   |                   +++++++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:27:18
+   |
+LL |     let v: i32 = func();
+   |            ---   ^^^^^^ expected `i32`, found `Option<i32>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<i32>`
+help: consider using `Option::expect` to unwrap the `Option<i32>` value, panicking if the value is an `Option::None`
+   |
+LL |     let v: i32 = func().expect("REASON");
+   |                        +++++++++++++++++
+
+error[E0308]: mismatched types
+  --> $DIR/mismatch-ty-unwrap-expect.rs:30:18
+   |
+LL |     let v: i32 = a;
+   |            ---   ^ expected `i32`, found `Option<_>`
+   |            |
+   |            expected due to this
+   |
+   = note: expected type `i32`
+              found enum `Option<_>`
+help: consider using `Option::expect` to unwrap the `Option<_>` value, panicking if the value is an `Option::None`
+   |
+LL |     let v: i32 = a.expect("REASON");
+   |                   +++++++++++++++++
+
+error: aborting due to 6 previous errors
+
+For more information about this error, try `rustc --explain E0308`.

tests/ui/noexporttypeexe.stderr

@@ -8,6 +8,10 @@ LL |   let x: isize = noexporttypelib::foo();
    |
    = note: expected type `isize`
               found enum `Option<isize>`
+help: consider using `Option::expect` to unwrap the `Option<isize>` value, panicking if the value is an `Option::None`
+   |
+LL |   let x: isize = noexporttypelib::foo().expect("REASON");
+   |                                        +++++++++++++++++
 
 error: aborting due to previous error
 

tests/ui/typeck/tag-that-dare-not-speak-its-name.stderr

@@ -8,6 +8,10 @@ LL |     let x : char = last(y);
    |
    = note: expected type `char`
               found enum `Option<_>`
+help: consider using `Option::expect` to unwrap the `Option<_>` value, panicking if the value is an `Option::None`
+   |
+LL |     let x : char = last(y).expect("REASON");
+   |                           +++++++++++++++++
 
 error: aborting due to previous error