On: Don't unwrap options: There are better ways (2024)

[mplanchard]

This is nice, but fairly miserable to deal with in in-module unit tests, IMO.

We get around it by using conditional compilation and putting the lints in our entrypoints (`main.rs` or `lib.rs`), which is done automatically for any new entrypoint in the codebase via a Make target and some awk magic.

As an example, the following forbids print and dbg statements in release builds (all output should go through logging), allows it with a warning in debug builds, and allows it unconditionally in tests:

    #![cfg_attr(not(debug_assertions), deny(clippy::dbg_macro))]
    #![cfg_attr(not(debug_assertions), deny(clippy::print_stdout))]
    #![cfg_attr(not(debug_assertions), deny(clippy::print_stderr))]
    #![cfg_attr(debug_assertions, warn(clippy::dbg_macro))]
    #![cfg_attr(debug_assertions, warn(clippy::print_stdout))]
    #![cfg_attr(debug_assertions, warn(clippy::print_stderr))]
    #![cfg_attr(test, allow(clippy::dbg_macro))]
    #![cfg_attr(test, allow(clippy::print_stdout))]
    #![cfg_attr(test, allow(clippy::print_stderr))]

AFAIK there isn't currently a way to configure per-profile lints in the top-level Cargo configs. I wish there were.

[indiv0 → mplanchard]

We just set all the lints to `warn` by default then `RUSTFLAGS="--deny warnings"` when building for release (or in CI).

[mplanchard → indiv0]

Yeah that is nice too, and that should also skip all the test code. I think all the clippy warnings on tests for unwrapping and such when working locally would bug me though. And at least the eglot LSP client tends to get bogged down when there are more than a thousand or so clippy warnings/errors, I have found.