From e1cf160e2a2fba4cf7625dab1a52af5adfc534f5 Mon Sep 17 00:00:00 2001 From: flip1995 <9744647+flip1995@users.noreply.github.com> Date: Mon, 3 Sep 2018 15:34:12 +0200 Subject: [PATCH 1/9] Add cfg_attr(rustfmt) lint --- clippy_lints/src/attrs.rs | 33 +++++++++++++++++++++++++++++++-- clippy_lints/src/lib.rs | 3 +++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 89d676f3f4ce..4ea13f649d7f 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -12,13 +12,13 @@ use crate::reexport::*; use crate::utils::{ - in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, + in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, without_block_comments, }; use if_chain::if_chain; use crate::rustc::hir::*; use crate::rustc::lint::{ - CheckLintNameResult, LateContext, LateLintPass, LintArray, LintContext, LintPass, + CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray, LintContext, LintPass, }; use crate::rustc::ty::{self, TyCtxt}; use crate::rustc::{declare_tool_lint, lint_array}; @@ -169,6 +169,35 @@ declare_clippy_lint! { "unknown_lints for scoped Clippy lints" } +/// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it +/// with `#[rustfmt::skip]`. +/// +/// **Why is this bad?** Since tool_attributes (rust-lang/rust#44690) are stable now, they should +/// be used instead of the old `cfg_attr(rustfmt)` attribute. +/// +/// **Known problems:** It currently only detects outer attributes. But since it does not really +/// makes sense to have `#![cfg_attr(rustfmt, rustfmt_skip)]` as an inner attribute, this should be +/// ok. +/// +/// **Example:** +/// +/// Bad: +/// ```rust +/// #[cfg_attr(rustfmt, rustfmt_skip)] +/// fn main() { } +/// ``` +/// +/// Good: +/// ```rust +/// #[rustfmt::skip] +/// fn main() { } +/// ``` +declare_clippy_lint! { + pub DEPRECATED_CFG_ATTR, + complexity, + "usage of `cfg_attr(rustfmt)` instead of `tool_attributes`" +} + #[derive(Copy, Clone)] pub struct AttrPass; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 863c06234871..f3fce54f910e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -220,6 +220,7 @@ pub fn register_pre_expansion_lints(session: &rustc::session::Session, store: &m store.register_pre_expansion_pass(Some(session), box non_expressive_names::NonExpressiveNames { single_char_binding_names_threshold: conf.single_char_binding_names_threshold, }); + store.register_pre_expansion_pass(Some(session), box attrs::CfgAttrPass); } pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf { @@ -532,6 +533,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { approx_const::APPROX_CONSTANT, assign_ops::ASSIGN_OP_PATTERN, assign_ops::MISREFACTORED_ASSIGN_OP, + attrs::DEPRECATED_CFG_ATTR, attrs::DEPRECATED_SEMVER, attrs::UNKNOWN_CLIPPY_LINTS, attrs::USELESS_ATTRIBUTE, @@ -839,6 +841,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![ assign_ops::MISREFACTORED_ASSIGN_OP, + attrs::DEPRECATED_CFG_ATTR, booleans::NONMINIMAL_BOOL, cyclomatic_complexity::CYCLOMATIC_COMPLEXITY, double_comparison::DOUBLE_COMPARISONS, From 7bd8c303d34369124b6550ad6c645d2e1a149214 Mon Sep 17 00:00:00 2001 From: flip1995 <9744647+flip1995@users.noreply.github.com> Date: Mon, 3 Sep 2018 15:34:33 +0200 Subject: [PATCH 2/9] Add tests --- tests/ui/cfg_attr_lint.rs | 9 +++++++++ tests/ui/cfg_attr_lint.stderr | 10 ++++++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/ui/cfg_attr_lint.rs create mode 100644 tests/ui/cfg_attr_lint.stderr diff --git a/tests/ui/cfg_attr_lint.rs b/tests/ui/cfg_attr_lint.rs new file mode 100644 index 000000000000..d543db06d6cc --- /dev/null +++ b/tests/ui/cfg_attr_lint.rs @@ -0,0 +1,9 @@ +#![feature(tool_lints)] + +#![warn(clippy::deprecated_cfg_attr)] + +// This doesn't get linted, see known problems +#![cfg_attr(rustfmt, rustfmt_skip)] + +#[cfg_attr(rustfmt, rustfmt_skip)] +fn main() {} diff --git a/tests/ui/cfg_attr_lint.stderr b/tests/ui/cfg_attr_lint.stderr new file mode 100644 index 000000000000..3a515f155c1f --- /dev/null +++ b/tests/ui/cfg_attr_lint.stderr @@ -0,0 +1,10 @@ +error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes + --> $DIR/cfg_attr_lint.rs:8:1 + | +8 | #[cfg_attr(rustfmt, rustfmt_skip)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]` + | + = note: `-D clippy::deprecated-cfg-attr` implied by `-D warnings` + +error: aborting due to previous error + From a770d8edd0f5f6d1bc4b2abb0842f900f121c186 Mon Sep 17 00:00:00 2001 From: flip1995 <9744647+flip1995@users.noreply.github.com> Date: Tue, 18 Sep 2018 11:35:53 +0200 Subject: [PATCH 3/9] Differ between inner and outer attributes --- clippy_lints/src/attrs.rs | 53 +++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 4ea13f649d7f..24edf6d84952 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -172,12 +172,12 @@ declare_clippy_lint! { /// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it /// with `#[rustfmt::skip]`. /// -/// **Why is this bad?** Since tool_attributes (rust-lang/rust#44690) are stable now, they should -/// be used instead of the old `cfg_attr(rustfmt)` attribute. +/// **Why is this bad?** Since tool_attributes ([rust-lang/rust#44690](https://github.com/rust-lang/rust/issues/44690)) +/// are stable now, they should be used instead of the old `cfg_attr(rustfmt)` attributes. /// -/// **Known problems:** It currently only detects outer attributes. But since it does not really -/// makes sense to have `#![cfg_attr(rustfmt, rustfmt_skip)]` as an inner attribute, this should be -/// ok. +/// **Known problems:** This lint doesn't detect crate level inner attributes, because they get +/// processed before the PreExpansionPass lints get executed. See +/// [#3123](https://github.com/rust-lang-nursery/rust-clippy/pull/3123#issuecomment-422321765) /// /// **Example:** /// @@ -495,3 +495,46 @@ fn is_present_in_source(cx: &LateContext<'_, '_>, span: Span) -> bool { } true } + +#[derive(Copy, Clone)] +pub struct CfgAttrPass; + +impl LintPass for CfgAttrPass { + fn get_lints(&self) -> LintArray { + lint_array!( + DEPRECATED_CFG_ATTR, + ) + } +} + +impl EarlyLintPass for CfgAttrPass { + fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { + if_chain! { + // check cfg_attr + if attr.name() == "cfg_attr"; + if let Some(ref items) = attr.meta_item_list(); + if items.len() == 2; + // check for `rustfmt` + if let Some(feature_item) = items[0].meta_item(); + if feature_item.name() == "rustfmt"; + // check for `rustfmt_skip` + if let Some(skip_item) = &items[1].meta_item(); + if skip_item.name() == "rustfmt_skip"; + then { + let attr_style = match attr.style { + AttrStyle::Outer => "#[", + AttrStyle::Inner => "#![", + }; + span_lint_and_sugg( + cx, + DEPRECATED_CFG_ATTR, + attr.span, + "`cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes", + "use", + format!("{}rustfmt::skip]", attr_style), + ); + } + } + } +} + From 352da1d33dd3f568c2c7bf6887879e3b569209bb Mon Sep 17 00:00:00 2001 From: flip1995 <9744647+flip1995@users.noreply.github.com> Date: Tue, 18 Sep 2018 11:36:59 +0200 Subject: [PATCH 4/9] Add test for non-crate-level inner attributes --- tests/ui/cfg_attr_lint.rs | 10 +++++++++- tests/ui/cfg_attr_lint.stderr | 8 +++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/ui/cfg_attr_lint.rs b/tests/ui/cfg_attr_lint.rs index d543db06d6cc..1005af8b9ea5 100644 --- a/tests/ui/cfg_attr_lint.rs +++ b/tests/ui/cfg_attr_lint.rs @@ -6,4 +6,12 @@ #![cfg_attr(rustfmt, rustfmt_skip)] #[cfg_attr(rustfmt, rustfmt_skip)] -fn main() {} +fn main() { + foo::f(); +} + +mod foo { + #![cfg_attr(rustfmt, rustfmt_skip)] + + pub fn f() {} +} diff --git a/tests/ui/cfg_attr_lint.stderr b/tests/ui/cfg_attr_lint.stderr index 3a515f155c1f..b166d4028d90 100644 --- a/tests/ui/cfg_attr_lint.stderr +++ b/tests/ui/cfg_attr_lint.stderr @@ -6,5 +6,11 @@ error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes | = note: `-D clippy::deprecated-cfg-attr` implied by `-D warnings` -error: aborting due to previous error +error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes + --> $DIR/cfg_attr_lint.rs:14:5 + | +14 | #![cfg_attr(rustfmt, rustfmt_skip)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#![rustfmt::skip]` + +error: aborting due to 2 previous errors From bb4083c412f0d7e687b3ae7ab4cc4dc57e6ea804 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Mon, 15 Oct 2018 15:18:26 +0200 Subject: [PATCH 5/9] Run update_lints.py script --- CHANGELOG.md | 1 + README.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72183f68ee0b..bdd01bfeecdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -651,6 +651,7 @@ All notable changes to this project will be documented in this file. [`decimal_literal_representation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#declare_interior_mutable_const [`default_trait_access`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#default_trait_access +[`deprecated_cfg_attr`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deprecated_cfg_attr [`deprecated_semver`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deprecated_semver [`deref_addrof`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#deref_addrof [`derive_hash_xor_eq`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#derive_hash_xor_eq diff --git a/README.md b/README.md index 8af7a20f66dc..0525788e64e1 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 286 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) +[There are 287 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: From 7df7a0a86ebf290f6f2650955f2479aaaa39beef Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Nov 2018 20:06:25 +0100 Subject: [PATCH 6/9] Add tests from rustfmt::skip test file --- tests/ui/cfg_attr_lint.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/ui/cfg_attr_lint.rs b/tests/ui/cfg_attr_lint.rs index 1005af8b9ea5..614cd3e30ec3 100644 --- a/tests/ui/cfg_attr_lint.rs +++ b/tests/ui/cfg_attr_lint.rs @@ -1,10 +1,31 @@ -#![feature(tool_lints)] +// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(stmt_expr_attributes)] #![warn(clippy::deprecated_cfg_attr)] // This doesn't get linted, see known problems #![cfg_attr(rustfmt, rustfmt_skip)] +#[rustfmt::skip] +trait Foo +{ +fn foo( +); +} + +fn skip_on_statements() { + #[cfg_attr(rustfmt, rustfmt::skip)] + 5+3; +} + #[cfg_attr(rustfmt, rustfmt_skip)] fn main() { foo::f(); From cadb367a5cb36590c54d90c4cad5ebf67e796477 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Nov 2018 20:06:59 +0100 Subject: [PATCH 7/9] Also lint cfg_attr(.., rustfmt::skip) --- clippy_lints/src/attrs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 24edf6d84952..593484aa1c61 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -517,9 +517,9 @@ impl EarlyLintPass for CfgAttrPass { // check for `rustfmt` if let Some(feature_item) = items[0].meta_item(); if feature_item.name() == "rustfmt"; - // check for `rustfmt_skip` + // check for `rustfmt_skip` and `rustfmt::skip` if let Some(skip_item) = &items[1].meta_item(); - if skip_item.name() == "rustfmt_skip"; + if skip_item.name() == "rustfmt_skip" || skip_item.name() == "skip"; then { let attr_style = match attr.style { AttrStyle::Outer => "#[", From 5c1385249e9ca227b469f83232c771ddfde8b0e6 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Nov 2018 20:17:04 +0100 Subject: [PATCH 8/9] Rename test files --- tests/ui/{cfg_attr_lint.rs => cfg_attr_rustfmt.rs} | 0 tests/ui/{cfg_attr_lint.stderr => cfg_attr_rustfmt.stderr} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/ui/{cfg_attr_lint.rs => cfg_attr_rustfmt.rs} (100%) rename tests/ui/{cfg_attr_lint.stderr => cfg_attr_rustfmt.stderr} (100%) diff --git a/tests/ui/cfg_attr_lint.rs b/tests/ui/cfg_attr_rustfmt.rs similarity index 100% rename from tests/ui/cfg_attr_lint.rs rename to tests/ui/cfg_attr_rustfmt.rs diff --git a/tests/ui/cfg_attr_lint.stderr b/tests/ui/cfg_attr_rustfmt.stderr similarity index 100% rename from tests/ui/cfg_attr_lint.stderr rename to tests/ui/cfg_attr_rustfmt.stderr From 318f84ffcf4d919373ab4a025891d591e81070d6 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 2 Nov 2018 13:43:16 +0100 Subject: [PATCH 9/9] Update stderr --- tests/ui/cfg_attr_rustfmt.stderr | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/ui/cfg_attr_rustfmt.stderr b/tests/ui/cfg_attr_rustfmt.stderr index b166d4028d90..a6a27bd2ee8f 100644 --- a/tests/ui/cfg_attr_rustfmt.stderr +++ b/tests/ui/cfg_attr_rustfmt.stderr @@ -1,16 +1,22 @@ error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes - --> $DIR/cfg_attr_lint.rs:8:1 - | -8 | #[cfg_attr(rustfmt, rustfmt_skip)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]` - | - = note: `-D clippy::deprecated-cfg-attr` implied by `-D warnings` + --> $DIR/cfg_attr_rustfmt.rs:25:5 + | +25 | #[cfg_attr(rustfmt, rustfmt::skip)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]` + | + = note: `-D clippy::deprecated-cfg-attr` implied by `-D warnings` error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes - --> $DIR/cfg_attr_lint.rs:14:5 + --> $DIR/cfg_attr_rustfmt.rs:29:1 | -14 | #![cfg_attr(rustfmt, rustfmt_skip)] +29 | #[cfg_attr(rustfmt, rustfmt_skip)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#[rustfmt::skip]` + +error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes + --> $DIR/cfg_attr_rustfmt.rs:35:5 + | +35 | #![cfg_attr(rustfmt, rustfmt_skip)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `#![rustfmt::skip]` -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors