Auto merge of #142487 - estebank:serde-attr-5, r=<try>

Detect missing `derive` on unresolved attribute even when not imported

When encountering unresolved attributes, ensure the proc-macros for every crate in scope are added to the `macro_map` so that typos and missing `derive`s are properly detected.

```
error: cannot find attribute `sede` in this scope
  --> $DIR/missing-derive-3.rs:20:7
   |
LL |     #[sede(untagged)]
   |       ^^^^
   |
help: the derive macros `Deserialize` and `Serialize` accept the similarly named `serde` attribute
   |
LL |     #[serde(untagged)]
   |         +

error: cannot find attribute `serde` in this scope
  --> $DIR/missing-derive-3.rs:14:7
   |
LL |     #[serde(untagged)]
   |       ^^^^^
   |
note: `serde` is imported here, but it is a crate, not an attribute
  --> $DIR/missing-derive-3.rs:4:1
   |
LL | extern crate serde;
   | ^^^^^^^^^^^^^^^^^^^
help: `serde` is an attribute that can be used by the derive macros `Deserialize` and `Serialize`, you might be missing a `derive` attribute
   |
LL + #[derive(Deserialize, Serialize)]
LL | enum B {
   |
```

Follow up to #134841.

Author: rust-bors[bot]

compiler/rustc_metadata/src/creader.rs

@@ -30,6 +30,7 @@ use rustc_session::cstore::{CrateDepKind, CrateSource, ExternCrate, ExternCrateS
 use rustc_session::lint::{self, BuiltinLintDiag};
 use rustc_session::output::validate_crate_name;
 use rustc_session::search_paths::PathKind;
+use rustc_span::def_id::DefId;
 use rustc_span::edition::Edition;
 use rustc_span::{DUMMY_SP, Ident, Span, Symbol, sym};
 use rustc_target::spec::{PanicStrategy, Target};
@@ -274,6 +275,12 @@ impl CStore {
             .filter_map(|(cnum, data)| data.as_deref().map(|data| (cnum, data)))
     }
 
+    pub fn all_proc_macro_def_ids(&self) -> Vec<DefId> {
+        self.iter_crate_data()
+            .flat_map(|(krate, data)| data.proc_macros_for_crate(krate, self))
+            .collect()
+    }
+
     fn push_dependencies_in_postorder(&self, deps: &mut IndexSet<CrateNum>, cnum: CrateNum) {
         if !deps.contains(&cnum) {
             let data = self.get_crate_data(cnum);

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -2010,6 +2010,16 @@ impl CrateMetadata {
         self.root.is_proc_macro_crate()
     }
 
+    pub(crate) fn proc_macros_for_crate(&self, krate: CrateNum, cstore: &CStore) -> Vec<DefId> {
+        let Some(data) = self.root.proc_macro_data.as_ref() else {
+            return vec![];
+        };
+        data.macros
+            .decode(CrateMetadataRef { cdata: self, cstore })
+            .map(|index| DefId { index, krate })
+            .collect()
+    }
+
     pub(crate) fn name(&self) -> Symbol {
         self.root.header.name
     }

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -188,6 +188,19 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
+    /// Add every proc macro accessible from the current crate to the `macro_map` so diagnostics can
+    /// find them for suggestions.
+    pub(crate) fn register_macros_for_all_crates(&mut self) {
+        if self.all_crate_macros_already_registered {
+            return;
+        }
+        self.all_crate_macros_already_registered = true;
+        let def_ids = self.cstore().all_proc_macro_def_ids();
+        for def_id in def_ids {
+            self.get_macro_by_def_id(def_id);
+        }
+    }
+
     pub(crate) fn build_reduced_graph(
         &mut self,
         fragment: &AstFragment,

compiler/rustc_resolve/src/diagnostics.rs

@@ -1483,32 +1483,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         krate: &Crate,
         sugg_span: Option<Span>,
     ) {
-        // Bring imported but unused `derive` macros into `macro_map` so we ensure they can be used
-        // for suggestions.
-        self.visit_scopes(
-            ScopeSet::Macro(MacroKind::Derive),
-            &parent_scope,
-            ident.span.ctxt(),
-            |this, scope, _use_prelude, _ctxt| {
-                let Scope::Module(m, _) = scope else {
-                    return None;
-                };
-                for (_, resolution) in this.resolutions(m).borrow().iter() {
-                    let Some(binding) = resolution.borrow().binding else {
-                        continue;
-                    };
-                    let Res::Def(DefKind::Macro(MacroKind::Derive | MacroKind::Attr), def_id) =
-                        binding.res()
-                    else {
-                        continue;
-                    };
-                    // By doing this all *imported* macros get added to the `macro_map` even if they
-                    // are *unused*, which makes the later suggestions find them and work.
-                    let _ = this.get_macro_by_def_id(def_id);
-                }
-                None::<()>
-            },
-        );
+        // Bring all unused `derive` macros into `macro_map` so we ensure they can be used for
+        // suggestions.
+        self.register_macros_for_all_crates();
 
         let is_expected = &|res: Res| res.macro_kind() == Some(macro_kind);
         let suggestion = self.early_lookup_typo_candidate(

compiler/rustc_resolve/src/lib.rs

@@ -14,6 +14,7 @@
 #![doc(rust_logo)]
 #![feature(assert_matches)]
 #![feature(box_patterns)]
+#![feature(default_field_values)]
 #![feature(if_let_guard)]
 #![feature(iter_intersperse)]
 #![feature(rustc_attrs)]
@@ -1036,7 +1037,7 @@ pub struct Resolver<'ra, 'tcx> {
 
     graph_root: Module<'ra>,
 
-    prelude: Option<Module<'ra>>,
+    prelude: Option<Module<'ra>> = None,
     extern_prelude: FxIndexMap<Ident, ExternPreludeEntry<'ra>>,
 
     /// N.B., this is used only for better diagnostics, not name resolution itself.
@@ -1047,10 +1048,10 @@ pub struct Resolver<'ra, 'tcx> {
     field_visibility_spans: FxHashMap<DefId, Vec<Span>>,
 
     /// All imports known to succeed or fail.
-    determined_imports: Vec<Import<'ra>>,
+    determined_imports: Vec<Import<'ra>> = Vec::new(),
 
     /// All non-determined imports.
-    indeterminate_imports: Vec<Import<'ra>>,
+    indeterminate_imports: Vec<Import<'ra>> = Vec::new(),
 
     // Spans for local variables found during pattern resolution.
     // Used for suggestions during error reporting.
@@ -1096,23 +1097,23 @@ pub struct Resolver<'ra, 'tcx> {
     module_map: FxIndexMap<DefId, Module<'ra>>,
     binding_parent_modules: FxHashMap<NameBinding<'ra>, Module<'ra>>,
 
-    underscore_disambiguator: u32,
+    underscore_disambiguator: u32 = 0,
 
     /// Maps glob imports to the names of items actually imported.
     glob_map: FxIndexMap<LocalDefId, FxIndexSet<Symbol>>,
-    glob_error: Option<ErrorGuaranteed>,
-    visibilities_for_hashing: Vec<(LocalDefId, ty::Visibility)>,
+    glob_error: Option<ErrorGuaranteed> = None,
+    visibilities_for_hashing: Vec<(LocalDefId, ty::Visibility)> = Vec::new(),
     used_imports: FxHashSet<NodeId>,
     maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
 
     /// Privacy errors are delayed until the end in order to deduplicate them.
-    privacy_errors: Vec<PrivacyError<'ra>>,
+    privacy_errors: Vec<PrivacyError<'ra>> = Vec::new(),
     /// Ambiguity errors are delayed for deduplication.
-    ambiguity_errors: Vec<AmbiguityError<'ra>>,
+    ambiguity_errors: Vec<AmbiguityError<'ra>> = Vec::new(),
     /// `use` injections are delayed for better placement and deduplication.
-    use_injections: Vec<UseError<'tcx>>,
+    use_injections: Vec<UseError<'tcx>> = Vec::new(),
     /// Crate-local macro expanded `macro_export` referred to by a module-relative path.
-    macro_expanded_macro_export_errors: BTreeSet<(Span, Span)>,
+    macro_expanded_macro_export_errors: BTreeSet<(Span, Span)> = BTreeSet::new(),
 
     arenas: &'ra ResolverArenas<'ra>,
     dummy_binding: NameBinding<'ra>,
@@ -1142,10 +1143,11 @@ pub struct Resolver<'ra, 'tcx> {
     proc_macro_stubs: FxHashSet<LocalDefId>,
     /// Traces collected during macro resolution and validated when it's complete.
     single_segment_macro_resolutions:
-        Vec<(Ident, MacroKind, ParentScope<'ra>, Option<NameBinding<'ra>>, Option<Span>)>,
+        Vec<(Ident, MacroKind, ParentScope<'ra>, Option<NameBinding<'ra>>, Option<Span>)>
+            = Vec::new(),
     multi_segment_macro_resolutions:
-        Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'ra>, Option<Res>, Namespace)>,
-    builtin_attrs: Vec<(Ident, ParentScope<'ra>)>,
+        Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'ra>, Option<Res>, Namespace)> = Vec::new(),
+    builtin_attrs: Vec<(Ident, ParentScope<'ra>)> = Vec::new(),
     /// `derive(Copy)` marks items they are applied to so they are treated specially later.
     /// Derive macros cannot modify the item themselves and have to store the markers in the global
     /// context, so they attach the markers to derive container IDs using this resolver table.
@@ -1167,9 +1169,9 @@ pub struct Resolver<'ra, 'tcx> {
     /// Avoid duplicated errors for "name already defined".
     name_already_seen: FxHashMap<Symbol, Span>,
 
-    potentially_unused_imports: Vec<Import<'ra>>,
+    potentially_unused_imports: Vec<Import<'ra>> = Vec::new(),
 
-    potentially_unnecessary_qualifications: Vec<UnnecessaryQualification<'ra>>,
+    potentially_unnecessary_qualifications: Vec<UnnecessaryQualification<'ra>> = Vec::new(),
 
     /// Table for mapping struct IDs into struct constructor IDs,
     /// it's not used during normal resolution, only for better error reporting.
@@ -1178,7 +1180,7 @@ pub struct Resolver<'ra, 'tcx> {
 
     lint_buffer: LintBuffer,
 
-    next_node_id: NodeId,
+    next_node_id: NodeId = CRATE_NODE_ID,
 
     node_id_to_def_id: NodeMap<Feed<'tcx, LocalDefId>>,
 
@@ -1196,17 +1198,17 @@ pub struct Resolver<'ra, 'tcx> {
     item_generics_num_lifetimes: FxHashMap<LocalDefId, usize>,
     delegation_fn_sigs: LocalDefIdMap<DelegationFnSig>,
 
-    main_def: Option<MainDefinition>,
+    main_def: Option<MainDefinition> = None,
     trait_impls: FxIndexMap<DefId, Vec<LocalDefId>>,
     /// A list of proc macro LocalDefIds, written out in the order in which
     /// they are declared in the static array generated by proc_macro_harness.
-    proc_macros: Vec<LocalDefId>,
+    proc_macros: Vec<LocalDefId> = Vec::new(),
     confused_type_with_std_module: FxIndexMap<Span, Span>,
     /// Whether lifetime elision was successful.
     lifetime_elision_allowed: FxHashSet<NodeId>,
 
     /// Names of items that were stripped out via cfg with their corresponding cfg meta item.
-    stripped_cfg_items: Vec<StrippedCfgItem<NodeId>>,
+    stripped_cfg_items: Vec<StrippedCfgItem<NodeId>> = Vec::new(),
 
     effective_visibilities: EffectiveVisibilities,
     doc_link_resolutions: FxIndexMap<LocalDefId, DocLinkResMap>,
@@ -1228,6 +1230,10 @@ pub struct Resolver<'ra, 'tcx> {
 
     mods_with_parse_errors: FxHashSet<DefId>,
 
+    /// Whether `Resolver::register_macros_for_all_crates` has been called once already, as we
+    /// don't need to run it more than once.
+    all_crate_macros_already_registered: bool = false,
+
     // Stores pre-expansion and pre-placeholder-fragment-insertion names for `impl Trait` types
     // that were encountered during resolution. These names are used to generate item names
     // for APITs, so we don't want to leak details of resolution into these names.
@@ -1475,15 +1481,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             // The outermost module has def ID 0; this is not reflected in the
             // AST.
             graph_root,
-            prelude: None,
             extern_prelude,
 
             field_names: Default::default(),
             field_visibility_spans: FxHashMap::default(),
 
-            determined_imports: Vec::new(),
-            indeterminate_imports: Vec::new(),
-
             pat_span_map: Default::default(),
             partial_res_map: Default::default(),
             import_res_map: Default::default(),
@@ -1494,24 +1496,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             extern_crate_map: Default::default(),
             module_children: Default::default(),
             trait_map: NodeMap::default(),
-            underscore_disambiguator: 0,
             empty_module,
             module_map,
             block_map: Default::default(),
             binding_parent_modules: FxHashMap::default(),
             ast_transform_scopes: FxHashMap::default(),
 
             glob_map: Default::default(),
-            glob_error: None,
-            visibilities_for_hashing: Default::default(),
             used_imports: FxHashSet::default(),
             maybe_unused_trait_imports: Default::default(),
 
-            privacy_errors: Vec::new(),
-            ambiguity_errors: Vec::new(),
-            use_injections: Vec::new(),
-            macro_expanded_macro_export_errors: BTreeSet::new(),
-
             arenas,
             dummy_binding: (Res::Err, pub_vis, DUMMY_SP, LocalExpnId::ROOT).to_name_binding(arenas),
             builtin_types_bindings: PrimTy::ALL
@@ -1559,27 +1553,19 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             derive_data: Default::default(),
             local_macro_def_scopes: FxHashMap::default(),
             name_already_seen: FxHashMap::default(),
-            potentially_unused_imports: Vec::new(),
-            potentially_unnecessary_qualifications: Default::default(),
             struct_constructors: Default::default(),
             unused_macros: Default::default(),
             unused_macro_rules: Default::default(),
             proc_macro_stubs: Default::default(),
-            single_segment_macro_resolutions: Default::default(),
-            multi_segment_macro_resolutions: Default::default(),
-            builtin_attrs: Default::default(),
             containers_deriving_copy: Default::default(),
             lint_buffer: LintBuffer::default(),
-            next_node_id: CRATE_NODE_ID,
             node_id_to_def_id,
             disambiguator: DisambiguatorState::new(),
             placeholder_field_indices: Default::default(),
             invocation_parents,
             legacy_const_generic_args: Default::default(),
             item_generics_num_lifetimes: Default::default(),
-            main_def: Default::default(),
             trait_impls: Default::default(),
-            proc_macros: Default::default(),
             confused_type_with_std_module: Default::default(),
             lifetime_elision_allowed: Default::default(),
             stripped_cfg_items: Default::default(),
@@ -1594,6 +1580,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             current_crate_outer_attr_insert_span,
             mods_with_parse_errors: Default::default(),
             impl_trait_names: Default::default(),
+            ..
         };
 
         let root_parent_scope = ParentScope::module(graph_root, &resolver);

tests/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr

@@ -559,6 +559,12 @@ error: cannot find attribute `error` in this scope
    |
 LL | #[error(no_crate_example, code = E0123)]
    |   ^^^^^
+   |
+help: `error` is an attribute that can be used by the derive macro `Error`, you might be missing a `derive` attribute
+   |
+LL + #[derive(Error)]
+LL | struct ErrorAttribute {}
+   |
 
 error: cannot find attribute `warn_` in this scope
   --> $DIR/diagnostic-derive.rs:590:3

tests/ui/macros/missing-derive-3.stderr

@@ -3,6 +3,11 @@ error: cannot find attribute `sede` in this scope
    |
 LL |     #[sede(untagged)]
    |       ^^^^
+   |
+help: the derive macros `Deserialize` and `Serialize` accept the similarly named `serde` attribute
+   |
+LL |     #[serde(untagged)]
+   |         +
 
 error: cannot find attribute `serde` in this scope
   --> $DIR/missing-derive-3.rs:14:7
@@ -15,6 +20,11 @@ note: `serde` is imported here, but it is a crate, not an attribute
    |
 LL | extern crate serde;
    | ^^^^^^^^^^^^^^^^^^^
+help: `serde` is an attribute that can be used by the derive macros `Deserialize` and `Serialize`, you might be missing a `derive` attribute
+   |
+LL + #[derive(Deserialize, Serialize)]
+LL | enum B {
+   |
 
 error: cannot find attribute `serde` in this scope
   --> $DIR/missing-derive-3.rs:6:3
@@ -27,6 +37,11 @@ note: `serde` is imported here, but it is a crate, not an attribute
    |
 LL | extern crate serde;
    | ^^^^^^^^^^^^^^^^^^^
+help: `serde` is an attribute that can be used by the derive macros `Deserialize` and `Serialize`, you might be missing a `derive` attribute
+   |
+LL + #[derive(Deserialize, Serialize)]
+LL | enum A {
+   |
 
 error: aborting due to 3 previous errors
 

tests/ui/proc-macro/derive-helper-legacy-spurious.stderr

@@ -9,6 +9,12 @@ error: cannot find attribute `empty_helper` in this scope
    |
 LL | #[empty_helper]
    |   ^^^^^^^^^^^^
+   |
+help: `empty_helper` is an attribute that can be used by the derive macro `Empty`, you might be missing a `derive` attribute
+   |
+LL + #[derive(Empty)]
+LL | struct Foo {}
+   |
 
 error: aborting due to 2 previous errors