refactor: code quality (some refactors) (#74)

* refactor(parser): remove some unnecessary clones

* refactor(code): change "generate_for_table" to take in a table reference

because it is only used as a reference anyway

* refactor(parser): change "parse_and_generate_code" to take in a "&str" instead of "String"

* style(code): remove some unnecessary extra calls

* refactor(file): remove clone from "new"

* refactor(lib): remove clone from "autogenerated_columns"

* fix(lib): change "generate_files" to take in "&Path" instead of "PathBuf"

* style(lib): inline variable renames in "generate_files"

* style(lib): remove some unnecessary extra calls

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - replace `structopt` with `clap`
 - add subcommand to generate shell completions
+- function `generate_files` now takes in `&Path`s instead of `PathBuf`s
 
 ## 0.0.16
 

src/bin/main.rs

@@ -145,8 +145,8 @@ fn actual_main() -> dsync::Result<()> {
     }
 
     dsync::generate_files(
-        args.input,
-        args.output,
+        &args.input,
+        &args.output,
         GenerationConfig {
             default_table_options,
             table_options: HashMap::from([]),

src/code.rs

@@ -40,7 +40,7 @@ impl StructType {
     /// Format a struct with all prefix- and suffixes
     ///
     /// Example: `UpdateTodos`
-    pub fn format(&self, name: &'_ str) -> String {
+    pub fn format(&self, name: &str) -> String {
         format!(
             "{struct_prefix}{struct_name}{struct_suffix}",
             struct_prefix = self.prefix(),
@@ -222,7 +222,7 @@ impl<'a> Struct<'a> {
     fn render(&mut self) {
         let ty = self.ty;
         let table = &self.table;
-        let _opts = self.config.table(table.name.to_string().as_str());
+        let _opts = self.config.table(&table.name.to_string());
 
         let primary_keys: Vec<String> = table.primary_key_column_names();
 
@@ -250,7 +250,7 @@ impl<'a> Struct<'a> {
             tsync_attr = self.attr_tsync(),
             derive_attr = self.attr_derive(),
             table_name = table.name,
-            struct_name = ty.format(table.struct_name.as_str()),
+            struct_name = ty.format(&table.struct_name),
             primary_key = if ty != StructType::Read {
                 "".to_string()
             } else {
@@ -547,12 +547,12 @@ fn build_imports(table: &ParsedTableMacro, config: &GenerationConfig) -> String
 }
 
 /// Generate a full file for a given diesel table
-pub fn generate_for_table(table: ParsedTableMacro, config: &GenerationConfig) -> String {
+pub fn generate_for_table(table: &ParsedTableMacro, config: &GenerationConfig) -> String {
     let table_options = config.table(&table.name.to_string());
     // first, we generate struct code
-    let read_struct = Struct::new(StructType::Read, &table, config);
-    let update_struct = Struct::new(StructType::Update, &table, config);
-    let create_struct = Struct::new(StructType::Create, &table, config);
+    let read_struct = Struct::new(StructType::Read, table, config);
+    let update_struct = Struct::new(StructType::Update, table, config);
+    let create_struct = Struct::new(StructType::Create, table, config);
 
     let mut structs = String::new();
     structs.push_str(read_struct.code());
@@ -562,11 +562,11 @@ pub fn generate_for_table(table: ParsedTableMacro, config: &GenerationConfig) ->
     structs.push_str(update_struct.code());
 
     let functions = if table_options.get_fns() {
-        build_table_fns(&table, config, create_struct, update_struct)
+        build_table_fns(table, config, create_struct, update_struct)
     } else {
         "".to_string()
     };
-    let imports = build_imports(&table, config);
+    let imports = build_imports(table, config);
 
     format!("{FILE_SIGNATURE}\n\n{imports}\n\n{structs}\n{functions}")
 }

src/file.rs

@@ -13,14 +13,15 @@ impl MarkedFile {
     ///
     /// If the file does not exist, a empty file is created
     pub fn new(path: PathBuf) -> Result<MarkedFile> {
+        let file_contents = if !path.exists() {
+            std::fs::write(&path, "").attach_path_err(&path)?;
+            "".to_string()
+        } else {
+            std::fs::read_to_string(&path).attach_path_err(&path)?
+        };
         Ok(MarkedFile {
-            path: path.clone(),
-            file_contents: if !path.exists() {
-                std::fs::write(&path, "").attach_path_err(&path)?;
-                "".to_string()
-            } else {
-                std::fs::read_to_string(&path).attach_path_err(&path)?
-            },
+            path,
+            file_contents,
         })
     }
 

src/lib.rs

@@ -9,7 +9,7 @@ use file::MarkedFile;
 use parser::ParsedTableMacro;
 pub use parser::FILE_SIGNATURE;
 use std::collections::HashMap;
-use std::path::PathBuf;
+use std::path::Path;
 
 /// Options for a individual table
 #[derive(Debug, Clone)]
@@ -97,7 +97,7 @@ impl<'a> TableOptions<'a> {
 
     pub fn autogenerated_columns(self, cols: Vec<&'a str>) -> Self {
         Self {
-            autogenerated_columns: Some(cols.clone()),
+            autogenerated_columns: Some(cols),
             ..self
         }
     }
@@ -171,7 +171,7 @@ impl GenerationConfig<'_> {
 ///
 /// Model is returned and not saved to disk yet
 pub fn generate_code(
-    diesel_schema_file_contents: String,
+    diesel_schema_file_contents: &str,
     config: GenerationConfig,
 ) -> Result<Vec<ParsedTableMacro>> {
     parser::parse_and_generate_code(diesel_schema_file_contents, &config)
@@ -181,33 +181,31 @@ pub fn generate_code(
 ///
 /// Models are saved to disk
 pub fn generate_files(
-    input_diesel_schema_file: PathBuf,
-    output_models_dir: PathBuf,
+    input_diesel_schema_file: &Path,
+    output_models_dir: &Path,
     config: GenerationConfig,
 ) -> Result<()> {
-    let input = input_diesel_schema_file;
-    let output_dir = output_models_dir;
-
     let generated = generate_code(
-        std::fs::read_to_string(&input).attach_path_err(&input)?,
+        &std::fs::read_to_string(input_diesel_schema_file)
+            .attach_path_err(input_diesel_schema_file)?,
         config,
     )?;
 
-    if !output_dir.exists() {
-        std::fs::create_dir(&output_dir).attach_path_err(&output_dir)?;
-    } else if !output_dir.is_dir() {
+    if !output_models_dir.exists() {
+        std::fs::create_dir(output_models_dir).attach_path_err(output_models_dir)?;
+    } else if !output_models_dir.is_dir() {
         return Err(Error::not_a_directory(
             "Expected output argument to be a directory or non-existent.",
-            output_dir,
+            output_models_dir,
         ));
     }
 
     // check that the mod.rs file exists
-    let mut mod_rs = MarkedFile::new(output_dir.join("mod.rs"))?;
+    let mut mod_rs = MarkedFile::new(output_models_dir.join("mod.rs"))?;
 
     // pass 1: add code for new tables
     for table in generated.iter() {
-        let table_dir = output_dir.join(table.name.to_string());
+        let table_dir = output_models_dir.join(table.name.to_string());
 
         if !table_dir.exists() {
             std::fs::create_dir(&table_dir).attach_path_err(&table_dir)?;
@@ -228,12 +226,12 @@ pub fn generate_files(
         table_mod_rs.ensure_use_stmt("generated::*");
         table_mod_rs.write()?;
 
-        mod_rs.ensure_mod_stmt(table.name.to_string().as_str());
+        mod_rs.ensure_mod_stmt(&table.name.to_string());
     }
 
     // pass 2: delete code for removed tables
-    for item in std::fs::read_dir(&output_dir).attach_path_err(&output_dir)? {
-        let item = item.attach_path_err(&output_dir)?;
+    for item in std::fs::read_dir(output_models_dir).attach_path_err(output_models_dir)? {
+        let item = item.attach_path_err(output_models_dir)?;
 
         // check if item is a directory
         let file_type = item

src/parser.rs

@@ -64,10 +64,10 @@ pub struct ParsedJoinMacro {
 
 /// Try to parse a diesel schema file contents
 pub fn parse_and_generate_code(
-    schema_file_contents: String,
+    schema_file_contents: &str,
     config: &GenerationConfig,
 ) -> Result<Vec<ParsedTableMacro>> {
-    let schema_file = syn::parse_file(&schema_file_contents).unwrap();
+    let schema_file = syn::parse_file(schema_file_contents).unwrap();
 
     let mut tables: Vec<ParsedTableMacro> = vec![];
 
@@ -115,7 +115,7 @@ pub fn parse_and_generate_code(
     }
 
     for table in tables.iter_mut() {
-        table.generated_code = code::generate_for_table(table.clone(), config);
+        table.generated_code = code::generate_for_table(table, config);
     }
 
     Ok(tables)
@@ -129,12 +129,12 @@ fn handle_joinable_macro(macro_item: syn::ItemMacro) -> Result<ParsedJoinMacro>
     let mut table2_join_column: Option<String> = None;
 
     for item in macro_item.mac.tokens.into_iter() {
-        match &item {
+        match item {
             proc_macro2::TokenTree::Ident(ident) => {
                 if table1_name.is_none() {
-                    table1_name = Some(ident.clone());
+                    table1_name = Some(ident);
                 } else if table2_name.is_none() {
-                    table2_name = Some(ident.clone());
+                    table2_name = Some(ident);
                 }
             }
             proc_macro2::TokenTree::Group(group) => {
@@ -185,7 +185,7 @@ fn handle_table_macro(
             continue;
         }
 
-        match &item {
+        match item {
             proc_macro2::TokenTree::Punct(punct) => {
                 // skip any "#[]"
                 if punct.to_string().as_str() == "#" {
@@ -200,7 +200,7 @@ fn handle_table_macro(
                     continue;
                 }
 
-                table_name_ident = Some(ident.clone());
+                table_name_ident = Some(ident);
             }
             proc_macro2::TokenTree::Group(group) => {
                 if skip_square_brackets {
@@ -234,13 +234,13 @@ fn handle_table_macro(
                             }
                             proc_macro2::TokenTree::Ident(ident) => {
                                 if column_name.is_none() {
-                                    column_name = Some(ident.clone());
+                                    column_name = Some(ident);
                                 } else if ident.to_string().eq_ignore_ascii_case("Nullable") {
                                     column_nullable = true;
                                 } else if ident.to_string().eq_ignore_ascii_case("Unsigned") {
                                     column_unsigned = true;
                                 } else {
-                                    column_type = Some(ident.clone());
+                                    column_type = Some(ident);
                                 }
                             }
                             proc_macro2::TokenTree::Punct(punct) => {