Be able to disable/enable Clippy lints globally

[repi]

In our monorepo we want to have the same baseline of Clippy lints enabled & disabled across our crates. Right now we need to set this up manually for every crate in lib.rs / bin.rs which is error prone and cumbersome.

This is what almost all of our crates have right now:

#![warn(clippy::all)]
#![warn(rust_2018_idioms)]
#![allow(clippy::new_ret_no_self)] // believe this is fixed in nightly https://github.com/rust-lang-nursery/rust-clippy/issues/3313

We would strongly prefer to instead be able to specify this in our workspace Cargo.toml or root Clippy.toml.

This is tracked in:

• Support defining enabled and disabled lints in a configuration file rust-lang/cargo#5034
• Allow/deny lints in clippy.toml rust-lang/rust-clippy#1313
• It is impossible to understand from the readme file how to supress lints using clippy.toml rust-lang/rust-clippy#3164
• [Roadmap] Configuration file for lints rust-lang/rust-clippy#6625
• Implicit Inheritance for Workspapce Inheritance rust-lang/cargo#12208

[repi]

Ran into this again today with trying to upgrade to Rust 1.34 and where the redundant_closure had been enabled.

All the cases it triggered on was for small member wrapper functions for simple conversions and where it would be a lot more verbose and quite annoying to use the full written out types, examples:

warning: redundant closure found
   --> br/src/network/ml_service.rs:147:45
    |
147 |     TcpListener::bind(&socket_addr).map_err(|e| e.into())
    |                                             ^^^^^^^^^^^^ help: remove closure as shown: `std::convert::Into::into`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

///////////////////////////////////////////

warning: redundant closure found
   --> texture-synth/src/applet.rs:505:18
    |
505 |             .map(|s| s.to_string())
    |                  ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string`
    |
note: lint level defined here
   --> texture-synth/src/lib.rs:1:9
    |
1   | #![warn(clippy::all)]
    |         ^^^^^^^^^^^
    = note: #[warn(clippy::redundant_closure)] implied by #[warn(clippy::all)]
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

////////////////////////////////////////

warning: redundant closure found
   --> texture-synthesis/src/service.rs:110:62
    |
110 |                     example_guides: source_guides.iter().map(|a| a.to_rgba()).collect(),
    |                                                              ^^^^^^^^^^^^^^^ help: remove closure as shown: `image::dynimage::DynamicImage::to_rgba`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

We also had a few of these cases where the suggested code didn't work because the types it suggested didn't have the crate:: prefix. Looks like it is tracked in this issue: rust-lang/rust-clippy#3071.

Regardless, this is a case where we would like to disable the redundant_closure lint by default across our crates. But for now likely will have to just add it to existing crates that run into this issue, and maybe manually propagate that lint toggle to the rest of our crates to avoid our devs running into this clippy warning and changing the code in a non-ideal way.

[repi]

We now have 81 crates in our monorepo, all having to specify #![warn(clippy::all)] & co in every lib.rs to get the behavior we want here. It works but it is a bit messy :(

[yorickpeterse]

@repi I ran into this myself for a project I am working on. When running Clippy you can disable lints via the CLI. I ended up putting this in an already existing Makefile, for example:

clippy:
	touch src/lib.rs
	${CARGO_CMD} clippy -- -Dwarnings \
		-Arenamed_and_removed_lints \
		-Aclippy::new_without_default \
		-Aclippy::needless_range_loop

Here the touch src/lib.rs is needed because otherwise Clippy does not (or at least did not in the past) build the changed files. Here the flag -A is used to globally disable certain lints.

Clippy also supports using a configuration file, but last I checked you can't disable lints using it.

[repi]

@yorickpeterse thanks, that does look like a workable solution for CI.

Though for working locally for users and with tools like RLS I really want a solution where everyone can just run cargo clippy and it has our set of additionally enabled or disabled lints so everyone runs the same lints. Hopefully the clippy config can be extended for that. Or even better: in Cargo.toml

[yorickpeterse]

@repi There is a discussion about adding support for enabling/disabling lints in Cargo.toml in rust-lang/cargo#5034, but it seems that at least since 2018 (example: rust-lang/cargo#5728) not a lot of progress has been made. I wouldn't be surprised if it will take a while for configuration file support to land in Clippy.

[repi]

Yeah doesn't look like very active development of it unfortunately, but hope Cargo/Clippy gets it eventually.

Just added these lines on top of 30 lib.rs files in our workspace to enable more lints for our codebase.

#![warn(clippy::all)]
#![warn(clippy::doc_markdown)]
#![warn(clippy::dbg_macro)]
#![warn(clippy::todo)]
#![warn(clippy::empty_enum)]
#![warn(clippy::enum_glob_use)]
#![warn(clippy::pub_enum_variant_names)]
#![warn(clippy::mem_forget)]
#![warn(clippy::use_self)]
#![warn(clippy::filter_map_next)]
#![warn(clippy::needless_continue)]
#![warn(clippy::needless_borrow)]
#![warn(rust_2018_idioms)]

Copy'n'paste is not pretty, but works.

[repi]

This is getting a bit more silly without this support, we have like 30 crates in our repo where all of their lib.rs start something like this:

#![forbid(unsafe_code)]
#![warn(
    clippy::all,
    clippy::doc_markdown,
    clippy::dbg_macro,
    clippy::todo,
    clippy::empty_enum,
    clippy::enum_glob_use,
    clippy::pub_enum_variant_names,
    clippy::mem_forget,
    clippy::use_self,
    clippy::filter_map_next,
    clippy::needless_continue,
    clippy::needless_borrow,
    clippy::match_wildcard_for_single_variants,
    clippy::if_let_mutex,
    clippy::mismatched_target_os,
    clippy::await_holding_lock,
    clippy::match_on_vec_items,
    clippy::imprecise_flops,
    clippy::suboptimal_flops,
    clippy::lossy_float_literal,
    clippy::rest_pat_in_fully_bound_structs,
    clippy::fn_params_excessive_bools,
    clippy::exit,
    clippy::inefficient_to_string,
    clippy::linkedlist,
    clippy::macro_use_imports,
    clippy::option_option,
    clippy::verbose_file_reads,
    clippy::unnested_or_patterns,
    rust_2018_idioms,
    future_incompatible,
    nonstandard_style
)]

Does work though!

[repi]

We now have ~170 crates across ~20 repositories so have an even larger list of Clippy lints we enable by default across them now scattered and duplicated across 170 locations, which makes it hard to add and esp easy to miss when creating new crates.

We really do need the proper Cargo/Clippy solution for a clippy.toml where we can specify the above list of warn/deny/forbid lints that can be applied for the entire repository: rust-lang/cargo#5034. We'll try and investigate if we can help push the development of that and clippy further. It is a fantastic tool that we want to use more of!

[repi]

We had a good sync with the Clippy team a couple of weeks ago and this is one of the top issues in the Clippy 2021 Roadmap!