Skip to content

Commit

Permalink
Suggest removing .unwrap() or .expect() if followed by a failed `…
Browse files Browse the repository at this point in the history
…?` operator
  • Loading branch information
trevyn committed Jul 15, 2024
1 parent 7caf672 commit 8e21088
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {

self.suggest_floating_point_literal(&obligation, &mut err, leaf_trait_ref);
self.suggest_dereferencing_index(&obligation, &mut err, leaf_trait_predicate);
suggested |= self.suggest_remove_unwrap(&obligation, &mut err, leaf_trait_predicate);
suggested |= self.suggest_dereferences(&obligation, &mut err, leaf_trait_predicate);
suggested |= self.suggest_fn_call(&obligation, &mut err, leaf_trait_predicate);
let impl_candidates = self.find_similar_impl_candidates(leaf_trait_predicate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,55 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
false
}

/// When a failed try operator follows an `unwrap()` or `expect()`, suggest removing it
fn suggest_remove_unwrap(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut Diag<'_>,
trait_pred: ty::PolyTraitPredicate<'tcx>,
) -> bool {
if !matches!(obligation.cause.code(), ObligationCauseCode::QuestionMark) {
return false;
}

let Some(body) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) else {
return false;
};

let mut expr_finder = super::FindExprBySpan::new(obligation.cause.span, self.tcx);
expr_finder.visit_expr(body.value);

if let Some(expr) = expr_finder.result
&& let Some(node) = self.tcx.hir().parent_iter(expr.hir_id).nth(2)
&& let hir::Node::Expr(expr) = node.1
&& let hir::ExprKind::Match(expr, _, _) = expr.kind
&& let hir::ExprKind::Call(_, args) = expr.kind
&& let Some(expr) = args.get(0)
&& let hir::ExprKind::MethodCall(pathsegment, receiver, _, span) = expr.kind
&& let Some(typeck_results) = &self.typeck_results
&& let receiver_ty = typeck_results.expr_ty_adjusted(receiver)
&& let ty::Adt(adt, _) = receiver_ty.kind()
&& (self.tcx.is_diagnostic_item(sym::Option, adt.did())
|| self.tcx.is_diagnostic_item(sym::Result, adt.did()))
&& (pathsegment.ident.name == sym::unwrap || pathsegment.ident.name == sym::expect)
&& self.predicate_may_hold(&self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
trait_pred.map_bound(|trait_pred| (trait_pred, receiver_ty)),
))
{
err.span_suggestion_verbose(
receiver.span.shrink_to_hi().to(span),
format!("remove the `.{}()`", pathsegment.ident.name),
"",
Applicability::MaybeIncorrect,
);

return true;
}

false
}

fn suggest_remove_await(&self, obligation: &PredicateObligation<'tcx>, err: &mut Diag<'_>) {
let hir = self.tcx.hir();
if let ObligationCauseCode::AwaitableExpr(hir_id) = obligation.cause.code().peel_derives()
Expand Down
62 changes: 62 additions & 0 deletions tests/ui/try-trait/try-after-unwrap-issue-127345.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
fn foo() -> Option<usize> {
let x = Some(42).expect("moop")?;
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.expect()`
let x = Some(42).unwrap()?;
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.unwrap()`
x
}

fn bar() -> Option<usize> {
foo().or(Some(43)).unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `usize`
//~| HELP remove the `.unwrap()`
}

fn baz() -> Result<usize, ()> {
Ok(44).unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.unwrap()`
}

fn baz2() -> Result<String, ()> {
Ok(44).unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.unwrap()`
}

fn baz3() -> Option<usize> {
Ok(44).unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.unwrap()`
}

fn baz4() {
Ok(44).unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `{integer}`
//~| HELP remove the `.unwrap()`
//~| ERROR the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
//~| HELP the trait `FromResidual<_>` is not implemented for `()`
}

struct FakeUnwrappable;

impl FakeUnwrappable {
fn unwrap(self) -> () {}
}

fn qux() -> Option<usize> {
FakeUnwrappable.unwrap()?
//~^ ERROR the `?` operator can only be applied to values that implement `Try`
//~| HELP the trait `Try` is not implemented for `()`
}

fn main() {}
112 changes: 112 additions & 0 deletions tests/ui/try-trait/try-after-unwrap-issue-127345.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:2:13
|
LL | let x = Some(42).expect("moop")?;
| ^^^^^^^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.expect()`
|
LL - let x = Some(42).expect("moop")?;
LL + let x = Some(42)?;
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:6:13
|
LL | let x = Some(42).unwrap()?;
| ^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.unwrap()`
|
LL - let x = Some(42).unwrap()?;
LL + let x = Some(42)?;
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:14:5
|
LL | foo().or(Some(43)).unwrap()?
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `usize`
|
= help: the trait `Try` is not implemented for `usize`
help: remove the `.unwrap()`
|
LL - foo().or(Some(43)).unwrap()?
LL + foo().or(Some(43))?
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:21:5
|
LL | Ok(44).unwrap()?
| ^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.unwrap()`
|
LL - Ok(44).unwrap()?
LL + Ok(44)?
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:28:5
|
LL | Ok(44).unwrap()?
| ^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.unwrap()`
|
LL - Ok(44).unwrap()?
LL + Ok(44)?
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:35:5
|
LL | Ok(44).unwrap()?
| ^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.unwrap()`
|
LL - Ok(44).unwrap()?
LL + Ok(44)?
|

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:42:5
|
LL | Ok(44).unwrap()?
| ^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `{integer}`
|
= help: the trait `Try` is not implemented for `{integer}`
help: remove the `.unwrap()`
|
LL - Ok(44).unwrap()?
LL + Ok(44)?
|

error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
--> $DIR/try-after-unwrap-issue-127345.rs:42:20
|
LL | fn baz4() {
| --------- this function should return `Result` or `Option` to accept `?`
LL | Ok(44).unwrap()?
| ^ cannot use the `?` operator in a function that returns `()`
|
= help: the trait `FromResidual<_>` is not implemented for `()`

error[E0277]: the `?` operator can only be applied to values that implement `Try`
--> $DIR/try-after-unwrap-issue-127345.rs:57:5
|
LL | FakeUnwrappable.unwrap()?
| ^^^^^^^^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `()`
|
= help: the trait `Try` is not implemented for `()`

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0277`.

0 comments on commit 8e21088

Please sign in to comment.