Skip to content
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/expect for let binding type mismatch #116841

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 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
80 changes: 80 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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;
}
}
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

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/typeck/tag-that-dare-not-speak-its-name.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading