Detect type mismatch due to loop that might never iterate

[estebank]

Given

fn foo() -> i32 {
    for i in 0..0 {
        return i;
    }
}

the current output is

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 |   fn foo() -> i32 {
  |               --- expected `i32` because of return type
2 | /     for i in 0..0 {
3 | |         return i;
4 | |     }
  | |_____^ expected `i32`, found `()`

If a return statement is present in the tail expression, and that tail expression is a loop, it should provide more information for newcomers:

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 |   fn foo() -> i32 {
  |               --- expected `i32` because of return type
2 | /     for i in 0..0 {
3 | |         return i;
4 | |     }
  | |_____^ expected `i32`, found `()`
note: the function expects a value to always be returned, but loops might run zero times
 --> src/lib.rs:2:5
  |
2 |     for i in 0..0 {
  |              ^^^^ this might have zero elements to iterate on
3 |         return i;
  |                - if the loop doesn't execute, this value would never get returned
help: return a value for the case when the loop has zero elements to iterate on, and consider changing the return type to account for that possibility
  |
1 ~ fn foo() -> Option<i32> {
...
5 +     None
  |

[lyming2007]

@rustbot claim

[lyming2007]

do we need to duplicate the statement of loop to emit more information? How about the output like this?

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 |   fn foo() -> i32 {
  |               --- expected `i32` because of return type
2 | /     for i in 0..0 {
   |           ^^^^ this might have zero elements to iterate on
3 | |         return i;
   |                - if the loop doesn't execute, this value would never get returned
help: return a value for the case when the loop has zero elements to iterate on, and consider changing the return type to account for that possibility
4 | |     }
  | |_____^ expected `i32`, found `()`
note: the function expects a value to always be returned, but loops might run zero times

[estebank]

@lyming2007 that might work as well, but I'd like to see what it looks like in the terminal, as too much text density can affect readability. The note for example is redundant with the label in line 2, so that could be removed when we show the label.

[lyming2007]

now the output looks like:

error[E0308]: mismatched types
 --> main.rs:2:5
  |
1 |   fn foo() -> i32 {
  |               --- expected `i32` because of return type
2 | /     for i in 0..0 {
3 | |         return i;
4 | |     }
  | |_____^ expected `i32`, found `()`

note: the function expects a value to always be returned, but loops might run zero times
 --> main.rs:2:5
  |
2 |       for i in 0..0 {
  |       ^------------
  |       |
  |  _____this might have zero elements to iterate on
  | |
3 | |         return i;
  | |         -------- if the loop doesn't execute, this value would never get returned
4 | |     }
  | |_____^
  |
  = help: return a value for the case when the loop has zero elements to iterate on, and consider changing the return type to account for that possibility

error: aborting due to previous error

Is that good enough? @estebank

[lyming2007]

#100094

[estebank]

@lyming2007 that seems very close! I'll add comments in the PR.

[lyming2007]

@lyming2007 that seems very close! I'll add comments in the PR.

committed the changes as you suggested which is very helpful. Please check if I missed anything. @estebank

[lyming2007]

@estebank thanks for the review and suggestions.
Can you take a look at #99064 by the way? It's been a while since I fixed it

[estebank]

Removing D-newcomer-roadblock as the current output is

error[E0308]: mismatched types
 --> src/lib.rs:2:5
  |
1 |   fn foo() -> i32 {
  |               --- expected `i32` because of return type
2 | /     for i in 0..0 {
3 | |         return i;
4 | |     }
  | |_____^ expected `i32`, found `()`
  |
note: the function expects a value to always be returned, but loops might run zero times
 --> src/lib.rs:2:5
  |
2 |     for i in 0..0 {
  |     ^^^^^^^^^^^^^ this might have zero elements to iterate on
3 |         return i;
  |         -------- if the loop doesn't execute, this value would never get returned
  = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility

and that's already great. Only thing left to do might be to add a structured suggestion.