anti pattern: #![deny(warnings)] about patterns HOT 5 CLOSED

@llogiq: Hear me Rust bard,

I can't help but be very criticial of this guideline. Let me explain why.

By disallowing the compiler to build with warnings, a crate author opts out of Rust's famed stability.

Rust's stability is rather mythical at this point. Only the syntax and standard library API are stable, and only on a best effort basis. There have been numerous breaking changes. That can be good, as Rust is still a young project. In the future, the Rust epochs system will be the principled solution to managing backward compatibility. At present, if there is any reason to not forbid all warnings during a build given #![deny(warnings)] in the source code, then this directive can be overridden with the relevant command line parameters of rustc.

Sometimes new features or old misfeatures need a change in how things are done, thus lints are written that warn for a certain grace period before being turned to deny.

This grace period is a courtesy from those responsible for the change to the affected. It is not an obligation of the affected themselves to adhere to that grace period. With this guideline, you imply that those affected by breaking changes to lint checking should not enforce the changed lint checking logic. But they normally want to. There's a reason they started out with #![deny(warnings)].

The proper way to prevent broken builds due to dependencies with strict linting is to apply rustc --cap-lints to dependencies.

Furthermore, crates that supply additional lints (e.g. rust-clippy) can no longer be used unless the annotation is removed.

That should be elaborated.

Moreover, the downsides of the alternative solutions need to be pointed out as well. Changing command line parameters to opt in to strict lint checking doesn't really solve the problem of forward compatibility. After all, when and where should these command line parameters be set then? If during CI, then the build will break still after a breaking change to lint checks. Explicitly listing numerous lints in the source code is not scalable across many crates. Furthermore, it results in another kind of forward incompatibility risk, other than breakage: risk of not keeping current with new lints. Lints are added for a reason, they address real world quality issues. Grace periods are only a compromise in the interest of keeping things going without maintenance, but it is certainly preferable to aggressively perform all lint checks available and address the results head-on.

from patterns.

@llogiq / @nrc: Can you reopen, to give others more opportunity to join the discussion?

from patterns.

In my rust2018 post, I propose using Epochs on warnings groups. I'm going to be posting on the epoch issue to find out the right venue for discussion.

from patterns.

Furthermore, crates that supply additional lints (e.g. rust-clippy) can no longer be used unless the annotation is removed.

from: https://github.com/rust-unofficial/patterns/blob/master/anti_patterns/deny-warnings.md#drawbacks

Is that still true? it doesn't seem to be, or I don't know what they meant, but I've only tested this example:

#![deny(clippy::pedantic, clippy::all, clippy::correctness, clippy::nursery)]
//^ no effect on: unreachable_patterns

#![deny(warnings)]
//^ this works!

fn main() {
    let num = Some(4);

    match num {
        Some(x) if x < 5 => println!("less than five: {}", x),
        Some(x) => println!("{}", x),
        None => (),
    }

    match num {
        Some(x) => println!("{}", x),
        Some(x) if x < 5 => println!("less than five: {}", x), // warning: unreachable pattern
        None => (),
    }                                                                                                                         
}

from patterns.

I think the "additional lints" refer to the possibility of the new lints breaking compilation.

WRT whether it is an anti-pattern… Being an ex-packager and seeing all the packages broken due to a well-intentioned -Wall -Werror has lead me to firmly see anything similar as actively harmful. Some compilation flags are only useful for development and have no place in your user's build instructions.

from patterns.