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

Only suggest removing semicolon when expression is compatible with impl Trait #95758

Merged
merged 1 commit into from
Apr 11, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/block-result/consider-removing-last-semi.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/block-result/issue-11714.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/block-result/issue-13428.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | let _x: i32 = {
| ___________________^
LL | |
LL | | foo();
| | - help: consider removing this semicolon
| | - help: remove this semicolon
LL | | };
| |_____^ expected `i32`, found `()`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 `()`
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-6458-4.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 `()`
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/liveness/liveness-return-last-stmt-semi.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 19 additions & 2 deletions src/test/ui/suggestions/impl-trait-return-trailing-semicolon.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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 `()`
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a span label pointing at the last statement saying "this is a statement, which are always of type ()" and maybe mention the type of the expression in that statement if it were a tail expression, explaining that it doesn't match the trait?

|
= 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`.
4 changes: 3 additions & 1 deletion src/test/ui/suggestions/issue-81098.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down