From 8e21088505a543381174e98cff5f0a087ffb51e5 Mon Sep 17 00:00:00 2001 From: trevyn <230691+trevyn@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:13:07 +0400 Subject: [PATCH] Suggest removing `.unwrap()` or `.expect()` if followed by a failed `?` operator --- .../traits/fulfillment_errors.rs | 1 + .../src/error_reporting/traits/suggestions.rs | 49 ++++++++ .../try-after-unwrap-issue-127345.rs | 62 ++++++++++ .../try-after-unwrap-issue-127345.stderr | 112 ++++++++++++++++++ 4 files changed, 224 insertions(+) create mode 100644 tests/ui/try-trait/try-after-unwrap-issue-127345.rs create mode 100644 tests/ui/try-trait/try-after-unwrap-issue-127345.stderr diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index f7ec5f1ff325f..692c29bb2323d 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -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); diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 2cf808f962f08..07f8bb0df1565 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -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() diff --git a/tests/ui/try-trait/try-after-unwrap-issue-127345.rs b/tests/ui/try-trait/try-after-unwrap-issue-127345.rs new file mode 100644 index 0000000000000..760f3c74dcc82 --- /dev/null +++ b/tests/ui/try-trait/try-after-unwrap-issue-127345.rs @@ -0,0 +1,62 @@ +fn foo() -> Option { + 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 { + 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 { + 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 { + 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 { + 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 { + FakeUnwrappable.unwrap()? + //~^ ERROR the `?` operator can only be applied to values that implement `Try` + //~| HELP the trait `Try` is not implemented for `()` +} + +fn main() {} diff --git a/tests/ui/try-trait/try-after-unwrap-issue-127345.stderr b/tests/ui/try-trait/try-after-unwrap-issue-127345.stderr new file mode 100644 index 0000000000000..772072117e77c --- /dev/null +++ b/tests/ui/try-trait/try-after-unwrap-issue-127345.stderr @@ -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`.