From a0581de2300dbbfc58f7489882b20b78a5a1ffdd Mon Sep 17 00:00:00 2001 From: Ali Aliyev Date: Sun, 19 Jan 2025 01:29:15 +0400 Subject: [PATCH 1/3] feat: update dependencies and improve error handling in various modules --- Cargo.toml | 1 + src/cli.rs | 3 +-- src/config.rs | 9 +++----- src/dialoguer.rs | 27 ++++++----------------- src/error.rs | 44 ++++++++++--------------------------- src/hooks.rs | 22 ++++++++++--------- src/ignore.rs | 16 +++++--------- src/ioutils.rs | 46 +++++++++++++++++++++++++++++++++++---- src/loader.rs | 2 +- src/renderer.rs | 26 +++++++--------------- src/template/processor.rs | 14 +++++------- 11 files changed, 98 insertions(+), 112 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a462c78..d44dcbc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ serde_derive = "1.0" serde_yaml = "0.9" url = "2.5" dialoguer = { version = "0.10.0", features = ["fuzzy-select"] } +anyhow = { version = "1.0.95" } [dev-dependencies] tempfile = "3.15" diff --git a/src/cli.rs b/src/cli.rs index c00f91a..059c8e9 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -221,8 +221,7 @@ pub fn run(args: Args) -> Result<()> { // Process template files for dir_entry in WalkDir::new(&template_root) { - let raw_entry = dir_entry.map_err(|e| Error::TemplateError(e.to_string()))?; - let template_entry = raw_entry.path().to_path_buf(); + let template_entry = dir_entry?.path().to_path_buf(); match processor.process(&template_entry) { Ok(file_operation) => { let user_confirmed_overwrite = match &file_operation { diff --git a/src/config.rs b/src/config.rs index d5f5adb..a955208 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,5 @@ use crate::error::{Error, Result}; +use crate::ioutils::path_to_str; use crate::renderer::TemplateRenderer; use indexmap::IndexMap; use serde::Deserialize; @@ -75,17 +76,13 @@ impl Config { pub fn load_config>(template_root: P) -> Result { let template_root = template_root.as_ref().to_path_buf(); - let template_dir = template_root - .to_str() - .to_owned() - .ok_or_else(|| Error::TemplateSourceInvalidError)? - .to_string(); + let template_dir = path_to_str(&template_root)?.to_string(); for config_file in CONFIG_LIST.iter() { if let Some(config) = Config::from_file(template_root.join(config_file)) { return Ok(config); } } - Err(Error::ConfigError { template_dir, config_files: CONFIG_LIST.join(", ") }) + Err(Error::ConfigNotFound { template_dir, config_files: CONFIG_LIST.join(", ") }) } } diff --git a/src/dialoguer.rs b/src/dialoguer.rs index 5d309fc..e54a392 100644 --- a/src/dialoguer.rs +++ b/src/dialoguer.rs @@ -1,7 +1,4 @@ -use crate::{ - config::Question, - error::{Error, Result}, -}; +use crate::{config::Question, error::Result}; use dialoguer::{Confirm, Input, MultiSelect, Password, Select}; @@ -9,7 +6,7 @@ pub fn confirm(skip: bool, prompt: String) -> Result { if skip { return Ok(true); } - Confirm::new().with_prompt(prompt).default(false).interact().map_err(Error::IoError) + Ok(Confirm::new().with_prompt(prompt).default(false).interact()?) } pub fn prompt_multiple_choice( @@ -34,8 +31,7 @@ pub fn prompt_multiple_choice( .with_prompt(prompt) .items(&choices) .defaults(&defaults) - .interact() - .map_err(Error::IoError)?; + .interact()?; let selected: Vec = indices.iter().map(|&i| serde_json::Value::String(choices[i].clone())).collect(); @@ -48,11 +44,7 @@ pub fn prompt_boolean( prompt: String, ) -> Result { let default_value = default_value.as_bool().unwrap(); - let result = Confirm::new() - .with_prompt(prompt) - .default(default_value) - .interact() - .map_err(Error::IoError)?; + let result = Confirm::new().with_prompt(prompt).default(default_value).interact()?; Ok(serde_json::Value::Bool(result)) } @@ -72,8 +64,7 @@ pub fn prompt_single_choice( .with_prompt(prompt) .default(default_value) .items(&choices) - .interact() - .map_err(Error::IoError)?; + .interact()?; Ok(serde_json::Value::String(choices[selection].clone())) } @@ -104,13 +95,9 @@ pub fn prompt_text( ); } - password.interact().map_err(Error::IoError)? + password.interact()? } else { - Input::new() - .with_prompt(&prompt) - .default(default_str) - .interact_text() - .map_err(Error::IoError)? + Input::new().with_prompt(&prompt).default(default_str).interact_text()? }; Ok(serde_json::Value::String(input)) diff --git a/src/error.rs b/src/error.rs index e9b9173..8f2de08 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,9 +6,6 @@ pub enum Error { #[error("IO error: {0}.")] IoError(#[from] std::io::Error), - #[error("Failed to parse config file.")] - ConfigParseError, - #[error("Failed to parse .bakerignore file. Original error: {0}")] GlobSetParseError(#[from] globset::Error), @@ -18,48 +15,31 @@ pub enum Error { #[error("Failed to render. Original error: {0}")] MinijinjaError(#[from] minijinja::Error), - #[error("Template error: {0}.")] - TemplateError(String), - - #[error("No configuration file found in '{template_dir}'. Tried: {config_files}.")] - ConfigError { template_dir: String, config_files: String }, + #[error("Failed to extract dir entry. Original error: {0}")] + WalkdirError(#[from] walkdir::Error), - /// When the Hook has executed but finished with an error. - #[error("Hook execution failed with status: {status}")] - HookExecutionError { status: ExitStatus }, + #[error( + "Configuration file not found. Searched in '{template_dir}' for: {config_files}" + )] + ConfigNotFound { template_dir: String, config_files: String }, - /// Represents validation failures in user input or data - #[error("Validation error: {0}.")] - ValidationError(String), - - /// Represents errors in processing .bakerignore files - #[error("BakerIgnore error: {0}.")] - BakerIgnoreError(String), + #[error("Hook script '{script}' failed with exit code: {status}")] + HookExecutionError { script: String, status: ExitStatus }, #[error("Cannot proceed: output directory '{output_dir}' already exists. Use --force to overwrite it.")] OutputDirectoryExistsError { output_dir: String }, #[error("Cannot proceed: template directory '{template_dir}' does not exist.")] TemplateDoesNotExistsError { template_dir: String }, - #[error("Cannot proceed: invalid type of template source.")] - TemplateSourceInvalidError, #[error("Cannot process the source path: '{source_path}'. Original error: {e}")] ProcessError { source_path: String, e: String }, + + #[error(transparent)] + Other(#[from] anyhow::Error), } -/// Convenience type alias for Results with BakerError as the error type. -/// -/// # Type Parameters -/// * `T` - The type of the success value -pub type Result = std::result::Result; +pub type Result = core::result::Result; -/// Default error handler that prints the error and exits the program. -/// -/// # Arguments -/// * `err` - The BakerError to handle -/// -/// # Behavior -/// Prints the error message to stderr and exits with status code 1 pub fn default_error_handler(err: Error) { eprintln!("{}", err); std::process::exit(1); diff --git a/src/hooks.rs b/src/hooks.rs index ccff2b8..7f6ceae 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -5,6 +5,7 @@ use std::process::{ChildStdout, Command, Stdio}; use crate::dialoguer::confirm; use crate::error::{Error, Result}; +use crate::ioutils::path_to_str; /// Structure representing data passed to hook scripts. /// @@ -72,11 +73,10 @@ pub fn run_hook>( ) -> Result> { let script_path = script_path.as_ref(); - let output = Output { - template_dir: template_dir.as_ref().to_str().unwrap(), - output_dir: output_dir.as_ref().to_str().unwrap(), - answers, - }; + let template_dir = path_to_str(&template_dir)?; + let output_dir = path_to_str(&output_dir)?; + + let output = Output { template_dir, output_dir, answers }; let output_data = serde_json::to_vec(&output).unwrap(); @@ -88,20 +88,22 @@ pub fn run_hook>( .stdin(Stdio::piped()) .stdout(if is_piped_stdout { Stdio::piped() } else { Stdio::inherit() }) .stderr(Stdio::inherit()) - .spawn() - .map_err(Error::IoError)?; + .spawn()?; // Write context to stdin if let Some(mut stdin) = child.stdin.take() { - stdin.write_all(&output_data).map_err(Error::IoError)?; + stdin.write_all(&output_data)?; stdin.write_all(b"\n")?; } // Wait for the process to complete - let status = child.wait().map_err(Error::IoError)?; + let status = child.wait()?; if !status.success() { - return Err(Error::HookExecutionError { status }); + return Err(Error::HookExecutionError { + script: script_path.display().to_string(), + status, + }); } Ok(child.stdout) diff --git a/src/ignore.rs b/src/ignore.rs index 2eabb55..bea58c0 100644 --- a/src/ignore.rs +++ b/src/ignore.rs @@ -1,4 +1,4 @@ -use crate::error::{Error, Result}; +use crate::{error::Result, ioutils::path_to_str}; use globset::{Glob, GlobSet, GlobSetBuilder}; use log::debug; use std::{fs::read_to_string, path::Path}; @@ -31,10 +31,9 @@ pub fn parse_bakerignore_file>(template_root: P) -> Result>(template_root: P) -> Result>(output_dir: P, force: bool) -> Result>(dest_path: P) -> Result<()> { let dest_path = dest_path.as_ref(); - std::fs::create_dir_all(dest_path).map_err(Error::IoError) + Ok(std::fs::create_dir_all(dest_path)?) } pub fn write_file>(content: &str, dest_path: P) -> Result<()> { @@ -30,7 +30,7 @@ pub fn write_file>(content: &str, dest_path: P) -> Result<()> { if let Some(parent) = abs_path.parent() { create_dir_all(parent)?; } - std::fs::write(abs_path, content).map_err(Error::IoError) + Ok(std::fs::write(abs_path, content)?) } pub fn copy_file>(source_path: P, dest_path: P) -> Result<()> { @@ -46,7 +46,7 @@ pub fn copy_file>(source_path: P, dest_path: P) -> Result<()> { if let Some(parent) = abs_dest.parent() { create_dir_all(parent)?; } - std::fs::copy(source_path, abs_dest).map(|_| ()).map_err(Error::IoError) + Ok(std::fs::copy(source_path, abs_dest).map(|_| ())?) } pub fn parse_string_to_json( @@ -63,6 +63,44 @@ pub fn parse_string_to_json( pub fn read_from(mut reader: impl std::io::Read) -> Result { let mut buf = String::new(); - reader.read_to_string(&mut buf).map_err(Error::IoError)?; + reader.read_to_string(&mut buf)?; Ok(buf) } + +/// Converts a path to a string slice, returning an error if the path contains invalid Unicode characters. +/// +/// # Arguments +/// * `path` - A reference to a type that can be converted to a [`Path`] +/// +/// # Returns +/// * `Ok(&str)` - A string slice representing the path +/// * `Err(Error)` - If the path contains invalid Unicode characters +/// +/// # Examples +/// ``` +/// use std::path::Path; +/// use std::ffi::OsStr; +/// use std::os::unix::ffi::OsStrExt; +/// use baker::ioutils::path_to_str; +/// +/// let valid_path = Path::new("/tmp/test.txt"); +/// let str_path = path_to_str(valid_path).unwrap(); +/// assert_eq!(str_path, "/tmp/test.txt"); +/// +/// // Path with invalid Unicode will return an error +/// let invalid_bytes = vec![0x2F, 0x74, 0x6D, 0x70, 0xFF, 0xFF]; // "/tmp��" +/// let invalid_path = Path::new(OsStr::from_bytes(&invalid_bytes)); +/// assert!(path_to_str(invalid_path).is_err()); +/// ``` +/// +/// # Errors +/// Returns an error if the path contains any invalid Unicode characters +/// +pub fn path_to_str + ?Sized>(path: &P) -> Result<&str> { + Ok(path.as_ref().to_str().ok_or_else(|| { + anyhow::anyhow!( + "Path '{}' contains invalid Unicode characters", + path.as_ref().display() + ) + })?) +} diff --git a/src/loader.rs b/src/loader.rs index 9c27a1b..5b23399 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -144,7 +144,7 @@ impl> TemplateLoader for GitLoader { format!("Directory '{}' already exists. Replace it?", repo_name), )?; if response { - fs::remove_dir_all(&clone_path).map_err(Error::IoError)?; + fs::remove_dir_all(&clone_path)?; } else { debug!("Using existing directory '{}'.", clone_path.display()); return Ok(clone_path); diff --git a/src/renderer.rs b/src/renderer.rs index 886e1a3..8fd95e1 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -1,4 +1,4 @@ -use crate::error::{Error, Result}; +use crate::{error::Result, ioutils::path_to_str}; use minijinja::Environment; use std::path::Path; @@ -41,9 +41,9 @@ impl MiniJinjaRenderer { context: &serde_json::Value, ) -> Result { let mut env = self.env.clone(); - env.add_template("temp", template).map_err(Error::MinijinjaError)?; - let tmpl = env.get_template("temp").map_err(Error::MinijinjaError)?; - tmpl.render(context).map_err(Error::MinijinjaError) + env.add_template("temp", template)?; + let tmpl = env.get_template("temp")?; + Ok(tmpl.render(context)?) } } @@ -62,25 +62,15 @@ impl TemplateRenderer for MiniJinjaRenderer { template_path: &Path, context: &serde_json::Value, ) -> Result { - let path_str = template_path.to_str().ok_or_else(|| Error::ProcessError { - source_path: template_path.display().to_string(), - e: "Cannot convert source_path to string.".to_string(), - })?; - - self.render_internal(path_str, context).map_err(|e| Error::ProcessError { - source_path: path_str.to_string(), - e: e.to_string(), - }) + let path_str = path_to_str(template_path)?; + self.render_internal(path_str, context) } fn execute_expression( &self, expr_str: &str, context: &serde_json::Value, ) -> Result { - let expr = self.env.compile_expression(expr_str).map_err(|e| { - Error::ProcessError { source_path: "".to_string(), e: e.to_string() } - })?; - let result = expr.eval(context).unwrap(); - Ok(result.is_true()) + let expr = self.env.compile_expression(expr_str)?; + Ok(expr.eval(context)?.is_true()) } } diff --git a/src/template/processor.rs b/src/template/processor.rs index 85cccf3..08f6d62 100644 --- a/src/template/processor.rs +++ b/src/template/processor.rs @@ -3,6 +3,7 @@ use std::fs; use std::path::{Path, PathBuf}; use crate::error::{Error, Result}; +use crate::ioutils::path_to_str; use crate::renderer::TemplateRenderer; use super::operation::TemplateOperation; @@ -127,13 +128,9 @@ impl<'a, P: AsRef> TemplateProcessor<'a, P> { let rendered_entry = self.engine.render_path(template_entry, self.answers)?; let rendered_entry = rendered_entry.as_str(); - if !self.has_valid_rendered_path_parts( - template_entry.to_str().ok_or_else(|| Error::ProcessError { - source_path: template_entry.display().to_string(), - e: "Invalid template path".to_string(), - })?, - rendered_entry, - ) { + if !self + .has_valid_rendered_path_parts(path_to_str(&template_entry)?, rendered_entry) + { return Err(Error::ProcessError { source_path: rendered_entry.to_string(), e: "The rendered path is not valid".to_string(), @@ -206,8 +203,7 @@ impl<'a, P: AsRef> TemplateProcessor<'a, P> { match (template_entry.is_file(), self.is_template_file(&template_entry)) { // Template file (true, true) => { - let template_content = - fs::read_to_string(&template_entry).map_err(Error::IoError)?; + let template_content = fs::read_to_string(&template_entry)?; let content = self.engine.render(&template_content, self.answers)?; Ok(TemplateOperation::Write { From 1ef2dd629da28fe11aefa0821db30740281ea66e Mon Sep 17 00:00:00 2001 From: Ali Aliyev Date: Sun, 19 Jan 2025 01:30:01 +0400 Subject: [PATCH 2/3] fix: support for YAML configuration files in config loader --- src/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config.rs b/src/config.rs index a955208..0ca0c98 100644 --- a/src/config.rs +++ b/src/config.rs @@ -69,6 +69,8 @@ impl Config { if let Ok(contents) = std::fs::read_to_string(path) { if let Ok(config) = serde_json::from_str(&contents) { return Some(config); + } else if let Ok(config) = serde_yaml::from_str(&contents) { + return Some(config); } } None From 19f7435f8b82594c4befc08d362ea5cc0472c259 Mon Sep 17 00:00:00 2001 From: Ali Aliyev Date: Sun, 19 Jan 2025 01:42:44 +0400 Subject: [PATCH 3/3] fix: improve path handling in bakerignore file parsing --- src/ignore.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ignore.rs b/src/ignore.rs index bea58c0..3569f4d 100644 --- a/src/ignore.rs +++ b/src/ignore.rs @@ -40,8 +40,11 @@ pub fn parse_bakerignore_file>(template_root: P) -> Result