Fix some things with builtin derives

ChayimFriedman2 · ChayimFriedman2 · commit 9c4a7705b13d · 2025-07-03T23:05:56.000+03:00
1. Err on unions on derive where it's required.
 2. Err on `#[derive(Default)]` on enums without `#[default]` variant.
 3. Don't add where bounds `T: Default` when expanding `Default` on enums (structs need that, enums not).

Also, because I was annoyed by that, in minicore, add a way to filter on multiple flags in the line-filter (`// :`). This is required for the `Debug` and `Hash` derives, because the derive should be in the prelude but the trait not.
diff --git a/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs b/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs
@@ -746,3 +746,83 @@ struct Struct9<#[pointee] T, U>(T) where T: ?Sized;
             623..690: `derive(CoercePointee)` requires `T` to be marked `?Sized`"#]],
     );
 }
+
+#[test]
+fn union_derive() {
+    check_errors(
+        r#"
+//- minicore: clone, copy, default, fmt, hash, ord, eq, derive
+
+#[derive(Copy)]
+union Foo1 { _v: () }
+#[derive(Clone)]
+union Foo2 { _v: () }
+#[derive(Default)]
+union Foo3 { _v: () }
+#[derive(Debug)]
+union Foo4 { _v: () }
+#[derive(Hash)]
+union Foo5 { _v: () }
+#[derive(Ord)]
+union Foo6 { _v: () }
+#[derive(PartialOrd)]
+union Foo7 { _v: () }
+#[derive(Eq)]
+union Foo8 { _v: () }
+#[derive(PartialEq)]
+union Foo9 { _v: () }
+    "#,
+        expect![[r#"
+            78..118: this trait cannot be derived for unions
+            119..157: this trait cannot be derived for unions
+            158..195: this trait cannot be derived for unions
+            196..232: this trait cannot be derived for unions
+            233..276: this trait cannot be derived for unions
+            313..355: this trait cannot be derived for unions"#]],
+    );
+}
+
+#[test]
+fn default_enum_without_default_attr() {
+    check_errors(
+        r#"
+//- minicore: default, derive
+
+#[derive(Default)]
+enum Foo {
+    Bar,
+}
+    "#,
+        expect!["1..41: `#[derive(Default)]` on enum with no `#[default]`"],
+    );
+}
+
+#[test]
+fn generic_enum_default() {
+    check(
+        r#"
+//- minicore: default, derive
+
+#[derive(Default)]
+enum Foo<T> {
+    Bar(T),
+    #[default]
+    Baz,
+}
+"#,
+        expect![[r#"
+
+#[derive(Default)]
+enum Foo<T> {
+    Bar(T),
+    #[default]
+    Baz,
+}
+
+impl <T, > $crate::default::Default for Foo<T, > where {
+    fn default() -> Self {
+        Foo::Baz
+    }
+}"#]],
+    );
+}
diff --git a/crates/hir-expand/src/builtin/derive_macro.rs b/crates/hir-expand/src/builtin/derive_macro.rs
@@ -458,6 +458,7 @@ fn expand_simple_derive(
     invoc_span: Span,
     tt: &tt::TopSubtree,
     trait_path: tt::TopSubtree,
+    allow_unions: bool,
     make_trait_body: impl FnOnce(&BasicAdtInfo) -> tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let info = match parse_adt(db, tt, invoc_span) {
@@ -469,6 +470,12 @@ fn expand_simple_derive(
             );
         }
     };
+    if !allow_unions && matches!(info.shape, AdtShape::Union) {
+        return ExpandResult::new(
+            tt::TopSubtree::empty(tt::DelimSpan::from_single(invoc_span)),
+            ExpandError::other(invoc_span, "this trait cannot be derived for unions"),
+        );
+    }
     ExpandResult::ok(expand_simple_derive_with_parsed(
         invoc_span,
         info,
@@ -535,7 +542,14 @@ fn copy_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::marker::Copy }, |_| quote! {span =>})
+    expand_simple_derive(
+        db,
+        span,
+        tt,
+        quote! {span => #krate::marker::Copy },
+        true,
+        |_| quote! {span =>},
+    )
 }
 
 fn clone_expand(
@@ -544,7 +558,7 @@ fn clone_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::clone::Clone }, |adt| {
+    expand_simple_derive(db, span, tt, quote! {span => #krate::clone::Clone }, true, |adt| {
         if matches!(adt.shape, AdtShape::Union) {
             let star = tt::Punct { char: '*', spacing: ::tt::Spacing::Alone, span };
             return quote! {span =>
@@ -599,41 +613,63 @@ fn default_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = &dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::default::Default }, |adt| {
-        let body = match &adt.shape {
-            AdtShape::Struct(fields) => {
-                let name = &adt.name;
-                fields.as_pattern_map(
-                    quote!(span =>#name),
+    let adt = match parse_adt(db, tt, span) {
+        Ok(info) => info,
+        Err(e) => {
+            return ExpandResult::new(
+                tt::TopSubtree::empty(tt::DelimSpan { open: span, close: span }),
+                e,
+            );
+        }
+    };
+    let (body, constrain_to_trait) = match &adt.shape {
+        AdtShape::Struct(fields) => {
+            let name = &adt.name;
+            let body = fields.as_pattern_map(
+                quote!(span =>#name),
+                span,
+                |_| quote!(span =>#krate::default::Default::default()),
+            );
+            (body, true)
+        }
+        AdtShape::Enum { default_variant, variants } => {
+            if let Some(d) = default_variant {
+                let (name, fields) = &variants[*d];
+                let adt_name = &adt.name;
+                let body = fields.as_pattern_map(
+                    quote!(span =>#adt_name :: #name),
                     span,
                     |_| quote!(span =>#krate::default::Default::default()),
-                )
+                );
+                (body, false)
+            } else {
+                return ExpandResult::new(
+                    tt::TopSubtree::empty(tt::DelimSpan::from_single(span)),
+                    ExpandError::other(span, "`#[derive(Default)]` on enum with no `#[default]`"),
+                );
             }
-            AdtShape::Enum { default_variant, variants } => {
-                if let Some(d) = default_variant {
-                    let (name, fields) = &variants[*d];
-                    let adt_name = &adt.name;
-                    fields.as_pattern_map(
-                        quote!(span =>#adt_name :: #name),
-                        span,
-                        |_| quote!(span =>#krate::default::Default::default()),
-                    )
-                } else {
-                    // FIXME: Return expand error here
-                    quote!(span =>)
+        }
+        AdtShape::Union => {
+            return ExpandResult::new(
+                tt::TopSubtree::empty(tt::DelimSpan::from_single(span)),
+                ExpandError::other(span, "this trait cannot be derived for unions"),
+            );
+        }
+    };
+    ExpandResult::ok(expand_simple_derive_with_parsed(
+        span,
+        adt,
+        quote! {span => #krate::default::Default },
+        |_adt| {
+            quote! {span =>
+                fn default() -> Self {
+                    #body
                 }
             }
-            AdtShape::Union => {
-                // FIXME: Return expand error here
-                quote!(span =>)
-            }
-        };
-        quote! {span =>
-            fn default() -> Self {
-                #body
-            }
-        }
-    })
+        },
+        constrain_to_trait,
+        tt::TopSubtree::empty(tt::DelimSpan::from_single(span)),
+    ))
 }
 
 fn debug_expand(
@@ -642,7 +678,7 @@ fn debug_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = &dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::fmt::Debug }, |adt| {
+    expand_simple_derive(db, span, tt, quote! {span => #krate::fmt::Debug }, false, |adt| {
         let for_variant = |name: String, v: &VariantShape| match v {
             VariantShape::Struct(fields) => {
                 let for_fields = fields.iter().map(|it| {
@@ -697,10 +733,7 @@ fn debug_expand(
                     }
                 })
                 .collect(),
-            AdtShape::Union => {
-                // FIXME: Return expand error here
-                vec![]
-            }
+            AdtShape::Union => unreachable!(),
         };
         quote! {span =>
             fn fmt(&self, f: &mut #krate::fmt::Formatter) -> #krate::fmt::Result {
@@ -718,11 +751,7 @@ fn hash_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = &dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::hash::Hash }, |adt| {
-        if matches!(adt.shape, AdtShape::Union) {
-            // FIXME: Return expand error here
-            return quote! {span =>};
-        }
+    expand_simple_derive(db, span, tt, quote! {span => #krate::hash::Hash }, false, |adt| {
         if matches!(&adt.shape, AdtShape::Enum { variants, .. } if variants.is_empty()) {
             let star = tt::Punct { char: '*', spacing: ::tt::Spacing::Alone, span };
             return quote! {span =>
@@ -769,7 +798,14 @@ fn eq_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Eq }, |_| quote! {span =>})
+    expand_simple_derive(
+        db,
+        span,
+        tt,
+        quote! {span => #krate::cmp::Eq },
+        true,
+        |_| quote! {span =>},
+    )
 }
 
 fn partial_eq_expand(
@@ -778,11 +814,7 @@ fn partial_eq_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialEq }, |adt| {
-        if matches!(adt.shape, AdtShape::Union) {
-            // FIXME: Return expand error here
-            return quote! {span =>};
-        }
+    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialEq }, false, |adt| {
         let name = &adt.name;
 
         let (self_patterns, other_patterns) = self_and_other_patterns(adt, name, span);
@@ -854,7 +886,7 @@ fn ord_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = &dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Ord }, |adt| {
+    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Ord }, false, |adt| {
         fn compare(
             krate: &tt::Ident,
             left: tt::TopSubtree,
@@ -873,10 +905,6 @@ fn ord_expand(
                 }
             }
         }
-        if matches!(adt.shape, AdtShape::Union) {
-            // FIXME: Return expand error here
-            return quote!(span =>);
-        }
         let (self_patterns, other_patterns) = self_and_other_patterns(adt, &adt.name, span);
         let arms = izip!(self_patterns, other_patterns, adt.shape.field_names(span)).map(
             |(pat1, pat2, fields)| {
@@ -916,7 +944,7 @@ fn partial_ord_expand(
     tt: &tt::TopSubtree,
 ) -> ExpandResult<tt::TopSubtree> {
     let krate = &dollar_crate(span);
-    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialOrd }, |adt| {
+    expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialOrd }, false, |adt| {
         fn compare(
             krate: &tt::Ident,
             left: tt::TopSubtree,
@@ -935,10 +963,6 @@ fn partial_ord_expand(
                 }
             }
         }
-        if matches!(adt.shape, AdtShape::Union) {
-            // FIXME: Return expand error here
-            return quote!(span =>);
-        }
         let left = quote!(span =>#krate::intrinsics::discriminant_value(self));
         let right = quote!(span =>#krate::intrinsics::discriminant_value(other));
 
diff --git a/crates/ide-completion/src/tests/attribute.rs b/crates/ide-completion/src/tests/attribute.rs
@@ -878,6 +878,7 @@ mod derive {
             expect![[r#"
                 de Clone              macro Clone
                 de Clone, Copy
+                de Debug              macro Debug
                 de Default          macro Default
                 de PartialEq      macro PartialEq
                 de PartialEq, Eq
@@ -900,6 +901,7 @@ mod derive {
             expect![[r#"
                 de Clone     macro Clone
                 de Clone, Copy
+                de Debug     macro Debug
                 de Default macro Default
                 de Eq
                 de Eq, PartialOrd, Ord
@@ -921,6 +923,7 @@ mod derive {
             expect![[r#"
                 de Clone     macro Clone
                 de Clone, Copy
+                de Debug     macro Debug
                 de Default macro Default
                 de Eq
                 de Eq, PartialOrd, Ord
@@ -942,6 +945,7 @@ mod derive {
             expect![[r#"
                 de Clone     macro Clone
                 de Clone, Copy
+                de Debug     macro Debug
                 de Default macro Default
                 de PartialOrd
                 de PartialOrd, Ord
diff --git a/crates/test-utils/src/fixture.rs b/crates/test-utils/src/fixture.rs
@@ -435,14 +435,16 @@ impl MiniCore {
                 continue;
             }
 
-            let mut active_line_region = false;
-            let mut inactive_line_region = false;
+            let mut active_line_region = 0;
+            let mut inactive_line_region = 0;
             if let Some(idx) = trimmed.find("// :!") {
-                inactive_line_region = true;
-                inactive_regions.push(&trimmed[idx + "// :!".len()..]);
+                let regions = trimmed[idx + "// :!".len()..].split(", ");
+                inactive_line_region += regions.clone().count();
+                inactive_regions.extend(regions);
             } else if let Some(idx) = trimmed.find("// :") {
-                active_line_region = true;
-                active_regions.push(&trimmed[idx + "// :".len()..]);
+                let regions = trimmed[idx + "// :".len()..].split(", ");
+                active_line_region += regions.clone().count();
+                active_regions.extend(regions);
             }
 
             let mut keep = true;
@@ -462,11 +464,11 @@ impl MiniCore {
             if keep {
                 buf.push_str(line);
             }
-            if active_line_region {
-                active_regions.pop().unwrap();
+            if active_line_region > 0 {
+                active_regions.drain(active_regions.len() - active_line_region..);
             }
-            if inactive_line_region {
-                inactive_regions.pop().unwrap();
+            if inactive_line_region > 0 {
+                inactive_regions.drain(inactive_regions.len() - active_line_region..);
             }
         }
 
diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs
