diff --git a/Cargo.lock b/Cargo.lock index facd10d..ce14e9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -78,9 +78,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.100" +version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" +checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" [[package]] name = "async-trait" @@ -432,6 +432,7 @@ dependencies = [ name = "dela" version = "0.0.6" dependencies = [ + "anyhow", "chrono", "clap", "colored", @@ -452,6 +453,7 @@ dependencies = [ "serial_test", "shell-words", "tempfile", + "thiserror 2.0.18", "tokio", "toml", "tracing", @@ -877,7 +879,7 @@ checksum = "8fe90c1150662e858c7d5f945089b7517b0a80d8bf7ba4b1b5ffc984e7230a5b" dependencies = [ "hashbrown 0.16.1", "portable-atomic", - "thiserror 2.0.16", + "thiserror 2.0.18", ] [[package]] @@ -1322,7 +1324,7 @@ dependencies = [ "kasuari", "lru", "strum", - "thiserror 2.0.16", + "thiserror 2.0.18", "unicode-segmentation", "unicode-truncate", "unicode-width", @@ -1396,7 +1398,7 @@ checksum = "a4e608c6638b9c18977b00b475ac1f28d14e84b27d8d42f70e0bf1e3dec127ac" dependencies = [ "getrandom 0.2.16", "libredox", - "thiserror 2.0.16", + "thiserror 2.0.18", ] [[package]] @@ -1464,7 +1466,7 @@ dependencies = [ "schemars", "serde", "serde_json", - "thiserror 2.0.16", + "thiserror 2.0.18", "tokio", "tokio-util", "tracing", @@ -1925,11 +1927,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.16" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3467d614147380f2e4e374161426ff399c91084acd2363eaf549172b3d5e60c0" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl 2.0.16", + "thiserror-impl 2.0.18", ] [[package]] @@ -1945,9 +1947,9 @@ dependencies = [ [[package]] name = "thiserror-impl" -version = "2.0.16" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c5e1be1c48b9172ee610da68fd9cd2770e7a4056cb3fc98710ee6906f0c7960" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 747aeb0..f52d638 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,8 @@ nix = { version = "0.31", features = ["signal"] } dirs = "6.0.0" shell-words = "1.1.1" chrono = { version = "0.4.42", default-features = false, features = ["clock", "std"] } +thiserror = "2.0.18" +anyhow = "1.0.102" [dev-dependencies] tempfile = "3.27.0" diff --git a/dev_docs/style.md b/dev_docs/style.md index 47cb7a0..5e4ee40 100644 --- a/dev_docs/style.md +++ b/dev_docs/style.md @@ -18,3 +18,6 @@ Comments should explain why something is done, not what is done. Prefer no or fe When working on feature, do a git diff against main, and make sure that no unnecessary or temporary code snuck in. +## Error Handling + +For error handling, avoid stringly-typed errors and instead define domain-specific, strongly-typed error enums using the `thiserror` crate for internal library logic. At the application boundaries (like CLI commands), use `anyhow::Result` to provide ergonomic context and robust error reporting. The `?` operator should be used to seamlessly convert the strongly-typed internal errors into `anyhow` errors as they propagate up. diff --git a/src/allowlist.rs b/src/allowlist.rs index 0d69639..37ca28c 100644 --- a/src/allowlist.rs +++ b/src/allowlist.rs @@ -5,19 +5,21 @@ use std::fs; use std::path::{Path, PathBuf}; /// Returns the path to the active allowlist.toml. -fn allowlist_path() -> Result { - active_allowlist_path() +fn allowlist_path() -> anyhow::Result { + Ok(active_allowlist_path()?) } /// Load the allowlist from the active allowlist path. /// If the file does not exist, return an empty allowlist. -pub fn load_allowlist() -> Result { +pub fn load_allowlist() -> anyhow::Result { let path = allowlist_path()?; let dela_dir = active_dela_config_dir()?; // Check if the dela config directory exists if !dela_dir.exists() { - return Err("Dela is not initialized. Please run 'dela init' first.".to_string()); + return Err(anyhow::anyhow!( + "Dela is not initialized. Please run 'dela init' first." + )); } // If allowlist file doesn't exist but the config directory does, return empty allowlist @@ -25,28 +27,29 @@ pub fn load_allowlist() -> Result { return Ok(Allowlist::default()); } - let contents = - fs::read_to_string(&path).map_err(|e| format!("Failed to read allowlist file: {}", e))?; + let contents = fs::read_to_string(&path) + .map_err(|e| anyhow::anyhow!("Failed to read allowlist file: {}", e))?; match toml::from_str::(&contents) { Ok(allowlist) => Ok(allowlist), - Err(e) => Err(format!("Failed to parse allowlist TOML: {}", e)), + Err(e) => Err(anyhow::anyhow!("Failed to parse allowlist TOML: {}", e)), } } /// Save the allowlist to ~/.config/dela/allowlist.toml. -pub fn save_allowlist(allowlist: &Allowlist) -> Result<(), String> { +pub fn save_allowlist(allowlist: &Allowlist) -> anyhow::Result<()> { let path = preferred_allowlist_path()?; // Create the config directory if it doesn't exist if let Some(parent) = path.parent() { fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create dela config directory: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to create dela config directory: {}", e))?; } let toml = toml::to_string_pretty(&allowlist) - .map_err(|e| format!("Failed to serialize allowlist: {}", e))?; - fs::write(&path, toml).map_err(|e| format!("Failed to create allowlist file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to serialize allowlist: {}", e))?; + fs::write(&path, toml) + .map_err(|e| anyhow::anyhow!("Failed to create allowlist file: {}", e))?; Ok(()) } @@ -126,7 +129,7 @@ pub fn allowlist_entry_for_task(task: &Task, scope: AllowScope) -> AllowlistEntr /// Check if a given task is explicitly allowed or denied by the allowlist /// Returns (explicitly_allowed, explicitly_denied) - both false means not found in allowlist /// This is the core allowlist checking logic without any prompting or persistence -pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { +pub fn is_task_allowed(task: &Task) -> anyhow::Result<(bool, bool)> { // Only proceed with allowlist operations if dela is initialized let allowlist = load_allowlist()?; Ok(match evaluate_task_against_allowlist(task, &allowlist) { @@ -138,7 +141,7 @@ pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { /// Check if a given task is allowed, based on the loaded allowlist /// If the task is not in the allowlist, prompt the user for a decision -pub fn check_task_allowed(task: &Task) -> Result { +pub fn check_task_allowed(task: &Task) -> anyhow::Result { // Check if task is explicitly allowed or denied let (explicitly_allowed, explicitly_denied) = is_task_allowed(task)?; @@ -183,7 +186,7 @@ pub fn check_task_allowed(task: &Task) -> Result { } /// Check if a given task is allowed with a specific scope, without prompting -pub fn check_task_allowed_with_scope(task: &Task, scope: AllowScope) -> Result { +pub fn check_task_allowed_with_scope(task: &Task, scope: AllowScope) -> anyhow::Result { // Only proceed with allowlist operations if dela is initialized let mut allowlist = load_allowlist()?; diff --git a/src/commands/allow_command.rs b/src/commands/allow_command.rs index 7924e05..69d73b5 100644 --- a/src/commands/allow_command.rs +++ b/src/commands/allow_command.rs @@ -2,23 +2,27 @@ use crate::allowlist; use crate::config::preferred_allowlist_path; use crate::task_discovery; use crate::types::AllowScope; +use anyhow::Context; use std::env; -pub fn execute(task_with_args: &str, allow: Option) -> Result<(), String> { +pub fn execute(task_with_args: &str, allow: Option) -> anyhow::Result<()> { let task_name = task_with_args .split_whitespace() .next() - .ok_or_else(|| "No task name provided".to_string())?; + .context("No task name provided")?; - let current_dir = - env::current_dir().map_err(|e| format!("Failed to get current directory: {}", e))?; + let current_dir = env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))?; let discovered = task_discovery::discover_tasks(¤t_dir); // Find all tasks with the given name (both original and disambiguated) let matching_tasks = task_discovery::get_matching_tasks(&discovered, task_name); match matching_tasks.len() { - 0 => Err(format!("dela: command or task not found: {}", task_name)), + 0 => Err(anyhow::anyhow!( + "dela: command or task not found: {}", + task_name + )), 1 => { // Single task found, check allowlist let task = matching_tasks[0]; @@ -40,13 +44,13 @@ pub fn execute(task_with_args: &str, allow: Option) -> Result<(), String> { } 5 => { eprintln!("Task '{}' was denied by the allowlist.", task.name); - Err(format!( + Err(anyhow::anyhow!( "Dela task '{}' was denied by the {}", task.name, preferred_allowlist_path()?.display() )) } - _ => Err(format!( + _ => Err(anyhow::anyhow!( "Invalid allow choice {}. Please use a number between 2 and 5.", choice )), @@ -55,7 +59,7 @@ pub fn execute(task_with_args: &str, allow: Option) -> Result<(), String> { // Otherwise, use the interactive prompt if !allowlist::check_task_allowed(task)? { eprintln!("Task '{}' was denied by the allowlist.", task.name); - return Err(format!( + return Err(anyhow::anyhow!( "Dela task '{}' was denied by the {}", task.name, preferred_allowlist_path()?.display() @@ -68,7 +72,10 @@ pub fn execute(task_with_args: &str, allow: Option) -> Result<(), String> { // Multiple tasks found, print error and list them let error_msg = task_discovery::format_ambiguous_task_error(task_name, &matching_tasks); eprintln!("{}", error_msg); - Err(format!("Multiple tasks named '{}' found", task_name)) + Err(anyhow::anyhow!( + "Multiple tasks named '{}' found", + task_name + )) } } } @@ -189,7 +196,7 @@ test: ## Running tests let result = execute("test", None); assert!(result.is_err(), "Should fail when task is denied"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), format!( "Dela task 'test' was denied by the {}", preferred_allowlist_path_for(home_dir.path()).display() @@ -209,7 +216,7 @@ test: ## Running tests let result = execute("nonexistent", None); assert!(result.is_err(), "Should fail when no task found"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "dela: command or task not found: nonexistent" ); @@ -242,7 +249,7 @@ test: ## Running tests let result = execute("test", Some(5)); assert!(result.is_err(), "Should fail with allow=5"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), format!( "Dela task 'test' was denied by the {}", preferred_allowlist_path_for(home_dir.path()).display() @@ -253,7 +260,7 @@ test: ## Running tests let result = execute("test", Some(1)); assert!(result.is_err(), "Should fail with allow=1"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "Invalid allow choice 1. Please use a number between 2 and 5." ); @@ -261,7 +268,7 @@ test: ## Running tests let result = execute("test", Some(6)); assert!(result.is_err(), "Should fail with allow=6"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "Invalid allow choice 6. Please use a number between 2 and 5." ); @@ -300,7 +307,7 @@ test: ## Running tests "Should fail when the dela config directory doesn't exist" ); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "Dela is not initialized. Please run 'dela init' first." ); @@ -329,7 +336,7 @@ test: ## Running tests let result = execute("test --verbose --coverage", Some(5)); assert!(result.is_err(), "Should fail with allow=5 and arguments"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), format!( "Dela task 'test' was denied by the {}", preferred_allowlist_path_for(home_dir.path()).display() @@ -370,6 +377,7 @@ test: ## Running tests assert!( result .unwrap_err() + .to_string() .contains("Multiple tasks named 'test' found"), "Error should mention multiple tasks" ); diff --git a/src/commands/configure_shell.rs b/src/commands/configure_shell.rs index 6fcf54e..6079042 100644 --- a/src/commands/configure_shell.rs +++ b/src/commands/configure_shell.rs @@ -1,4 +1,5 @@ use crate::environment::get_current_shell; +use anyhow::Context; const ZSH_CONFIG: &str = include_str!("../../resources/zsh.sh"); const BASH_CONFIG: &str = include_str!("../../resources/bash.sh"); @@ -15,12 +16,12 @@ enum Shell { } impl Shell { - fn from_path(path: &str) -> Result { + fn from_path(path: &str) -> anyhow::Result { let shell_path = std::path::PathBuf::from(path); let shell_name = shell_path .file_name() .and_then(|name| name.to_str()) - .ok_or_else(|| "Invalid shell path".to_string())?; + .context("Invalid shell path")?; match shell_name { "zsh" => Ok(Shell::Zsh), @@ -32,9 +33,9 @@ impl Shell { } } -pub fn execute() -> Result<(), String> { +pub fn execute() -> anyhow::Result<()> { // Get the current shell from environment - let shell = get_current_shell().ok_or("SHELL environment variable not set".to_string())?; + let shell = get_current_shell().context("SHELL environment variable not set")?; // Parse the shell type let shell_type = Shell::from_path(&shell)?; @@ -57,7 +58,7 @@ pub fn execute() -> Result<(), String> { print!("{}", PWSH_CONFIG); Ok(()) } - Shell::Unknown(name) => Err(format!("Unsupported shell: {}", name)), + Shell::Unknown(name) => Err(anyhow::anyhow!("Unsupported shell: {}", name)), } } @@ -114,7 +115,10 @@ mod tests { setup_test_env("/bin/unknown"); let result = execute(); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Unsupported shell: unknown"); + assert_eq!( + result.unwrap_err().to_string(), + "Unsupported shell: unknown" + ); reset_to_real_environment(); } @@ -136,7 +140,10 @@ mod tests { let result = execute(); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "SHELL environment variable not set"); + assert_eq!( + result.unwrap_err().to_string(), + "SHELL environment variable not set" + ); reset_to_real_environment(); } } diff --git a/src/commands/get_command.rs b/src/commands/get_command.rs index 81804ba..95d6959 100644 --- a/src/commands/get_command.rs +++ b/src/commands/get_command.rs @@ -1,31 +1,38 @@ use crate::runner::is_runner_available; use crate::task_discovery; +use anyhow::Context; use std::env; -pub fn execute(task_with_args: &str) -> Result<(), String> { +pub fn execute(task_with_args: &str) -> anyhow::Result<()> { let mut parts = task_with_args.split_whitespace(); - let task_name = parts - .next() - .ok_or_else(|| "No task name provided".to_string())?; + let task_name = parts.next().context("No task name provided")?; let args: Vec<&str> = parts.collect(); - let current_dir = - env::current_dir().map_err(|e| format!("Failed to get current directory: {}", e))?; + let current_dir = env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))?; let discovered = task_discovery::discover_tasks(¤t_dir); // Find all tasks with the given name (both original and disambiguated) let matching_tasks = task_discovery::get_matching_tasks(&discovered, task_name); match matching_tasks.len() { - 0 => Err(format!("dela: command or task not found: {}", task_name)), + 0 => Err(anyhow::anyhow!( + "dela: command or task not found: {}", + task_name + )), 1 => { // Single task found, check if runner is available let task = matching_tasks[0]; if !is_runner_available(&task.runner) { if task.runner == crate::types::TaskRunner::TravisCi { - return Err("Travis CI tasks cannot be executed locally - they are only available for discovery".to_string()); + return Err(anyhow::anyhow!( + "Travis CI tasks cannot be executed locally - they are only available for discovery" + )); } - return Err(format!("Runner '{}' not found", task.runner.short_name())); + return Err(anyhow::anyhow!( + "Runner '{}' not found", + task.runner.short_name() + )); } let mut command = task.runner.get_command(task); if !args.is_empty() { @@ -38,7 +45,7 @@ pub fn execute(task_with_args: &str) -> Result<(), String> { _ => { // Multiple matches (should not happen with get_matching_tasks, but handle for safety) let error_msg = task_discovery::format_ambiguous_task_error(task_name, &matching_tasks); - Err(error_msg) + Err(anyhow::anyhow!(error_msg)) } } } @@ -141,7 +148,7 @@ test: ## Running tests let result = execute("nonexistent"); assert!(result.is_err(), "Should fail when no task found"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "dela: command or task not found: nonexistent" ); @@ -163,7 +170,7 @@ test: ## Running tests let result = execute("test"); assert!(result.is_err(), "Should fail when runner is missing"); - assert_eq!(result.unwrap_err(), "Runner 'make' not found"); + assert_eq!(result.unwrap_err().to_string(), "Runner 'make' not found"); reset_mock(); reset_to_real_environment(); @@ -210,6 +217,7 @@ test: ## Running tests assert!( result .unwrap_err() + .to_string() .contains("Multiple tasks named 'test' found"), "Error should mention multiple tasks" ); diff --git a/src/commands/init.rs b/src/commands/init.rs index 0f881e7..b745406 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -1,13 +1,14 @@ use crate::config::{legacy_dela_config_dir, preferred_allowlist_path, preferred_config_dir_path}; use crate::environment::{get_current_home, get_current_shell as env_get_current_shell}; use crate::types::Allowlist; +use anyhow::Context; use std::env; use std::fs; use std::io::Write; use std::path::PathBuf; /// Get the current shell name by checking the parent process -fn get_current_shell() -> Result { +fn get_current_shell() -> anyhow::Result { // Try to get shell from BASH_VERSION or ZSH_VERSION first if env::var("BASH_VERSION").is_ok() { return Ok("bash".to_string()); @@ -17,20 +18,20 @@ fn get_current_shell() -> Result { } // Fallback to $SHELL if version variables aren't set - let shell = env_get_current_shell().ok_or("SHELL environment variable not set".to_string())?; + let shell = env_get_current_shell().context("SHELL environment variable not set")?; let shell_path = std::path::PathBuf::from(&shell); shell_path .file_name() .and_then(|name| name.to_str()) .map(|s| s.to_string()) - .ok_or_else(|| "Invalid shell path".to_string()) + .context("Invalid shell path") } /// Get the appropriate shell config path based on current shell -fn get_shell_config_path() -> Result { +fn get_shell_config_path() -> anyhow::Result { let shell_name = get_current_shell()?; - let home = get_current_home().ok_or("HOME environment variable not set".to_string())?; + let home = get_current_home().context("HOME environment variable not set")?; let home_path = PathBuf::from(&home); match shell_name.as_str() { @@ -41,16 +42,18 @@ fn get_shell_config_path() -> Result { .join(".config") .join("powershell") .join("Microsoft.PowerShell_profile.ps1")), - name => Err(format!("Unsupported shell: {}", name)), + name => Err(anyhow::anyhow!("Unsupported shell: {}", name)), } } /// Add dela shell integration to the shell config file -fn add_shell_integration(config_path: &PathBuf) -> Result<(), String> { +fn add_shell_integration(config_path: &PathBuf) -> anyhow::Result<()> { // Read the current content - let content = fs::read_to_string(config_path) - .map_err(|e| format!("Failed to read shell config: {}", e))?; - + let content = match fs::read_to_string(config_path) { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(e) => return Err(anyhow::anyhow!("Failed to read shell config: {}", e)), + }; // Get the shell type from the path let shell = get_current_shell()?; @@ -74,7 +77,7 @@ fn add_shell_integration(config_path: &PathBuf) -> Result<(), String> { && !parent.exists() { fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create config directory: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to create config directory: {}", e))?; } // Open file in append mode @@ -82,19 +85,19 @@ fn add_shell_integration(config_path: &PathBuf) -> Result<(), String> { .create(true) .append(true) .open(config_path) - .map_err(|e| format!("Failed to open shell config: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to open shell config: {}", e))?; // Add dela integration with shell-specific syntax - writeln!(file).map_err(|e| format!("Failed to write to shell config: {}", e))?; + writeln!(file).map_err(|e| anyhow::anyhow!("Failed to write to shell config: {}", e))?; writeln!(file, "# dela shell integration") - .map_err(|e| format!("Failed to write to shell config: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to write to shell config: {}", e))?; writeln!(file, "{}", integration_pattern) - .map_err(|e| format!("Failed to write to shell config: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to write to shell config: {}", e))?; Ok(()) } -pub fn execute() -> Result<(), String> { +pub fn execute() -> anyhow::Result<()> { println!("Initializing dela..."); // Get the shell config path first to validate shell support @@ -107,7 +110,7 @@ pub fn execute() -> Result<(), String> { config_path.display() ); - get_current_home().ok_or("HOME environment variable not set".to_string())?; + get_current_home().context("HOME environment variable not set")?; let dela_dir = preferred_config_dir_path()?; let legacy_dela_dir = legacy_dela_config_dir()?; @@ -118,11 +121,12 @@ pub fn execute() -> Result<(), String> { dela_dir.display() ); if let Some(parent) = dela_dir.parent() { - fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create dela config parent directory: {}", e))?; + fs::create_dir_all(parent).map_err(|e| { + anyhow::anyhow!("Failed to create dela config parent directory: {}", e) + })?; } fs::rename(&legacy_dela_dir, &dela_dir).map_err(|e| { - format!( + anyhow::anyhow!( "Failed to migrate dela configuration to {}: {}", dela_dir.display(), e @@ -136,7 +140,7 @@ pub fn execute() -> Result<(), String> { dela_dir.display() ); fs::create_dir_all(&dela_dir) - .map_err(|e| format!("Failed to create dela config directory: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to create dela config directory: {}", e))?; } else { println!( "Using existing dela configuration directory at {}", @@ -150,9 +154,9 @@ pub fn execute() -> Result<(), String> { println!("Creating empty allowlist at {}", allowlist_path.display()); let empty_allowlist = Allowlist::default(); let toml = toml::to_string_pretty(&empty_allowlist) - .map_err(|e| format!("Failed to serialize empty allowlist: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to serialize empty allowlist: {}", e))?; fs::write(&allowlist_path, toml) - .map_err(|e| format!("Failed to create allowlist file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to create allowlist file: {}", e))?; } // Add shell integration @@ -262,7 +266,10 @@ mod tests { let result = execute(); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Unsupported shell: unsupported"); + assert_eq!( + result.unwrap_err().to_string(), + "Unsupported shell: unsupported" + ); reset_to_real_environment(); } diff --git a/src/commands/list.rs b/src/commands/list.rs index 8ff7939..348b245 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -17,9 +17,9 @@ macro_rules! test_println { ($($arg:tt)*) => { println!($($arg)*) }; } -pub fn execute(verbose: bool) -> Result<(), String> { - let current_dir = - env::current_dir().map_err(|e| format!("Failed to get current directory: {}", e))?; +pub fn execute(verbose: bool) -> anyhow::Result<()> { + let current_dir = env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))?; let discovered = task_discovery::discover_tasks(¤t_dir); // Only show task definition files status in verbose mode @@ -188,8 +188,8 @@ pub fn execute(verbose: bool) -> Result<(), String> { Box::new(std::io::stdout()) }; - let mut write_line = |line: &str| -> Result<(), String> { - writeln!(writer, "{}", line).map_err(|e| format!("Failed to write output: {}", e)) + let mut write_line = |line: &str| -> anyhow::Result<()> { + writeln!(writer, "{}", line).map_err(|e| anyhow::anyhow!("Failed to write output: {}", e)) }; // Group tasks by runner for the new format diff --git a/src/commands/mcp.rs b/src/commands/mcp.rs index fa35aba..ff0c2f0 100644 --- a/src/commands/mcp.rs +++ b/src/commands/mcp.rs @@ -1,4 +1,5 @@ use crate::mcp; +use anyhow::Context; use std::fs; use std::path::PathBuf; @@ -106,13 +107,13 @@ impl Editor { } /// Merge dela into an existing JSON config file (Cursor, VSCode, Gemini, Claude Code) -fn merge_dela_into_json(editor: Editor, existing: &str) -> Result { +fn merge_dela_into_json(editor: Editor, existing: &str) -> anyhow::Result { let mut root: serde_json::Value = serde_json::from_str(existing) - .map_err(|e| format!("Failed to parse config as JSON: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to parse config as JSON: {}", e))?; let obj = root .as_object_mut() - .ok_or_else(|| "Config file is not a JSON object".to_string())?; + .context("Config file is not a JSON object")?; let key = editor.servers_key(); if !obj.contains_key(key) { @@ -125,20 +126,20 @@ fn merge_dela_into_json(editor: Editor, existing: &str) -> Result Result { - let mut table: toml::Table = - toml::from_str(existing).map_err(|e| format!("Failed to parse config as TOML: {}", e))?; +fn merge_dela_into_toml(existing: &str) -> anyhow::Result { + let mut table: toml::Table = toml::from_str(existing) + .map_err(|e| anyhow::anyhow!("Failed to parse config as TOML: {}", e))?; if !table.contains_key("mcp_servers") { table.insert( @@ -150,7 +151,7 @@ fn merge_dela_into_toml(existing: &str) -> Result { let mcp_table = table .get_mut("mcp_servers") .and_then(|v| v.as_table_mut()) - .ok_or_else(|| "'mcp_servers' in config is not a table".to_string())?; + .context("'mcp_servers' in config is not a table")?; let mut dela = toml::map::Map::new(); dela.insert( @@ -163,23 +164,23 @@ fn merge_dela_into_toml(existing: &str) -> Result { ); mcp_table.insert("dela".to_string(), toml::Value::Table(dela)); - toml::to_string_pretty(&table).map_err(|e| format!("Failed to serialize config: {}", e)) + toml::to_string_pretty(&table).map_err(|e| anyhow::anyhow!("Failed to serialize config: {}", e)) } /// Generate MCP config file for an editor at a specific path -fn generate_config_at(editor: Editor, config_path: &PathBuf) -> Result<(), String> { +fn generate_config_at(editor: Editor, config_path: &PathBuf) -> anyhow::Result<()> { // Create parent directory if it doesn't exist if let Some(parent) = config_path.parent() && !parent.exists() { fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create {} directory: {}", editor.name(), e))?; + .map_err(|e| anyhow::anyhow!("Failed to create {} directory: {}", editor.name(), e))?; } // Check if config already exists if config_path.exists() { let existing = fs::read_to_string(config_path) - .map_err(|e| format!("Failed to read existing config: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to read existing config: {}", e))?; if existing.contains(editor.dela_marker()) { eprintln!( @@ -199,7 +200,7 @@ fn generate_config_at(editor: Editor, config_path: &PathBuf) -> Result<(), Strin match merged { Ok(content) => { fs::write(config_path, content) - .map_err(|e| format!("Failed to write config file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to write config file: {}", e))?; eprintln!( "✓ Added dela to {} config at {}", editor.name(), @@ -221,7 +222,7 @@ fn generate_config_at(editor: Editor, config_path: &PathBuf) -> Result<(), Strin // Write the config file fs::write(config_path, editor.template()) - .map_err(|e| format!("Failed to write config file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to write config file: {}", e))?; eprintln!( "✓ Created {} config at {}", @@ -233,7 +234,7 @@ fn generate_config_at(editor: Editor, config_path: &PathBuf) -> Result<(), Strin } /// Generate MCP config file for an editor at its default global path -fn generate_config(editor: Editor) -> Result<(), String> { +fn generate_config(editor: Editor) -> anyhow::Result<()> { let config_path = editor.config_path(); generate_config_at(editor, &config_path) } @@ -246,10 +247,11 @@ pub async fn execute( init_codex: bool, init_gemini: bool, init_claude_code: bool, -) -> Result<(), String> { +) -> anyhow::Result<()> { // Resolve the path relative to the current working directory let root_path = if cwd == "." { - std::env::current_dir().map_err(|e| format!("Failed to get current directory: {}", e))? + std::env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))? } else { PathBuf::from(&cwd) }; @@ -277,19 +279,21 @@ pub async fn execute( } crate::allowlist::load_allowlist().map_err(|e| { - crate::mcp::DelaError::mcp_not_ready(format!( - "MCP server cannot start because dela configuration is unavailable: {}", - e - )) - .to_error_data() - .message - .into_owned() + anyhow::anyhow!( + crate::mcp::DelaError::mcp_not_ready(format!( + "MCP server cannot start because dela configuration is unavailable: {}", + e + )) + .to_error_data() + .message + .into_owned() + ) })?; // Start the MCP server mcp::run_stdio_server(root_path) .await - .map_err(|e| e.to_string()) + .map_err(|e| anyhow::anyhow!(e)) } #[cfg(test)] diff --git a/src/commands/run.rs b/src/commands/run.rs index b7fc315..89a1b54 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -1,6 +1,6 @@ use crate::commands::run_command; -pub fn execute(task_name: &str) -> Result<(), String> { +pub fn execute(task_name: &str) -> anyhow::Result<()> { println!("Note: The 'dela run' command is meant to be intercepted by shell integration."); println!("If you're seeing this message, it means either:"); println!("1. Shell integration is not installed (run 'dela init' to set it up)"); diff --git a/src/commands/run_command.rs b/src/commands/run_command.rs index 5c914f6..7f47b58 100644 --- a/src/commands/run_command.rs +++ b/src/commands/run_command.rs @@ -1,20 +1,21 @@ use crate::runner::is_runner_available; use crate::runner::split_command_words; use crate::task_discovery; +use anyhow::Context; use std::env; use std::process::{Command, Stdio}; -pub fn execute(task_with_args: &str) -> Result<(), String> { - let mut invocation_parts = - shell_words::split(task_with_args).map_err(|e| format!("Failed to parse args: {}", e))?; +pub fn execute(task_with_args: &str) -> anyhow::Result<()> { + let mut invocation_parts = shell_words::split(task_with_args) + .map_err(|e| anyhow::anyhow!("Failed to parse args: {}", e))?; let task_name = invocation_parts .first() - .ok_or_else(|| "No task name provided".to_string())? + .context("No task name provided")? .to_string(); let task_args: Vec = invocation_parts.drain(1..).collect(); - let current_dir = - env::current_dir().map_err(|e| format!("Failed to get current directory: {}", e))?; + let current_dir = env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))?; let discovered = task_discovery::discover_tasks(¤t_dir); // Find all tasks with the given name (both original and disambiguated) @@ -22,7 +23,10 @@ pub fn execute(task_with_args: &str) -> Result<(), String> { // Check if there are no matching tasks if matching_tasks.is_empty() { - return Err(format!("dela: command or task not found: {}", task_name)); + return Err(anyhow::anyhow!( + "dela: command or task not found: {}", + task_name + )); } // Check if there are multiple matching tasks @@ -30,13 +34,16 @@ pub fn execute(task_with_args: &str) -> Result<(), String> { let error_msg = task_discovery::format_ambiguous_task_error(task_name.as_str(), &matching_tasks); println!("{}", error_msg); - return Err(format!("Ambiguous task name: '{}'", task_name)); + return Err(anyhow::anyhow!("Ambiguous task name: '{}'", task_name)); } // Single task found, check if runner is available let task = matching_tasks[0]; if !is_runner_available(&task.runner) { - return Err(format!("Runner '{}' not found", task.runner.short_name())); + return Err(anyhow::anyhow!( + "Runner '{}' not found", + task.runner.short_name() + )); } // Get the command to run @@ -45,9 +52,7 @@ pub fn execute(task_with_args: &str) -> Result<(), String> { command_parts.extend(task_args.clone()); let mut parts_iter = command_parts.iter(); - let executable = parts_iter - .next() - .ok_or_else(|| "Empty command generated".to_string())?; + let executable = parts_iter.next().context("Empty command generated")?; let remaining_args: Vec<&String> = parts_iter.collect(); println!("Running: {}", shell_words::join(command_parts.clone())); @@ -59,10 +64,10 @@ pub fn execute(task_with_args: &str) -> Result<(), String> { .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) .status() - .map_err(|e| format!("Failed to execute command: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to execute command: {}", e))?; if !status.success() { - return Err(format!("Command failed with exit code: {}", status)); + return Err(anyhow::anyhow!("Command failed with exit code: {}", status)); } Ok(()) @@ -123,7 +128,7 @@ test: ## Running tests let result = execute("nonexistent"); assert!(result.is_err(), "Should fail when no task found"); assert_eq!( - result.unwrap_err(), + result.unwrap_err().to_string(), "dela: command or task not found: nonexistent" ); @@ -145,7 +150,7 @@ test: ## Running tests let result = execute("test"); assert!(result.is_err(), "Should fail when runner is missing"); - assert_eq!(result.unwrap_err(), "Runner 'make' not found"); + assert_eq!(result.unwrap_err().to_string(), "Runner 'make' not found"); reset_mock(); reset_to_real_environment(); @@ -189,7 +194,10 @@ test: ## Running tests let result = execute("test"); assert!(result.is_err(), "Should fail with ambiguous task name"); assert!( - result.unwrap_err().contains("Ambiguous task name: 'test'"), + result + .unwrap_err() + .to_string() + .contains("Ambiguous task name: 'test'"), "Error should mention ambiguous task name" ); @@ -221,10 +229,7 @@ test: ## Running tests let tasks = task_discovery::get_matching_tasks(&discovered, "test"); assert_eq!(tasks.len(), 1, "Should find exactly one task"); - // Instead of trying to execute the command, which causes make to print help output, - // we'll just mock the behavior we expect - that the command would be constructed - // correctly but would fail in the test environment. - let result: Result<(), String> = Err("Command failed with exit code: 127".to_string()); + let result = super::execute("test --invalid-arg-for-make"); assert!( result.is_err(), "Command execution should fail in test environment" @@ -273,7 +278,10 @@ test: ## Running tests let result = execute("test"); assert!(result.is_err(), "Should fail with ambiguous task name"); assert!( - result.unwrap_err().contains("Ambiguous task name: 'test'"), + result + .unwrap_err() + .to_string() + .contains("Ambiguous task name: 'test'"), "Error should mention ambiguous task name" ); diff --git a/src/config.rs b/src/config.rs index 7d5771c..9dfdaf8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,12 @@ use crate::environment::get_current_home; use std::path::{Path, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum ConfigError { + #[error("HOME environment variable not set")] + HomeNotSet, +} pub fn preferred_config_dir_path_for(home: impl AsRef) -> PathBuf { home.as_ref().join(".config").join("dela") @@ -14,22 +21,22 @@ pub fn legacy_allowlist_path_for(home: impl AsRef) -> PathBuf { home.as_ref().join(".dela").join("allowlist.toml") } -pub fn preferred_config_dir_path() -> Result { - let home = get_current_home().ok_or("HOME environment variable not set".to_string())?; +pub fn preferred_config_dir_path() -> Result { + let home = get_current_home().ok_or(ConfigError::HomeNotSet)?; Ok(preferred_config_dir_path_for(PathBuf::from(home))) } -pub fn legacy_dela_config_dir() -> Result { - let home = get_current_home().ok_or("HOME environment variable not set".to_string())?; +pub fn legacy_dela_config_dir() -> Result { + let home = get_current_home().ok_or(ConfigError::HomeNotSet)?; Ok(PathBuf::from(home).join(".dela")) } -pub fn legacy_allowlist_path() -> Result { - let home = get_current_home().ok_or("HOME environment variable not set".to_string())?; +pub fn legacy_allowlist_path() -> Result { + let home = get_current_home().ok_or(ConfigError::HomeNotSet)?; Ok(legacy_allowlist_path_for(PathBuf::from(home))) } -pub fn active_dela_config_dir() -> Result { +pub fn active_dela_config_dir() -> Result { let dela_dir = preferred_config_dir_path()?; if dela_dir.exists() { return Ok(dela_dir); @@ -43,11 +50,11 @@ pub fn active_dela_config_dir() -> Result { Ok(dela_dir) } -pub fn preferred_allowlist_path() -> Result { +pub fn preferred_allowlist_path() -> Result { Ok(preferred_config_dir_path()?.join("allowlist.toml")) } -pub fn active_allowlist_path() -> Result { +pub fn active_allowlist_path() -> Result { let preferred_path = preferred_allowlist_path()?; if preferred_path.exists() { return Ok(preferred_path); diff --git a/src/main.rs b/src/main.rs index e1a8ea4..018420b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -184,7 +184,7 @@ async fn main() { Commands::Run { task } => commands::run::execute(&task), Commands::GetCommand { args } => { if args.is_empty() { - Err("No task name provided".to_string()) + Err(anyhow::anyhow!("No task name provided")) } else { commands::get_command::execute(&args.join(" ")) } @@ -193,7 +193,10 @@ async fn main() { }; if let Err(err) = result { - if err.starts_with("dela: command or task not found") { + if err + .to_string() + .starts_with("dela: command or task not found") + { eprintln!("{}", err); } else { eprintln!("Error: {}", err); diff --git a/src/mcp/allowlist.rs b/src/mcp/allowlist.rs index 0074d26..95fb1f9 100644 --- a/src/mcp/allowlist.rs +++ b/src/mcp/allowlist.rs @@ -15,7 +15,7 @@ pub struct McpAllowlistEvaluator { impl McpAllowlistEvaluator { /// Create a new MCP allowlist evaluator by loading the allowlist from disk - pub fn new() -> Result { + pub fn new() -> anyhow::Result { let allowlist = load_allowlist()?; Ok(Self { allowlist }) } @@ -33,7 +33,7 @@ impl McpAllowlistEvaluator { /// 3. File scope allow entries /// 4. Task scope allow entries /// 5. Not found in allowlist (deny by default for MCP) - pub fn is_task_allowed(&self, task: &Task) -> Result { + pub fn is_task_allowed(&self, task: &Task) -> anyhow::Result { Ok(matches!( evaluate_task_against_allowlist(task, &self.allowlist), AllowlistMatch::Allowed diff --git a/src/mcp/job_manager.rs b/src/mcp/job_manager.rs index 7482f72..7f2df0f 100644 --- a/src/mcp/job_manager.rs +++ b/src/mcp/job_manager.rs @@ -266,12 +266,12 @@ impl JobManager { } /// Check if we can start a new job (concurrency limit check) - pub async fn can_start_job(&self) -> Result<(), String> { + pub async fn can_start_job(&self) -> anyhow::Result<()> { self.garbage_collect().await; let jobs = self.jobs.read().await; let running_jobs = jobs.values().filter(|job| job.is_running()).count(); if running_jobs >= self.config.max_concurrent_jobs { - return Err(format!( + return Err(anyhow::anyhow!( "Maximum concurrent jobs limit reached: {}", self.config.max_concurrent_jobs )); @@ -285,19 +285,19 @@ impl JobManager { pid: u32, metadata: JobMetadata, process: Child, - ) -> Result<(), String> { + ) -> anyhow::Result<()> { let mut jobs = self.jobs.write().await; // Check concurrent job limit if jobs.len() >= self.config.max_concurrent_jobs { - return Err(format!( + return Err(anyhow::anyhow!( "Maximum concurrent jobs limit reached: {}", self.config.max_concurrent_jobs )); } if matches!(jobs.get(&pid), Some(existing) if existing.is_running()) { - return Err(format!( + return Err(anyhow::anyhow!( "Refusing to overwrite running job with PID {}", pid )); @@ -326,10 +326,10 @@ impl JobManager { pid: u32, metadata: JobMetadata, state: JobState, - ) -> Result<(), String> { + ) -> anyhow::Result<()> { let mut jobs = self.jobs.write().await; if matches!(jobs.get(&pid), Some(existing) if existing.is_running()) { - return Err(format!( + return Err(anyhow::anyhow!( "Refusing to overwrite running job with PID {}", pid )); @@ -381,7 +381,7 @@ impl JobManager { } /// Update a job's state - pub async fn update_job_state(&self, pid: u32, state: JobState) -> Result<(), String> { + pub async fn update_job_state(&self, pid: u32, state: JobState) -> anyhow::Result<()> { let mut jobs = self.jobs.write().await; if let Some(job) = jobs.get_mut(&pid) { match state { @@ -396,24 +396,24 @@ impl JobManager { } Ok(()) } else { - Err(format!("Job with PID {} not found", pid)) + Err(anyhow::anyhow!("Job with PID {} not found", pid)) } } /// Add output to a job - pub async fn add_job_output(&self, pid: u32, output: String) -> Result<(), String> { + pub async fn add_job_output(&self, pid: u32, output: String) -> anyhow::Result<()> { let mut jobs = self.jobs.write().await; if let Some(job) = jobs.get_mut(&pid) { job.add_output(output); Ok(()) } else { - Err(format!("Job with PID {} not found", pid)) + Err(anyhow::anyhow!("Job with PID {} not found", pid)) } } /// Stop a job (send SIGTERM) #[allow(dead_code)] - pub async fn stop_job(&self, pid: u32) -> Result<(), String> { + pub async fn stop_job(&self, pid: u32) -> anyhow::Result<()> { let mut processes = self.processes.write().await; if let Some(mut process) = processes.remove(&pid) { if let Err(e) = process.kill().await { @@ -431,7 +431,7 @@ impl JobManager { } Ok(()) } else { - Err(format!("Job with PID {} not found", pid)) + Err(anyhow::anyhow!("Job with PID {} not found", pid)) } } @@ -440,7 +440,7 @@ impl JobManager { &self, pid: u32, grace_period_seconds: u64, - ) -> Result { + ) -> anyhow::Result { use tokio::time::{Duration, timeout}; // First, try to get the process from our managed processes @@ -455,7 +455,7 @@ impl JobManager { Ok(()) => {} Err(nix::errno::Errno::ESRCH) => { let exit_status = process.wait().await.map_err(|e| { - format!("Process already exited but wait failed: {}", e) + anyhow::anyhow!("Process already exited but wait failed: {}", e) })?; let exit_code = exit_status.code().unwrap_or(0); let mut jobs = self.jobs.write().await; @@ -614,7 +614,7 @@ impl JobManager { /// Remove a job #[allow(dead_code)] - pub async fn remove_job(&self, pid: u32) -> Result<(), String> { + pub async fn remove_job(&self, pid: u32) -> anyhow::Result<()> { let mut jobs = self.jobs.write().await; let mut processes = self.processes.write().await; @@ -624,7 +624,7 @@ impl JobManager { if job_removed || process_removed { Ok(()) } else { - Err(format!("Job with PID {} not found", pid)) + Err(anyhow::anyhow!("Job with PID {} not found", pid)) } } @@ -1008,6 +1008,7 @@ mod tests { assert!( result .unwrap_err() + .to_string() .contains("Refusing to overwrite running job") ); diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index f3ddbad..0f7de7d 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -4,12 +4,6 @@ mod errors; mod job_manager; mod server; -// Re-export DTO types for public API consumers -#[allow(unused_imports)] -pub use dto::{ - ListTasksArgs, StartResultDto, TaskOutputArgs, TaskStartArgs, TaskStatusArgs, TaskStopArgs, -}; -#[allow(unused_imports)] pub use errors::DelaError; pub use server::DelaMcpServer; diff --git a/src/mcp/server.rs b/src/mcp/server.rs index e16dc92..5444bd8 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -2537,8 +2537,12 @@ mod tests { // Assert assert!(result.is_err()); let error = result.unwrap_err(); - assert!(error.contains("Maximum concurrent jobs limit reached")); - assert!(error.contains("2")); + assert!( + error + .to_string() + .contains("Maximum concurrent jobs limit reached") + ); + assert!(error.to_string().contains("2")); } #[tokio::test] diff --git a/src/parsers/errors.rs b/src/parsers/errors.rs new file mode 100644 index 0000000..2518f1a --- /dev/null +++ b/src/parsers/errors.rs @@ -0,0 +1,25 @@ +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum DelaParseError { + #[error("Regex error: {0}")] + Regex(#[from] regex::Error), + + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + + #[error("JSON parsing error: {0}")] + Json(#[from] serde_json::Error), + + #[error("YAML parsing error: {0}")] + Yaml(#[from] serde_yaml::Error), + + #[error("TOML parsing error: {0}")] + Toml(#[from] toml::de::Error), + + #[error("XML parsing error: {0}")] + Xml(#[from] roxmltree::Error), + + #[error("Syntax error: {0}")] + Syntax(String), +} diff --git a/src/parsers/mod.rs b/src/parsers/mod.rs index c630f75..1b06b25 100644 --- a/src/parsers/mod.rs +++ b/src/parsers/mod.rs @@ -23,3 +23,5 @@ pub use parse_pyproject_toml::parse as parse_pyproject_toml; pub use parse_taskfile::parse as parse_taskfile; pub use parse_travis_ci::parse as parse_travis_ci; pub use parse_turbo_json::parse as parse_turbo_json; + +pub mod errors; diff --git a/src/parsers/parse_cmake.rs b/src/parsers/parse_cmake.rs index 41e9c95..d46d4d6 100644 --- a/src/parsers/parse_cmake.rs +++ b/src/parsers/parse_cmake.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use regex::Regex; use std::fs::File; @@ -8,17 +9,16 @@ use std::path::Path; /// /// This function parses a CMakeLists.txt file and extracts each custom target /// as a separate task. It uses regex patterns to find add_custom_target() calls. -pub fn parse(file_path: &Path) -> Result, String> { - let mut file = File::open(file_path).map_err(|e| format!("Failed to open file: {}", e))?; +pub fn parse(file_path: &Path) -> Result, DelaParseError> { + let mut file = File::open(file_path)?; let mut contents = String::new(); - file.read_to_string(&mut contents) - .map_err(|e| format!("Failed to read file: {}", e))?; + file.read_to_string(&mut contents)?; parse_cmake_string(&contents, file_path) } /// Parse CMakeLists.txt content from a string -fn parse_cmake_string(content: &str, file_path: &Path) -> Result, String> { +fn parse_cmake_string(content: &str, file_path: &Path) -> Result, DelaParseError> { let mut tasks = Vec::new(); // First, let's normalize the content by removing comments and extra whitespace @@ -36,11 +36,9 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result, Stri .join("\n"); // Use a simpler regex that just finds the target names - let target_pattern = Regex::new(r#"add_custom_target\s*\(\s*([a-zA-Z_][a-zA-Z0-9_-]*)"#) - .map_err(|e| format!("Failed to compile regex: {}", e))?; + let target_pattern = Regex::new(r#"add_custom_target\s*\(\s*([a-zA-Z_][a-zA-Z0-9_-]*)"#)?; - let comment_pattern = Regex::new(r#"COMMENT\s+"([^"]*)"#) - .map_err(|e| format!("Failed to compile comment regex: {}", e))?; + let comment_pattern = Regex::new(r#"COMMENT\s+"([^"]*)"#)?; // Find all matches in the content for captures in target_pattern.captures_iter(&normalized_content) { @@ -220,7 +218,7 @@ add_custom_target(build COMMENT "Build the project") let result = parse(&cmake_path); assert!(result.is_err()); - assert!(result.unwrap_err().contains("Failed to open file")); + assert!(result.unwrap_err().to_string().contains("I/O error")); } #[test] diff --git a/src/parsers/parse_docker_compose.rs b/src/parsers/parse_docker_compose.rs index 0ecde5c..6672063 100644 --- a/src/parsers/parse_docker_compose.rs +++ b/src/parsers/parse_docker_compose.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -31,17 +32,10 @@ struct DockerCompose { } /// Parse a docker-compose.yml file at the given path and extract services as tasks -pub fn parse(path: &PathBuf) -> Result, String> { - let file_name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("docker-compose.yml"); +pub fn parse(path: &PathBuf) -> Result, DelaParseError> { + let contents = std::fs::read_to_string(path)?; - let contents = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read {}: {}", file_name, e))?; - - let docker_compose: DockerCompose = serde_yaml::from_str(&contents) - .map_err(|e| format!("Failed to parse {}: {}", file_name, e))?; + let docker_compose: DockerCompose = serde_yaml::from_str(&contents)?; let mut tasks = Vec::new(); @@ -267,7 +261,12 @@ invalid: yaml: here let result = parse(&temp_dir.path().join("docker-compose.yml")); assert!(result.is_err()); // YAML should fail to parse with invalid structure - assert!(result.unwrap_err().contains("Failed to parse")); + assert!( + result + .unwrap_err() + .to_string() + .contains("YAML parsing error") + ); } #[test] @@ -275,7 +274,7 @@ invalid: yaml: here let temp_dir = TempDir::new().unwrap(); let result = parse(&temp_dir.path().join("docker-compose.yml")); assert!(result.is_err()); - assert!(result.unwrap_err().contains("Failed to read")); + assert!(result.unwrap_err().to_string().contains("I/O error")); } #[test] diff --git a/src/parsers/parse_github_actions.rs b/src/parsers/parse_github_actions.rs index 9510246..07ba5c9 100644 --- a/src/parsers/parse_github_actions.rs +++ b/src/parsers/parse_github_actions.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde_yaml::Value; use std::fs::File; @@ -8,23 +9,25 @@ use std::path::Path; /// /// This function parses a GitHub Actions workflow file and extracts the entire workflow as a single task. /// The tasks can be executed using the `act` command-line tool. -pub fn parse(file_path: &Path) -> Result, String> { - let mut file = File::open(file_path).map_err(|e| format!("Failed to open file: {}", e))?; +pub fn parse(file_path: &Path) -> Result, DelaParseError> { + let mut file = File::open(file_path)?; let mut contents = String::new(); - file.read_to_string(&mut contents) - .map_err(|e| format!("Failed to read file: {}", e))?; + file.read_to_string(&mut contents)?; parse_workflow_string(&contents, file_path) } /// Parse GitHub Actions workflow content from a string -fn parse_workflow_string(content: &str, file_path: &Path) -> Result, String> { - let workflow: Value = serde_yaml::from_str(content) - .map_err(|e| format!("Failed to parse workflow YAML: {}", e))?; +fn parse_workflow_string(content: &str, file_path: &Path) -> Result, DelaParseError> { + let workflow: Value = serde_yaml::from_str(content)?; let workflow_map = match workflow { Value::Mapping(map) => map, - _ => return Err("Workflow YAML is not a mapping".to_string()), + _ => { + return Err(DelaParseError::Syntax( + "Workflow YAML is not a mapping".to_string(), + )); + } }; // Try to get workflow name for description @@ -38,11 +41,17 @@ fn parse_workflow_string(content: &str, file_path: &Path) -> Result, S // Extract jobs to confirm the workflow is valid let jobs = match workflow_map.get(Value::String("jobs".to_string())) { Some(Value::Mapping(jobs_map)) => jobs_map, - _ => return Err("No jobs found in workflow file".to_string()), + _ => { + return Err(DelaParseError::Syntax( + "No jobs found in workflow file".to_string(), + )); + } }; if jobs.is_empty() { - return Err("Workflow contains no jobs".to_string()); + return Err(DelaParseError::Syntax( + "Workflow contains no jobs".to_string(), + )); } // Extract filename without path for task name @@ -328,6 +337,6 @@ on: [push] let result = parse(&file_path); assert!(result.is_err()); - assert!(result.unwrap_err().contains("No jobs found")); + assert!(result.unwrap_err().to_string().contains("No jobs found")); } } diff --git a/src/parsers/parse_gradle.rs b/src/parsers/parse_gradle.rs index 62c1ba8..755bb80 100644 --- a/src/parsers/parse_gradle.rs +++ b/src/parsers/parse_gradle.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use regex::Regex; use std::fs; use std::path::Path; @@ -5,10 +6,9 @@ use std::path::Path; use crate::types::{Task, TaskDefinitionType, TaskRunner}; /// Parse a Gradle build file (build.gradle or build.gradle.kts) and extract tasks -pub fn parse(file_path: &Path) -> Result, String> { +pub fn parse(file_path: &Path) -> Result, DelaParseError> { // Read the file - let content = fs::read_to_string(file_path) - .map_err(|e| format!("Failed to read Gradle build file: {}", e))?; + let content = fs::read_to_string(file_path)?; // Parse tasks using regex patterns for both Groovy and Kotlin DSL let mut tasks = Vec::new(); @@ -64,18 +64,15 @@ fn extract_custom_tasks( content: &str, tasks: &mut Vec, file_path: &Path, -) -> Result<(), String> { +) -> Result<(), DelaParseError> { // Look for task definitions in Groovy DSL (build.gradle) - let groovy_task_regex = Regex::new(r"task\s+(\w+)(?:\s*\{|\s*\(|\s+.*?\{)") - .map_err(|e| format!("Failed to compile regex: {}", e))?; + let groovy_task_regex = Regex::new(r"task\s+(\w+)(?:\s*\{|\s*\(|\s+.*?\{)")?; // Regular expressions for finding tasks in Gradle Kotlin DSL files - let kotlin_task_regex = Regex::new(r#"tasks\s*\.\s*register\s*<.*>\s*\(\s*"(\w+)"\s*\)"#) - .map_err(|e| format!("Failed to compile Kotlin task regex: {}", e))?; + let kotlin_task_regex = Regex::new(r#"tasks\s*\.\s*register\s*<.*>\s*\(\s*"(\w+)"\s*\)"#)?; // Alternative Kotlin syntax: task("taskName") - let kotlin_task_alt_regex = Regex::new(r#"task\s*\(\s*"(\w+)"\s*\)"#) - .map_err(|e| format!("Failed to compile alternative Kotlin task regex: {}", e))?; + let kotlin_task_alt_regex = Regex::new(r#"task\s*\(\s*"(\w+)"\s*\)"#)?; // Process Groovy-style tasks for cap in groovy_task_regex.captures_iter(content) { @@ -178,7 +175,7 @@ fn extract_plugin_tasks( content: &str, tasks: &mut Vec, file_path: &Path, -) -> Result<(), String> { +) -> Result<(), DelaParseError> { // Common plugins and their tasks let plugins = [ ( @@ -203,14 +200,11 @@ fn extract_plugin_tasks( ]; // Regular expressions to extract plugin information - let apply_plugin_regex = Regex::new(r#"apply\s+plugin\s*:\s*['"]([^'"]+)['"]"#) - .map_err(|e| format!("Failed to compile apply plugin regex: {}", e))?; + let apply_plugin_regex = Regex::new(r#"apply\s+plugin\s*:\s*['"]([^'"]+)['"]"#)?; - let plugins_id_regex = Regex::new(r#"plugins\s*\{\s*.*?id\s*\(\s*["']([^"']+)["']\s*\)"#) - .map_err(|e| format!("Failed to compile plugins id regex: {}", e))?; + let plugins_id_regex = Regex::new(r#"plugins\s*\{\s*.*?id\s*\(\s*["']([^"']+)["']\s*\)"#)?; - let plugins_id_alt_regex = Regex::new(r#"plugins\s*\{\s*.*?id\s*["']([^"']+)["']"#) - .map_err(|e| format!("Failed to compile alternative plugins id regex: {}", e))?; + let plugins_id_alt_regex = Regex::new(r#"plugins\s*\{\s*.*?id\s*["']([^"']+)["']"#)?; // Identify plugins used in the build file let mut identified_plugins = Vec::new(); diff --git a/src/parsers/parse_justfile.rs b/src/parsers/parse_justfile.rs index ce2f7a3..084bbdf 100644 --- a/src/parsers/parse_justfile.rs +++ b/src/parsers/parse_justfile.rs @@ -1,16 +1,16 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use regex::Regex; use std::path::PathBuf; /// Parse a Justfile at the given path and extract tasks -pub fn parse(path: &PathBuf) -> Result, String> { +pub fn parse(path: &PathBuf) -> Result, DelaParseError> { let file_name = path .file_name() .and_then(|n| n.to_str()) .unwrap_or("Justfile"); - let contents = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read {}: {}", file_name, e))?; + let contents = std::fs::read_to_string(path)?; let mut tasks = Vec::new(); let lines: Vec<&str> = contents.lines().collect(); @@ -39,7 +39,16 @@ pub fn parse(path: &PathBuf) -> Result, String> { // Validate indentation for this recipe if let Err(indent_error) = validate_recipe_indentation(&lines, line_num + 1) { - return Err(format!("{}: {}", file_name, indent_error)); + return match indent_error { + DelaParseError::Syntax(inner_msg) => Err(DelaParseError::Syntax(format!( + "{}: {}", + file_name, inner_msg + ))), + _ => Err(DelaParseError::Syntax(format!( + "{}: {}", + file_name, indent_error + ))), + }; } tasks.push(Task { @@ -60,7 +69,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { } /// Validate that a recipe's lines use consistent indentation -fn validate_recipe_indentation(lines: &[&str], task_line_num: usize) -> Result<(), String> { +fn validate_recipe_indentation(lines: &[&str], task_line_num: usize) -> Result<(), DelaParseError> { let mut recipe_lines = Vec::new(); let mut current_line = task_line_num; @@ -97,10 +106,10 @@ fn validate_recipe_indentation(lines: &[&str], task_line_num: usize) -> Result<( for (line_num, line) in recipe_lines.iter().skip(1) { let indent_type = get_indentation_type(line); if indent_type != first_indent_type { - return Err(format!( + return Err(DelaParseError::Syntax(format!( "line {}: mixed indentation in recipe - found both spaces and tabs", line_num - )); + ))); } } @@ -669,7 +678,12 @@ build: # Build the project let result = parse(&justfile_path); assert!(result.is_err()); - assert!(result.unwrap_err().contains("mixed indentation in recipe")); + assert!( + result + .unwrap_err() + .to_string() + .contains("mixed indentation in recipe") + ); } #[test] @@ -692,7 +706,12 @@ build: # Build the project let result = parse(&justfile_path); assert!(result.is_err()); - assert!(result.unwrap_err().contains("mixed indentation in recipe")); + assert!( + result + .unwrap_err() + .to_string() + .contains("mixed indentation in recipe") + ); } #[test] diff --git a/src/parsers/parse_makefile.rs b/src/parsers/parse_makefile.rs index 573fdad..7450a9d 100644 --- a/src/parsers/parse_makefile.rs +++ b/src/parsers/parse_makefile.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use makefile_lossless::Makefile; use regex::Regex; @@ -12,13 +13,12 @@ pub struct MakefileInclude { } /// Parse a Makefile at the given path and extract tasks -pub fn parse(path: &Path) -> Result, String> { - let content = - std::fs::read_to_string(path).map_err(|e| format!("Failed to read Makefile: {}", e))?; +pub fn parse(path: &Path) -> Result, DelaParseError> { + let content = std::fs::read_to_string(path)?; // Special case for the test_discover_tasks_with_invalid_makefile test if content.contains("not a make file") { - return Err("Failed to parse Makefile: Invalid syntax".to_string()); + return Err(DelaParseError::Syntax("Invalid syntax".to_string())); } // Special case for testing regex parsing - look for a marker in the content @@ -33,14 +33,17 @@ pub fn parse(path: &Path) -> Result, String> { // If standard parsing fails, try regex-based parsing as fallback match extract_tasks_regex(&content, path) { Ok(tasks) => Ok(tasks), - Err(_) => Err(format!("Failed to parse Makefile: {}", e)), + Err(_) => Err(DelaParseError::Syntax(format!( + "Failed to parse Makefile: {}", + e + ))), } } } } /// Extract tasks from a parsed Makefile -fn extract_tasks(makefile: &Makefile, path: &Path) -> Result, String> { +fn extract_tasks(makefile: &Makefile, path: &Path) -> Result, DelaParseError> { // Use a HashMap to track tasks by name to avoid duplicates let mut tasks_map: HashMap = HashMap::new(); @@ -95,9 +98,8 @@ fn extract_tasks(makefile: &Makefile, path: &Path) -> Result, String> } /// Extract Makefile include directives from a file. -pub fn extract_include_directives(path: &Path) -> Result, String> { - let content = - std::fs::read_to_string(path).map_err(|e| format!("Failed to read Makefile: {}", e))?; +pub fn extract_include_directives(path: &Path) -> Result, DelaParseError> { + let content = std::fs::read_to_string(path)?; Ok(extract_include_directives_from_str(&content)) } @@ -224,7 +226,7 @@ fn contains_dynamic_make_syntax(token: &str) -> bool { } /// Extract tasks using regex as a fallback method when standard parsing fails -fn extract_tasks_regex(content: &str, path: &Path) -> Result, String> { +fn extract_tasks_regex(content: &str, path: &Path) -> Result, DelaParseError> { let mut tasks_map: HashMap = HashMap::new(); // Pre-process content to handle line continuations @@ -233,8 +235,7 @@ fn extract_tasks_regex(content: &str, path: &Path) -> Result, String> // Simple rule pattern to match task names // Matches start of line, then allowed target characters, then a colon, then the rest of the line let rule_pattern = r"(?m)^([a-zA-Z0-9_-][^:$\n]*?):([^\n]*)"; - let rule_regex = - Regex::new(rule_pattern).map_err(|e| format!("Failed to create regex: {}", e))?; + let rule_regex = Regex::new(rule_pattern)?; for cap in rule_regex.captures_iter(&processed_content) { if cap.len() < 3 { diff --git a/src/parsers/parse_package_json.rs b/src/parsers/parse_package_json.rs index d43f1a4..d53ccbe 100644 --- a/src/parsers/parse_package_json.rs +++ b/src/parsers/parse_package_json.rs @@ -1,13 +1,12 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType}; use std::path::PathBuf; /// Parse a package.json file at the given path and extract tasks -pub fn parse(path: &PathBuf) -> Result, String> { - let contents = - std::fs::read_to_string(path).map_err(|e| format!("Failed to read package.json: {}", e))?; +pub fn parse(path: &PathBuf) -> Result, DelaParseError> { + let contents = std::fs::read_to_string(path)?; - let json: serde_json::Value = serde_json::from_str(&contents) - .map_err(|e| format!("Failed to parse package.json: {}", e))?; + let json: serde_json::Value = serde_json::from_str(&contents)?; let parent = path.parent().unwrap_or(path); let runner = match crate::runners::runners_package_json::detect_package_manager(parent) { diff --git a/src/parsers/parse_pom_xml.rs b/src/parsers/parse_pom_xml.rs index 92270b7..5a8659d 100644 --- a/src/parsers/parse_pom_xml.rs +++ b/src/parsers/parse_pom_xml.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use roxmltree::{Document, Node}; use std::fs; use std::path::Path; @@ -5,13 +6,12 @@ use std::path::Path; use crate::types::{Task, TaskDefinitionType, TaskRunner}; /// Parse a Maven pom.xml file and return a list of tasks -pub fn parse(file_path: &Path) -> Result, String> { +pub fn parse(file_path: &Path) -> Result, DelaParseError> { // Read the file - let content = - fs::read_to_string(file_path).map_err(|e| format!("Error reading pom.xml file: {}", e))?; + let content = fs::read_to_string(file_path)?; // Parse the XML - let doc = Document::parse(&content).map_err(|e| format!("Error parsing pom.xml: {}", e))?; + let doc = Document::parse(&content)?; let root = doc.root_element(); @@ -53,7 +53,11 @@ fn add_default_maven_goals(tasks: &mut Vec, file_path: &Path) { } /// Add tasks from Maven profiles -fn add_profile_tasks(tasks: &mut Vec, root: Node, file_path: &Path) -> Result<(), String> { +fn add_profile_tasks( + tasks: &mut Vec, + root: Node, + file_path: &Path, +) -> Result<(), DelaParseError> { // Find section if let Some(profiles_node) = root.children().find(|n| n.has_tag_name("profiles")) { // Iterate over each profile @@ -84,7 +88,11 @@ fn add_profile_tasks(tasks: &mut Vec, root: Node, file_path: &Path) -> Res } /// Add tasks from Maven plugins -fn add_plugin_tasks(tasks: &mut Vec, root: Node, file_path: &Path) -> Result<(), String> { +fn add_plugin_tasks( + tasks: &mut Vec, + root: Node, + file_path: &Path, +) -> Result<(), DelaParseError> { // Find section and then if let Some(build_node) = root.children().find(|n| n.has_tag_name("build")) && let Some(plugins_node) = build_node.children().find(|n| n.has_tag_name("plugins")) diff --git a/src/parsers/parse_pyproject_toml.rs b/src/parsers/parse_pyproject_toml.rs index b6368a8..84a00ab 100644 --- a/src/parsers/parse_pyproject_toml.rs +++ b/src/parsers/parse_pyproject_toml.rs @@ -1,14 +1,13 @@ +use crate::parsers::errors::DelaParseError; use crate::task_shadowing::check_path_executable; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use std::path::Path; /// Parse a pyproject.toml file at the given path and extract tasks -pub fn parse(path: &Path) -> Result, String> { - let content = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read pyproject.toml: {}", e))?; +pub fn parse(path: &Path) -> Result, DelaParseError> { + let content = std::fs::read_to_string(path)?; - let toml: toml::Value = - toml::from_str(&content).map_err(|e| format!("Failed to parse pyproject.toml: {}", e))?; + let toml: toml::Value = toml::from_str(&content)?; let mut tasks = Vec::new(); diff --git a/src/parsers/parse_taskfile.rs b/src/parsers/parse_taskfile.rs index a3296f7..4315b64 100644 --- a/src/parsers/parse_taskfile.rs +++ b/src/parsers/parse_taskfile.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -103,7 +104,7 @@ pub fn resolve_taskfile_include_path(candidate: &Path) -> PathBuf { } /// Parse a Taskfile.yml file at the given path and extract tasks -pub fn parse(path: &Path) -> Result, String> { +pub fn parse(path: &Path) -> Result, DelaParseError> { let taskfile = load_taskfile(path)?; let mut task_entries: Vec<_> = taskfile.tasks.into_iter().collect(); task_entries.sort_by(|a, b| a.0.cmp(&b.0)); @@ -148,7 +149,7 @@ pub fn parse(path: &Path) -> Result, String> { Ok(tasks) } -pub fn extract_include_directives(path: &Path) -> Result, String> { +pub fn extract_include_directives(path: &Path) -> Result, DelaParseError> { let taskfile = load_taskfile(path)?; let mut include_entries: Vec<_> = taskfile.includes.into_iter().collect(); include_entries.sort_by(|a, b| a.0.cmp(&b.0)); @@ -185,17 +186,10 @@ pub fn extract_include_directives(path: &Path) -> Result, S Ok(includes) } -fn load_taskfile(path: &Path) -> Result { - let file_name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("Taskfile"); +fn load_taskfile(path: &Path) -> Result { + let contents = std::fs::read_to_string(path)?; - let contents = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read {}: {}", file_name, e))?; - - let taskfile = serde_yaml::from_str(&contents) - .map_err(|e| format!("Failed to parse {}: {}", file_name, e))?; + let taskfile = serde_yaml::from_str(&contents)?; Ok(taskfile) } diff --git a/src/parsers/parse_travis_ci.rs b/src/parsers/parse_travis_ci.rs index d8149bb..f45e367 100644 --- a/src/parsers/parse_travis_ci.rs +++ b/src/parsers/parse_travis_ci.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde_yaml::Value; use std::fs::File; @@ -8,27 +9,29 @@ use std::path::Path; /// /// This function parses a .travis.yml file and extracts each job as a separate task. /// Note: Travis CI tasks are listed for discovery but cannot be executed locally. -pub fn parse(file_path: &Path) -> Result, String> { - let mut file = File::open(file_path).map_err(|e| format!("Failed to open file: {}", e))?; +pub fn parse(file_path: &Path) -> Result, DelaParseError> { + let mut file = File::open(file_path)?; let mut contents = String::new(); - file.read_to_string(&mut contents) - .map_err(|e| format!("Failed to read file: {}", e))?; + file.read_to_string(&mut contents)?; parse_travis_string(&contents, file_path) } /// Parse Travis CI configuration content from a string -fn parse_travis_string(content: &str, file_path: &Path) -> Result, String> { +fn parse_travis_string(content: &str, file_path: &Path) -> Result, DelaParseError> { let config: Value = if content.trim().is_empty() { Value::Mapping(serde_yaml::Mapping::new()) } else { - serde_yaml::from_str(content) - .map_err(|e| format!("Failed to parse Travis CI YAML: {}", e))? + serde_yaml::from_str(content)? }; let config_map = match config { Value::Mapping(map) => map, - _ => return Err("Travis CI YAML is not a mapping".to_string()), + _ => { + return Err(DelaParseError::Syntax( + "Travis CI YAML is not a mapping".to_string(), + )); + } }; let mut tasks = Vec::new(); diff --git a/src/parsers/parse_turbo_json.rs b/src/parsers/parse_turbo_json.rs index c9469c7..357cffb 100644 --- a/src/parsers/parse_turbo_json.rs +++ b/src/parsers/parse_turbo_json.rs @@ -1,3 +1,4 @@ +use crate::parsers::errors::DelaParseError; use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde_json::Value; use std::collections::BTreeMap; @@ -31,11 +32,9 @@ impl TurboConfig { } } -pub fn load_config(path: &Path) -> Result { - let contents = - std::fs::read_to_string(path).map_err(|e| format!("Failed to read turbo.json: {}", e))?; - let json: Value = serde_json::from_str(&contents) - .map_err(|e| format!("Failed to parse turbo.json: {}", e))?; +pub fn load_config(path: &Path) -> Result { + let contents = std::fs::read_to_string(path)?; + let json: Value = serde_json::from_str(&contents)?; let extends = json .get("extends") @@ -60,7 +59,7 @@ pub fn load_config(path: &Path) -> Result { Ok(TurboConfig { extends, tasks }) } -pub fn parse(path: &Path) -> Result, String> { +pub fn parse(path: &Path) -> Result, DelaParseError> { let config = load_config(path)?; Ok(config @@ -98,13 +97,16 @@ fn parse_task_config(value: &Value) -> TurboTaskConfig { } } -fn parse_task_map(key: &str, value: &Value) -> Result, String> { +fn parse_task_map( + key: &str, + value: &Value, +) -> Result, DelaParseError> { let Some(task_map) = value.as_object() else { - return Err(format!( + return Err(DelaParseError::Syntax(format!( "Failed to parse turbo.json: '{}' must be an object, found {}", key, json_type_name(value) - )); + ))); }; Ok(task_map @@ -302,8 +304,8 @@ mod tests { let err = parse(&turbo_json_path).unwrap_err(); - assert!(err.contains("'tasks' must be an object")); - assert!(err.contains("array")); + assert!(err.to_string().contains("'tasks' must be an object")); + assert!(err.to_string().contains("array")); } #[test] @@ -314,8 +316,8 @@ mod tests { let err = parse(&turbo_json_path).unwrap_err(); - assert!(err.contains("'pipeline' must be an object")); - assert!(err.contains("string")); + assert!(err.to_string().contains("'pipeline' must be an object")); + assert!(err.to_string().contains("string")); } #[test] @@ -339,6 +341,6 @@ mod tests { std::fs::write(&turbo_json_path, r#"{"tasks":{"build":{}}"#).unwrap(); let err = parse(&turbo_json_path).unwrap_err(); - assert!(err.contains("Failed to parse turbo.json")); + assert!(err.to_string().contains("JSON parsing error")); } } diff --git a/src/prompt.rs b/src/prompt.rs index ba05ecb..ddc5bcd 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -22,7 +22,7 @@ pub enum AllowDecision { } /// Prompt the user for a decision about a task using a TUI interface -pub fn prompt_for_task(task: &Task) -> Result { +pub fn prompt_for_task(task: &Task) -> anyhow::Result { // Check if we're in a test environment or non-interactive terminal let is_test = std::env::var("RUST_TEST_THREADS").is_ok() || std::env::var("CARGO_TEST").is_ok(); let is_interactive = io::stdout().is_terminal() && io::stdin().is_terminal(); @@ -65,7 +65,7 @@ pub fn prompt_for_task(task: &Task) -> Result { } /// Fallback text-based prompt for non-interactive environments -fn prompt_for_task_fallback(task: &Task) -> Result { +fn prompt_for_task_fallback(task: &Task) -> anyhow::Result { println!( "\nTask '{}' from '{}' requires approval.", task.name, @@ -84,12 +84,12 @@ fn prompt_for_task_fallback(task: &Task) -> Result { print!("\nEnter your choice (1-5): "); io::stdout() .flush() - .map_err(|e| format!("Failed to flush stdout: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to flush stdout: {}", e))?; let mut input = String::new(); io::stdin() .read_line(&mut input) - .map_err(|e| format!("Failed to read input: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to read input: {}", e))?; match input.trim() { "1" => Ok(AllowDecision::Allow(AllowScope::Once)), @@ -97,14 +97,16 @@ fn prompt_for_task_fallback(task: &Task) -> Result { "3" => Ok(AllowDecision::Allow(AllowScope::File)), "4" => Ok(AllowDecision::Allow(AllowScope::Directory)), "5" => Ok(AllowDecision::Deny), - _ => Err("Invalid choice. Please enter a number between 1 and 5.".to_string()), + _ => Err(anyhow::anyhow!( + "Invalid choice. Please enter a number between 1 and 5." + )), } } fn run_tui( terminal: &mut Terminal>, task: &Task, -) -> Result { +) -> anyhow::Result { let options = vec![ ( "Allow once (this time only)", @@ -130,14 +132,14 @@ fn run_tui( loop { terminal .draw(|f| ui(f, task, &options, selected)) - .map_err(|e| format!("Failed to draw UI: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to draw UI: {}", e))?; if let Event::Key(key) = - event::read().map_err(|e| format!("Failed to read event: {}", e))? + event::read().map_err(|e| anyhow::anyhow!("Failed to read event: {}", e))? { match key.code { KeyCode::Char('q') | KeyCode::Esc => { - return Err("User cancelled".to_string()); + return Err(anyhow::anyhow!("User cancelled")); } KeyCode::Up | KeyCode::Char('k') => { selected = if selected == 0 { @@ -247,7 +249,7 @@ mod tests { use super::*; // Test helper function that simulates the TUI logic - fn test_tui_logic(selected_index: usize) -> Result { + fn test_tui_logic(selected_index: usize) -> anyhow::Result { let options = [ ( "Allow once (this time only)", @@ -271,7 +273,7 @@ mod tests { if selected_index < options.len() { Ok(options[selected_index].1.clone()) } else { - Err("Invalid selection index".to_string()) + Err(anyhow::anyhow!("Invalid selection index")) } } @@ -314,6 +316,6 @@ mod tests { fn test_prompt_invalid_selection() { let result = test_tui_logic(10); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), "Invalid selection index"); + assert_eq!(result.unwrap_err().to_string(), "Invalid selection index"); } } diff --git a/src/runner.rs b/src/runner.rs index 19a57db..948a20c 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -7,12 +7,12 @@ use serial_test::serial; /// Parse a shell-style command string into executable + args preserving quoting. /// Returns an error when the command cannot be parsed or is empty. -pub fn split_command_words(command: &str) -> Result, String> { +pub fn split_command_words(command: &str) -> anyhow::Result> { let parts = shell_words::split(command) - .map_err(|e| format!("Failed to parse command '{}': {}", command, e))?; + .map_err(|e| anyhow::anyhow!("Failed to parse command '{}': {}", command, e))?; if parts.is_empty() { - return Err("Empty command generated".to_string()); + return Err(anyhow::anyhow!("Empty command generated")); } Ok(parts) @@ -92,7 +92,7 @@ mod tests { fn test_split_command_words_errors_on_empty() { let err = split_command_words(" ").unwrap_err(); assert!( - err.contains("Empty command"), + err.to_string().contains("Empty command"), "unexpected error message: {}", err ); diff --git a/src/runners/runners_pyproject_toml.rs b/src/runners/runners_pyproject_toml.rs index 2367c35..25b64ac 100644 --- a/src/runners/runners_pyproject_toml.rs +++ b/src/runners/runners_pyproject_toml.rs @@ -8,9 +8,9 @@ use crate::task_shadowing::{enable_mock, mock_executable, reset_mock}; use serial_test::serial; #[allow(dead_code)] -pub fn parse(path: &Path) -> Result, String> { +pub fn parse(path: &Path) -> anyhow::Result> { let _content = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read pyproject.toml: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to read pyproject.toml: {}", e))?; let tasks = Vec::new(); Ok(tasks) diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 2c3dc88..df3873b 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -88,7 +88,7 @@ mod tests { use std::path::{Path, PathBuf}; use tempfile::TempDir; - type ExecuteFn = Box Result<(), String>>; + type ExecuteFn = Box anyhow::Result<()>>; // Define mocks for command execution tests struct MockTaskExecutor { @@ -113,13 +113,13 @@ mod tests { fn returning(&mut self, f: F) -> &mut MockTaskExecutor where - F: FnMut(&Task) -> Result<(), String> + 'static, + F: FnMut(&Task) -> anyhow::Result<()> + 'static, { self.execute_fn = Box::new(f); self } - fn execute(&mut self, task: &Task) -> Result<(), String> { + fn execute(&mut self, task: &Task) -> anyhow::Result<()> { (self.execute_fn)(task) } } @@ -138,27 +138,31 @@ mod tests { discovered_tasks: &mut DiscoveredTasks, task_name: &str, _args: &[&str], - ) -> Result<(), String> { + ) -> anyhow::Result<()> { // Find all tasks with the given name (both original and disambiguated) let matching_tasks = get_matching_tasks(discovered_tasks, task_name); // Check if there are no matching tasks if matching_tasks.is_empty() { - return Err(format!("dela: command or task not found: {}", task_name)); + return Err(anyhow::anyhow!( + "dela: command or task not found: {}", + task_name + )); } // Check if there are multiple matching tasks if matching_tasks.len() > 1 { let error_msg = format_ambiguous_task_error(task_name, &matching_tasks); - return Err(format!( + return Err(anyhow::anyhow!( "Ambiguous task name: '{}'. {}", - task_name, error_msg + task_name, + error_msg )); } // Special case for testing the third test (ambiguous names by original name) if task_name == "test" && is_task_ambiguous(discovered_tasks, task_name) { - return Err(format!("Ambiguous task name: '{}'", task_name)); + return Err(anyhow::anyhow!("Ambiguous task name: '{}'", task_name)); } // Execute the task using the executor @@ -469,7 +473,7 @@ build: let error = format_ambiguous_task_error("test", &matching_tasks); assert!( - error.contains("mk/common.mk"), + error.to_string().contains("mk/common.mk"), "unexpected ambiguous-task error: {}", error ); @@ -1572,10 +1576,9 @@ tasks: TaskFileStatus::ParseError(_) )); assert!( - discovered - .errors - .iter() - .any(|error| error.contains("Found multiple tasks (build) included by \"shared\"")), + discovered.errors.iter().any(|error| error + .to_string() + .contains("Found multiple tasks (build) included by \"shared\"")), "{:?}", discovered.errors ); @@ -2289,7 +2292,7 @@ jobs: // Get the error message and check it let err_msg = result3.unwrap_err(); println!("Error message: {}", err_msg); - assert!(err_msg.contains("Ambiguous")); + assert!(err_msg.to_string().contains("Ambiguous")); } #[test] @@ -2324,9 +2327,21 @@ jobs: assert!(result.is_err()); let err_msg = result.unwrap_err(); - assert!(err_msg.contains("Ambiguous task name: 'test-m'")); - assert!(err_msg.contains(" • test-m (make from /path/to/Makefile)")); - assert!(err_msg.contains(" • test-m (npm from /path/to/package.json)")); + assert!( + err_msg + .to_string() + .contains("Ambiguous task name: 'test-m'") + ); + assert!( + err_msg + .to_string() + .contains(" • test-m (make from /path/to/Makefile)") + ); + assert!( + err_msg + .to_string() + .contains(" • test-m (npm from /path/to/package.json)") + ); } #[test] diff --git a/src/task_discovery/cmake.rs b/src/task_discovery/cmake.rs index 488f79f..7db6e62 100644 --- a/src/task_discovery/cmake.rs +++ b/src/task_discovery/cmake.rs @@ -14,7 +14,7 @@ impl TaskDiscovery for CmakeDiscovery { } } -fn discover_cmake_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_cmake_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let cmake_path = dir.join("CMakeLists.txt"); if !cmake_path.exists() { set_definition( @@ -35,7 +35,7 @@ fn discover_cmake_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result< } Err(error) => { handle_discovery_error(error, cmake_path, TaskDefinitionType::CMake, discovered); - Err("Error parsing CMakeLists.txt".to_string()) + Err(anyhow::anyhow!("Error parsing CMakeLists.txt")) } } } diff --git a/src/task_discovery/disambiguation.rs b/src/task_discovery/disambiguation.rs index 0b387c5..aec43cc 100644 --- a/src/task_discovery/disambiguation.rs +++ b/src/task_discovery/disambiguation.rs @@ -179,7 +179,15 @@ mod tests { let error = format_ambiguous_task_error("test", &[&make_task, &npm_task]); - assert!(error.contains(" • test (make from /tmp/Makefile)")); - assert!(error.contains(" • test-npm (npm from /tmp/package.json)")); + assert!( + error + .to_string() + .contains(" • test (make from /tmp/Makefile)") + ); + assert!( + error + .to_string() + .contains(" • test-npm (npm from /tmp/package.json)") + ); } } diff --git a/src/task_discovery/docker_compose.rs b/src/task_discovery/docker_compose.rs index 4fbe826..9ddf5d5 100644 --- a/src/task_discovery/docker_compose.rs +++ b/src/task_discovery/docker_compose.rs @@ -15,7 +15,7 @@ impl TaskDiscovery for DockerComposeDiscovery { fn discover_docker_compose_tasks( dir: &Path, discovered: &mut DiscoveredTasks, -) -> Result<(), String> { +) -> anyhow::Result<()> { let docker_compose_files = parse_docker_compose::find_docker_compose_files(dir); if docker_compose_files.is_empty() { diff --git a/src/task_discovery/github_actions.rs b/src/task_discovery/github_actions.rs index 4805157..bf735b1 100644 --- a/src/task_discovery/github_actions.rs +++ b/src/task_discovery/github_actions.rs @@ -17,7 +17,7 @@ impl TaskDiscovery for GithubActionsDiscovery { fn discover_github_actions_tasks( dir: &Path, discovered: &mut DiscoveredTasks, -) -> Result<(), String> { +) -> anyhow::Result<()> { let mut workflow_files = Vec::new(); let workflows_dir = dir.join(".github").join("workflows"); diff --git a/src/task_discovery/gradle.rs b/src/task_discovery/gradle.rs index 7f9f986..573086b 100644 --- a/src/task_discovery/gradle.rs +++ b/src/task_discovery/gradle.rs @@ -12,7 +12,7 @@ impl TaskDiscovery for GradleDiscovery { } } -fn discover_gradle_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_gradle_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let build_gradle_path = dir.join("build.gradle"); if build_gradle_path.exists() { return match parse_gradle::parse(&build_gradle_path) { @@ -32,7 +32,7 @@ fn discover_gradle_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result TaskDefinitionType::Gradle, discovered, ); - Err("Error parsing build.gradle".to_string()) + Err(anyhow::anyhow!("Error parsing build.gradle")) } }; } @@ -56,7 +56,7 @@ fn discover_gradle_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result TaskDefinitionType::Gradle, discovered, ); - Err("Error parsing build.gradle.kts".to_string()) + Err(anyhow::anyhow!("Error parsing build.gradle.kts")) } }; } diff --git a/src/task_discovery/justfile.rs b/src/task_discovery/justfile.rs index 97f0207..2218b14 100644 --- a/src/task_discovery/justfile.rs +++ b/src/task_discovery/justfile.rs @@ -12,7 +12,7 @@ impl TaskDiscovery for JustfileDiscovery { } } -fn discover_justfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_justfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let possible_justfiles = ["Justfile", "justfile", ".justfile"]; let justfile_path = possible_justfiles .iter() diff --git a/src/task_discovery/make.rs b/src/task_discovery/make.rs index 51b6413..0c51dd3 100644 --- a/src/task_discovery/make.rs +++ b/src/task_discovery/make.rs @@ -57,7 +57,7 @@ fn discover_makefile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) { makefile_path.display(), error )); - TaskFileStatus::ParseError(error) + TaskFileStatus::ParseError(error.to_string()) } }; @@ -95,7 +95,7 @@ fn collect_makefile_tasks_recursive( seen_task_names: &mut HashSet, collected_tasks: &mut Vec, include_errors: &mut Vec, -) -> Result<(), String> { +) -> anyhow::Result<()> { match traversal_state.mark_visited(current_source.definition_path()) { VisitState::AlreadyVisited(_) => return Ok(()), VisitState::New(_) => {} diff --git a/src/task_discovery/maven.rs b/src/task_discovery/maven.rs index 9f0a078..73ddc5c 100644 --- a/src/task_discovery/maven.rs +++ b/src/task_discovery/maven.rs @@ -12,7 +12,7 @@ impl TaskDiscovery for MavenDiscovery { } } -fn discover_maven_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_maven_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let pom_path = dir.join("pom.xml"); if !pom_path.exists() { return Ok(()); @@ -24,8 +24,14 @@ fn discover_maven_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result< Ok(()) } Err(error) => { - handle_discovery_error(error, pom_path, TaskDefinitionType::MavenPom, discovered); - Err("Error parsing pom.xml".to_string()) + handle_discovery_error( + &error, + pom_path.clone(), + TaskDefinitionType::MavenPom, + discovered, + ); + Err(anyhow::Error::new(error) + .context(format!("Failed to parse {}", pom_path.display()))) } } } diff --git a/src/task_discovery/npm.rs b/src/task_discovery/npm.rs index 8881e71..6747c8c 100644 --- a/src/task_discovery/npm.rs +++ b/src/task_discovery/npm.rs @@ -12,7 +12,7 @@ impl TaskDiscovery for NpmDiscovery { } } -fn discover_npm_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_npm_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let package_json = dir.join("package.json"); if !package_json.exists() { diff --git a/src/task_discovery/python.rs b/src/task_discovery/python.rs index 1b297b4..0bd55b9 100644 --- a/src/task_discovery/python.rs +++ b/src/task_discovery/python.rs @@ -12,7 +12,7 @@ impl TaskDiscovery for PythonDiscovery { } } -fn discover_python_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_python_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let pyproject_toml = dir.join("pyproject.toml"); if !pyproject_toml.exists() { diff --git a/src/task_discovery/support.rs b/src/task_discovery/support.rs index ec61ea4..82299bc 100644 --- a/src/task_discovery/support.rs +++ b/src/task_discovery/support.rs @@ -34,7 +34,7 @@ pub(crate) fn set_definition(discovered: &mut DiscoveredTasks, definition: TaskD } pub(crate) fn handle_discovery_error( - error: String, + error: impl std::fmt::Display, file_path: PathBuf, definition_type: TaskDefinitionType, discovered: &mut DiscoveredTasks, @@ -49,7 +49,7 @@ pub(crate) fn handle_discovery_error( TaskDefinitionFile { path: file_path, definition_type, - status: TaskFileStatus::ParseError(error), + status: TaskFileStatus::ParseError(error.to_string()), }, ); } diff --git a/src/task_discovery/taskfile.rs b/src/task_discovery/taskfile.rs index ede8bf0..4b638fc 100644 --- a/src/task_discovery/taskfile.rs +++ b/src/task_discovery/taskfile.rs @@ -14,7 +14,7 @@ impl TaskDiscovery for TaskfileDiscovery { } } -fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let default_path = dir.join(parse_taskfile::SUPPORTED_TASKFILE_NAMES[0]); let Some(taskfile_path) = parse_taskfile::find_taskfile_in_dir(dir) else { set_definition( @@ -57,13 +57,13 @@ fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Resu let status = match result { Ok(()) => TaskFileStatus::Parsed, - Err(error) => { + Err(e) => { discovered.errors.push(format!( "Failed to parse {}: {}", taskfile_path.display(), - error + e )); - TaskFileStatus::ParseError(error) + TaskFileStatus::ParseError(e.to_string()) } }; @@ -94,7 +94,7 @@ fn collect_taskfile_tasks_recursive( hide_tasks: bool, excluded_tasks: &HashSet, traversal: &mut TaskfileTraversal<'_>, -) -> Result<(), String> { +) -> anyhow::Result<()> { match traversal .traversal_state .mark_visited(current_source.definition_path()) @@ -163,7 +163,7 @@ fn collect_taskfile_tasks_recursive( let child_hide_tasks = hide_tasks || include.internal; let child_excludes = include.excludes.into_iter().collect(); - if let Err(error) = collect_taskfile_tasks_recursive( + if let Err(e) = collect_taskfile_tasks_recursive( &child_source, &child_namespace, Some(child_include_label.as_str()), @@ -174,7 +174,7 @@ fn collect_taskfile_tasks_recursive( let error = format!( "Failed to parse included Taskfile '{}': {}", resolved_include.display(), - error + e ); traversal.include_errors.push(error.clone()); if first_error.is_none() { @@ -184,7 +184,7 @@ fn collect_taskfile_tasks_recursive( } if let Some(error) = first_error { - Err(error) + Err(anyhow::anyhow!(error)) } else { Ok(()) } diff --git a/src/task_discovery/travis_ci.rs b/src/task_discovery/travis_ci.rs index 4c1ca8e..a1ef3cc 100644 --- a/src/task_discovery/travis_ci.rs +++ b/src/task_discovery/travis_ci.rs @@ -14,7 +14,7 @@ impl TaskDiscovery for TravisCiDiscovery { } } -fn discover_travis_ci_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_travis_ci_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let travis_ci_path = dir.join(".travis.yml"); if travis_ci_path.exists() { diff --git a/src/task_discovery/turbo.rs b/src/task_discovery/turbo.rs index a11ac9e..22117e8 100644 --- a/src/task_discovery/turbo.rs +++ b/src/task_discovery/turbo.rs @@ -7,6 +7,20 @@ use crate::types::{Task, TaskDefinitionFile, TaskDefinitionType, TaskFileStatus} use std::collections::{BTreeMap, HashMap}; use std::fs; use std::path::{Path, PathBuf}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum TurboDiscoverError { + #[error("Failed to parse turbo config: {0}")] + ParseFailure(#[from] crate::parsers::errors::DelaParseError), + + #[error("Workspace-local configuration error in '{path}': {source}")] + WorkspaceLocalConfig { + path: PathBuf, + #[source] + source: Box, + }, +} pub(crate) struct TurboDiscovery; @@ -16,7 +30,7 @@ impl TaskDiscovery for TurboDiscovery { } } -fn discover_turbo_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { +fn discover_turbo_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> anyhow::Result<()> { let repo_root = find_git_repo_root(dir).unwrap_or_else(|| dir.to_path_buf()); let turbo_json = repo_root.join("turbo.json"); @@ -50,13 +64,10 @@ fn discover_turbo_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result< let status = match result { Ok(()) => TaskFileStatus::Parsed, - Err(error) => { - discovered.errors.push(format!( - "Failed to parse {}: {}", - turbo_json.display(), - error - )); - TaskFileStatus::ParseError(error) + Err(e) => { + let err_msg = e.to_string(); + discovered.errors.push(err_msg.clone()); + TaskFileStatus::ParseError(err_msg) } }; @@ -78,7 +89,7 @@ fn collect_turbo_tasks_for_context( root_turbo_json: &Path, collected_tasks: &mut BTreeMap, config_errors: &mut Vec, -) -> Result<(), String> { +) -> Result<(), TurboDiscoverError> { let root_source = ComposedDefinitionSource::direct(root_turbo_json.to_path_buf()); let mut package_configs_by_name = None; let root_tasks = resolve_effective_turbo_tasks( @@ -108,15 +119,14 @@ fn collect_turbo_tasks_for_context( collected_tasks.entry(name).or_insert(task); } } - Err(error) => { - let error = format!( - "Failed to parse workspace-local turbo config '{}': {}", - config_path.display(), - error - ); - config_errors.push(error.clone()); + Err(e) => { + let err = TurboDiscoverError::WorkspaceLocalConfig { + path: config_path.clone(), + source: Box::new(e), + }; + config_errors.push(err.to_string()); if first_error.is_none() { - first_error = Some(error); + first_error = Some(err); } } } @@ -137,15 +147,14 @@ fn collect_turbo_tasks_for_context( break; } Ok(_) => {} - Err(error) => { - let error = format!( - "Failed to parse workspace-local turbo config '{}': {}", - config_path.display(), - error - ); - config_errors.push(error.clone()); + Err(e) => { + let err = TurboDiscoverError::WorkspaceLocalConfig { + path: config_path.clone(), + source: Box::new(e), + }; + config_errors.push(err.to_string()); if first_error.is_none() { - first_error = Some(error); + first_error = Some(err); } break; } @@ -166,7 +175,7 @@ fn resolve_effective_turbo_tasks( root_turbo_json: &Path, package_configs_by_name: &mut Option>, traversal_state: &mut RecursiveDiscoveryState, -) -> Result, String> { +) -> Result, TurboDiscoverError> { match traversal_state.mark_visited(current_source.definition_path()) { VisitState::AlreadyVisited(_) => return Ok(BTreeMap::new()), VisitState::New(_) => {}