Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect unknown features inside #[cfg(feature = "…")] blocks #11649

Closed
Tracked by #79
emilk opened this issue Oct 9, 2023 · 6 comments
Closed
Tracked by #79

Detect unknown features inside #[cfg(feature = "…")] blocks #11649

emilk opened this issue Oct 9, 2023 · 6 comments
Labels
A-lint Area: New lints

Comments

@emilk
Copy link

emilk commented Oct 9, 2023

What it does

Look for #[cfg(feature = "NAME")], cfg!(feature = "NAME"), #[cfg_attr(feature = "NAME", …)] etc and check that NAME is a real feature found in Cargo.toml

Advantage

  • Catches typos (#[cfg(feature = "sedre")])
  • Catches refactors that removes or renames a feature

Example

#[cfg_attr(feature = "sedre", derive(serde::Deserialize, serde::Serialize))]
struct Foo {
}

Suggested output:

Clippy: "No feature called sedre. Available features are: default, serde

Related

May sounds similar, but are different:

@emilk emilk added the A-lint Area: New lints label Oct 9, 2023
@emilk
Copy link
Author

emilk commented Oct 9, 2023

Tagging @GuillaumeGomez, as they suggested I do so

@GuillaumeGomez
Copy link
Member

I'll check what can be done about it. https://rust-lang.github.io/rfcs/3013-conditional-compilation-checking.html seems to be covering this case as well. The big blocker here is in case a project is not using Cargo. But I guess the lint can be skipped in this case.

@Centri3
Copy link
Member

Centri3 commented Oct 9, 2023

I'd say this is ok. I don't think you can easily run clippy without cargo anyway, or is even supported.

Couple things of note:

  • This data must be available in cargo metadata (which I believe it is).
  • This will be allow-by-default as it would be in the cargo category (I don't think it should be disabled by default, though)
  • This must be a pre-expansion lint as these attributes would be stripped during macro expansion since it will always evaluate to false (since the feature is missing). This would probably be in attrs.rs despite being based around cargo metadata due to this, but I'm not sure.

@GuillaumeGomez
Copy link
Member

There is a work in progress for this to be done in rust directly: rust-lang/rust#82450

@epage
Copy link

epage commented Mar 4, 2024

FYI the cargo/rustc feature has been proposed for merge. Cargo has approved its side of it and its waiting on rustc.

@Alexendoo
Copy link
Member

Closing in favour of rust-lang/rust#82450

@Alexendoo Alexendoo closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

5 participants