Merge pull request diesel-rs#1110 from Eijebong/auto_renaming

Eijebong · web-flow · commit 07c6cefb86ae · 2017-08-24T15:39:50.000+02:00
Automatically rename columns which name would collide with a keyword
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,7 +29,7 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 * `infer_schema!` will now automatically detect which tables can be joined based
   on the presence of foreign key constraints.
-  
+
 * Added support for `Add` and `Sub` to timestamp types.
 
 * Added a way to rename columns in the table macro with `#[sql_name="the_column_name"]`
@@ -39,6 +39,11 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
   Diesel's CLI tool, pass the new `--with-docs` parameter:
   `diesel print-schema --with-docs`.
 
+* infer_schema! now automatically renames columns that conflict with
+  a Rust keyword by placing a _ at the end of the name. For example,
+  a column called type will be referenced as type_ in Rust.
+
+
 ### Changed
 
 * The deprecated `debug_sql!` and `print_sql!` functions will now generate
diff --git a/diesel/src/macros/macros_from_codegen.rs b/diesel/src/macros/macros_from_codegen.rs
@@ -9,8 +9,12 @@
 /// limitations of the Macros 1.1 system, but you can pass a string in the form
 /// `"env:SOME_ENV_VAR"` or `"dotenv:SOME_ENV_VAR"` to achieve the same effect.
 ///
-/// This macro can only be used in combination with the `diesel_codegen` or
-/// `diesel_codegen_syntex` crates. It will not work on its own.
+/// This macro can only be used in combination with the `diesel_codegen` crate.
+/// It will not work on its own.
+///
+/// If any column name would collide with a rust keyword, a `_` will
+/// automatically be placed at the end of the name. For example, a column called
+/// `type` will be referenced as `type_` in the generated module.
 macro_rules! infer_schema {
     ($database_url: expr) => {
         mod __diesel_infer_schema {
diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs
@@ -155,7 +155,12 @@ impl<'a> Display for ColumnDefinitions<'a> {
                         writeln!(out, "///{}{}", if d.is_empty() { "" } else { " " }, d)?;
                     }
                 }
-                writeln!(out, "{} -> {},", column.name, column.ty)?;
+                if let Some(ref rust_name) = column.rust_name {
+                    writeln!(out, r#"#[sql_name = {}]"#, column.sql_name)?;
+                    writeln!(out, "{} -> {},", rust_name, column.ty)?;
+                } else {
+                    writeln!(out, "{} -> {},", column.sql_name, column.ty)?;
+                }
             }
         }
         writeln!(f, "}}")?;
diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs
@@ -46,6 +46,11 @@ fn print_schema_with_foreign_keys() {
     test_print_schema("print_schema_with_foreign_keys", vec!["--with-docs"]);
 }
 
+#[test]
+fn print_schema_column_renaming() {
+    test_print_schema("print_schema_column_renaming", vec!["--with-docs"]);
+}
+
 #[cfg(feature = "sqlite")]
 const BACKEND: &str = "sqlite";
 #[cfg(feature = "postgres")]
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/mysql/expected.rs b/diesel_cli/tests/print_schema/print_schema_column_renaming/mysql/expected.rs
@@ -0,0 +1,28 @@
+table! {
+    /// Representation of the `with_keywords` table.
+    ///
+    /// (Automatically generated by Diesel.)
+    with_keywords (fn_) {
+        /// The `fn` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = fn]
+        fn_ -> Integer,
+        /// The `let` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = let]
+        let_ -> Integer,
+        /// The `extern` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = extern]
+        extern_ -> Integer,
+    }
+}
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/mysql/schema.sql b/diesel_cli/tests/print_schema/print_schema_column_renaming/mysql/schema.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER NOT NULL PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_column_renaming/postgres/expected.rs
@@ -0,0 +1,28 @@
+table! {
+    /// Representation of the `with_keywords` table.
+    ///
+    /// (Automatically generated by Diesel.)
+    with_keywords (fn_) {
+        /// The `fn` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Int4`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = fn]
+        fn_ -> Int4,
+        /// The `let` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Int4`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = let]
+        let_ -> Int4,
+        /// The `extern` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Int4`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = extern]
+        extern_ -> Int4,
+    }
+}
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/postgres/schema.sql b/diesel_cli/tests/print_schema/print_schema_column_renaming/postgres/schema.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/sqlite/expected.rs b/diesel_cli/tests/print_schema/print_schema_column_renaming/sqlite/expected.rs
@@ -0,0 +1,28 @@
+table! {
+    /// Representation of the `with_keywords` table.
+    ///
+    /// (Automatically generated by Diesel.)
+    with_keywords (fn_) {
+        /// The `fn` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = fn]
+        fn_ -> Integer,
+        /// The `let` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = let]
+        let_ -> Integer,
+        /// The `extern` column of the `with_keywords` table.
+        ///
+        /// Its SQL type is `Integer`.
+        ///
+        /// (Automatically generated by Diesel.)
+        #[sql_name = extern]
+        extern_ -> Integer,
+    }
+}
diff --git a/diesel_cli/tests/print_schema/print_schema_column_renaming/sqlite/schema.sql b/diesel_cli/tests/print_schema/print_schema_column_renaming/sqlite/schema.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER NOT NULL PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);
diff --git a/diesel_codegen/src/schema_inference.rs b/diesel_codegen/src/schema_inference.rs
@@ -107,11 +107,23 @@ fn table_name_to_tokens(table_name: TableName) -> quote::Tokens {
 
 fn column_data_to_tokens(column_data: ColumnDefinition) -> quote::Tokens {
     let docs = to_doc_comment_tokens(&column_data.docs);
-    let name = syn::Ident::new(column_data.name);
     let ty = column_ty_to_tokens(column_data.ty);
-    quote! {
-        #(#docs)*
-        #name -> #ty
+    if let Some(rust_name) = column_data.rust_name {
+        let rust_name = syn::Ident::new(rust_name);
+        let sql_name = column_data.sql_name;
+
+        quote!(
+            #(#docs)*
+            #[sql_name = #sql_name]
+            #rust_name -> #ty
+        )
+    } else {
+        let name = syn::Ident::new(column_data.sql_name);
+
+        quote!(
+            #(#docs)*
+            #name -> #ty
+        )
     }
 }
 
diff --git a/diesel_infer_schema/src/data_structures.rs b/diesel_infer_schema/src/data_structures.rs
@@ -46,9 +46,10 @@ impl fmt::Display for ColumnType {
 
 #[derive(Debug)]
 pub struct ColumnDefinition {
-    pub name: String,
+    pub sql_name: String,
     pub ty: ColumnType,
     pub docs: String,
+    pub rust_name: Option<String>,
 }
 
 impl ColumnInformation {
diff --git a/diesel_infer_schema/src/inference.rs b/diesel_infer_schema/src/inference.rs
@@ -6,6 +6,13 @@ use diesel::result::Error::NotFound;
 use table_data::*;
 use data_structures::*;
 
+static RESERVED_NAMES: &[&str] = &["abstract", "alignof", "as", "become", "box", "break",
+"const", "continue", "crate", "do", "else", "enum", "extern", "false", "final", "fn", "for", "if",
+"impl", "in", "let", "loop", "macro", "match", "mod", "move", "mut", "offsetof", "override",
+"priv", "proc", "pub", "pure", "ref", "return", "Self", "self", "sizeof", "static", "struct",
+"super", "trait", "true", "type", "typeof", "unsafe", "unsized", "use", "virtual", "where",
+"while", "yield", ];
+
 pub(crate) enum InferConnection {
     #[cfg(feature = "sqlite")]
     Sqlite(SqliteConnection),
@@ -165,20 +172,34 @@ pub fn load_table_data(database_url: &str, name: TableName)
         (Automatically generated by Diesel.)",
         name);
     let primary_key = get_primary_keys(&connection, &name)?;
+    let primary_key = primary_key.iter().map(|k| {
+        if RESERVED_NAMES.contains(&k.as_str()) {
+            format!("{}_", k)
+        } else {
+            k.clone()
+        }
+    }).collect();
 
     let column_data = get_column_information(&connection, &name)?
         .into_iter()
         .map(|c| {
             let ty = determine_column_type(&c, &connection)?;
+            let rust_name = if RESERVED_NAMES.contains(&c.column_name.as_str()) {
+                Some(format!("{}_", c.column_name))
+            } else {
+                None
+            };
+
             Ok(ColumnDefinition {
                 docs: doc_comment!("The `{}` column of the `{}` table.
 
                     Its SQL type is `{}`.
 
                     (Automatically generated by Diesel.)",
                     c.column_name, name, ty),
-                name: c.column_name,
+                sql_name: c.column_name,
                 ty,
+                rust_name
             })
         })
         .collect::<Result<_, Box<Error>>>()?;
diff --git a/diesel_tests/tests/schema_inference.rs b/diesel_tests/tests/schema_inference.rs
@@ -256,3 +256,27 @@ mod mysql {
         assert_eq!(Ok(vec![blobs]), all_the_blobs::table.load(&conn));
     }
 }
+
+#[test]
+fn columns_named_as_reserved_keywords_are_renamed() {
+    use diesel::*;
+    use schema::*;
+
+    #[derive(Queryable, Insertable, Debug, PartialEq)]
+    #[table_name = "with_keywords"]
+    struct WithKeywords {
+        fn_: i32,
+        let_: i32,
+        extern_: i32,
+    }
+
+    let value = WithKeywords {
+        fn_: 1,
+        let_: 42,
+        extern_: 51,
+    };
+
+    let conn = connection();
+    insert(&value).into(with_keywords::table).execute(&conn).unwrap();
+    assert_eq!(Ok(vec![value]), with_keywords::table.load(&conn));
+}
diff --git a/migrations/mysql/20170816164349_add_keywords_table/down.sql b/migrations/mysql/20170816164349_add_keywords_table/down.sql
@@ -0,0 +1 @@
+DROP TABLE with_keywords;
diff --git a/migrations/mysql/20170816164349_add_keywords_table/up.sql b/migrations/mysql/20170816164349_add_keywords_table/up.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER NOT NULL PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);
diff --git a/migrations/postgresql/20170816164352_add_keywords_table/down.sql b/migrations/postgresql/20170816164352_add_keywords_table/down.sql
@@ -0,0 +1 @@
+DROP TABLE with_keywords;
diff --git a/migrations/postgresql/20170816164352_add_keywords_table/up.sql b/migrations/postgresql/20170816164352_add_keywords_table/up.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);
diff --git a/migrations/sqlite/20170816164356_add_keywords_table/down.sql b/migrations/sqlite/20170816164356_add_keywords_table/down.sql
@@ -0,0 +1 @@
+DROP TABLE with_keywords;
diff --git a/migrations/sqlite/20170816164356_add_keywords_table/up.sql b/migrations/sqlite/20170816164356_add_keywords_table/up.sql
@@ -0,0 +1,5 @@
+CREATE TABLE with_keywords (
+    fn INTEGER NOT NULL PRIMARY KEY,
+    let INTEGER NOT NULL,
+    extern INTEGER NOT NULL
+);

Original file line number	Diff line number	Diff line change
`@@ -155,7 +155,12 @@ impl<'a> Display for ColumnDefinitions<'a> {`
`155`	`155`	`writeln!(out, "///{}{}", if d.is_empty() { "" } else { " " }, d)?;`
`156`	`156`	`}`
`157`	`157`	`}`
`158`		`- writeln!(out, "{} -> {},", column.name, column.ty)?;`
	`158`	`+ if let Some(ref rust_name) = column.rust_name {`
	`159`	`+ writeln!(out, r#"#[sql_name = {}]"#, column.sql_name)?;`
	`160`	`+ writeln!(out, "{} -> {},", rust_name, column.ty)?;`
	`161`	`+ } else {`
	`162`	`+ writeln!(out, "{} -> {},", column.sql_name, column.ty)?;`
	`163`	`+ }`
`159`	`164`	`}`
`160`	`165`	`}`
`161`	`166`	`writeln!(f, "}}")?;`