Merge #3123

3123: Lint #[cfg_attr(rustfmt, rustfmt_skip)] and suggest #[rustfmt::skip] r=phansch a=flip1995

Closes #3121 

Co-authored-by: flip1995 <9744647+flip1995@users.noreply.github.com>
Co-authored-by: flip1995 <hello@philkrones.com>

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

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:
 

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](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:** 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:**
+///
+/// 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;
 
@@ -466,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` and `rustfmt::skip`
+            if let Some(skip_item) = &items[1].meta_item();
+            if skip_item.name() == "rustfmt_skip" || skip_item.name() == "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),
+                );
+            }
+        }
+    }
+}
+

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,

tests/ui/cfg_attr_rustfmt.rs

@@ -0,0 +1,38 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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();
+}
+
+mod foo {
+    #![cfg_attr(rustfmt, rustfmt_skip)]
+
+    pub fn f() {}
+}

tests/ui/cfg_attr_rustfmt.stderr

@@ -0,0 +1,22 @@
+error: `cfg_attr` is deprecated for rustfmt and got replaced by tool_attributes
+  --> $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_rustfmt.rs:29:1
+   |
+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 3 previous errors
+