Add new lint `DEPRECATED_CLIPPY_CFG_ATTR` #12292

GuillaumeGomez · 2024-02-14T16:56:25Z

As discussed on zulip.

This lint suggests to replace feature = "cargo-clippy" with clippy.

r? @flip1995

changelog: Add new lint DEPRECATED_CLIPPY_CFG_ATTR

flip1995

Impl LGTM. 2 open questions.

I think we want to wait with merging this, until cfg(clippy) is known by rustc/cargo? Then we'll also add --cfg clippy to the clippy-driver.

clippy_lints/src/attrs.rs

Urgau · 2024-02-14T20:19:43Z

I think we want to wait with merging this, until cfg(clippy) is known by rustc/cargo?

We can do it at the same time. It won't be an issue until clippy is synced in rust-lang/rust.
It will also probably be easier to convince rust-lang/rust folks if this PR is merged first.

GuillaumeGomez · 2024-02-15T00:25:56Z

Finally fixed doc test and dogfood. ^^'

flip1995 · 2024-02-15T10:37:47Z

It won't be an issue until clippy is synced in rust-lang/rust.

That's in exactly one week. Will rustc be ready until then. I'd rather approve the PR and leave it as S-blocked until rustc is ready. I'm happy to do an out of cycle sync for this. We need to get all of those changes in, in the same release cycle. Just merging and hoping that rustc (or any other OSS project) is ready in time doesn't work out great in my experience 😅

GuillaumeGomez · 2024-02-15T10:38:41Z

Sending the PR for rust in a few minutes.

GuillaumeGomez · 2024-02-15T10:42:33Z

Opened rust-lang/rust#121137.

tests/ui/useless_attribute.fixed

src/driver.rs

tests/ui/useless_attribute.stderr

…-clippy`

Urgau · 2024-02-15T11:02:33Z

It won't be an issue until clippy is synced in rust-lang/rust.

That's in exactly one week. Will rustc be ready until then. I'd rather approve the PR and leave it as S-blocked until rustc is ready. I'm happy to do an out of cycle sync for this. We need to get all of those changes in, in the same release cycle. Just merging and hoping that rustc (or any other OSS project) is ready in time doesn't work out great in my experience 😅

-Zcheck-cfg is unstable and opt-in, so we have technically more than a week, but yes, I had planned to submit the PR just after this one merged; as to get them in the same-ish nightly.

flip1995

LGTM and approved.

The last open question is, if this should be an extra lint or if it should be merged into DEPRECATED_CFG_ARG?

flip1995 · 2024-02-16T09:52:01Z

clippy_lints/src/attrs.rs

+            "replace with",
+            "clippy".to_string(),


One possible enhancement to this lint could be to check if the second part of the cfg_attr is an allow/warn/... and suggest to remove the cfg_attr completely.

OTOH we might want to add an extra lint for this, once people moved to cfg_attr(clippy, allow(...)).

So I wouldn't do this in this PR.

I made the same suggestion to @GuillaumeGomez in private and we also thought that it would be better done in a follow-up, as to also catch the pattern with cfg_attr(clippy, ...).

I confirm we did. ;)

I'll likely send a follow-up PR.

flip1995 · 2024-02-16T09:56:26Z

-Zcheck-cfg is unstable and opt-in

I would propose to keep -Zcheck-cfg unstable for at least one more release cycle after this PR and the cfg(clippy) PR in rustc are merged and synced. I'm not sure what timeline you have in mind for stabilization though.

Urgau · 2024-02-16T10:13:47Z

No problem, I have not yet proposed stabilization of the feature. We are currently running a Call for Testing, we may run it for a second week, after that and if all goes well, I will propose stabilization in rustc and Cargo, I expect this to take several weeks; and will make sure to have at least one full release for cfg(clippy).

flip1995 · 2024-02-16T10:18:18Z

src/driver.rs

@@ -271,7 +271,9 @@ pub fn main() {
                },
                _ => Some(s.to_string()),
            })
+            // FIXME: remove this line in 1.79 to only keep `--cfg clippy`.
            .chain(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()])


@Urgau Can I ask you to remove this line as part of the stabilization PR of the -Zcheck-cfg feature in rust-lang/rust please? That way, the time people have to transition is as long as possible.

Sure, no problem.

flip1995 · 2024-02-16T10:23:07Z

The last open question is, if this should be an extra lint or if it should be merged into DEPRECATED_CFG_ARG?

Answering my final question myself:

After thinking about this, I prefer to use a new lint for this, because:

People that allowed the old DEPRECATED_CFG_ARG lint for some reason, still get a warning with the new lint
It will be easier for people to transition with cargo clippy --fix -- -Aclippy::all -Wclippy::deprecated_clippy_cfg_arg, without interfering with the old lint.

With that and as the cfg(clippy) PR is already in the bors queue:

@bors r+

bors · 2024-02-16T10:23:10Z

📌 Commit f4a3db8 has been approved by flip1995

It is now in the queue for this repository.

bors · 2024-02-16T10:24:18Z

⌛ Testing commit f4a3db8 with merge 32c006c...

bors · 2024-02-16T10:34:40Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 32c006c to master...

Urgau · 2024-02-16T10:36:28Z

Thank you @flip1995 (and @GuillaumeGomez) for your help, I really appreciate it ❤️.

Without your help this issue could very well have delayed check-cfg or even put it in limbo. Thank you very much.

GuillaumeGomez · 2024-02-16T10:37:21Z

It allows me to take a break from some compiler stuff so it's very welcome. 😄

flip1995 · 2024-02-16T11:21:06Z

rust-lang/blog.rust-lang.org#1253

I thought I'd write a blog post about this.

…rgau,Nilstrieb Add clippy into the known `cfg` list In clippy, we are removing the `feature = "cargo-clippy"` cfg to replace it with `clippy` in rust-lang/rust-clippy#12292. But for it to work, we need to declare `clippy` as cfg. It makes it more coherent with other existing tools like rustdoc. cc `@flip1995`

Rollup merge of rust-lang#121137 - GuillaumeGomez:add-clippy-cfg, r=Urgau,Nilstrieb Add clippy into the known `cfg` list In clippy, we are removing the `feature = "cargo-clippy"` cfg to replace it with `clippy` in rust-lang/rust-clippy#12292. But for it to work, we need to declare `clippy` as cfg. It makes it more coherent with other existing tools like rustdoc. cc `@flip1995`

…ip1995 Add `unnecessary_clippy_cfg` lint Follow-up of #12292. r? `@flip1995` changelog: Add `unnecessary_clippy_cfg` lint

`cfg(feature = "cargo-clippy")` has been [deprecated]. The correct way to achieve the same result is now `cfg(clippy)`. [deprecated]: rust-lang/rust-clippy#12292

rustbot assigned flip1995 Feb 14, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 14, 2024

flip1995 reviewed Feb 14, 2024

View reviewed changes

clippy_lints/src/attrs.rs Outdated Show resolved Hide resolved

clippy_lints/src/attrs.rs Show resolved Hide resolved

GuillaumeGomez force-pushed the DEPRECATED_CLIPPY_CFG_ATTR branch 3 times, most recently from 7f1e19f to 22cd0dd Compare February 14, 2024 22:40

Add new lint DEPRECATED_CLIPPY_CFG_ATTR

f35eec8

GuillaumeGomez force-pushed the DEPRECATED_CLIPPY_CFG_ATTR branch from 22cd0dd to 51dc344 Compare February 14, 2024 23:40

GuillaumeGomez mentioned this pull request Feb 15, 2024

Add clippy into the known cfg list rust-lang/rust#121137

Merged

flip1995 reviewed Feb 15, 2024

View reviewed changes

tests/ui/useless_attribute.fixed Outdated Show resolved Hide resolved

src/driver.rs Show resolved Hide resolved

tests/ui/useless_attribute.stderr Outdated Show resolved Hide resolved

GuillaumeGomez added 3 commits February 15, 2024 11:52

Pass --cfg clippy to clippy-driver

e0f82af

Add ui tests for DEPRECATED_CLIPPY_CFG_ATTR

cd6f03a

Update clippy book to mention cfg(clippy) instead of feature `cargo…

f4a3db8

…-clippy`

GuillaumeGomez force-pushed the DEPRECATED_CLIPPY_CFG_ATTR branch from 51dc344 to f4a3db8 Compare February 15, 2024 10:52

flip1995 approved these changes Feb 16, 2024

View reviewed changes

flip1995 reviewed Feb 16, 2024

View reviewed changes

bors merged commit 32c006c into rust-lang:master Feb 16, 2024
8 checks passed

GuillaumeGomez deleted the DEPRECATED_CLIPPY_CFG_ATTR branch February 16, 2024 10:35

GuillaumeGomez mentioned this pull request Feb 16, 2024

Add unnecessary_clippy_cfg lint #12303

Merged

bors added a commit that referenced this pull request Feb 22, 2024

Auto merge of #12303 - GuillaumeGomez:unneedeed_clippy_cfg_attr, r=fl…

d554bca

…ip1995 Add `unnecessary_clippy_cfg` lint Follow-up of #12292. r? `@flip1995` changelog: Add `unnecessary_clippy_cfg` lint

This was referenced Feb 28, 2024

Replace feature = "cargo-clippy" danielparks/matchgen#41

Draft

Clippy ignores feature = "cargo-clippy" in generated code #12392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new lint `DEPRECATED_CLIPPY_CFG_ATTR` #12292

Add new lint `DEPRECATED_CLIPPY_CFG_ATTR` #12292

GuillaumeGomez commented Feb 14, 2024

flip1995 left a comment •

edited

Urgau commented Feb 14, 2024

GuillaumeGomez commented Feb 15, 2024

flip1995 commented Feb 15, 2024

GuillaumeGomez commented Feb 15, 2024

GuillaumeGomez commented Feb 15, 2024

Urgau commented Feb 15, 2024 •

edited

flip1995 left a comment

flip1995 Feb 16, 2024

Urgau Feb 16, 2024

GuillaumeGomez Feb 16, 2024

flip1995 commented Feb 16, 2024

Urgau commented Feb 16, 2024 •

edited

flip1995 Feb 16, 2024

Urgau Feb 16, 2024

flip1995 commented Feb 16, 2024

bors commented Feb 16, 2024

bors commented Feb 16, 2024

bors commented Feb 16, 2024

Urgau commented Feb 16, 2024 •

edited

GuillaumeGomez commented Feb 16, 2024

flip1995 commented Feb 16, 2024

Add new lint DEPRECATED_CLIPPY_CFG_ATTR #12292

Add new lint DEPRECATED_CLIPPY_CFG_ATTR #12292

Conversation

GuillaumeGomez commented Feb 14, 2024

flip1995 left a comment • edited

Choose a reason for hiding this comment

Urgau commented Feb 14, 2024

GuillaumeGomez commented Feb 15, 2024

flip1995 commented Feb 15, 2024

GuillaumeGomez commented Feb 15, 2024

GuillaumeGomez commented Feb 15, 2024

Urgau commented Feb 15, 2024 • edited

flip1995 left a comment

Choose a reason for hiding this comment

flip1995 Feb 16, 2024

Choose a reason for hiding this comment

Urgau Feb 16, 2024

Choose a reason for hiding this comment

GuillaumeGomez Feb 16, 2024

Choose a reason for hiding this comment

flip1995 commented Feb 16, 2024

Urgau commented Feb 16, 2024 • edited

flip1995 Feb 16, 2024

Choose a reason for hiding this comment

Urgau Feb 16, 2024

Choose a reason for hiding this comment

flip1995 commented Feb 16, 2024

bors commented Feb 16, 2024

bors commented Feb 16, 2024

bors commented Feb 16, 2024

Urgau commented Feb 16, 2024 • edited

GuillaumeGomez commented Feb 16, 2024

flip1995 commented Feb 16, 2024

Add new lint `DEPRECATED_CLIPPY_CFG_ATTR` #12292

Add new lint `DEPRECATED_CLIPPY_CFG_ATTR` #12292

flip1995 left a comment •

edited

Urgau commented Feb 15, 2024 •

edited

Urgau commented Feb 16, 2024 •

edited

Urgau commented Feb 16, 2024 •

edited