Skip to content

Commit

Permalink
suggest unwrap/expect for let binding type mismatch
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Oct 18, 2023
1 parent a483969 commit 4e9d93f
Show file tree
Hide file tree
Showing 9 changed files with 326 additions and 1 deletion.
82 changes: 81 additions & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/lifetimes/issue-26638.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/mismatched_types/mismatch-ty-dont-suggest.rs
Original file line number Diff line number Diff line change
@@ -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

}
55 changes: 55 additions & 0 deletions tests/ui/mismatched_types/mismatch-ty-dont-suggest.stderr
Original file line number Diff line number Diff line change
@@ -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`.
31 changes: 31 additions & 0 deletions tests/ui/mismatched_types/mismatch-ty-unwrap-expect.fixed
Original file line number Diff line number Diff line change
@@ -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
}
31 changes: 31 additions & 0 deletions tests/ui/mismatched_types/mismatch-ty-unwrap-expect.rs
Original file line number Diff line number Diff line change
@@ -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
}
93 changes: 93 additions & 0 deletions tests/ui/mismatched_types/mismatch-ty-unwrap-expect.stderr
Original file line number Diff line number Diff line change
@@ -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`.
4 changes: 4 additions & 0 deletions tests/ui/noexporttypeexe.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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