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

lint errors make rustc abort early #82761

Closed
SNCPlay42 opened this issue Mar 4, 2021 · 4 comments · Fixed by #87337
Closed

lint errors make rustc abort early #82761

SNCPlay42 opened this issue Mar 4, 2021 · 4 comments · Fixed by #87337
Labels
A-clippy Area: Clippy A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@SNCPlay42
Copy link
Contributor

SNCPlay42 commented Mar 4, 2021

In general, the compiler tries to keep going after errors to emit as many helpful diagnostics as possible. This isn't always possible because later stages of compilation might need certain conditions to be upheld, so sometimes the compiler aborts early.

It should never be necessary to abort early due to a lint from an external tool (e.g. clippy) because the compiler's hard errors should be sufficient to enforce conditions needed by later stages.

At present, however, error-level lints emitted by external tools can cause compilation to abort early. It seems unfortunate that, after fixing all the problems reported by one run of clippy, you could run clippy again (perhaps by committing your "fixed" code and updating a PR, causing CI to run) and discover there are more problems (when CI fails).

As an example, this code:

fn main() {
    if true {} {}
    
    let x;
    if true {
        x = 1;
    } else {
        x = 1;
    }
    println!("{}", x);
}

Emits two lints with clippy:

warning: this looks like an `else {..}` but the `else` is missing
 --> src/main.rs:4:15
  |
4 |     if true {} {}
  |               ^
  |
  = note: `#[warn(clippy::suspicious_else_formatting)]` on by default
  = note: to remove this lint, add the missing `else` or add a new line before the next block
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

error: this `if` has identical blocks
  --> src/main.rs:9:12
   |
9  |       } else {
   |  ____________^
10 | |         x = 1;
11 | |     }
   | |_____^
   |
   = note: `#[deny(clippy::if_same_then_else)]` on by default
note: same as this
  --> src/main.rs:7:13
   |
7  |       if true {
   |  _____________^
8  | |         x = 1;
9  | |     } else {
   | |_____^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else

error: aborting due to previous error; 1 warning emitted

error: could not compile `playground`

If #![deny(clippy::all)] is added, only the first lint is emitted, and the compiler aborts after it:

error: this looks like an `else {..}` but the `else` is missing
 --> src/main.rs:4:15
  |
4 |     if true {} {}
  |               ^
  |
note: the lint level is defined here
 --> src/main.rs:1:8
  |
1 | #[deny(clippy::all)]
  |        ^^^^^^^^^^^
  = note: `#[deny(clippy::suspicious_else_formatting)]` implied by `#[deny(clippy::all)]`
  = note: to remove this lint, add the missing `else` or add a new line before the next block
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

error: aborting due to previous error

error: could not compile `playground`

@rustbot label A-lint

@rustbot rustbot added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Mar 4, 2021
@SNCPlay42
Copy link
Contributor Author

Come to think of it, if a lint from any source, including internally, could have its level be set to warn or lower, that implies the compiler doesn't require it to be enforced in later stages.

@SNCPlay42 SNCPlay42 changed the title external lint errors make rustc abort early lint errors make rustc abort early Mar 4, 2021
@jyn514
Copy link
Member

jyn514 commented Jul 21, 2021

Note that this would need cargo support if you have a path dependency. rustc would print all lints for the first crate, error out, and then cargo would see the error and stop before compiling the next crate in the workspace. For cargo to avoid that, rustc needs to tell it somehow that these are 'lint-only errors'.

@jyn514 jyn514 added A-clippy Area: Clippy C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 21, 2021
@bors bors closed this as completed in 214cd1f Nov 9, 2021
@jyn514
Copy link
Member

jyn514 commented Nov 9, 2021

@sxlijin @SNCPlay42 feel free to open a new issue for showing all lint errors across a workspace :) it should probably be under rust-lang/cargo.

@sxlijin
Copy link

sxlijin commented Nov 9, 2021

Hurrah! Many thanks :)

flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Nov 18, 2021
Don't abort compilation after giving a lint error

The only reason to use `abort_if_errors` is when the program is so broken that either:
1. later passes get confused and ICE
2. any diagnostics from later passes would be noise

This is never the case for lints, because the compiler has to be able to deal with `allow`-ed lints.
So it can continue to lint and compile even if there are lint errors.

Closes rust-lang/rust#82761. This is a WIP because I have a feeling it will exit with 0 even if there were lint errors; I don't have a computer that can build rustc locally at the moment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-clippy Area: Clippy A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants