Replace cfg_attr in AST with rustc-cfg-placeholder for accurate s…

…pan 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 +
   |
```

compiler/rustc_ast/src/ast.rs

@@ -3006,6 +3006,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

@@ -226,6 +226,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

@@ -340,6 +340,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::is_builtin_attr(attr)

compiler/rustc_expand/src/config.rs

@@ -296,7 +296,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

@@ -750,6 +750,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         template!(Word, List: r#""...""#), DuplicatesOk,
         EncodeCrossCrate::No, 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

@@ -927,8 +927,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/context/diagnostics.rs

@@ -288,8 +288,8 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
         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

@@ -2907,7 +2907,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

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

compiler/rustc_passes/src/check_attr.rs

@@ -273,6 +273,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

compiler/rustc_resolve/src/check_unused.rs

@@ -155,6 +155,7 @@ impl<'a, 'ra, 'tcx> UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
                         extern_crate.id,
                         span,
                         BuiltinLintDiag::UnusedExternCrate {
+                            span: extern_crate.span,
                             removal_span: extern_crate.span_with_attributes,
                         },
                     );
@@ -221,6 +222,7 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
             // compiler and we don't need to consider them.
             ast::ItemKind::Use(..) if item.span.is_dummy() => return,
             ast::ItemKind::ExternCrate(orig_name) => {
+                tracing::info!(?item);
                 self.extern_crate_items.push(ExternCrateToLint {
                     id: item.id,
                     span: item.span,
@@ -397,6 +399,7 @@ impl Resolver<'_, '_> {
                     }
                 }
                 ImportKind::ExternCrate { id, .. } => {
+                    tracing::info!("import={import:#?}");
                     let def_id = self.local_def_id(id);
                     if self.extern_crate_map.get(&def_id).map_or(true, |&cnum| {
                         !tcx.is_compiler_builtins(cnum)

compiler/rustc_span/src/symbol.rs

@@ -1667,6 +1667,8 @@ symbols! {
         rustc_box,
         rustc_builtin_macro,
         rustc_capture_analysis,
+        // We ensure that the attribute can't be written by end users by adding `-` to the name.
+        rustc_cfg_placeholder: "rustc-cfg-placeholder",
         rustc_clean,
         rustc_coherence_is_core,
         rustc_coinductive,

tests/ui/editions/edition-extern-crate-allowed.rs

@@ -5,6 +5,6 @@
 #![warn(rust_2018_idioms)]
 
 extern crate edition_extern_crate_allowed;
-//~^ WARNING unused extern crate
+//~^ WARNING unused `extern crate`
 
 fn main() {}

tests/ui/editions/edition-extern-crate-allowed.stderr

@@ -1,15 +1,19 @@
-warning: unused extern crate
+warning: unused `extern crate`
   --> $DIR/edition-extern-crate-allowed.rs:7:1
    |
 LL | extern crate edition_extern_crate_allowed;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
    |
 note: the lint level is defined here
   --> $DIR/edition-extern-crate-allowed.rs:5:9
    |
 LL | #![warn(rust_2018_idioms)]
    |         ^^^^^^^^^^^^^^^^
    = note: `#[warn(unused_extern_crates)]` implied by `#[warn(rust_2018_idioms)]`
+help: remove the unused `extern crate`
+   |
+LL - extern crate edition_extern_crate_allowed;
+   |
 
 warning: 1 warning emitted
 

tests/ui/imports/extern-crate-used.rs

@@ -15,7 +15,7 @@ extern crate core as iso3;
 extern crate core as iso4;
 
 // Doesn't introduce its extern prelude entry, so it's still considered unused.
-extern crate core; //~ ERROR unused extern crate
+extern crate core; //~ ERROR unused `extern crate`
 
 mod m {
     use iso1::any as are_you_okay1;

tests/ui/imports/extern-crate-used.stderr

@@ -1,14 +1,19 @@
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/extern-crate-used.rs:18:1
    |
 LL | extern crate core;
-   | ^^^^^^^^^^^^^^^^^^ help: remove it
+   | ^^^^^^^^^^^^^^^^^^ unused
    |
 note: the lint level is defined here
   --> $DIR/extern-crate-used.rs:6:9
    |
 LL | #![deny(unused_extern_crates)]
    |         ^^^^^^^^^^^^^^^^^^^^
+help: remove the unused `extern crate`
+   |
+LL - extern crate core;
+LL +
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/lint/unnecessary-extern-crate.rs

@@ -4,10 +4,10 @@
 #![feature(test)]
 
 extern crate core;
-//~^ ERROR unused extern crate
+//~^ ERROR unused `extern crate`
 //~| HELP remove
 extern crate core as x;
-//~^ ERROR unused extern crate
+//~^ ERROR unused `extern crate`
 //~| HELP remove
 
 extern crate proc_macro;
@@ -29,11 +29,11 @@ mod foo {
     pub(super) extern crate alloc as d;
 
     extern crate core;
-    //~^ ERROR unused extern crate
+    //~^ ERROR unused `extern crate`
     //~| HELP remove
 
     extern crate core as x;
-    //~^ ERROR unused extern crate
+    //~^ ERROR unused `extern crate`
     //~| HELP remove
 
     pub extern crate test;
@@ -42,11 +42,11 @@ mod foo {
 
     mod bar {
         extern crate core;
-        //~^ ERROR unused extern crate
+        //~^ ERROR unused `extern crate`
         //~| HELP remove
 
         extern crate core as x;
-        //~^ ERROR unused extern crate
+        //~^ ERROR unused `extern crate`
         //~| HELP remove
 
         pub(in crate::foo::bar) extern crate alloc as e;

tests/ui/lint/unnecessary-extern-crate.stderr

@@ -1,44 +1,73 @@
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:6:1
    |
 LL | extern crate core;
-   | ^^^^^^^^^^^^^^^^^^ help: remove it
+   | ^^^^^^^^^^^^^^^^^^ unused
    |
 note: the lint level is defined here
   --> $DIR/unnecessary-extern-crate.rs:3:9
    |
 LL | #![deny(unused_extern_crates)]
    |         ^^^^^^^^^^^^^^^^^^^^
+help: remove the unused `extern crate`
+   |
+LL - extern crate core;
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:9:1
    |
 LL | extern crate core as x;
-   | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   | ^^^^^^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL - extern crate core as x;
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:31:5
    |
 LL |     extern crate core;
-   |     ^^^^^^^^^^^^^^^^^^ help: remove it
+   |     ^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL -     extern crate core;
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:35:5
    |
 LL |     extern crate core as x;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL -     extern crate core as x;
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:44:9
    |
 LL |         extern crate core;
-   |         ^^^^^^^^^^^^^^^^^^ help: remove it
+   |         ^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL -         extern crate core;
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/unnecessary-extern-crate.rs:48:9
    |
 LL |         extern crate core as x;
-   |         ^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   |         ^^^^^^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL -         extern crate core as x;
+   |
 
 error: aborting due to 6 previous errors
 

tests/ui/lint/unused/lint-unused-extern-crate.rs

@@ -8,7 +8,7 @@
 #![allow(unused_variables)]
 #![allow(deprecated)]
 
-extern crate lint_unused_extern_crate5; //~ ERROR: unused extern crate
+extern crate lint_unused_extern_crate5; //~ ERROR: unused `extern crate`
 
 pub extern crate lint_unused_extern_crate4; // no error, it is re-exported
 
@@ -26,7 +26,7 @@ use other::*;
 
 mod foo {
     // Test that this is unused even though an earlier `extern crate` is used.
-    extern crate lint_unused_extern_crate2; //~ ERROR unused extern crate
+    extern crate lint_unused_extern_crate2; //~ ERROR unused `extern crate`
 }
 
 fn main() {

tests/ui/lint/unused/lint-unused-extern-crate.stderr

@@ -1,20 +1,31 @@
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/lint-unused-extern-crate.rs:11:1
    |
 LL | extern crate lint_unused_extern_crate5;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
    |
 note: the lint level is defined here
   --> $DIR/lint-unused-extern-crate.rs:7:9
    |
 LL | #![deny(unused_extern_crates)]
    |         ^^^^^^^^^^^^^^^^^^^^
+help: remove the unused `extern crate`
+   |
+LL - extern crate lint_unused_extern_crate5;
+LL +
+   |
 
-error: unused extern crate
+error: unused `extern crate`
   --> $DIR/lint-unused-extern-crate.rs:29:5
    |
 LL |     extern crate lint_unused_extern_crate2;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unused
+   |
+help: remove the unused `extern crate`
+   |
+LL -     extern crate lint_unused_extern_crate2;
+LL +
+   |
 
 error: aborting due to 2 previous errors