Merge pull request #4030 from z33ky/raw-sqlidentifier

Allow raw identifiers for SqlIdentifier (column-name)
diesel-rs · May 24, 2024 · 3db4ed2 · 3db4ed2
2 parents ee44189 + 5a044a6
commit 3db4ed2
Show file tree

Hide file tree

Showing 16 changed files with 756 additions and 146 deletions.
diff --git a/.typos.toml b/.typos.toml
@@ -20,7 +20,9 @@ extend-ignore-re = [
     "cannot find value `titel` in module `posts`",
     "cannot find type `titel` in module `posts`",
     "[0-9]+[[:space]]+|[[:space:]]+titel: String",
-    "big_sur"
+    "big_sur",
+    # That's Spanish for "type" (used in a unit-test)
+    "tipe",
 ]
 
 [type.md]

diff --git a/diesel_compile_tests/tests/fail/derive/bad_column_name.rs b/diesel_compile_tests/tests/fail/derive/bad_column_name.rs
@@ -5,8 +5,8 @@ table! {
     users {
         id -> Integer,
         name -> Text,
-        #[sql_name = "type"]
-        tpe -> Text,
+        #[sql_name = "spa ce"]
+        space -> Text,
     }
 }
 
@@ -35,16 +35,16 @@ struct User3 {
 #[derive(Insertable)]
 #[diesel(table_name = users)]
 struct User4 {
-    #[diesel(column_name = "type")]
-    ty: String,
+    #[diesel(column_name = "spa ce")]
+    space: String,
 }
 
 
 #[derive(AsChangeset)]
 #[diesel(table_name = users)]
 struct User5 {
-    #[diesel(column_name = "type")]
-    ty: String,
+    #[diesel(column_name = "spa ce")]
+    space: String,
 }
 
 fn main() {}
diff --git a/diesel_compile_tests/tests/fail/derive/bad_column_name.stderr b/diesel_compile_tests/tests/fail/derive/bad_column_name.stderr
@@ -17,14 +17,14 @@ error: expected string literal
 30 |     #[diesel(column_name = true)]
    |                            ^^^^
 
-error: Expected valid identifier, found `type`. Diesel automatically renames invalid identifiers, perhaps you meant to write `type_`?
+error: Expected valid identifier, found `spa ce`. Diesel does not support column names with whitespaces yet
   --> tests/fail/derive/bad_column_name.rs:38:28
    |
-38 |     #[diesel(column_name = "type")]
-   |                            ^^^^^^
+38 |     #[diesel(column_name = "spa ce")]
+   |                            ^^^^^^^^
 
-error: Expected valid identifier, found `type`. Diesel automatically renames invalid identifiers, perhaps you meant to write `type_`?
+error: Expected valid identifier, found `spa ce`. Diesel does not support column names with whitespaces yet
   --> tests/fail/derive/bad_column_name.rs:46:28
    |
-46 |     #[diesel(column_name = "type")]
-   |                            ^^^^^^
+46 |     #[diesel(column_name = "spa ce")]
+   |                            ^^^^^^^^
diff --git a/diesel_derives/src/as_changeset.rs b/diesel_derives/src/as_changeset.rs
@@ -157,8 +157,7 @@ fn field_changeset_ty(
     lifetime: Option<TokenStream>,
     treat_none_as_null: bool,
 ) -> Result<TokenStream> {
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     if !treat_none_as_null && is_option_ty(&field.ty) {
         let field_ty = inner_of_option_ty(&field.ty);
         Ok(
@@ -177,8 +176,7 @@ fn field_changeset_expr(
     treat_none_as_null: bool,
 ) -> Result<TokenStream> {
     let field_name = &field.name;
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     if !treat_none_as_null && is_option_ty(&field.ty) {
         if lifetime.is_some() {
             Ok(quote!(self.#field_name.as_ref().map(|x| #table_name::#column_name.eq(x))))
@@ -196,8 +194,7 @@ fn field_changeset_ty_serialize_as(
     ty: &Type,
     treat_none_as_null: bool,
 ) -> Result<TokenStream> {
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     if !treat_none_as_null && is_option_ty(&field.ty) {
         let inner_ty = inner_of_option_ty(ty);
         Ok(quote!(std::option::Option<diesel::dsl::Eq<#table_name::#column_name, #inner_ty>>))
@@ -213,8 +210,7 @@ fn field_changeset_expr_serialize_as(
     treat_none_as_null: bool,
 ) -> Result<TokenStream> {
     let field_name = &field.name;
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     let column: Expr = parse_quote!(#table_name::#column_name);
     if !treat_none_as_null && is_option_ty(&field.ty) {
         Ok(quote!(self.#field_name.map(|x| #column.eq(::std::convert::Into::<#ty>::into(x)))))

diff --git a/diesel_derives/src/attrs.rs b/diesel_derives/src/attrs.rs
@@ -56,32 +56,50 @@ impl SqlIdentifier {
         self.span
     }
 
-    pub fn valid_ident(&self) -> Result<()> {
-        if syn::parse_str::<Ident>(&self.field_name).is_err() {
-            Err(syn::Error::new(
+    pub fn to_ident(&self) -> Result<Ident> {
+        match syn::parse_str::<Ident>(&format!("r#{}", self.field_name)) {
+            Ok(mut ident) => {
+                ident.set_span(self.span);
+                Ok(ident)
+            }
+            Err(_e) if self.field_name.contains(' ') => Err(syn::Error::new(
+                self.span(),
+                format!(
+                    "Expected valid identifier, found `{0}`. \
+                 Diesel does not support column names with whitespaces yet",
+                    self.field_name
+                ),
+            )),
+            Err(_e) => Err(syn::Error::new(
                 self.span(),
                 format!(
                     "Expected valid identifier, found `{0}`. \
                  Diesel automatically renames invalid identifiers, \
                  perhaps you meant to write `{0}_`?",
                     self.field_name
                 ),
-            ))
-        } else {
-            Ok(())
+            )),
         }
     }
 }
 
 impl ToTokens for SqlIdentifier {
     fn to_tokens(&self, tokens: &mut TokenStream) {
-        Ident::new(&self.field_name, self.span).to_tokens(tokens)
+        if self.field_name.starts_with("r#") {
+            Ident::new_raw(&self.field_name[2..], self.span).to_tokens(tokens)
+        } else {
+            Ident::new(&self.field_name, self.span).to_tokens(tokens)
+        }
     }
 }
 
 impl Display for SqlIdentifier {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        f.write_str(&self.field_name)
+        let mut start = 0;
+        if self.field_name.starts_with("r#") {
+            start = 2;
+        }
+        f.write_str(&self.field_name[start..])
     }
 }
 
@@ -93,6 +111,8 @@ impl PartialEq<Ident> for SqlIdentifier {
 
 impl From<&'_ Ident> for SqlIdentifier {
     fn from(ident: &'_ Ident) -> Self {
+        use syn::ext::IdentExt;
+        let ident = ident.unraw();
         Self {
             span: ident.span(),
             field_name: ident.to_string(),

diff --git a/diesel_derives/src/insertable.rs b/diesel_derives/src/insertable.rs
@@ -194,8 +194,7 @@ fn field_ty_serialize_as(
     ty: &Type,
     treat_none_as_default_value: bool,
 ) -> Result<TokenStream> {
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     let span = field.span;
     if treat_none_as_default_value {
         let inner_ty = inner_of_option_ty(ty);
@@ -223,8 +222,7 @@ fn field_expr_serialize_as(
     treat_none_as_default_value: bool,
 ) -> Result<TokenStream> {
     let field_name = &field.name;
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     let column = quote!(#table_name::#column_name);
     if treat_none_as_default_value {
         if is_option_ty(ty) {
@@ -245,8 +243,7 @@ fn field_ty(
     lifetime: Option<TokenStream>,
     treat_none_as_default_value: bool,
 ) -> Result<TokenStream> {
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
     let span = field.span;
     if treat_none_as_default_value {
         let inner_ty = inner_of_option_ty(&field.ty);
@@ -276,8 +273,7 @@ fn field_expr(
     treat_none_as_default_value: bool,
 ) -> Result<TokenStream> {
     let field_name = &field.name;
-    let column_name = field.column_name()?;
-    column_name.valid_ident()?;
+    let column_name = field.column_name()?.to_ident()?;
 
     let column: Expr = parse_quote!(#table_name::#column_name);
     if treat_none_as_default_value {

diff --git a/diesel_derives/src/queryable_by_name.rs b/diesel_derives/src/queryable_by_name.rs
@@ -120,7 +120,7 @@ fn sql_type(field: &Field, model: &Model) -> Result<Type> {
     match field.sql_type {
         Some(AttributeSpanWrapper { item: ref st, .. }) => Ok(st.clone()),
         None => {
-            let column_name = field.column_name()?;
+            let column_name = field.column_name()?.to_ident()?;
             Ok(parse_quote!(diesel::dsl::SqlTypeOf<#table_name::#column_name>))
         }
     }

diff --git a/diesel_derives/src/selectable.rs b/diesel_derives/src/selectable.rs
@@ -151,7 +151,7 @@ fn field_column_ty(
         Ok(quote!(<#embed_ty as Selectable<__DB>>::SelectExpression))
     } else {
         let table_name = &model.table_names()[0];
-        let column_name = field.column_name()?;
+        let column_name = field.column_name()?.to_ident()?;
         Ok(quote!(#table_name::#column_name))
     }
 }
@@ -165,7 +165,7 @@ fn field_column_inst(field: &Field, model: &Model) -> Result<TokenStream> {
         Ok(quote!(<#embed_ty as Selectable<__DB>>::construct_selection()))
     } else {
         let table_name = &model.table_names()[0];
-        let column_name = field.column_name()?;
+        let column_name = field.column_name()?.to_ident()?;
         Ok(quote!(#table_name::#column_name))
     }
 }