rust-lang
Lint against unexpected cfgs in `[target.'cfg(...)']`
What does this PR try to resolve?
Since https://github.com/rust-lang/cargo/pull/13571, we now lint by default on unexpected cfgs in Rust code, but one area that is missing are cfg expressions in Cargo.toml and .cargo/config.toml. This PR address this missing piece by (unstably) introducing -Zcheck-target-cfgs based on rustc --print=check-cfg.
The feature ~~voluntarily does not always follow RUSTFLAGS (to avoid mistakes with one-off and for consistency)~~ but instead follows Rust unexpected_cfgs lint configuration:
[target.'cfg(foo)'.dependencies] # will not get linted about
cfg-if = "1.0"
[lints.rust.unexpected_cfgs]
level = "warn"
check-cfg = ['cfg(foo)'] # because it's mark as expected here
In terms of implementation, one global --print=check-cfg is retrieved and fetched, packages that have their own check-cfg inputs have each their own --print=check-cfg, otherwise the expected cfgs will get mixed up. Each --print=check-cfg is cached and only done if necessary.
How should we test and review this PR?
As asked I tried putting everything into small commits, they may not reflect the de-coupling possible as they follow the implementation but they are still IMO very useful. However I strongly recommand doing a first-pass of the complete diff first.
In terms of tests, I added some combinations that I though would be useful, I can add more if desired.
Additional information
Documentation for --print=check-cfg. It was one of motivation of the compiler MCP https://github.com/rust-lang/compiler-team/issues/743.
r? @epage
This probably could have used a conversation with the Cargo team first to ensure alignment with where we are going with lints. If there are questions about the direction I'm pointing this towards, feel free to reach out!
:umbrella: The latest upstream changes (presumably #14630) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #14137) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #14752) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably ed31dad4307145f82a17eebe56a17a4d801a3382) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably 88edf01fe39d6d15c8e15dbf4ea478661b7b85e3) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably 10c255ad07766377c269058d40405bc17d79b50e) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly a4c0d39826f972fc3d974d9918269e865b74841e) made this pull request unmergeable. Please resolve the merge conflicts.
Hello,
Not sure if this is the best place to discuss this, but the zulip stream linked in the MCP is pretty empty, the MCP has already been accepted, and I don't see prior discussion on it, so I'm putting my concern here.
We recently encountered a case that would be linted by this PR if I understand correctly. We have in the Cargo.toml:
[target.'cfg(loom)'.dependencies]
loom = "0.5"
and some cfg(loom) in the rust code.
Rustc linted the rust code and generated the warning for that. We can silence them through the [lints.rust.unexpected_cfgs] section in Cargo.toml. If I understand correctly, with this PR, the lint would also have warned for the [target.'cfg(loom)'.dependencies] section, and [lints.rust.unexpected_cfgs] would also silence that.
My intuition was different, I would have expected Cargo to automatically add cfg(loom) to the check-cfg config of the lint, based on the fact that there is a dependency for this specific cfg that is defined.
This goes with my intuition that Cargo.toml would define all that can be used for the cfg() and minimize the amount of manual bookkeeping of what is and isn't linted, and also minimize the amount of "false positives" for this lint with the default configuration.
To me the presence of the [target.cfg(loom)'] section implies that the cfg(loom)` is intended to be used, so shouldn't be linted against, rather than being a possible mistake.
My intuition was different, I would have expected Cargo to automatically add cfg(loom) to the check-cfg config of the lint, based on the fact that there is a dependency for this specific cfg that is defined.
--cfg / --check-cfg is operating at a layer of abstraction below Cargo which is why check-cfg is configured through either build scripts or the rust lints table. While we support conditional dependencies, I don't expect it to be likely to infer cfg's from them, especially since the case you gave is fairly special. IN a lot of other cases, users can benefit from being told what cfg's are supported vs accidentally creating a new one.
Currently, the only feature at Cargo's level of abstraction is features. global, mutually exclusive features is an idea for another one.
:umbrella: The latest upstream changes (possibly 321f14e03ec24ee30ae8fb7d22ac409ff874c6b3) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 3b784a42e3075ebf34591ebfd5b79457e680e69d) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly ef12f10bfcd9af9791234f7380bf2325a794c57f) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 856f1bae5d77ad653d756fbed7c72555927c6d09) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 7220445a9d7d2ac7a51a2a8c56679ce3b2ae5cd5) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 69eeb6ad94c643b6f60cc00ff9a2cc99f27442e2) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 7bdd872ceccca971c969f0d7e2c07204f14bb48c) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 47c911e9e6f6461f90ce19142031fe16876a3b95) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly f19321e343196f0f0096a69a4413b23ced8a058c) made this pull request unmergeable. Please resolve the merge conflicts.
@epage @Muscraft I have updated the PR with the conclusion of our discussion at the All Hands. That is, the lint is now under Cargo cargo::unexpected_cfgs.
I don't remember if we had settle on the lint config position, ie if we should share check-cfg = [...] from rust.unexpected_cfgs or if cargo.unexpected_cfgs should have a distinct one. I've left the sharing for now.
:umbrella: The latest upstream changes (possibly 6ffc01f9fa7aa50d07a148aa95666bc87d8b4b81) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (possibly 409fed7dc1553d49cb9a8c0637d12d65571346ce) made this pull request unmergeable. Please resolve the merge conflicts.