Detect missing ; on methods with return type ()

[estebank]

On a given file foo.rs:

fn foo() {
    return 1;
}

fn bar() {
    3
}

fn main() {
    3
}

Provide the following output:

error[E0308]: mismatched types
 --> foo.rs:2:12
  |
2 |     return 1;
  |            ^ expected (), found integral variable
  |
  = note: expected type `()`
  = note:    found type `{integer}`

error[E0308]: mismatched types
 --> foo.rs:6:5
  |
5 | fn bar() {
  |          - possibly return type `{integer}` missing here
6 |     3
  |     ^- consider adding a semicolon here
  |     |
  |     expected (), found integral variable
  |
  = note: expected type `()`
  = note:    found type `{integer}`

error[E0308]: mismatched types
  --> foo.rs:10:5
   |
10 |     3
   |     ^- consider adding a semicolon here
   |     |
   |     expected (), found integral variable
   |
   = note: expected type `()`
   = note:    found type `{integer}`

error: aborting due to 3 previous errors

Fixes: #25133, #40891.

Previous discussion: #36409.

[rust-highfive]

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

[estebank]

Compiler output:

[nrc]

I agree with @brson and @nikomatsakis on the previous PR that adding a semicolon is likely to be incorrect more often than it is correct. OTOH, significant semicolons are a novel feature of Rust so perhaps this is more useful for newcomers than I might think.

cc @rust-lang/compiler and @jonathandturner what do the rest of you think?

And r? @eddyb because he reviewed this PR last time around.

(I also don't think this fully addresses #25133, it seems to be a partial fix).

[dotdash]

I agree with the previous discussion, that without at least looking at the type of expression that causes the error, the hints can seem pretty nonsensical and rather confusing than helpful. At least when there is a literal of some kind, a plain variable or a constructor, I'd make more sense to me to only suggest to add a return type to the function.

Also, why does foo not get the possibly reutrn type '{integer}' missing here message?

[nikomatsakis]

This definitely seems like an improvement, but I worry that we might overwhelm users a bit too. Something about "possibly return type {integer} missing" feels less than clear. For one thing, the fact that we print {integer} is sub-optimal, but I'd also consider suggesting ->. Perhaps just say "you may wish to add a return type here" and don't try to suggest what return type it is?

In the case of 3 as the expression, it seems obvious that they meant to return the value, so I think we could skip suggesting a ;.

Otherwise, perhaps something like "add a ; here to ignore this value rather than return it"?

[JordiPolo]

I'm new to the language and the optional ; does cause a lot of early errors.
Without a historical view on how they were used before (I have no idea), for me , lack of ; == return . It's just a difficult to see return as the glyph for ; is not big.
That said, if ; will still have this role, a check that verifies your returns and tells you where those are missing or probably wrong seems very useful (as a beginner) to me.

[bors]

☔ The latest upstream changes (presumably #39485) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

Closing due to inactivity, but feel free to resubmit with a rebase!