autoformatting

also fix duplicate trait bounds

macros/src/lib.rs

@@ -1,10 +1,10 @@
+use std::collections::HashMap;
 use proc_macro::TokenStream;
 use quote::{quote, ToTokens};
 use syn::{parse_macro_input, Data, DeriveInput, Fields};
 
-// TODO: wrap functions in a trait
+// TODO: wrap functions in a trait? would probably use the other crate
 
-// TODO: doc comments
 
 #[proc_macro_derive(Table)]
 pub fn derive_table(input: TokenStream) -> TokenStream {
@@ -29,8 +29,8 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     let mut field_getters = Vec::new();
     let mut field_accessors = Vec::new();
 
-    let mut to_sql_trait_bounds = Vec::new();
-    let mut from_sql_trait_bounds = Vec::new();
+    let mut to_sql_trait_bounds = HashMap::new();
+    let mut from_sql_trait_bounds = HashMap::new();
 
     for field in fields.named.iter() {
         let field_name = field.ident.as_ref().unwrap();
@@ -40,8 +40,8 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
         field_getters.push(quote!(#field_name: row.get(stringify!(#field_name))?));
         field_accessors.push(quote!(self.#field_name));
 
-        to_sql_trait_bounds.push(quote!(#field_type: rusqlite::types::ToSql));
-        from_sql_trait_bounds.push(quote!(#field_type: rusqlite::types::FromSql));
+        to_sql_trait_bounds.insert(stringify!(#field_type), quote!(#field_type: rusqlite::types::ToSql));
+        from_sql_trait_bounds.insert(stringify!(#field_type), quote!(#field_type: rusqlite::types::FromSql));
 
         if field_name == "id" {
             if let syn::Type::Path(type_path) = field_type {
@@ -59,6 +59,9 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
         }
     }
 
+    let to_sql_trait_bounds = to_sql_trait_bounds.values().collect::<Vec<_>>();
+    let from_sql_trait_bounds = from_sql_trait_bounds.values().collect::<Vec<_>>();
+
     if !field_names.iter().map(|id| id.to_string()).any(|id| &id == "id") {
         panic!("Structs annotated with `Table` require a primary key field `id: Option<i64>`.");
     }
@@ -71,6 +74,8 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     );
 
     let create_table_fn = quote! {
+        /// Use a `Connection` to create a table named after the struct (`#struct_name`)
+        /// If the table already exists, this returns `Ok(())` and does nothing.
         pub fn create_table(conn: &rusqlite::Connection) -> rusqlite::Result<()>
             where #(#to_sql_trait_bounds),*
         {
@@ -88,10 +93,13 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     );
 
     let insert_fn = quote! {
+        /// Insert struct instance into the table, setting `self.id` to
+        /// `Some(last_insert_rowid())` if it was `None`.
         pub fn insert(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<i64>
             where #(#to_sql_trait_bounds),*
         {
             conn.execute(#insert_sql, rusqlite::params![#(#field_accessors),*])?;
+            // TODO: test this with manually set id. also test that this can't update!!!
             let id = conn.last_insert_rowid();
             self.id = Some(id);
             Ok(id)
@@ -99,8 +107,10 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     };
 
 
-    let upsert_fn = quote! {
-        pub fn upsert(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<i64>
+    let update_or_insert_fn = quote! {
+        /// Update a table row using the calling struct instance.
+        /// If the row does not yet exist, it is inserted into the table.
+        pub fn update_or_insert(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<i64>
             where #(#to_sql_trait_bounds),*
         {
             match self.id {
@@ -124,10 +134,17 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     );
 
     let update_fn = quote! {
+        /// Update a table row using the calling struct instance.
+        ///
+        /// If the row does not yet exist, this fails.
+        /// A version that inserts a new row instead also exists. See `update_or_insert`.
+        ///
+        /// Result contains `true` if a row was updated.
         pub fn update(&self, conn: &rusqlite::Connection) -> rusqlite::Result<bool>
             where #(#to_sql_trait_bounds),*
         {
             if self.id.is_none() {
+                // TODO: bad design, should probably fail instead
                 return Ok(false);
             }
             let updated_count = conn.execute(#update_sql, rusqlite::params![#(#field_accessors),*])?;
@@ -137,23 +154,45 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
 
 
     let sync_fn = quote! {
+        /// Sync a struct instance with the database state.
+        /// This "updates" the structs fields using its database entry.
+        ///
+        /// Result contains `false` if `self.id == None` or if no row with that `id` was found.
+        ///
+        /// To update database entry using the structs fields, see `update`.
         pub fn sync(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<bool>
             where #(#from_sql_trait_bounds),*
         {
             if self.id.is_none() {
                 return Ok(false);
             }
-            match #struct_name::get_by_id(conn, self.id.unwrap())? {
-                Some(person) => *self = person,
-                _ => return Ok(false),
-            };
-            Ok(true)
+            match #struct_name::get_by_id(conn, self.id.unwrap()) {
+                Err(rusqlite::Error::QueryReturnedNoRows) => Ok(false),
+                Ok(person) => {
+                    *self = person;
+                    Ok(true)
+                },
+                Err(e) => Err(e),
+            }
+        }
+    };
+
+
+    let from_sql_row_fn = quote! {
+        /// Convert a `rusqlite::Row` received through a query to an instance of the struct
+        pub fn from_sql_row(row: &rusqlite::Row) -> rusqlite::Result<Self>
+        where
+            Self: Sized,
+            #(#from_sql_trait_bounds),*
+        {
+            Ok(Self { #(#field_getters),* })
         }
     };
 
 
     let get_by_id_fn = quote! {
-        pub fn get_by_id(conn: &rusqlite::Connection, id: i64) -> rusqlite::Result<Option<Self>>
+        /// Get a person from the table using their `id` (corresponding to the sqlite rowid)
+        pub fn get_by_id(conn: &rusqlite::Connection, id: i64) -> rusqlite::Result<Self>
         where
             Self: Sized,
             #(#from_sql_trait_bounds),*
@@ -162,18 +201,20 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
             let mut rows = stmt.query(rusqlite::params![id])?;
 
             if let Some(row) = rows.next()? {
-                Ok(Some(Self { #(#field_getters),* }))
+                Self::from_sql_row(row)
             } else {
-                Ok(None)
+                Err(rusqlite::Error::QueryReturnedNoRows)
             }
         }
     };
 
 
     let delete_fn = quote! {
-        pub fn delete(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<bool>
-            where #(#to_sql_trait_bounds),*
-        {
+        /// Delete row corresponding to the struct instance from the database.
+        /// Deletes the entry with rowid equal to `self.id` without further checks.
+        ///
+        /// Result contains `true` if a row was deleted.
+        pub fn delete(&mut self, conn: &rusqlite::Connection) -> rusqlite::Result<bool> {
             if self.id.is_none() {
                 return Ok(false);
             }
@@ -187,6 +228,9 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
     };
 
     let delete_by_id_fn = quote! {
+        /// Delete a row from the database by rowid.
+        ///
+        /// Result contains `true` if a row was deleted.
         pub fn delete_by_id(conn: &rusqlite::Connection, id: i64) -> rusqlite::Result<()> {
             conn.execute(&format!(
                     "DELETE FROM {} WHERE id = ?",
@@ -198,6 +242,7 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
 
 
     let drop_table_fn = quote! {
+        /// Use a `Connection` to drop the table named after the struct (`#struct_name`)
         pub fn drop_table(conn: &rusqlite::Connection) -> rusqlite::Result<()> {
             conn.execute(&format!("DROP TABLE {}", #table_name), [])?;
             Ok(())
@@ -209,9 +254,10 @@ pub fn derive_table(input: TokenStream) -> TokenStream {
         impl #struct_name {
             #create_table_fn
             #insert_fn
-            #upsert_fn
+            #update_or_insert_fn
             #update_fn
             #sync_fn
+            #from_sql_row_fn
             #get_by_id_fn
             #delete_fn
             #delete_by_id_fn

src/lib.rs

@@ -1,2 +1,4 @@
 pub use squail_macros::*;
 mod test;
+
+pub trait SquailTable {}

src/test/macros.rs

@@ -73,7 +73,7 @@ fn test_table_derive_macro() {
     let larry_copy = Person::get_by_id(&conn, larry_id).expect("Querying a row should work");
     assert_eq!(
         larry_copy,
-        Some(larry.clone()),
+        larry.clone(),
         "Retrieving inserted row should give an identical row"
     );
 
@@ -81,14 +81,17 @@ fn test_table_derive_macro() {
     // also works: `Person::delete_by_id(&conn, larry_id).unwrap();`
     assert!(deleted_something, "Should have deleted something");
 
-    let deleted_larry = Person::get_by_id(&conn, larry_id)
-        .expect("Querying a deleted row should return Ok(None), not Err(_)");
+    let err = Person::get_by_id(&conn, larry_id)
+        .expect_err("Querying a deleted row should Err(QueryReturnedNoRows)");
     assert_eq!(
-        deleted_larry, None,
+        err,
+        rusqlite::Error::QueryReturnedNoRows,
         "Received row that should have been deleted"
     );
 
-    let id = larry.upsert(&conn).expect("Upsertion (insert) should work");
+    let id = larry
+        .update_or_insert(&conn)
+        .expect("Upsertion (insert) should work");
     let larry_id = larry
         .id
         .expect("After (mutable) upsertion, id should not be None");
@@ -97,12 +100,14 @@ fn test_table_derive_macro() {
     assert_eq!(id, larry_id, "Upsert should return correct id");
     assert_eq!(
         larry_copy,
-        Some(larry.clone()),
+        larry.clone(),
         "Retrieving upserted row should give an identical row"
     );
 
     larry.age += 1;
-    let id = larry.upsert(&conn).expect("Upsertion (update) should work");
+    let id = larry
+        .update_or_insert(&conn)
+        .expect("Upsertion (update) should work");
     let larry_id = larry
         .id
         .expect("After (mutable) upsertion, id should not be None");
@@ -111,7 +116,7 @@ fn test_table_derive_macro() {
     let larry_copy = Person::get_by_id(&conn, larry_id).expect("Querying a row should work");
     assert_eq!(
         larry_copy,
-        Some(larry.clone()),
+        larry.clone(),
         "Retrieving upserted row should give an identical row"
     );
 
@@ -138,6 +143,16 @@ fn test_table_derive_macro() {
         )
         .expect("Explicit Sqlite statement (not a library test) failed");
     assert!(!exists, "Deleted table should not exist anymore but does");
+
+    /// Another example struct to assure this works more than once
+    #[derive(Table, Default)]
+    #[allow(unused)]
+    struct Car {
+        id: Option<i64>,
+        name: String,
+        brand: String,
+        year: i32,
+    }
 }
 
 // TODO: implement compile-error test(s) -- perhaps with `trybuild`?