Add lints as .cargo/config.toml section

[repi]

Adds an alternate way to enable our standard lints (#59) for all this crates in a repository through .cargo/config.toml section. Inspired by rust-lang/cargo#5034 (comment).

We are testing this as a workaround for #22 until Cargo/Clippy supports a proper mechanism to configure lints for an entire repository/workspace, and seems to work really well so far. One of our big repositories has 80+ crates and all of them had the previous lint code definitions in every lib.rs/main.rs copy'n'pasted and this will replace all of that and make it dramatically easier to manage and less error prone.

[MarijnS95]

@repi Thanks for this!

We're test-driving this change in our own projects now, but have found one major deficiency: the change in rustflags seem to invalidate "the cache" and cause cargo check and cargo clippy to repeatedly recompile/check each and every dependency in our tree if they're run in turns. This is especially noticeable with rust-analyzer, which uses cargo-check under the hood.

Have you found a solution to this already or not even ran into it yet?

[repi]

Ouch just tested and we ran into the same problem, switching between cargo clippy and then doing cargo check is a full rebuild 😢

We actually configure Rust Analyzer to use clippy instead, so didn't notice that, and then almost never then run cargo check manually as use clippy instead.

This is what we have for configuring RA in VS Code:
"rust-analyzer.checkOnSave.command": "clippy",

[MarijnS95]

@repi Thanks, I wasn't even aware that we could get RA to use clippy instead - if it also gives us the clippy lints in addition to check warnings directly in-line that'd be awesome!

We'll see if this works out for us - haven't checked if there are conflicts with cargo run.

[repi]

Looks like cargo build (debug build) and cargo clippy also causes full rebuilds with this, which is even worse as harder to live without debug builds.

For Rust Analyzer in VS Code one can set a different target directory for that and avoid it (which is good in general to be able to build on command-line same time as RA runs in the background. But don't know of any way to force just clippy by itself to use a different target directory to avoid this behavior.

So looks like this entire approach may not work properly in practice and one have to continue to wait for a mechanism in Cargo to set the lints properly (been waiting for 2.5 years 😢 #59).

I'm closing this for now as we wont' merge this as is without a proper workaround for debug builds being force rebuilt when switching between cargo build and cargo clippy with this. Would be great if can be further investigated in the background though, maybe there is some way to get it to work properly. And can continue to discuss in here

[MarijnS95]

@repi I'm now successfully pingponging between cargo b/c/clippy --all on our project without rebuilds, by simply not hiding the lint flags behind cfg(feature = "cargo-clippy") but providing it project-wide with [build] rustflags = [...]. Nothing seems to break so far on unknown compiler options.

[repi]

ah interesting! thought rustc would fail with unknown parameters / lints if it saw the clippy ones. Will test on our side also

[repi]

Changing to this definitely seems to work in my local tests also for our codebase, yay! Will deploy it out for wider testing in our team.

[build]
rustflags = [

big thanks @MarijnS95, was very close to giving up here again :)

[repi]

@MarijnS95 are you getting any Rust or Clippy lints to actually trigger with it as a [build] rustflags though? I'm not getting it to trigger on any of them in our repo hmm. Neither Rust ones like -Dunsafe_code or clippy ones

[MarijnS95]

@repi It's definitely triggering for us. Adding -Dunsafe_code for testing turns our entire codebase deep red.

We're on Rust 1.55, latest stable.

[repi]

Also on Rust 1.55 stable.

Hmm wonder if we have something else global causing it to not take, couldn't even set as env var and have it be used. Can't find anything obvious though.
Though setting the flag under [target.'cfg(all())'] instead does seem to work which can be another workaround.

Update 2: on my Windows machine the [build] setting doesn't work, but on my Mac it does. hmm

[repi]

@MarijnS95 do you have any other target-specific rustflags in .cargo/config also?

we do and it appears that if you have a section like this that sets rustflags:

[target.x86_64-pc-windows-msvc]
rustflags = [
    "-C", "target-feature=+crt-static", # use static MSVC CRT so we do not have to distribute vc_redist
    "-C", "link-args=-fuse-ld=lld-link",
    "-C", "lto=no"]                     # disable LTO for faster incremental link times

you can't have a [build] section also with rustflags, it will be ignored:

[build]
rustflags = [

If I remove the above [target] section it works (or build for a different target that it doesn't cover. Or if I change the [build] section to [target.'cfg(all())'] the flags get properly merged and activated

[Jasper-Bekkers]

@repi we don't have any other place that specifies buildflags, so that'll explain it.

[Jasper-Bekkers]

There are three mutually exclusive sources of extra flags. They are checked in order, with the first one being used:

• RUSTFLAGS environment variable.
• All matching target.<triple>.rustflags and target.<cfg>.rustflags config entries joined together.
• build.rustflags config value.

Looks like for this to work you ought to stick it in a target. so it gets joined properly

[repi]

Yeah quite cryptic, but glad found workaround. Wish Cargo would have warned or failed on this when it actively ignores flags

[MarijnS95]

@repi Glad you figured it out - it's unfortunate that these are mutually exclusive instead of being joined together - how else can you make global + target specific flags without duplicating the global ones? This is especially worrisome when joining config.toml from various places, when the docs mention that arrays for the same key are merged together. OTOH this might be necessary to properly override the confiuguration.

[emilk]

I'm trying out cargo cranky as an alternative to using .cargo/config.toml.

You can specify the desired lints in a Cranky.toml and check them by running cargo cranky. This plays well with both CI and local development (you can configure Rust Analyzer to run cargo cranky on save, for instance). See emilk/egui#1820