suggest unwrap/expect for let binding type mismatch

chenyukang · chenyukang · commit 4e9d93f56d01 · 2023-10-18T09:58:25.000+08:00
diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/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(
@@ -954,6 +955,85 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         );
     }
 
+    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;
+    }
+
     pub(crate) fn suggest_coercing_result_via_try_operator(
         &self,
         err: &mut Diagnostic,
diff --git a/tests/ui/lifetimes/issue-26638.stderr b/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
diff --git a/tests/ui/mismatched_types/mismatch-ty-dont-suggest.rs b/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
+
+}
diff --git a/tests/ui/mismatched_types/mismatch-ty-dont-suggest.stderr b/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`.
diff --git a/tests/ui/mismatched_types/mismatch-ty-unwrap-expect.fixed b/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
+}
diff --git a/tests/ui/mismatched_types/mismatch-ty-unwrap-expect.rs b/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
+}
diff --git a/tests/ui/mismatched_types/mismatch-ty-unwrap-expect.stderr b/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`.
diff --git a/tests/ui/noexporttypeexe.stderr b/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
 
diff --git a/tests/ui/typeck/tag-that-dare-not-speak-its-name.stderr b/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
 

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,10 @@ LL \| let x: isize = noexporttypelib::foo();`
`8`	`8`	`\|`
`9`	`9`	= note: expected type `isize`
`10`	`10`	found enum `Option<isize>`
	`11`	+help: consider using `Option::expect` to unwrap the `Option<isize>` value, panicking if the value is an `Option::None`
	`12`	`+ \|`
	`13`	`+LL \| let x: isize = noexporttypelib::foo().expect("REASON");`
	`14`	`+ \| +++++++++++++++++`
`11`	`15`
`12`	`16`	`error: aborting due to previous error`
`13`	`17`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,10 @@ LL \| let x : char = last(y);`
`8`	`8`	`\|`
`9`	`9`	= note: expected type `char`
`10`	`10`	found enum `Option<_>`
	`11`	+help: consider using `Option::expect` to unwrap the `Option<_>` value, panicking if the value is an `Option::None`
	`12`	`+ \|`
	`13`	`+LL \| let x : char = last(y).expect("REASON");`
	`14`	`+ \| +++++++++++++++++`
`11`	`15`
`12`	`16`	`error: aborting due to previous error`
`13`	`17`