Replace cfg_attr in AST with rustc-cfg-placeholder for accurate span tracking

Previously, when evaluating a `#[cfg_attr(..)]` to false, the entire attribute was removed from the AST. Afterwards, we insert in its place a `#[rustc-cfg-placeholder]` attribute so that checks for attributes can still know about their placement. This is particularly relevant when we suggest removing items with `cfg_attr`s (fix #56328). We use `rustc-cfg-placeholder` as it is an ident that can't be written by the end user to begin with. We tweak the wording of the existing "unused `extern crate`" lint.

```
warning: unused `extern crate`
  --> $DIR/removing-extern-crate.rs:9:1
   |
LL | extern crate removing_extern_crate as foo;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
   |
note: the lint level is defined here
  --> $DIR/removing-extern-crate.rs:6:9
   |
LL | #![warn(rust_2018_idioms)]
   |         ^^^^^^^^^^^^^^^^
   = note: `#[warn(unused_extern_crates)]` implied by `#[warn(rust_2018_idioms)]`
help: remove the unused `extern crate`
   |
LL - #[cfg_attr(test, macro_use)]
LL - extern crate removing_extern_crate as foo;
LL +
   |
```

Author: estebank

compiler/rustc_ast/src/ast.rs

@@ -3143,6 +3143,10 @@ impl AttrItem {
             || self.path == sym::allow
             || self.path == sym::deny
     }
+
+    pub fn is_cfg_placeholder(&self) -> bool {
+        self.path == sym::rustc_cfg_placeholder
+    }
 }
 
 /// `TraitRef`s appear in impls.

compiler/rustc_ast/src/attr/mod.rs

@@ -233,6 +233,7 @@ impl Attribute {
 
     pub fn token_trees(&self) -> Vec<TokenTree> {
         match self.kind {
+            AttrKind::Normal(ref normal) if normal.item.is_cfg_placeholder() => vec![],
             AttrKind::Normal(ref normal) => normal
                 .tokens
                 .as_ref()

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -339,6 +339,10 @@ impl<'a> AstValidator<'a> {
                     sym::deny,
                     sym::expect,
                     sym::forbid,
+                    // `cfg` and `cfg_attr` get replaced with an inert `rustc_cfg_placeholder` to
+                    // keep the attribute "spot" in the AST. This allows suggestions to remove an
+                    // item to provide a correct suggestion when `#[cfg_attr]`s are present.
+                    sym::rustc_cfg_placeholder,
                     sym::warn,
                 ];
                 !arr.contains(&attr.name_or_empty()) && rustc_attr_parsing::is_builtin_attr(*attr)

compiler/rustc_expand/src/config.rs

@@ -292,7 +292,25 @@ impl<'a> StripUnconfigured<'a> {
         }
 
         if !attr::cfg_matches(&cfg_predicate, &self.sess, self.lint_node_id, self.features) {
-            return vec![];
+            // `cfg` and `cfg_attr` gets replaced with an inert `rustc_cfg_placeholder` to keep the
+            // attribute "spot" in the AST. This allows suggestions to remove an item to provide a
+            // correct suggestion when `#[cfg_attr]`s are present.
+            let item = ast::AttrItem {
+                unsafety: ast::Safety::Default,
+                path: ast::Path::from_ident(rustc_span::symbol::Ident::with_dummy_span(
+                    sym::rustc_cfg_placeholder,
+                )),
+                args: ast::AttrArgs::Empty,
+                tokens: None,
+            };
+            let kind = ast::AttrKind::Normal(P(ast::NormalAttr { item, tokens: None }));
+            let attr: ast::Attribute = ast::Attribute {
+                kind,
+                id: self.sess.psess.attr_id_generator.mk_attr_id(),
+                style: ast::AttrStyle::Outer,
+                span: cfg_attr.span,
+            };
+            return vec![attr];
         }
 
         if recursive {

compiler/rustc_feature/src/builtin_attrs.rs

@@ -752,6 +752,12 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         template!(Word, List: r#""...""#), DuplicatesOk,
         EncodeCrossCrate::Yes, INTERNAL_UNSTABLE
     ),
+    // placeholder that replaces `cfg_attr` in the item's attribute list
+    ungated!(
+        rustc_cfg_placeholder, Normal, template!(Word /* irrelevant */), DuplicatesOk,
+        EncodeCrossCrate::No
+    ),
+
 
     // ==========================================================================
     // Internal attributes, Diagnostics related:

compiler/rustc_lint/messages.ftl

@@ -945,8 +945,9 @@ lint_unused_doc_comment = unused doc comment
     .label = rustdoc does not generate documentation for macro invocations
     .help = to document an item produced by a macro, the macro must produce the documentation as part of its expansion
 
-lint_unused_extern_crate = unused extern crate
-    .suggestion = remove it
+lint_unused_extern_crate = unused `extern crate`
+    .label = unused
+    .suggestion = remove the unused `extern crate`
 
 lint_unused_import_braces = braces around {$node} is unnecessary
 

compiler/rustc_lint/src/early/diagnostics.rs

@@ -292,8 +292,8 @@ pub(super) fn decorate_lint(
         BuiltinLintDiag::ByteSliceInPackedStructWithDerive { ty } => {
             lints::ByteSliceInPackedStructWithDerive { ty }.decorate_lint(diag);
         }
-        BuiltinLintDiag::UnusedExternCrate { removal_span } => {
-            lints::UnusedExternCrate { removal_span }.decorate_lint(diag);
+        BuiltinLintDiag::UnusedExternCrate { span, removal_span } => {
+            lints::UnusedExternCrate { span, removal_span }.decorate_lint(diag);
         }
         BuiltinLintDiag::ExternCrateNotIdiomatic { vis_span, ident_span } => {
             let suggestion_span = vis_span.between(ident_span);

compiler/rustc_lint/src/lints.rs

@@ -3001,7 +3001,9 @@ pub(crate) struct ByteSliceInPackedStructWithDerive {
 #[derive(LintDiagnostic)]
 #[diag(lint_unused_extern_crate)]
 pub(crate) struct UnusedExternCrate {
-    #[suggestion(code = "", applicability = "machine-applicable")]
+    #[label]
+    pub span: Span,
+    #[suggestion(code = "", applicability = "machine-applicable", style = "verbose")]
     pub removal_span: Span,
 }
 

compiler/rustc_lint_defs/src/lib.rs

@@ -735,6 +735,7 @@ pub enum BuiltinLintDiag {
         ty: String,
     },
     UnusedExternCrate {
+        span: Span,
         removal_span: Span,
     },
     ExternCrateNotIdiomatic {

compiler/rustc_passes/src/check_attr.rs

@@ -286,6 +286,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                             | sym::prelude_import
                             | sym::panic_handler
                             | sym::allow_internal_unsafe
+                            | sym::rustc_cfg_placeholder // Inert, it's a placeholder in the AST.
                             | sym::fundamental
                             | sym::lang
                             | sym::needs_allocator
@@ -575,6 +576,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             // conditional compilation
             sym::cfg,
             sym::cfg_attr,
+            sym::rustc_cfg_placeholder,
             // testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`)
             sym::test,
             sym::ignore,
@@ -2641,7 +2643,7 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
         // only `#[cfg]` and `#[cfg_attr]` are allowed, but it should be removed
         // if we allow more attributes (e.g., tool attributes and `allow/deny/warn`)
         // in where clauses. After that, only `self.check_attributes` should be enough.
-        const ATTRS_ALLOWED: &[Symbol] = &[sym::cfg, sym::cfg_attr];
+        const ATTRS_ALLOWED: &[Symbol] = &[sym::cfg, sym::cfg_attr, sym::rustc_cfg_placeholder];
         let spans = self
             .tcx
             .hir_attrs(where_predicate.hir_id)