Auto merge of #95758 - compiler-errors:issue-54771, r=estebank

bors · bors · commit d00e77078c0b · 2022-04-11T08:31:37.000Z
Only suggest removing semicolon when expression is compatible with `impl Trait` #54771 (comment) > It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there. Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?). cc #54771
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -1083,20 +1083,31 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         let parent_node = hir.get_parent_node(obligation.cause.body_id);
         let node = hir.find(parent_node);
         if let Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, _, body_id), .. })) = node
-            && let body = hir.body(*body_id)
-            && let hir::ExprKind::Block(blk, _) = &body.value.kind
+            && let hir::ExprKind::Block(blk, _) = &hir.body(*body_id).value.kind
             && sig.decl.output.span().overlaps(span)
             && blk.expr.is_none()
-            && *trait_pred.self_ty().skip_binder().kind() == ty::Tuple(ty::List::empty())
-            // FIXME(estebank): When encountering a method with a trait
-            // bound not satisfied in the return type with a body that has
-            // no return, suggest removal of semicolon on last statement.
-            // Once that is added, close #54771.
+            && trait_pred.self_ty().skip_binder().is_unit()
             && let Some(stmt) = blk.stmts.last()
-            && let hir::StmtKind::Semi(_) = stmt.kind
+            && let hir::StmtKind::Semi(expr) = stmt.kind
+            // Only suggest this if the expression behind the semicolon implements the predicate
+            && let Some(typeck_results) = self.in_progress_typeck_results
+            && let Some(ty) = typeck_results.borrow().expr_ty_opt(expr)
+            && self.predicate_may_hold(&self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred, ty))
         {
-            let sp = self.tcx.sess.source_map().end_point(stmt.span);
-            err.span_label(sp, "consider removing this semicolon");
+            err.span_label(
+                expr.span,
+                &format!(
+                    "this expression has type `{}`, which implements `{}`",
+                    ty,
+                    trait_pred.print_modifiers_and_trait_path()
+                )
+            );
+            err.span_suggestion(
+                self.tcx.sess.source_map().end_point(stmt.span),
+                "remove this semicolon",
+                String::new(),
+                Applicability::MachineApplicable
+            );
             return true;
         }
         false
diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
@@ -936,7 +936,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             } else {
                 err.span_suggestion_short(
                     span_semi,
-                    "consider removing this semicolon",
+                    "remove this semicolon",
                     String::new(),
                     Applicability::MachineApplicable,
                 );
diff --git a/src/test/ui/block-result/consider-removing-last-semi.stderr b/src/test/ui/block-result/consider-removing-last-semi.stderr
@@ -7,7 +7,7 @@ LL | pub fn f() -> String {
    |        implicitly returns `()` as its body has no tail or `return` expression
 LL |     0u8;
 LL |     "bla".to_string();
-   |                      - help: consider removing this semicolon
+   |                      - help: remove this semicolon
 
 error[E0308]: mismatched types
   --> $DIR/consider-removing-last-semi.rs:8:15
@@ -18,7 +18,7 @@ LL | pub fn g() -> String {
    |        implicitly returns `()` as its body has no tail or `return` expression
 LL |     "this won't work".to_string();
 LL |     "removeme".to_string();
-   |                           - help: consider removing this semicolon
+   |                           - help: remove this semicolon
 
 error[E0308]: mismatched types
   --> $DIR/consider-removing-last-semi.rs:13:25
@@ -29,7 +29,7 @@ LL | pub fn macro_tests() -> u32 {
    |        implicitly returns `()` as its body has no tail or `return` expression
 ...
 LL |     mac!();
-   |           - help: consider removing this semicolon
+   |           - help: remove this semicolon
 
 error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/block-result/issue-11714.stderr b/src/test/ui/block-result/issue-11714.stderr
@@ -7,7 +7,7 @@ LL | fn blah() -> i32 {
    |    implicitly returns `()` as its body has no tail or `return` expression
 ...
 LL |     ;
-   |     - help: consider removing this semicolon
+   |     - help: remove this semicolon
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/block-result/issue-13428.stderr b/src/test/ui/block-result/issue-13428.stderr
@@ -15,7 +15,7 @@ LL | fn bar() -> String {
    |    implicitly returns `()` as its body has no tail or `return` expression
 LL |     "foobar".to_string()
 LL |     ;
-   |     - help: consider removing this semicolon
+   |     - help: remove this semicolon
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/closures/old-closure-expression-remove-semicolon.fixed b/src/test/ui/closures/old-closure-expression-remove-semicolon.fixed
@@ -7,6 +7,6 @@ fn foo() -> i32 {
 fn main() {
     let _x: i32 = {
         //~^ ERROR mismatched types
-        foo() //~ HELP consider removing this semicolon
+        foo() //~ HELP remove this semicolon
     };
 }
diff --git a/src/test/ui/closures/old-closure-expression-remove-semicolon.rs b/src/test/ui/closures/old-closure-expression-remove-semicolon.rs
@@ -7,6 +7,6 @@ fn foo() -> i32 {
 fn main() {
     let _x: i32 = {
         //~^ ERROR mismatched types
-        foo(); //~ HELP consider removing this semicolon
+        foo(); //~ HELP remove this semicolon
     };
 }
diff --git a/src/test/ui/closures/old-closure-expression-remove-semicolon.stderr b/src/test/ui/closures/old-closure-expression-remove-semicolon.stderr
@@ -5,7 +5,7 @@ LL |       let _x: i32 = {
    |  ___________________^
 LL | |
 LL | |         foo();
-   | |              - help: consider removing this semicolon
+   | |              - help: remove this semicolon
 LL | |     };
    | |_____^ expected `i32`, found `()`
 
diff --git a/src/test/ui/coercion/coercion-missing-tail-expected-type.stderr b/src/test/ui/coercion/coercion-missing-tail-expected-type.stderr
@@ -6,7 +6,7 @@ LL | fn plus_one(x: i32) -> i32 {
    |    |
    |    implicitly returns `()` as its body has no tail or `return` expression
 LL |     x + 1;
-   |          - help: consider removing this semicolon
+   |          - help: remove this semicolon
 
 error[E0308]: mismatched types
   --> $DIR/coercion-missing-tail-expected-type.rs:8:13
@@ -16,7 +16,7 @@ LL | fn foo() -> Result<u8, u64> {
    |    |
    |    implicitly returns `()` as its body has no tail or `return` expression
 LL |     Ok(1);
-   |          - help: consider removing this semicolon
+   |          - help: remove this semicolon
    |
    = note:   expected enum `Result<u8, u64>`
            found unit type `()`
diff --git a/src/test/ui/issues/issue-6458-4.stderr b/src/test/ui/issues/issue-6458-4.stderr
@@ -6,7 +6,7 @@ LL | fn foo(b: bool) -> Result<bool,String> {
    |    |
    |    implicitly returns `()` as its body has no tail or `return` expression
 LL |     Err("bar".to_string());
-   |                           - help: consider removing this semicolon
+   |                           - help: remove this semicolon
    |
    = note:   expected enum `Result<bool, String>`
            found unit type `()`
diff --git a/src/test/ui/liveness/liveness-return-last-stmt-semi.stderr b/src/test/ui/liveness/liveness-return-last-stmt-semi.stderr
@@ -14,7 +14,7 @@ LL | fn bar(x: u32) -> u32 {
    |    |
    |    implicitly returns `()` as its body has no tail or `return` expression
 LL |     x * 2;
-   |          - help: consider removing this semicolon
+   |          - help: remove this semicolon
 
 error[E0308]: mismatched types
   --> $DIR/liveness-return-last-stmt-semi.rs:13:19
diff --git a/src/test/ui/suggestions/impl-trait-return-trailing-semicolon.rs b/src/test/ui/suggestions/impl-trait-return-trailing-semicolon.rs
@@ -1,8 +1,25 @@
 trait Bar {}
-impl Bar for u8 {}
+
+impl Bar for i32 {}
+
+struct Qux;
+
+impl Bar for Qux {}
+
 fn foo() -> impl Bar {
-    5; //~^ ERROR the trait bound `(): Bar` is not satisfied
+    //~^ ERROR the trait bound `(): Bar` is not satisfied
+    //~| ERROR the trait bound `(): Bar` is not satisfied
+    //~| HELP the following other types implement trait `Bar`:
+    5;
+    //~^ HELP remove this semicolon
+}
+
+fn bar() -> impl Bar {
+    //~^ ERROR the trait bound `(): Bar` is not satisfied
     //~| ERROR the trait bound `(): Bar` is not satisfied
+    //~| HELP the following other types implement trait `Bar`:
+    //~| HELP the following other types implement trait `Bar`:
+    "";
 }
 
 fn main() {}
diff --git a/src/test/ui/suggestions/impl-trait-return-trailing-semicolon.stderr b/src/test/ui/suggestions/impl-trait-return-trailing-semicolon.stderr
@@ -1,23 +1,58 @@
 error[E0277]: the trait bound `(): Bar` is not satisfied
-  --> $DIR/impl-trait-return-trailing-semicolon.rs:3:13
+  --> $DIR/impl-trait-return-trailing-semicolon.rs:9:13
    |
 LL | fn foo() -> impl Bar {
    |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
+...
 LL |     5;
-   |      - consider removing this semicolon
+   |     -- help: remove this semicolon
+   |     |
+   |     this expression has type `{integer}`, which implements `Bar`
 
 error[E0277]: the trait bound `(): Bar` is not satisfied
-  --> $DIR/impl-trait-return-trailing-semicolon.rs:3:22
+  --> $DIR/impl-trait-return-trailing-semicolon.rs:9:22
    |
 LL |   fn foo() -> impl Bar {
    |  ______________________^
+LL | |
+LL | |
+LL | |
 LL | |     5;
 LL | |
 LL | | }
    | |_^ the trait `Bar` is not implemented for `()`
    |
-   = help: the trait `Bar` is implemented for `u8`
+   = help: the following other types implement trait `Bar`:
+             Qux
+             i32
+
+error[E0277]: the trait bound `(): Bar` is not satisfied
+  --> $DIR/impl-trait-return-trailing-semicolon.rs:17:13
+   |
+LL | fn bar() -> impl Bar {
+   |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
+   |
+   = help: the following other types implement trait `Bar`:
+             Qux
+             i32
+
+error[E0277]: the trait bound `(): Bar` is not satisfied
+  --> $DIR/impl-trait-return-trailing-semicolon.rs:17:22
+   |
+LL |   fn bar() -> impl Bar {
+   |  ______________________^
+LL | |
+LL | |
+LL | |
+LL | |
+LL | |     "";
+LL | | }
+   | |_^ the trait `Bar` is not implemented for `()`
+   |
+   = help: the following other types implement trait `Bar`:
+             Qux
+             i32
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/suggestions/issue-81098.stderr b/src/test/ui/suggestions/issue-81098.stderr
@@ -27,7 +27,9 @@ LL | fn ok() -> impl core::fmt::Display {
    |            ^^^^^^^^^^^^^^^^^^^^^^^ `()` cannot be formatted with the default formatter
 LL |
 LL |     1;
-   |      - consider removing this semicolon
+   |     -- help: remove this semicolon
+   |     |
+   |     this expression has type `{integer}`, which implements `std::fmt::Display`
    |
    = help: the trait `std::fmt::Display` is not implemented for `()`
    = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
diff --git a/src/test/ui/suggestions/match-with-different-arm-types-as-stmt-instead-of-expr.stderr b/src/test/ui/suggestions/match-with-different-arm-types-as-stmt-instead-of-expr.stderr
@@ -7,7 +7,7 @@ LL | fn not_all_paths(a: &str) -> u32 {
    |    implicitly returns `()` as its body has no tail or `return` expression
 ...
 LL |     };
-   |      - help: consider removing this semicolon
+   |      - help: remove this semicolon
 
 error[E0308]: `match` arms have incompatible types
   --> $DIR/match-with-different-arm-types-as-stmt-instead-of-expr.rs:26:14

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,6 @@ fn foo() -> i32 {`
`7`	`7`	`fn main() {`
`8`	`8`	`let _x: i32 = {`
`9`	`9`	`//~^ ERROR mismatched types`
`10`		`- foo() //~ HELP consider removing this semicolon`
	`10`	`+ foo() //~ HELP remove this semicolon`
`11`	`11`	`};`
`12`	`12`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ LL \| fn foo(b: bool) -> Result<bool,String> {`
`6`	`6`	`\| \|`
`7`	`7`	\| implicitly returns `()` as its body has no tail or `return` expression
`8`	`8`	`LL \| Err("bar".to_string());`
`9`		`- \| - help: consider removing this semicolon`
	`9`	`+ \| - help: remove this semicolon`
`10`	`10`	`\|`
`11`	`11`	= note: expected enum `Result<bool, String>`
`12`	`12`	found unit type `()`