needless_return autofix does not remove semicolons when creating implicit returns, causing them to fail

[rowan-sl]

Summary

In some cases in functions using match statements where it would make a lot of sense to use a implicit return, when clippy attempts to autofix this, it does not correctly remove the semicolons necessary to crate a implicit return, or in some cases thinks it has not, but actuality has.

possibly useful:

• Rust edition: 2021
• no dependancies

also this is my first bug report here, so please give me feedback on it

Reproducer

when writing a match expression in a function like

fn foo() -> u64 {
  match bar {
      80 => {
          return 10;
      }
      _ => {
          return 100;
      }
  };
}

for a function, clippy suggests to remove the return statements, (which it should). however, after running cargo clippy --fix --broken-code,
it does fix the return, producing

fn foo() -> u64 {
  match bar {
      80 => {
          10
      }
      _ => {
          100
      }
  }
}

however it gives this error:

Checking clipy_err_test v0.1.0 ([path removed, contains personal info])
warning: failed to automatically apply fixes suggested by rustc to crate `clipy_err_test`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0308]: mismatched types
  --> src/main.rs:1:21
   |
1  | fn update_read() -> u128 {
   |    -----------      ^^^^ expected `u128`, found `()`
   |    |
   |    implicitly returns `()` as its body has no tail or `return` expression
...
10 |     };
   |      - help: consider removing this semicolon

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
Original diagnostics will follow.

       Fixed src/main.rs (1 fix)
    Finished dev [unoptimized + debuginfo] target(s) in 0.51s

even though the resulting code does not have a semicolon where it is pointing to.

By using a larger example, such as

struct Test {
    foo: i8
}


fn update_read() -> u128 {
    let read: Result<Test, ()> = Ok(Test{foo:10});
    match read {
        Ok(stat) => {
            match stat.foo {
                10 => {
                    return 80;
                }
                _ => {
                    return 100;
                }
            };
        }
        Err(_) => {
            return 100;
        }
    };
}

fn main() {
    println!("{}", update_read());
}

however, it gives a similar error:

    Checking clipy_err_test v0.1.0 
warning: failed to automatically apply fixes suggested by rustc to crate `clipy_err_test`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0308]: `match` arms have incompatible types
  --> src/main.rs:20:13
   |
8  |       match read {
   |       ---------- `match` arms have incompatible types
9  |           Ok(stat) => {
10 | /             match stat.foo {
11 | |                 10 => {
12 | |                     80
13 | |                 }
...  |
16 | |                 }
17 | |             };
   | |              -
   | |              |
   | |______________help: consider removing this semicolon
   |                this is found to be of type `()`
...
20 |               100
   |               ^^^ expected `()`, found integer

error[E0308]: mismatched types
 --> src/main.rs:6:21
  |
6 | fn update_read() -> u128 {
  |    -----------      ^^^^ expected `u128`, found `()`
  |    |
  |    implicitly returns `()` as its body has no tail or `return` expression

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
Original diagnostics will follow.

error[E0308]: mismatched types
  --> src/main.rs:6:21
   |
6  | fn update_read() -> u128 {
   |    -----------      ^^^^ expected `u128`, found `()`
   |    |
   |    implicitly returns `()` as its body has no tail or `return` expression
...
22 |     };
   |      - help: consider removing this semicolon

For more information about this error, try `rustc --explain E0308`.
error: could not compile `clipy_err_test` due to previous error
warning: build failed, waiting for other jobs to finish...
warning: failed to automatically apply fixes suggested by rustc to crate `clipy_err_test`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0308]: mismatched types
  --> src/main.rs:6:21
   |
6  | fn update_read() -> u128 {
   |    -----------      ^^^^ expected `u128`, found `()`
   |    |
   |    implicitly returns `()` as its body has no tail or `return` expression
...
22 |     };
   |      - help: consider removing this semicolon

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
Original diagnostics will follow.

error: build failed

but this time produces code this is broken, shown here:

struct Test {
    foo: i8
}


fn update_read() -> u128 {
    let read: Result<Test, ()> = Ok(Test{foo:10});
    match read {
        Ok(stat) => {
            match stat.foo {
                10 => {
                    80
                }
                _ => {
                    100
                }
            }
        }
        Err(_) => {
            100
        }
    };
}

fn main() {
    println!("{}", update_read());
}

where there is a semicolon preventing the implicit return.

Version

rustc 1.57.0 (f1edd0429 2021-11-29)
binary: rustc
commit-hash: f1edd0429582dd29cccacaf50fd134b05593bd9c
commit-date: 2021-11-29
host: x86_64-unknown-linux-gnu
release: 1.57.0
LLVM version: 13.0.0

Additional Labels

@rustbot label +I-suggestion-causes-error

[dswij]

@rustbot claim

[dswij]

Thanks for the bug report!

This is similar to #7358, it seems like needless_return does not consider semicolons when checking match at the end.

[llogiq]

Note that the match does not need to be the direct parent of the return expression.

[rowan-sl]

Thanks for the bug report!

This is similar to #7358, it seems like needless_return does not consider semicolons when checking match at the end.

I did later see this, but it seemed like the part where it says it fails but does not actually fail was new.