Forbid #[diesel(treat_none_as_{null, default_value}] on non-Option fi…

…elds
diesel-rs · Aug 3, 2023 · bb707bf · bb707bf
1 parent ef4b5e8
commit bb707bf
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 9 deletions.
diff --git a/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_default_value.rs b/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_default_value.rs
@@ -32,4 +32,12 @@ struct UserForm3 {
     name: String,
 }
 
+#[derive(Insertable)]
+#[diesel(table_name = users)]
+struct UserForm4 {
+    id: i32,
+    #[diesel(treat_none_as_default_value = false)]
+    name: String,
+}
+
 fn main() {}
diff --git a/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_default_value.stderr b/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_default_value.stderr
@@ -16,3 +16,9 @@ error: expected boolean literal
    |
 29 | #[diesel(treat_none_as_default_value = "foo")]
    |                                        ^^^^^
+
+error: expected `treat_none_as_default_value` field to be of type `Option<_>`
+  --> tests/fail/derive/bad_treat_none_as_default_value.rs:40:11
+   |
+40 |     name: String,
+   |           ^^^^^^
diff --git a/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_null.rs b/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_null.rs
@@ -32,4 +32,12 @@ struct UserForm3 {
     name: String,
 }
 
+#[derive(AsChangeset)]
+#[diesel(table_name = users)]
+struct UserForm4 {
+    id: i32,
+    #[diesel(treat_none_as_null = true)]
+    name: String,
+}
+
 fn main() {}
diff --git a/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_null.stderr b/diesel_compile_tests/tests/fail/derive/bad_treat_none_as_null.stderr
@@ -16,3 +16,9 @@ error: expected boolean literal
    |
 29 | #[diesel(treat_none_as_null = "foo")]
    |                               ^^^^^
+
+error: expected `treat_none_as_null` field to be of type `Option<_>`
+  --> tests/fail/derive/bad_treat_none_as_null.rs:40:11
+   |
+40 |     name: String,
+   |           ^^^^^^
diff --git a/diesel_derives/src/as_changeset.rs b/diesel_derives/src/as_changeset.rs
@@ -1,5 +1,6 @@
 use proc_macro2::TokenStream;
 use quote::quote;
+use syn::spanned::Spanned as _;
 use syn::{parse_quote, DeriveInput, Expr, Path, Result, Type};
 
 use crate::attrs::AttributeSpanWrapper;
@@ -47,7 +48,16 @@ pub fn derive(item: DeriveInput) -> Result<TokenStream> {
     for field in fields_for_update {
         // Use field-level attr. with fallback to the struct-level one.
         let treat_none_as_null = match &field.treat_none_as_null {
-            Some(attr) => attr.item,
+            Some(attr) => {
+                if !is_option_ty(&field.ty) {
+                    return Err(syn::Error::new(
+                        field.ty.span(),
+                        "expected `treat_none_as_null` field to be of type `Option<_>`",
+                    ));
+                }
+
+                attr.item
+            }
             None => treat_none_as_null,
         };
 

diff --git a/diesel_derives/src/insertable.rs b/diesel_derives/src/insertable.rs
@@ -6,6 +6,7 @@ use proc_macro2::TokenStream;
 use quote::quote;
 use quote::quote_spanned;
 use syn::parse_quote;
+use syn::spanned::Spanned as _;
 use syn::{DeriveInput, Expr, Path, Result, Type};
 
 pub fn derive(item: DeriveInput) -> Result<TokenStream> {
@@ -46,19 +47,28 @@ fn derive_into_single_table(
     for field in model.fields() {
         // Use field-level attr. with fallback to the struct-level one.
         let treat_none_as_default_value = match &field.treat_none_as_default_value {
-            Some(attr) => attr.item,
-            None => treat_none_as_default_value,
-        };
-
-        match (field.serialize_as.as_ref(), field.embed()) {
-            (None, true) => {
-                if field.treat_none_as_default_value.is_some() {
+            Some(attr) => {
+                if let Some(embed) = &field.embed {
                     return Err(syn::Error::new(
-                        field.embed.as_ref().unwrap().attribute_span,
+                        embed.attribute_span,
                         "`embed` and `treat_none_as_default_value` are mutually exclusive",
                     ));
                 }
 
+                if !is_option_ty(&field.ty) {
+                    return Err(syn::Error::new(
+                        field.ty.span(),
+                        "expected `treat_none_as_default_value` field to be of type `Option<_>`",
+                    ));
+                }
+
+                attr.item
+            }
+            None => treat_none_as_default_value,
+        };
+
+        match (field.serialize_as.as_ref(), field.embed()) {
+            (None, true) => {
                 direct_field_ty.push(field_ty_embed(field, None));
                 direct_field_assign.push(field_expr_embed(field, None));
                 ref_field_ty.push(field_ty_embed(field, Some(quote!(&'insert))));