Remove the pre-expansion pass

Author: Alexendoo

clippy_lints/src/attrs/deprecated_cfg_attr.rs

@@ -1,14 +1,16 @@
 use super::{Attribute, DEPRECATED_CFG_ATTR, DEPRECATED_CLIPPY_CFG_ATTR, unnecessary_clippy_cfg};
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::msrvs::{self, MsrvStack};
 use clippy_utils::sym;
 use rustc_ast::AttrStyle;
 use rustc_errors::Applicability;
-use rustc_lint::EarlyContext;
+use rustc_hir::attrs::CfgEntry;
+use rustc_hir::def_id::{LOCAL_CRATE, LocalDefId};
+use rustc_lint::{EarlyContext, LateContext};
 
-pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &MsrvStack) {
+pub(super) fn check_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &MsrvStack) {
     // check cfg_attr
-    if attr.has_name(sym::cfg_attr)
+    if attr.has_name(sym::cfg_attr_trace)
         && let Some(items) = attr.meta_item_list()
         && items.len() == 2
         && let Some(feature_item) = items[0].meta_item()
@@ -27,18 +29,23 @@ pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &MsrvStack) {
                     .ident
                     .name
                     == sym::skip)
-            // Only lint outer attributes, because custom inner attributes are unstable
-            // Tracking issue: https://github.com/rust-lang/rust/issues/54726
-            && attr.style == AttrStyle::Outer
         {
-            span_lint_and_sugg(
+            span_lint_and_then(
                 cx,
                 DEPRECATED_CFG_ATTR,
                 attr.span,
-                "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
-                "use",
-                "#[rustfmt::skip]".to_string(),
-                Applicability::MachineApplicable,
+                "`cfg_attr` is deprecated for rustfmt",
+                |diag| {
+                    diag.span_suggestion_verbose(
+                        attr.span,
+                        "use the `rustfmt::skip` tool attribute instead",
+                        match attr.style {
+                            AttrStyle::Outer => "#[rustfmt::skip]",
+                            AttrStyle::Inner => "#![rustfmt::skip]",
+                        },
+                        Applicability::MaybeIncorrect, // See Known Problems
+                    );
+                },
             );
         } else {
             check_deprecated_cfg_recursively(cx, feature_item);
@@ -49,8 +56,8 @@ pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &MsrvStack) {
     }
 }
 
-pub(super) fn check_clippy(cx: &EarlyContext<'_>, attr: &Attribute) {
-    if attr.has_name(sym::cfg)
+pub(super) fn check_cfg(cx: &EarlyContext<'_>, attr: &Attribute) {
+    if attr.has_name(sym::cfg_trace)
         && let Some(list) = attr.meta_item_list()
     {
         for item in list.iter().filter_map(|item| item.meta_item()) {
@@ -85,3 +92,40 @@ fn check_cargo_clippy_attr(cx: &EarlyContext<'_>, item: &rustc_ast::MetaItem) {
         );
     }
 }
+
+pub fn check_stripped(cx: &LateContext<'_>) {
+    for stripped in cx.tcx.stripped_cfg_items(LOCAL_CRATE) {
+        if let Some(parent_module) = stripped.parent_module.as_local() {
+            check_cfg_entry(cx, &stripped.cfg.0, parent_module);
+        }
+    }
+}
+
+fn check_cfg_entry(cx: &LateContext<'_>, cfg: &CfgEntry, parent_module: LocalDefId) {
+    match cfg {
+        &CfgEntry::NameValue {
+            name: sym::feature,
+            name_span: _,
+            value: Some((sym::cargo_clippy, _)),
+            span,
+        } => {
+            span_lint_hir_and_then(
+                cx,
+                DEPRECATED_CLIPPY_CFG_ATTR,
+                cx.tcx.local_def_id_to_hir_id(parent_module),
+                span,
+                "`feature = \"cargo-clippy\"` was replaced by `clippy`",
+                |diag| {
+                    diag.span_suggestion(span, "replace with", "clippy", Applicability::MachineApplicable);
+                },
+            );
+        },
+        CfgEntry::All(children, _) | CfgEntry::Any(children, _) => {
+            for child in children {
+                check_cfg_entry(cx, child, parent_module);
+            }
+        },
+        CfgEntry::Not(child, _) => check_cfg_entry(cx, child, parent_module),
+        _ => {},
+    }
+}

clippy_lints/src/attrs/mod.rs

@@ -160,9 +160,8 @@ declare_clippy_lint! {
     /// 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/rust-clippy/pull/3123#issuecomment-422321765)
+    /// `cfg_attr` is valid in some places that `rustfmt::skip` is not, e.g. outer attributes on
+    /// blocks and inner attributes on inline modules.
     ///
     /// ### Example
     /// ```no_run
@@ -474,22 +473,27 @@ declare_clippy_lint! {
     "ignored tests without messages"
 }
 
-pub struct Attributes {
+pub struct LateAttributes {
     msrv: Msrv,
 }
 
-impl_lint_pass!(Attributes => [
+impl_lint_pass!(LateAttributes => [
     INLINE_ALWAYS,
     REPR_PACKED_WITHOUT_ABI,
+    DEPRECATED_CLIPPY_CFG_ATTR,
 ]);
 
-impl Attributes {
+impl LateAttributes {
     pub fn new(conf: &'static Conf) -> Self {
         Self { msrv: conf.msrv }
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for Attributes {
+impl<'tcx> LateLintPass<'tcx> for LateAttributes {
+    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
+        deprecated_cfg_attr::check_stripped(cx);
+    }
+
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
         let attrs = cx.tcx.hir_attrs(item.hir_id());
         if let ItemKind::Fn { ident, .. } = item.kind
@@ -526,35 +530,6 @@ impl EarlyAttributes {
 }
 
 impl_lint_pass!(EarlyAttributes => [
-    DEPRECATED_CFG_ATTR,
-    NON_MINIMAL_CFG,
-    DEPRECATED_CLIPPY_CFG_ATTR,
-    UNNECESSARY_CLIPPY_CFG,
-]);
-
-impl EarlyLintPass for EarlyAttributes {
-    fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
-        deprecated_cfg_attr::check(cx, attr, &self.msrv);
-        deprecated_cfg_attr::check_clippy(cx, attr);
-        non_minimal_cfg::check(cx, attr);
-    }
-
-    extract_msrv_attr!();
-}
-
-pub struct PostExpansionEarlyAttributes {
-    msrv: MsrvStack,
-}
-
-impl PostExpansionEarlyAttributes {
-    pub fn new(conf: &'static Conf) -> Self {
-        Self {
-            msrv: MsrvStack::new(conf.msrv),
-        }
-    }
-}
-
-impl_lint_pass!(PostExpansionEarlyAttributes => [
     ALLOW_ATTRIBUTES,
     ALLOW_ATTRIBUTES_WITHOUT_REASON,
     DEPRECATED_SEMVER,
@@ -564,15 +539,23 @@ impl_lint_pass!(PostExpansionEarlyAttributes => [
     SHOULD_PANIC_WITHOUT_EXPECT,
     MIXED_ATTRIBUTES_STYLE,
     DUPLICATED_ATTRIBUTES,
+    DEPRECATED_CFG_ATTR,
+    DEPRECATED_CLIPPY_CFG_ATTR,
+    UNNECESSARY_CLIPPY_CFG,
+    NON_MINIMAL_CFG,
 ]);
 
-impl EarlyLintPass for PostExpansionEarlyAttributes {
+impl EarlyLintPass for EarlyAttributes {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &ast::Crate) {
         blanket_clippy_restriction_lints::check_command_line(cx);
         duplicated_attributes::check(cx, &krate.attrs);
     }
 
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
+        deprecated_cfg_attr::check_cfg_attr(cx, attr, &self.msrv);
+        deprecated_cfg_attr::check_cfg(cx, attr);
+        non_minimal_cfg::check(cx, attr);
+
         if let Some(items) = &attr.meta_item_list()
             && let Some(ident) = attr.ident()
         {

clippy_lints/src/attrs/non_minimal_cfg.rs

@@ -7,7 +7,7 @@ use rustc_lint::EarlyContext;
 use rustc_span::sym;
 
 pub(super) fn check(cx: &EarlyContext<'_>, attr: &Attribute) {
-    if attr.has_name(sym::cfg)
+    if attr.has_name(sym::cfg_trace)
         && let Some(items) = attr.meta_item_list()
     {
         check_nested_cfg(cx, &items);

clippy_lints/src/lib.rs

@@ -442,12 +442,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         store.register_removed(name, reason);
     }
 
-    // NOTE: Do not add any more pre-expansion passes. These should be removed eventually.
-    // Due to the architecture of the compiler, currently `cfg_attr` attributes on crate
-    // level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass.
-    store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes::new(conf)));
-
-    store.register_early_pass(move || Box::new(attrs::PostExpansionEarlyAttributes::new(conf)));
+    store.register_early_pass(move || Box::new(attrs::EarlyAttributes::new(conf)));
 
     let format_args_storage = FormatArgsStorage::default();
     let format_args = format_args_storage.clone();
@@ -480,7 +475,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(unnecessary_mut_passed::UnnecessaryMutPassed));
     store.register_late_pass(|_| Box::<significant_drop_tightening::SignificantDropTightening<'_>>::default());
     store.register_late_pass(move |_| Box::new(len_zero::LenZero::new(conf)));
-    store.register_late_pass(move |_| Box::new(attrs::Attributes::new(conf)));
+    store.register_late_pass(move |_| Box::new(attrs::LateAttributes::new(conf)));
     store.register_late_pass(|_| Box::new(blocks_in_conditions::BlocksInConditions));
     store.register_late_pass(|_| Box::new(unicode::Unicode));
     store.register_late_pass(|_| Box::new(uninit_vec::UninitVec));

tests/ui/cfg_attr_cargo_clippy.fixed

@@ -7,14 +7,22 @@
 //~^ deprecated_clippy_cfg_attr
 #[cfg_attr(not(clippy), derive(Debug))]
 //~^ deprecated_clippy_cfg_attr
-#[cfg(clippy)]
-//~^ deprecated_clippy_cfg_attr
+struct Attrs;
+
 #[cfg(not(clippy))]
 //~^ deprecated_clippy_cfg_attr
+struct Not;
+
+#[cfg(clippy)]
+//~^ deprecated_clippy_cfg_attr
+struct Stripped;
+
 #[cfg(any(clippy))]
 //~^ deprecated_clippy_cfg_attr
+struct StrippedAny;
+
 #[cfg(all(clippy))]
 //~^ deprecated_clippy_cfg_attr
-pub struct Bar;
+struct StrippedAll;
 
 fn main() {}

tests/ui/cfg_attr_cargo_clippy.rs

@@ -7,14 +7,22 @@
 //~^ deprecated_clippy_cfg_attr
 #[cfg_attr(not(feature = "cargo-clippy"), derive(Debug))]
 //~^ deprecated_clippy_cfg_attr
-#[cfg(feature = "cargo-clippy")]
-//~^ deprecated_clippy_cfg_attr
+struct Attrs;
+
 #[cfg(not(feature = "cargo-clippy"))]
 //~^ deprecated_clippy_cfg_attr
+struct Not;
+
+#[cfg(feature = "cargo-clippy")]
+//~^ deprecated_clippy_cfg_attr
+struct Stripped;
+
 #[cfg(any(feature = "cargo-clippy"))]
 //~^ deprecated_clippy_cfg_attr
+struct StrippedAny;
+
 #[cfg(all(feature = "cargo-clippy"))]
 //~^ deprecated_clippy_cfg_attr
-pub struct Bar;
+struct StrippedAll;
 
 fn main() {}

tests/ui/cfg_attr_cargo_clippy.stderr

@@ -19,26 +19,26 @@ error: `feature = "cargo-clippy"` was replaced by `clippy`
 LL | #[cfg_attr(not(feature = "cargo-clippy"), derive(Debug))]
    |                ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
 
-error: `feature = "cargo-clippy"` was replaced by `clippy`
-  --> tests/ui/cfg_attr_cargo_clippy.rs:10:7
-   |
-LL | #[cfg(feature = "cargo-clippy")]
-   |       ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
-
 error: `feature = "cargo-clippy"` was replaced by `clippy`
   --> tests/ui/cfg_attr_cargo_clippy.rs:12:11
    |
 LL | #[cfg(not(feature = "cargo-clippy"))]
    |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
 
 error: `feature = "cargo-clippy"` was replaced by `clippy`
-  --> tests/ui/cfg_attr_cargo_clippy.rs:14:11
+  --> tests/ui/cfg_attr_cargo_clippy.rs:16:7
+   |
+LL | #[cfg(feature = "cargo-clippy")]
+   |       ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
+
+error: `feature = "cargo-clippy"` was replaced by `clippy`
+  --> tests/ui/cfg_attr_cargo_clippy.rs:20:11
    |
 LL | #[cfg(any(feature = "cargo-clippy"))]
    |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`
 
 error: `feature = "cargo-clippy"` was replaced by `clippy`
-  --> tests/ui/cfg_attr_cargo_clippy.rs:16:11
+  --> tests/ui/cfg_attr_cargo_clippy.rs:24:11
    |
 LL | #[cfg(all(feature = "cargo-clippy"))]
    |           ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `clippy`

tests/ui/cfg_attr_rustfmt.fixed

@@ -1,11 +1,8 @@
-
-#![feature(stmt_expr_attributes)]
-
-#![allow(unused, clippy::no_effect, clippy::unnecessary_operation)]
+#![feature(stmt_expr_attributes, custom_inner_attributes)]
 #![warn(clippy::deprecated_cfg_attr)]
 
-// This doesn't get linted, see known problems
-#![cfg_attr(rustfmt, rustfmt_skip)]
+#![rustfmt::skip]
+//~^ deprecated_cfg_attr
 
 #[rustfmt::skip]
 trait Foo
@@ -14,33 +11,25 @@ fn foo(
 );
 }
 
-fn skip_on_statements() {
-    #[rustfmt::skip]
-    //~^ deprecated_cfg_attr
-    5+3;
-}
-
 #[rustfmt::skip]
 //~^ deprecated_cfg_attr
-fn main() {
-    foo::f();
-}
+fn outer() {}
 
-mod foo {
-    #![cfg_attr(rustfmt, rustfmt_skip)]
+// Ideally these would not lint
+fn invalid() {
+    #![rustfmt::skip]
+    //~^ deprecated_cfg_attr
 
-    pub fn f() {}
+    #[rustfmt::skip]
+    //~^ deprecated_cfg_attr
+    {}
 }
 
 #[clippy::msrv = "1.29"]
-fn msrv_1_29() {
-    #[cfg_attr(rustfmt, rustfmt::skip)]
-    1+29;
-}
+#[cfg_attr(rustfmt, rustfmt::skip)]
+fn msrv_1_29() {}
 
 #[clippy::msrv = "1.30"]
-fn msrv_1_30() {
-    #[rustfmt::skip]
-    //~^ deprecated_cfg_attr
-    1+30;
-}
+#[rustfmt::skip]
+//~^ deprecated_cfg_attr
+fn msrv_1_30() {}