Merge #10804

10804: fix: Diagnose using `derive` on non-adt items r=Veykril a=Veykril



Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

6 files changed, 99 insertions, 27 deletions

diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index 0f1963e8318..5c4104baaef 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -32,6 +32,7 @@ diagnostics![
     BreakOutsideOfLoop,
     InactiveCode,
     IncorrectCase,
+    InvalidDeriveTarget,
     MacroError,
     MismatchedArgCount,
     MissingFields,
@@ -99,6 +100,11 @@ pub struct UnimplementedBuiltinMacro {
 }
 
 #[derive(Debug)]
+pub struct InvalidDeriveTarget {
+    pub node: InFile<SyntaxNodePtr>,
+}
+
+#[derive(Debug)]
 pub struct NoSuchField {
     pub field: InFile<AstPtr<ast::RecordExprField>>,
 }
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 4fdcb88bbf0..cca9b08f5bc 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -83,10 +83,10 @@ pub use crate::{
     attrs::{HasAttrs, Namespace},
     diagnostics::{
         AddReferenceHere, AnyDiagnostic, BreakOutsideOfLoop, InactiveCode, IncorrectCase,
-        MacroError, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
-        MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
-        UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedModule, UnresolvedProcMacro,
+        InvalidDeriveTarget, MacroError, MismatchedArgCount, MissingFields, MissingMatchArms,
+        MissingOkOrSomeInTailExpr, MissingUnsafe, NoSuchField, RemoveThisSemicolon,
+        ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate,
+        UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo},
@@ -654,6 +654,21 @@ impl Module {
                         .into(),
                     );
                 }
+                DefDiagnosticKind::InvalidDeriveTarget { ast, id } => {
+                    let node = ast.to_node(db.upcast());
+                    let derive = node.attrs().nth(*id as usize);
+                    match derive {
+                        Some(derive) => {
+                            acc.push(
+                                InvalidDeriveTarget {
+                                    node: ast.with_value(SyntaxNodePtr::from(AstPtr::new(&derive))),
+                                }
+                                .into(),
+                            );
+                        }
+                        None => stdx::never!("derive diagnostic on item without derive attribute"),
+                    }
+                }
             }
         }
         for decl in self.declarations(db) {
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 964af8a8a42..343e189046e 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -1055,13 +1055,10 @@ impl DefCollector<'_> {
                         &resolver,
                         &mut |_err| (),
                     );
-                    match call_id {
-                        Ok(Ok(call_id)) => {
-                            resolved.push((directive.module_id, call_id, directive.depth));
-                            res = ReachedFixedPoint::No;
-                            return false;
-                        }
-                        Err(UnresolvedMacro { .. }) | Ok(Err(_)) => {}
+                    if let Ok(Ok(call_id)) = call_id {
+                        resolved.push((directive.module_id, call_id, directive.depth));
+                        res = ReachedFixedPoint::No;
+                        return false;
                     }
                 }
                 MacroDirectiveKind::Derive { ast_id, derive_attr } => {
@@ -1072,19 +1069,16 @@ impl DefCollector<'_> {
                         self.def_map.krate,
                         &resolver,
                     );
-                    match call_id {
-                        Ok(call_id) => {
-                            self.def_map.modules[directive.module_id].scope.add_derive_macro_invoc(
-                                ast_id.ast_id,
-                                call_id,
-                                *derive_attr,
-                            );
+                    if let Ok(call_id) = call_id {
+                        self.def_map.modules[directive.module_id].scope.add_derive_macro_invoc(
+                            ast_id.ast_id,
+                            call_id,
+                            *derive_attr,
+                        );
 
-                            resolved.push((directive.module_id, call_id, directive.depth));
-                            res = ReachedFixedPoint::No;
-                            return false;
-                        }
-                        Err(UnresolvedMacro { .. }) => {}
+                        resolved.push((directive.module_id, call_id, directive.depth));
+                        res = ReachedFixedPoint::No;
+                        return false;
                     }
                 }
                 MacroDirectiveKind::Attr { ast_id, mod_item, attr } => {
@@ -1125,7 +1119,6 @@ impl DefCollector<'_> {
                         if expander.is_derive()
                     ) {
                         // Resolved to `#[derive]`
-                        let file_id = ast_id.ast_id.file_id;
                         let item_tree = self.db.file_item_tree(file_id);
 
                         let ast_id: FileAstId<ast::Item> = match *mod_item {
@@ -1133,8 +1126,12 @@ impl DefCollector<'_> {
                             ModItem::Union(it) => item_tree[it].ast_id.upcast(),
                             ModItem::Enum(it) => item_tree[it].ast_id.upcast(),
                             _ => {
-                                // Cannot use derive on this item.
-                                // FIXME: diagnose
+                                let diag = DefDiagnostic::invalid_derive_target(
+                                    directive.module_id,
+                                    ast_id.ast_id,
+                                    attr.id,
+                                );
+                                self.def_map.diagnostics.push(diag);
                                 res = ReachedFixedPoint::No;
                                 return false;
                             }
@@ -1194,7 +1191,6 @@ impl DefCollector<'_> {
                                         ),
                                     );
 
-                                    let file_id = ast_id.ast_id.file_id;
                                     let item_tree = self.db.file_item_tree(file_id);
                                     return recollect_without(self, &item_tree);
                                 }
diff --git a/crates/hir_def/src/nameres/diagnostics.rs b/crates/hir_def/src/nameres/diagnostics.rs
index 95061f60154..1eb38447310 100644
--- a/crates/hir_def/src/nameres/diagnostics.rs
+++ b/crates/hir_def/src/nameres/diagnostics.rs
@@ -6,6 +6,7 @@ use la_arena::Idx;
 use syntax::ast;
 
 use crate::{
+    attr::AttrId,
     item_tree::{self, ItemTreeId},
     nameres::LocalModuleId,
     path::ModPath,
@@ -29,6 +30,8 @@ pub enum DefDiagnosticKind {
     MacroError { ast: MacroCallKind, message: String },
 
     UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
+
+    InvalidDeriveTarget { ast: AstId<ast::Item>, id: u32 },
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -102,4 +105,15 @@ impl DefDiagnostic {
     ) -> Self {
         Self { in_module: container, kind: DefDiagnosticKind::UnimplementedBuiltinMacro { ast } }
     }
+
+    pub(super) fn invalid_derive_target(
+        container: LocalModuleId,
+        ast: AstId<ast::Item>,
+        id: AttrId,
+    ) -> Self {
+        Self {
+            in_module: container,
+            kind: DefDiagnosticKind::InvalidDeriveTarget { ast, id: id.ast_index },
+        }
+    }
 }
diff --git a/crates/ide_diagnostics/src/handlers/invalid_derive_target.rs b/crates/ide_diagnostics/src/handlers/invalid_derive_target.rs
new file mode 100644
index 00000000000..556d2dc0cbd
--- /dev/null
+++ b/crates/ide_diagnostics/src/handlers/invalid_derive_target.rs
@@ -0,0 +1,39 @@
+use crate::{Diagnostic, DiagnosticsContext, Severity};
+
+// Diagnostic: invalid-derive-target
+//
+// This diagnostic is shown when the derive attribute is used on an item other than a `struct`,
+// `enum` or `union`.
+pub(crate) fn invalid_derive_target(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::InvalidDeriveTarget,
+) -> Diagnostic {
+    // Use more accurate position if available.
+    let display_range = ctx.sema.diagnostics_display_range(d.node.clone()).range;
+
+    Diagnostic::new(
+        "invalid-derive-target",
+        "`derive` may only be applied to `struct`s, `enum`s and `union`s",
+        display_range,
+    )
+    .severity(Severity::Error)
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::check_diagnostics;
+
+    #[test]
+    fn fails_on_function() {
+        check_diagnostics(
+            r#"
+//- minicore:derive
+mod __ {
+    #[derive()]
+  //^^^^^^^^^^^ error: `derive` may only be applied to `struct`s, `enum`s and `union`s
+    fn main() {}
+}
+            "#,
+        );
+    }
+}
diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs
index 2f53b647b11..5070f240036 100644
--- a/crates/ide_diagnostics/src/lib.rs
+++ b/crates/ide_diagnostics/src/lib.rs
@@ -28,6 +28,7 @@ mod handlers {
     pub(crate) mod break_outside_of_loop;
     pub(crate) mod inactive_code;
     pub(crate) mod incorrect_case;
+    pub(crate) mod invalid_derive_target;
     pub(crate) mod macro_error;
     pub(crate) mod mismatched_arg_count;
     pub(crate) mod missing_fields;
@@ -195,6 +196,7 @@ pub fn diagnostics(
             AnyDiagnostic::UnresolvedMacroCall(d) => handlers::unresolved_macro_call::unresolved_macro_call(&ctx, &d),
             AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d),
             AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d),
+            AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d),
 
             AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) {
                 Some(it) => it,