Clippy ignores `feature = "cargo-clippy"` in generated code

[danielparks]

Summary

The deprecated_clippy_cfg_attr lint does not seem to work in generated code. I have confirmed that other lints are working, and that the lint works on code in src/lib.rs.

Reference: PR #12292 — Add new lint DEPRECATED_CLIPPY_CFG_ATTR.

Lint Name

deprecated_clippy_cfg_attr

Reproducer

1. cargo init . --name foo --lib

2. build.rs:

   fn main() {
       std::fs::write(
           std::path::Path::new(&std::env::var("OUT_DIR").unwrap()).join("foo.rs"),
           r#"
   pub fn lint() {
       // Correct: Clippy will catch this.
       if 100 > i32::MAX {}
   }
   
   // INCORRECT: Clippy will not lint this.
   #[cfg(not(feature = "cargo-clippy"))]
   pub fn no_lint() {
       // Correct: Clippy will ignore this.
       if 200 > i32::MAX {}
   }
   "#,
       )
       .unwrap();
   }

3. src/lib.rs:

   include!(concat!(env!("OUT_DIR"), "/foo.rs"));
   
   // Correct: Clippy will catch this.
   #[cfg(not(feature = "cargo-clippy"))]
   pub fn reference() {
       // Correct: Clippy will ignore this.
       if 300 > i32::MAX {}
   }

Actual results

❯ cargo +nightly clippy --message-format short
    Checking foo v0.1.0 (/...)
src/lib.rs:4:11: warning: `feature = "cargo-clippy"` was replaced by `clippy`
/.../target/debug/build/.../out/foo.rs:4:8: error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
warning: `foo` (lib) generated 1 warning
error: could not compile `foo` (lib) due to 1 previous error; 1 warning emitted

This catches the use of feature = "cargo-clippy" in src/lib.rs, and one of the comparisons in the generated code, but does not warn about the use of feature = "cargo-clippy" in the generated code.

Expected results

❯ cargo +nightly clippy --message-format short
    Checking foo v0.1.0 (/...)
src/lib.rs:4:11: warning: `feature = "cargo-clippy"` was replaced by `clippy`
/.../target/debug/build/.../out/foo.rs:4:8: error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
/.../target/debug/build/.../out/foo.rs:8:11: warning: `feature = "cargo-clippy"` was replaced by `clippy`
warning: `foo` (lib) generated 1 warning
error: could not compile `foo` (lib) due to 1 previous error; 1 warning emitted

Version

rustc 1.78.0-nightly (c475e2303 2024-02-28)
binary: rustc
commit-hash: c475e2303b551d726307c646181e0677af1e0069
commit-date: 2024-02-28
host: x86_64-apple-darwin
release: 1.78.0-nightly
LLVM version: 18.1.0

[flip1995]

Oh I see what is going on here. But the bad news is, that we can't fix this:

To catch #[cfg] attributes, we have to run the lint before macros/attributes are expanded. This means, that the include! attribute didn't get expanded yet, when running the lint, so Clippy won't see the generated code, just the include! macro in its literal form.

The reason why this lint after macro expansion is pretty straightforward: After the cfg attribute is expanded it is completely gone, together with the code that was cfged out.

[flip1995]

@Urgau Does the -Zcheck-cfg feature catch #[cfg] attributes that get include!d? Or does it suffer from the same problem?

EDIT: Just tested this (sorry for the premature ping) and it works with generated code:

warning: unexpected `cfg` condition value: `cargo-clippy`
 --> /home/pkrones/cfg-lint/target/debug/build/cfg-lint-9e94349ba70314c0/out/foo.rs:8:11
  |
8 | #[cfg(not(feature = "cargo-clippy"))]
  |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the condition
  |
  = note: no expected values for `feature`
  = help: consider adding `cargo-clippy` as a feature in `Cargo.toml`
  = note: see <https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#check-cfg> for more information about checking conditional configuration

So you would get this error message once the -Zcheck-cfg feature is stabilized. Just not the auto-fix capabilities Clippy would give you...

[Urgau]

@flip1995 No worries.

Internally check-cfg is implemented directly at the evaluation phase of the configs, it is therefor able to catch all cfgs, no matter where they are coming from: #[cfg], #[cfg_attr], #[link], cfg!, generated code or not.

I unfortunately don't see an easy way for Clippy to hook itself there.

[danielparks]

Ah, makes sense. I suppose you could add a pass that runs after the expansion of each macro, at a performance cost that scales with the number of macros. It’s probably a small cost, but… bleh.

This must affect all of the lints in

rust-clippy/clippy_lints/src/attrs.rs

Lines 870 to 880 in 346b094

	impl_lint_pass!(EarlyAttributes => [
	DEPRECATED_CFG_ATTR,
	MISMATCHED_TARGET_OS,
	EMPTY_LINE_AFTER_OUTER_ATTR,
	EMPTY_LINE_AFTER_DOC_COMMENTS,
	NON_MINIMAL_CFG,
	MAYBE_MISUSED_CFG,
	DEPRECATED_CLIPPY_CFG_ATTR,
	UNNECESSARY_CLIPPY_CFG,
	MIXED_ATTRIBUTES_STYLE,
	]);

I usually turn down linting for generated code anyway, so it seems like fixing this probably isn’t worth the effort. Is this worth pursuing, or should this issue just be closed?

[flip1995]

at a performance cost that scales with the number of macros. It’s probably a small cost, but… bleh.

This would actually be a huge cost, as it would have to run after every #[derive], println!, ... macro. This is not doable.

IMO this issue can be closed. We can't fix this in Clippy and once -Zcheck-cfg gets stabilized you'll see warnings on those cfg_attrs anyway. Until then (and probably longer) they will continue to work. The situation is not ideal, but it is what it is.