diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 51ba44b..874e682 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -20,15 +20,15 @@ jobs: name: Build Base Image runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4 with: buildkitd-flags: --debug - name: Build and (locally) load base image - uses: docker/build-push-action@v7 + uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f # v7 with: context: . file: tests/Dockerfile.builder @@ -52,15 +52,15 @@ jobs: fail-fast: false steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd # v4 with: buildkitd-flags: --debug - name: Load cached builder image - uses: docker/build-push-action@v7 + uses: docker/build-push-action@bcafcacb16a39f128d818304e6c9c0c18556b85f # v7 with: context: . file: tests/Dockerfile.builder diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d2ab20c..5d5b471 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -14,15 +14,16 @@ jobs: name: Test runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # master with: - components: rustfmt + toolchain: 1.91.1 + components: rustfmt, clippy - name: Cache dependencies - uses: actions/cache@v5 + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5 with: path: | ~/.cargo/registry @@ -30,8 +31,8 @@ jobs: target key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - - name: Check formatting - run: cargo fmt --all -- --check + - name: Lint + run: make lint - name: Run tests run: cargo test --verbose diff --git a/Cargo.toml b/Cargo.toml index 7fde301..3783935 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [package] name = "dela" version = "0.0.6" +rust-version = "1.91.1" edition = "2024" license = "MIT" authors = ["Alexander Yankov"] diff --git a/Makefile b/Makefile index 2b76b47..c37d25b 100644 --- a/Makefile +++ b/Makefile @@ -10,6 +10,10 @@ build: @echo "Building dela..." cargo build +lint: + cargo fmt --all -- --check + cargo clippy --all-targets --all-features -- -D warnings + tests: @echo "Running unit tests." cargo test diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000..4f22047 --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "1.91.1" +components = ["rustfmt", "clippy"] diff --git a/src/allowlist.rs b/src/allowlist.rs index 927d97c..8c17f64 100644 --- a/src/allowlist.rs +++ b/src/allowlist.rs @@ -68,10 +68,10 @@ pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { // First pass: Check for deny entries (highest precedence) for entry in &allowlist.entries { - if let AllowScope::Deny = entry.scope { - if path_matches(&task.file_path, &entry.path, true) { - return Ok((false, true)); - } + if let AllowScope::Deny = entry.scope + && path_matches(&task.file_path, &entry.path, true) + { + return Ok((false, true)); } } @@ -89,12 +89,11 @@ pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { } } AllowScope::Task => { - if path_matches(&task.file_path, &entry.path, false) { - if let Some(ref tasks) = entry.tasks { - if tasks.contains(&task.name) { - return Ok((true, false)); - } - } + if path_matches(&task.file_path, &entry.path, false) + && let Some(ref tasks) = entry.tasks + && tasks.contains(&task.name) + { + return Ok((true, false)); } } AllowScope::Deny | AllowScope::Once => { diff --git a/src/commands/init.rs b/src/commands/init.rs index 9f7f43c..0f881e7 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -70,11 +70,11 @@ fn add_shell_integration(config_path: &PathBuf) -> Result<(), String> { } // Create parent directory if it doesn't exist (needed for PowerShell) - if let Some(parent) = config_path.parent() { - if !parent.exists() { - fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create config directory: {}", e))?; - } + if let Some(parent) = config_path.parent() + && !parent.exists() + { + fs::create_dir_all(parent) + .map_err(|e| format!("Failed to create config directory: {}", e))?; } // Open file in append mode @@ -174,7 +174,7 @@ mod tests { use serial_test::serial; use tempfile::TempDir; - fn setup_test_env(shell: &str, home: &PathBuf) -> Result<(), std::io::Error> { + fn setup_test_env(shell: &str, home: &std::path::Path) -> Result<(), std::io::Error> { let test_env = TestEnvironment::new() .with_shell(shell) .with_home(home.to_string_lossy()); diff --git a/src/commands/list.rs b/src/commands/list.rs index cc1c31d..5bb6cc9 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -230,7 +230,7 @@ pub fn execute(verbose: bool) -> Result<(), String> { // Ensure all task names will be padded to this width // Round up to nearest multiple of 5 for better alignment - let display_width = (max_task_name_width + 4) / 5 * 5; + let display_width = max_task_name_width.div_ceil(5) * 5; // Get a sorted list of runners for deterministic output let mut runners: Vec = tasks_by_runner.keys().cloned().collect(); @@ -390,7 +390,7 @@ fn format_task_entry(task: &Task, is_ambiguous: bool, name_width: usize) -> Stri }; // Create the task description part - let description_part = if let Some(_) = &task.disambiguated_name { + let description_part = if task.disambiguated_name.is_some() { // For disambiguated tasks, show the original name with footnotes let orig_with_footnotes = if !footnotes.is_empty() { format!("{} {}", task.name.dimmed().red(), footnotes.yellow()) @@ -422,9 +422,7 @@ fn format_task_entry(task: &Task, is_ambiguous: bool, name_width: usize) -> Stri } else if task.disambiguated_name.is_some() { // For disambiguated tasks, the display name (disambiguated) should be green display_name.green() - } else if is_ambiguous { - display_name.dimmed().red() - } else if task.shadowed_by.is_some() { + } else if is_ambiguous || task.shadowed_by.is_some() { display_name.dimmed().red() } else { display_name.green() @@ -644,8 +642,10 @@ mod tests { runners.sort(); // Mock the task discovery - let mut discovered_tasks = task_discovery::DiscoveredTasks::default(); - discovered_tasks.tasks = tasks; + let discovered_tasks = task_discovery::DiscoveredTasks { + tasks, + ..Default::default() + }; // Calculate max task name width across all runners let max_task_name_width = discovered_tasks @@ -658,7 +658,7 @@ mod tests { // Ensure all task names will be padded to this width // Round up to nearest multiple of 5 for better alignment - let display_width = (max_task_name_width + 4) / 5 * 5; + let display_width = max_task_name_width.div_ceil(5) * 5; // Process each runner for runner in runners { @@ -900,8 +900,10 @@ mod tests { let mut writer = TestWriter::new(); // Mock the task discovery with our GitHub Actions task - let mut discovered_tasks = task_discovery::DiscoveredTasks::default(); - discovered_tasks.tasks = vec![task]; + let discovered_tasks = task_discovery::DiscoveredTasks { + tasks: vec![task], + ..Default::default() + }; // Group tasks by runner let mut tasks_by_runner: HashMap> = HashMap::new(); @@ -922,7 +924,7 @@ mod tests { write!(writer, "{} — {}", runner.cyan(), display_path.dimmed()).unwrap(); // Write the task - let formatted_task = format_task_entry(&act_tasks[0], false, 20); + let formatted_task = format_task_entry(act_tasks[0], false, 20); writeln!(writer, "\n {}", formatted_task).unwrap(); // Get the output and verify it shows the full path diff --git a/src/commands/mcp.rs b/src/commands/mcp.rs index e72e547..fa35aba 100644 --- a/src/commands/mcp.rs +++ b/src/commands/mcp.rs @@ -169,11 +169,11 @@ fn merge_dela_into_toml(existing: &str) -> Result { /// Generate MCP config file for an editor at a specific path fn generate_config_at(editor: Editor, config_path: &PathBuf) -> Result<(), String> { // Create parent directory if it doesn't exist - if let Some(parent) = config_path.parent() { - if !parent.exists() { - fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create {} directory: {}", editor.name(), e))?; - } + 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))?; } // Check if config already exists diff --git a/src/mcp/allowlist.rs b/src/mcp/allowlist.rs index 641ff03..fd3d25b 100644 --- a/src/mcp/allowlist.rs +++ b/src/mcp/allowlist.rs @@ -35,10 +35,10 @@ impl McpAllowlistEvaluator { pub fn is_task_allowed(&self, task: &Task) -> Result { // First pass: Check for deny entries (highest precedence) for entry in &self.allowlist.entries { - if let AllowScope::Deny = entry.scope { - if self.path_matches(&task.file_path, &entry.path, true) { - return Ok(false); - } + if let AllowScope::Deny = entry.scope + && self.path_matches(&task.file_path, &entry.path, true) + { + return Ok(false); } } @@ -56,12 +56,11 @@ impl McpAllowlistEvaluator { } } AllowScope::Task => { - if self.path_matches(&task.file_path, &entry.path, false) { - if let Some(ref tasks) = entry.tasks { - if tasks.contains(&task.name) { - return Ok(true); - } - } + if self.path_matches(&task.file_path, &entry.path, false) + && let Some(ref tasks) = entry.tasks + && tasks.contains(&task.name) + { + return Ok(true); } } AllowScope::Deny | AllowScope::Once => { @@ -157,7 +156,7 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // With empty allowlist, task should be denied by default for MCP - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&task).unwrap()); reset_to_real_environment(); } @@ -180,7 +179,7 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // Task should be allowed - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), true); + assert!(evaluator.is_task_allowed(&task).unwrap()); drop(temp_dir); reset_to_real_environment(); @@ -204,11 +203,11 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // Task should be allowed - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), true); + assert!(evaluator.is_task_allowed(&task).unwrap()); // Create a different task that should be denied let other_task = create_test_task("other-task", std::path::PathBuf::from("Makefile")); - assert_eq!(evaluator.is_task_allowed(&other_task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&other_task).unwrap()); drop(temp_dir); reset_to_real_environment(); @@ -236,11 +235,11 @@ mod tests { "build", std::path::PathBuf::from("/project/subdir/Makefile"), ); - assert_eq!(evaluator.is_task_allowed(&subdir_task).unwrap(), true); + assert!(evaluator.is_task_allowed(&subdir_task).unwrap()); // Task outside directory should be denied let outside_task = create_test_task("build", std::path::PathBuf::from("/other/Makefile")); - assert_eq!(evaluator.is_task_allowed(&outside_task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&outside_task).unwrap()); drop(temp_dir); reset_to_real_environment(); @@ -264,7 +263,7 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // Task should be denied - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&task).unwrap()); drop(temp_dir); reset_to_real_environment(); @@ -300,7 +299,7 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // Task should be denied (deny takes precedence) - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&task).unwrap()); drop(temp_dir); reset_to_real_environment(); @@ -350,12 +349,12 @@ mod tests { let evaluator = McpAllowlistEvaluator::new().unwrap(); // Task should be denied due to file deny entry (higher precedence than directory allow) - assert_eq!(evaluator.is_task_allowed(&task).unwrap(), false); + assert!(!evaluator.is_task_allowed(&task).unwrap()); // But a task in a different file in the same directory should be allowed let other_file_task = create_test_task("build", std::path::PathBuf::from("/project/other.mk")); - assert_eq!(evaluator.is_task_allowed(&other_file_task).unwrap(), true); + assert!(evaluator.is_task_allowed(&other_file_task).unwrap()); drop(temp_dir); reset_to_real_environment(); diff --git a/src/mcp/dto.rs b/src/mcp/dto.rs index 1a0abf8..06bff81 100644 --- a/src/mcp/dto.rs +++ b/src/mcp/dto.rs @@ -85,19 +85,13 @@ impl TaskDto { } /// Parameters for the list_tasks MCP tool -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, Default)] pub struct ListTasksArgs { /// Optional runner filter - if provided, only return tasks for this runner /// Examples: "make", "npm", "gradle", "poetry" pub runner: Option, } -impl Default for ListTasksArgs { - fn default() -> Self { - Self { runner: None } - } -} - #[cfg(test)] mod tests { use super::*; @@ -293,7 +287,7 @@ mod tests { #[test] fn test_taskdto_from_multiple_tasks_batch() { // Arrange - simulate a disambiguation scenario - let tasks = vec![ + let tasks = [ Task { name: "test".to_string(), file_path: PathBuf::from("/project/Makefile"), @@ -410,10 +404,6 @@ mod tests { assert_eq!(dto.command, "make build"); assert_eq!(dto.file_path, "/project/Makefile"); assert_eq!(dto.description, Some("Build the project".to_string())); - - // These fields depend on system state but should be present - assert!(dto.runner_available == true || dto.runner_available == false); - assert!(dto.allowlisted == true || dto.allowlisted == false); } #[test] @@ -543,7 +533,7 @@ mod tests { dto.command, "# Travis CI task 'test' - not executable locally" ); - assert_eq!(dto.runner_available, false); // Travis CI is never available locally + assert!(!dto.runner_available); // Travis CI is never available locally } #[test] diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 3b51325..093837d 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -317,10 +317,10 @@ impl DelaMcpServer { async fn get_discovered_tasks(&self) -> task_discovery::DiscoveredTasks { { let cache = self.task_cache.read().await; - if let Some(entry) = cache.as_ref() { - if entry.cached_at.elapsed() < self.task_cache_ttl { - return entry.discovered.clone(); - } + if let Some(entry) = cache.as_ref() + && entry.cached_at.elapsed() < self.task_cache_ttl + { + return entry.discovered.clone(); } } @@ -372,7 +372,7 @@ impl DelaMcpServer { .collect(); Ok(CallToolResult::success(vec![ - Content::json(&serde_json::json!({ + Content::json(serde_json::json!({ "tasks": task_dtos })) .expect("Failed to serialize JSON"), @@ -401,7 +401,7 @@ impl DelaMcpServer { .collect(); Ok(CallToolResult::success(vec![ - Content::json(&serde_json::json!({ + Content::json(serde_json::json!({ "running": running_jobs })) .expect("Failed to serialize JSON"), @@ -743,7 +743,7 @@ impl DelaMcpServer { .await; // Check if process exited during initial capture - let process_exited = child.try_wait().map_or(false, |status| status.is_some()); + let process_exited = child.try_wait().is_ok_and(|status| status.is_some()); if process_exited { // Process completed within 1 second @@ -1066,7 +1066,7 @@ impl DelaMcpServer { .collect(); Ok(CallToolResult::success(vec![ - Content::json(&serde_json::json!({ + Content::json(serde_json::json!({ "jobs": job_statuses })) .expect("Failed to serialize JSON"), @@ -1155,7 +1155,7 @@ impl DelaMcpServer { response["lines"] = serde_json::Value::Array( truncated_lines .into_iter() - .map(|line| serde_json::Value::String(line)) + .map(serde_json::Value::String) .collect(), ); response["chunk_truncated"] = serde_json::Value::Bool(true); @@ -1217,7 +1217,7 @@ impl DelaMcpServer { }; Ok(CallToolResult::success(vec![ - Content::json(&serde_json::json!({ + Content::json(serde_json::json!({ "pid": args.pid, "status": status, "message": message, @@ -1794,7 +1794,7 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); @@ -1901,12 +1901,12 @@ mod tests { server .job_manager - .start_job(pid1 as u32, metadata1, child1) + .start_job(pid1, metadata1, child1) .await .unwrap(); server .job_manager - .start_job(pid2 as u32, metadata2, child2) + .start_job(pid2, metadata2, child2) .await .unwrap(); @@ -1975,14 +1975,14 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); // Mark job as exited server .job_manager - .update_job_state(pid as u32, JobState::Exited(0)) + .update_job_state(pid, JobState::Exited(0)) .await .unwrap(); @@ -2039,12 +2039,12 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); server .job_manager - .update_job_state(pid as u32, JobState::Failed("boom".to_string())) + .update_job_state(pid, JobState::Failed("boom".to_string())) .await .unwrap(); @@ -2097,19 +2097,19 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); // Add some output to the job server .job_manager - .add_job_output(pid as u32, "Line 1\nLine 2\nLine 3\n".to_string()) + .add_job_output(pid, "Line 1\nLine 2\nLine 3\n".to_string()) .await .unwrap(); let args = TaskOutputArgs { - pid: pid as u32, + pid, lines: Some(2), show_truncation: None, }; @@ -2163,22 +2163,19 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); // Add some output to the job server .job_manager - .add_job_output( - pid as u32, - "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n".to_string(), - ) + .add_job_output(pid, "Line 1\nLine 2\nLine 3\nLine 4\nLine 5\n".to_string()) .await .unwrap(); let args = TaskOutputArgs { - pid: pid as u32, + pid, lines: Some(3), show_truncation: Some(true), }; @@ -2238,19 +2235,19 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); // Add some output to the job server .job_manager - .add_job_output(pid as u32, "Line 1\nLine 2\n".to_string()) + .add_job_output(pid, "Line 1\nLine 2\n".to_string()) .await .unwrap(); let args = TaskOutputArgs { - pid: pid as u32, + pid, lines: Some(5), // Request more lines than available show_truncation: Some(true), }; @@ -2326,12 +2323,12 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); let args = TaskStopArgs { - pid: pid as u32, + pid, grace_period: Some(2), }; @@ -2382,12 +2379,12 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); let args = TaskStopArgs { - pid: pid as u32, + pid, grace_period: None, // Should use default 5 seconds }; @@ -2452,19 +2449,19 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); // Mark job as exited server .job_manager - .update_job_state(pid as u32, JobState::Exited(0)) + .update_job_state(pid, JobState::Exited(0)) .await .unwrap(); let args = TaskStopArgs { - pid: pid as u32, + pid, grace_period: Some(5), }; @@ -2510,7 +2507,7 @@ mod tests { let pid1 = child1.id().unwrap(); job_manager - .start_job(pid1 as u32, metadata.clone(), child1) + .start_job(pid1, metadata.clone(), child1) .await .unwrap(); @@ -2523,7 +2520,7 @@ mod tests { let pid2 = child2.id().unwrap(); job_manager - .start_job(pid2 as u32, metadata.clone(), child2) + .start_job(pid2, metadata.clone(), child2) .await .unwrap(); @@ -2535,7 +2532,7 @@ mod tests { let child3 = cmd3.spawn().unwrap(); let pid3 = child3.id().unwrap(); - let result = job_manager.start_job(pid3 as u32, metadata, child3).await; + let result = job_manager.start_job(pid3, metadata, child3).await; // Assert assert!(result.is_err()); @@ -2572,7 +2569,7 @@ mod tests { server .job_manager - .start_job(pid as u32, metadata, child) + .start_job(pid, metadata, child) .await .unwrap(); @@ -2580,12 +2577,12 @@ mod tests { let large_output = "x".repeat(10000); // 10KB line server .job_manager - .add_job_output(pid as u32, large_output) + .add_job_output(pid, large_output) .await .unwrap(); let args = TaskOutputArgs { - pid: pid as u32, + pid, lines: Some(1), show_truncation: Some(true), }; @@ -2625,7 +2622,6 @@ mod tests { // This test would require mocking the job manager or creating a custom server // with a low concurrency limit, which is complex. For now, we'll test the // can_start_job method directly as shown above. - assert!(true); // Placeholder test } #[tokio::test] @@ -3662,7 +3658,7 @@ add_custom_target(build-all COMMENT "Build everything") println!( "Task status immediately after start: {} jobs, first job state: {}", jobs.len(), - jobs.get(0) + jobs.first() .map(|j| j["state"].as_str().unwrap_or("unknown")) .unwrap_or("none") ); @@ -3734,7 +3730,7 @@ add_custom_target(build-all COMMENT "Build everything") println!( "Task status after completion: {} jobs, first job state: {}", jobs.len(), - jobs.get(0) + jobs.first() .map(|j| j["state"].as_str().unwrap_or("unknown")) .unwrap_or("none") ); diff --git a/src/parsers/parse_cmake.rs b/src/parsers/parse_cmake.rs index 384f14c..6be11cf 100644 --- a/src/parsers/parse_cmake.rs +++ b/src/parsers/parse_cmake.rs @@ -39,6 +39,9 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result, Stri 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 comment_pattern = Regex::new(r#"COMMENT\s+"([^"]*)"#) + .map_err(|e| format!("Failed to compile comment regex: {}", e))?; + // Find all matches in the content for captures in target_pattern.captures_iter(&normalized_content) { let target_name = captures.get(1).unwrap().as_str(); @@ -52,13 +55,10 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result, Stri let mut description = format!("CMake custom target: {}", target_name); // Look for COMMENT in this specific target block - let comment_pattern = Regex::new(r#"COMMENT\s+"([^"]*)"#) - .map_err(|e| format!("Failed to compile comment regex: {}", e))?; - - if let Some(comment_captures) = comment_pattern.captures(target_block) { - if let Some(comment) = comment_captures.get(1) { - description = comment.as_str().to_string(); - } + if let Some(comment_captures) = comment_pattern.captures(target_block) + && let Some(comment) = comment_captures.get(1) + { + description = comment.as_str().to_string(); } let task = Task { diff --git a/src/parsers/parse_github_actions.rs b/src/parsers/parse_github_actions.rs index 38e3f62..15fec04 100644 --- a/src/parsers/parse_github_actions.rs +++ b/src/parsers/parse_github_actions.rs @@ -29,14 +29,14 @@ fn parse_workflow_string(content: &str, file_path: &Path) -> Result, S // Try to get workflow name for description let workflow_name = workflow_map - .get(&Value::String("name".to_string())) + .get(Value::String("name".to_string())) .and_then(|v| match v { Value::String(s) => Some(s.clone()), _ => None, }); // Extract jobs to confirm the workflow is valid - let jobs = match workflow_map.get(&Value::String("jobs".to_string())) { + 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()), }; @@ -118,7 +118,7 @@ jobs: run: echo "Testing..." "#; - let file_path = create_test_workflow(&temp_dir.path(), "workflow.yml", workflow_content); + let file_path = create_test_workflow(temp_dir.path(), "workflow.yml", workflow_content); let tasks = parse(&file_path).expect("Failed to parse workflow"); @@ -196,7 +196,7 @@ jobs: "#; let file_path = - create_test_workflow(&temp_dir.path(), "complex-workflow.yml", workflow_content); + create_test_workflow(temp_dir.path(), "complex-workflow.yml", workflow_content); let tasks = parse(&file_path).expect("Failed to parse complex workflow"); @@ -284,7 +284,7 @@ jobs: "#; let file_path = - create_test_workflow(&temp_dir.path(), "unnamed-workflow.yml", workflow_content); + create_test_workflow(temp_dir.path(), "unnamed-workflow.yml", workflow_content); let tasks = parse(&file_path).expect("Failed to parse workflow without name"); @@ -312,7 +312,7 @@ jobs: "#; let file_path = - create_test_workflow(&temp_dir.path(), "invalid-workflow.yml", workflow_content); + create_test_workflow(temp_dir.path(), "invalid-workflow.yml", workflow_content); let result = parse(&file_path); assert!(result.is_err(), "Should fail with invalid YAML"); @@ -323,7 +323,7 @@ name: No Jobs on: [push] "#; - let file_path = create_test_workflow(&temp_dir.path(), "no-jobs.yml", workflow_content); + let file_path = create_test_workflow(temp_dir.path(), "no-jobs.yml", workflow_content); let result = parse(&file_path); assert!(result.is_err()); diff --git a/src/parsers/parse_gradle.rs b/src/parsers/parse_gradle.rs index af7dfb8..3124e51 100644 --- a/src/parsers/parse_gradle.rs +++ b/src/parsers/parse_gradle.rs @@ -131,31 +131,27 @@ fn extract_custom_tasks( fn extract_task_description(content: &str, task_name: &str) -> Option { // This is a simplified approach with basic regex let task_pattern = format!(r"task\s+{}", regex::escape(task_name)); - let description_single_quote_pattern = format!(r"description\s+'([^']*)'"); - let description_double_quote_pattern = format!(r#"description\s+"([^"]*)""#); + let description_single_quote_pattern = r"description\s+'([^']*)'".to_string(); + let description_double_quote_pattern = r#"description\s+"([^"]*)""#.to_string(); // Look for task with description using single quotes if let Ok(regex) = Regex::new(&format!( "{}.+?{}", task_pattern, description_single_quote_pattern - )) { - if let Some(caps) = regex.captures(content) { - if let Some(desc) = caps.get(1) { - return Some(desc.as_str().to_string()); - } - } + )) && let Some(caps) = regex.captures(content) + && let Some(desc) = caps.get(1) + { + return Some(desc.as_str().to_string()); } // Look for task with description using double quotes if let Ok(regex) = Regex::new(&format!( "{}.+?{}", task_pattern, description_double_quote_pattern - )) { - if let Some(caps) = regex.captures(content) { - if let Some(desc) = caps.get(1) { - return Some(desc.as_str().to_string()); - } - } + )) && let Some(caps) = regex.captures(content) + && let Some(desc) = caps.get(1) + { + return Some(desc.as_str().to_string()); } // For Kotlin DSL, look for description with equals @@ -163,12 +159,11 @@ fn extract_task_description(content: &str, task_name: &str) -> Option { r#"tasks.*?"{}".+?description\s*=\s*"([^"]*)""#, regex::escape(task_name) ); - if let Ok(regex) = Regex::new(&kotlin_pattern) { - if let Some(caps) = regex.captures(content) { - if let Some(desc) = caps.get(1) { - return Some(desc.as_str().to_string()); - } - } + if let Ok(regex) = Regex::new(&kotlin_pattern) + && let Some(caps) = regex.captures(content) + && let Some(desc) = caps.get(1) + { + return Some(desc.as_str().to_string()); } Some("Custom Gradle task".to_string()) diff --git a/src/parsers/parse_justfile.rs b/src/parsers/parse_justfile.rs index 363cc25..ad66769 100644 --- a/src/parsers/parse_justfile.rs +++ b/src/parsers/parse_justfile.rs @@ -304,9 +304,9 @@ test: # Run tests assert_eq!(tasks.len(), 2); // Only the valid tasks should be parsed - assert!(tasks.iter().find(|t| t.name == "build").is_some()); - assert!(tasks.iter().find(|t| t.name == "test").is_some()); - assert!(tasks.iter().find(|t| t.name == "invalid-task").is_none()); + assert!(tasks.iter().any(|t| t.name == "build")); + assert!(tasks.iter().any(|t| t.name == "test")); + assert!(!tasks.iter().any(|t| t.name == "invalid-task")); } #[test] diff --git a/src/parsers/parse_makefile.rs b/src/parsers/parse_makefile.rs index ea46850..a5ec234 100644 --- a/src/parsers/parse_makefile.rs +++ b/src/parsers/parse_makefile.rs @@ -11,7 +11,7 @@ pub fn parse(path: &Path) -> Result, String> { // Special case for the test_discover_tasks_with_invalid_makefile test if content.contains("not a make file") { - return Err(format!("Failed to parse Makefile: Invalid syntax")); + return Err("Failed to parse Makefile: Invalid syntax".to_string()); } // Special case for testing regex parsing - look for a marker in the content diff --git a/src/parsers/parse_package_json.rs b/src/parsers/parse_package_json.rs index cc90b70..6b848bf 100644 --- a/src/parsers/parse_package_json.rs +++ b/src/parsers/parse_package_json.rs @@ -20,20 +20,20 @@ pub fn parse(path: &PathBuf) -> Result, String> { let mut tasks = Vec::new(); - if let Some(scripts) = json.get("scripts") { - if let Some(scripts_obj) = scripts.as_object() { - for (name, cmd) in scripts_obj { - tasks.push(Task { - name: name.clone(), - file_path: path.clone(), - definition_type: TaskDefinitionType::PackageJson, - runner: runner.clone(), - source_name: name.clone(), - description: cmd.as_str().map(|s| s.to_string()), - shadowed_by: None, - disambiguated_name: None, - }); - } + if let Some(scripts) = json.get("scripts") + && let Some(scripts_obj) = scripts.as_object() + { + for (name, cmd) in scripts_obj { + tasks.push(Task { + name: name.clone(), + file_path: path.clone(), + definition_type: TaskDefinitionType::PackageJson, + runner: runner.clone(), + source_name: name.clone(), + description: cmd.as_str().map(|s| s.to_string()), + shadowed_by: None, + disambiguated_name: None, + }); } } diff --git a/src/parsers/parse_pom_xml.rs b/src/parsers/parse_pom_xml.rs index 506eca7..cc56e41 100644 --- a/src/parsers/parse_pom_xml.rs +++ b/src/parsers/parse_pom_xml.rs @@ -84,55 +84,53 @@ 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> { // Find section and then - if let Some(build_node) = root.children().find(|n| n.has_tag_name("build")) { - if let Some(plugins_node) = build_node.children().find(|n| n.has_tag_name("plugins")) { - // Iterate over each plugin - for plugin in plugins_node.children().filter(|n| n.has_tag_name("plugin")) { - // Get plugin artifact ID - let artifact_id = plugin + 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")) + { + // Iterate over each plugin + for plugin in plugins_node.children().filter(|n| n.has_tag_name("plugin")) { + // Get plugin artifact ID + let artifact_id = plugin + .children() + .find(|n| n.has_tag_name("artifactId")) + .and_then(|n| n.text()) + .unwrap_or("unknown") + .to_string(); + + // Find executions + if let Some(executions_node) = plugin.children().find(|n| n.has_tag_name("executions")) + { + for execution in executions_node .children() - .find(|n| n.has_tag_name("artifactId")) - .and_then(|n| n.text()) - .unwrap_or("unknown") - .to_string(); - - // Find executions - if let Some(executions_node) = - plugin.children().find(|n| n.has_tag_name("executions")) + .filter(|n| n.has_tag_name("execution")) { - for execution in executions_node + // Get execution ID + let exec_id = execution .children() - .filter(|n| n.has_tag_name("execution")) + .find(|n| n.has_tag_name("id")) + .and_then(|n| n.text()) + .unwrap_or("default") + .to_string(); + + // Get goals + if let Some(goals_node) = execution.children().find(|n| n.has_tag_name("goals")) { - // Get execution ID - let exec_id = execution - .children() - .find(|n| n.has_tag_name("id")) - .and_then(|n| n.text()) - .unwrap_or("default") - .to_string(); - - // Get goals - if let Some(goals_node) = - execution.children().find(|n| n.has_tag_name("goals")) - { - for goal in goals_node.children().filter(|n| n.has_tag_name("goal")) { - if let Some(goal_text) = goal.text() { - let task_name = format!("{}:{}", artifact_id, goal_text); - tasks.push(Task { - name: task_name.clone(), - file_path: file_path.to_path_buf(), - definition_type: TaskDefinitionType::MavenPom, - runner: TaskRunner::Maven, - source_name: task_name, - description: Some(format!( - "Maven plugin goal {} (execution: {})", - goal_text, exec_id - )), - shadowed_by: None, - disambiguated_name: None, - }); - } + for goal in goals_node.children().filter(|n| n.has_tag_name("goal")) { + if let Some(goal_text) = goal.text() { + let task_name = format!("{}:{}", artifact_id, goal_text); + tasks.push(Task { + name: task_name.clone(), + file_path: file_path.to_path_buf(), + definition_type: TaskDefinitionType::MavenPom, + runner: TaskRunner::Maven, + source_name: task_name, + description: Some(format!( + "Maven plugin goal {} (execution: {})", + goal_text, exec_id + )), + shadowed_by: None, + disambiguated_name: None, + }); } } } diff --git a/src/parsers/parse_pyproject_toml.rs b/src/parsers/parse_pyproject_toml.rs index 0f140aa..c64d2f5 100644 --- a/src/parsers/parse_pyproject_toml.rs +++ b/src/parsers/parse_pyproject_toml.rs @@ -13,26 +13,24 @@ pub fn parse(path: &Path) -> Result, String> { let mut tasks = Vec::new(); // Check for UV scripts - if let Some(project) = toml.get("project") { - if let Some(scripts) = project.get("scripts") { - if let Some(scripts_table) = scripts.as_table() { - if cfg!(test) || check_path_executable("uv").is_some() { - for (name, cmd) in scripts_table { - let description = cmd.as_str().map(|s| format!("python script: {}", s)); - - tasks.push(Task { - name: name.clone(), - file_path: path.to_path_buf(), - definition_type: TaskDefinitionType::PyprojectToml, - runner: TaskRunner::PythonUv, - source_name: name.clone(), - description, - shadowed_by: None, - disambiguated_name: None, - }); - } - } - } + if let Some(project) = toml.get("project") + && let Some(scripts) = project.get("scripts") + && let Some(scripts_table) = scripts.as_table() + && (cfg!(test) || check_path_executable("uv").is_some()) + { + for (name, cmd) in scripts_table { + let description = cmd.as_str().map(|s| format!("python script: {}", s)); + + tasks.push(Task { + name: name.clone(), + file_path: path.to_path_buf(), + definition_type: TaskDefinitionType::PyprojectToml, + runner: TaskRunner::PythonUv, + source_name: name.clone(), + description, + shadowed_by: None, + disambiguated_name: None, + }); } } @@ -45,72 +43,68 @@ pub fn parse(path: &Path) -> Result, String> { tool_val }; - if let Some(scripts) = poetry.get("scripts") { - if let Some(scripts_table) = scripts.as_table() { - let poetry_lock_exists = path - .parent() - .map(|dir| dir.join("poetry.lock").exists()) - .unwrap_or(false); - if cfg!(test) - || (check_path_executable("poetry").is_some() - && (cfg!(test) || poetry_lock_exists)) - { - for (name, cmd) in scripts_table { - let description = cmd.as_str().map(|s| format!("python script: {}", s)); - - tasks.push(Task { - name: name.clone(), - file_path: path.to_path_buf(), - definition_type: TaskDefinitionType::PyprojectToml, - runner: TaskRunner::PythonPoetry, - source_name: name.clone(), - description, - shadowed_by: None, - disambiguated_name: None, - }); - } + if let Some(scripts) = poetry.get("scripts") + && let Some(scripts_table) = scripts.as_table() + { + let poetry_lock_exists = path + .parent() + .map(|dir| dir.join("poetry.lock").exists()) + .unwrap_or(false); + if poetry_lock_exists && (cfg!(test) || check_path_executable("poetry").is_some()) { + for (name, cmd) in scripts_table { + let description = cmd.as_str().map(|s| format!("python script: {}", s)); + + tasks.push(Task { + name: name.clone(), + file_path: path.to_path_buf(), + definition_type: TaskDefinitionType::PyprojectToml, + runner: TaskRunner::PythonPoetry, + source_name: name.clone(), + description, + shadowed_by: None, + disambiguated_name: None, + }); } } } } // Check for poethepoet tasks - if let Some(poe) = toml.get("tool") { - if let Some(poe_section) = poe.get("poe") { - // If there is a nested "tasks" key, use that table; otherwise, use the poe_section table directly - if let Some(tasks_table) = if let Some(inner) = poe_section.get("tasks") { - inner.as_table() - } else { - poe_section.as_table() - } { - if cfg!(test) || check_path_executable("poe").is_some() { - for (name, task_def) in tasks_table { - let description = match task_def { - toml::Value::String(cmd) => Some(format!("command: {}", cmd)), - toml::Value::Table(table) => { - if let Some(script) = table.get("script") { - script.as_str().map(|s| format!("python script: {}", s)) - } else if let Some(shell) = table.get("shell") { - shell.as_str().map(|s| format!("shell script: {}", s)) - } else { - None - } - } - _ => None, - }; - - tasks.push(Task { - name: name.clone(), - file_path: path.to_path_buf(), - definition_type: TaskDefinitionType::PyprojectToml, - runner: TaskRunner::PythonPoe, - source_name: name.clone(), - description, - shadowed_by: None, - disambiguated_name: None, - }); + if let Some(poe) = toml.get("tool") + && let Some(poe_section) = poe.get("poe") + { + // If there is a nested "tasks" key, use that table; otherwise, use the poe_section table directly + if let Some(tasks_table) = if let Some(inner) = poe_section.get("tasks") { + inner.as_table() + } else { + poe_section.as_table() + } && (cfg!(test) || check_path_executable("poe").is_some()) + { + for (name, task_def) in tasks_table { + let description = match task_def { + toml::Value::String(cmd) => Some(format!("command: {}", cmd)), + toml::Value::Table(table) => { + if let Some(script) = table.get("script") { + script.as_str().map(|s| format!("python script: {}", s)) + } else if let Some(shell) = table.get("shell") { + shell.as_str().map(|s| format!("shell script: {}", s)) + } else { + None + } } - } + _ => None, + }; + + tasks.push(Task { + name: name.clone(), + file_path: path.to_path_buf(), + definition_type: TaskDefinitionType::PyprojectToml, + runner: TaskRunner::PythonPoe, + source_name: name.clone(), + description, + shadowed_by: None, + disambiguated_name: None, + }); } } } diff --git a/src/parsers/parse_taskfile.rs b/src/parsers/parse_taskfile.rs index 86ef744..2fa3387 100644 --- a/src/parsers/parse_taskfile.rs +++ b/src/parsers/parse_taskfile.rs @@ -59,7 +59,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { TaskCommand::String(cmd) => format!("command: {}", cmd), TaskCommand::Map(_map) => { // Just indicate it's a complex command without parsing details - format!("complex command") + "complex command".to_string() } } } else { @@ -202,13 +202,13 @@ tasks: assert_eq!(tasks.len(), 3); // Verify that the internal tasks are not included - assert!(tasks.iter().find(|t| t.name == "internal-task").is_none()); - assert!(tasks.iter().find(|t| t.name == "helper").is_none()); + assert!(!tasks.iter().any(|t| t.name == "internal-task")); + assert!(!tasks.iter().any(|t| t.name == "helper")); // Verify the normal tasks are included - assert!(tasks.iter().find(|t| t.name == "build").is_some()); - assert!(tasks.iter().find(|t| t.name == "test").is_some()); - assert!(tasks.iter().find(|t| t.name == "clean").is_some()); + assert!(tasks.iter().any(|t| t.name == "build")); + assert!(tasks.iter().any(|t| t.name == "test")); + assert!(tasks.iter().any(|t| t.name == "clean")); } #[test] fn test_parse_taskfile_with_nested_commands() { diff --git a/src/parsers/parse_travis_ci.rs b/src/parsers/parse_travis_ci.rs index d0d10f5..4c2f69b 100644 --- a/src/parsers/parse_travis_ci.rs +++ b/src/parsers/parse_travis_ci.rs @@ -34,7 +34,7 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str let mut tasks = Vec::new(); // Extract jobs from the configuration - if let Some(Value::Mapping(jobs_map)) = config_map.get(&Value::String("jobs".to_string())) { + if let Some(Value::Mapping(jobs_map)) = config_map.get(Value::String("jobs".to_string())) { // Parse jobs section for (job_key, job_value) in jobs_map { if let Value::String(job_name) = job_key { @@ -57,16 +57,16 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str } else { // If no jobs section, look for matrix or other job definitions if let Some(Value::Mapping(matrix_map)) = - config_map.get(&Value::String("matrix".to_string())) + config_map.get(Value::String("matrix".to_string())) { // Handle matrix configuration if let Some(Value::Sequence(include_list)) = - matrix_map.get(&Value::String("include".to_string())) + matrix_map.get(Value::String("include".to_string())) { for (i, include_item) in include_list.iter().enumerate() { if let Value::Mapping(include_map) = include_item { if let Some(Value::String(job_name)) = - include_map.get(&Value::String("name".to_string())) + include_map.get(Value::String("name".to_string())) { let description = extract_job_description(include_item); @@ -130,18 +130,18 @@ fn extract_job_description(job_value: &Value) -> Option { match job_value { Value::Mapping(job_map) => { // Try to get name first - if let Some(Value::String(name)) = job_map.get(&Value::String("name".to_string())) { + if let Some(Value::String(name)) = job_map.get(Value::String("name".to_string())) { return Some(format!("Travis CI job: {}", name)); } // Try to get stage - if let Some(Value::String(stage)) = job_map.get(&Value::String("stage".to_string())) { + if let Some(Value::String(stage)) = job_map.get(Value::String("stage".to_string())) { return Some(format!("Travis CI job in stage: {}", stage)); } // Try to get language if let Some(Value::String(language)) = - job_map.get(&Value::String("language".to_string())) + job_map.get(Value::String("language".to_string())) { return Some(format!("Travis CI {} job", language)); } @@ -185,7 +185,7 @@ jobs: stage: build "#; - let file_path = create_test_travis_config(&temp_dir.path(), ".travis.yml", travis_content); + let file_path = create_test_travis_config(temp_dir.path(), ".travis.yml", travis_content); let tasks = parse(&file_path).expect("Failed to parse Travis CI config"); @@ -231,7 +231,7 @@ matrix: python: "3.10" "#; - let file_path = create_test_travis_config(&temp_dir.path(), ".travis.yml", travis_content); + let file_path = create_test_travis_config(temp_dir.path(), ".travis.yml", travis_content); let tasks = parse(&file_path).expect("Failed to parse Travis CI config"); @@ -265,7 +265,7 @@ script: - bundle exec rspec "#; - let file_path = create_test_travis_config(&temp_dir.path(), ".travis.yml", travis_content); + let file_path = create_test_travis_config(temp_dir.path(), ".travis.yml", travis_content); let tasks = parse(&file_path).expect("Failed to parse Travis CI config"); @@ -301,7 +301,7 @@ jobs: invalid: yaml: content "#; - let file_path = create_test_travis_config(&temp_dir.path(), ".travis.yml", invalid_content); + let file_path = create_test_travis_config(temp_dir.path(), ".travis.yml", invalid_content); let result = parse(&file_path); assert!(result.is_err(), "Should fail to parse invalid YAML"); @@ -311,7 +311,7 @@ jobs: fn test_parse_empty_file() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); - let file_path = create_test_travis_config(&temp_dir.path(), ".travis.yml", ""); + let file_path = create_test_travis_config(temp_dir.path(), ".travis.yml", ""); let tasks = parse(&file_path).expect("Failed to parse empty Travis CI config"); diff --git a/src/prompt.rs b/src/prompt.rs index 58561e9..be3fc36 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -248,7 +248,7 @@ mod tests { // Test helper function that simulates the TUI logic fn test_tui_logic(selected_index: usize) -> Result { - let options = vec![ + let options = [ ( "Allow once (this time only)", AllowDecision::Allow(AllowScope::Once), diff --git a/src/task_discovery.rs b/src/task_discovery.rs index e22ce80..3a696a9 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -93,10 +93,7 @@ pub fn process_task_disambiguation(discovered: &mut DiscoveredTasks) { // Count occurrences of each task name for (i, task) in discovered.tasks.iter().enumerate() { *task_name_counts.entry(task.name.clone()).or_insert(0) += 1; - tasks_by_name - .entry(task.name.clone()) - .or_insert_with(Vec::new) - .push(i); + tasks_by_name.entry(task.name.clone()).or_default().push(i); } // Save task name counts for reference @@ -184,7 +181,7 @@ pub fn is_task_ambiguous(discovered: &DiscoveredTasks, task_name: &str) -> bool discovered .task_name_counts .get(task_name) - .map_or(false, |&count| count > 1) + .is_some_and(|&count| count > 1) } /// Returns a list of disambiguated task names for tasks with the given name @@ -206,7 +203,7 @@ pub fn get_matching_tasks<'a>(discovered: &'a DiscoveredTasks, task_name: &str) if let Some(task) = discovered.tasks.iter().find(|t| { t.disambiguated_name .as_ref() - .map_or(false, |dn| dn == task_name) + .is_some_and(|dn| dn == task_name) }) { result.push(task); return result; @@ -609,21 +606,22 @@ fn discover_github_actions_tasks( // 3. Check custom directories that might contain workflows for custom_dir in &["workflows", "custom/workflows", ".gitlab/workflows"] { let custom_path = dir.join(custom_dir); - if custom_path.exists() && custom_path.is_dir() { - if let Ok(entries) = fs::read_dir(&custom_path) { - let files: Vec = entries - .filter_map(Result::ok) - .map(|entry| entry.path()) - .filter(|path| { - if let Some(ext) = path.extension() { - ext == "yml" || ext == "yaml" - } else { - false - } - }) - .collect(); - workflow_files.extend(files); - } + if custom_path.exists() + && custom_path.is_dir() + && let Ok(entries) = fs::read_dir(&custom_path) + { + let files: Vec = entries + .filter_map(Result::ok) + .map(|entry| entry.path()) + .filter(|path| { + if let Some(ext) = path.extension() { + ext == "yml" || ext == "yaml" + } else { + false + } + }) + .collect(); + workflow_files.extend(files); } } @@ -821,33 +819,32 @@ fn discover_shell_script_tasks(dir: &Path, discovered: &mut DiscoveredTasks) { if let Ok(entries) = fs::read_dir(dir) { for entry in entries.flatten() { let path = entry.path(); - if path.is_file() { - if let Some(extension) = path.extension() { - if extension == "sh" { - // Use file stem (without extension) for task name and disambiguation - let name = path - .file_stem() - .unwrap_or_default() - .to_string_lossy() - .to_string(); - // Use full filename (with extension) for source_name since we execute ./script.sh - let source_name = path - .file_name() - .unwrap_or_default() - .to_string_lossy() - .to_string(); - discovered.tasks.push(Task { - name: name.clone(), - file_path: path, - definition_type: TaskDefinitionType::ShellScript, - runner: TaskRunner::ShellScript, - source_name, - description: None, - shadowed_by: check_shadowing(&name), - disambiguated_name: None, - }); - } - } + if path.is_file() + && let Some(extension) = path.extension() + && extension == "sh" + { + // Use file stem (without extension) for task name and disambiguation + let name = path + .file_stem() + .unwrap_or_default() + .to_string_lossy() + .to_string(); + // Use full filename (with extension) for source_name since we execute ./script.sh + let source_name = path + .file_name() + .unwrap_or_default() + .to_string_lossy() + .to_string(); + discovered.tasks.push(Task { + name: name.clone(), + file_path: path, + definition_type: TaskDefinitionType::ShellScript, + runner: TaskRunner::ShellScript, + source_name, + description: None, + shadowed_by: check_shadowing(&name), + disambiguated_name: None, + }); } } } @@ -864,10 +861,12 @@ mod tests { use std::io::Write; use tempfile::TempDir; + type ExecuteFn = Box Result<(), String>>; + // Define mocks for command execution tests struct MockTaskExecutor { // Mock implementation to handle execute() calls in tests - execute_fn: Box Result<(), String>>, + execute_fn: ExecuteFn, } impl MockTaskExecutor { diff --git a/src/types.rs b/src/types.rs index dea3ff2..b6c0f11 100644 --- a/src/types.rs +++ b/src/types.rs @@ -287,7 +287,7 @@ pub struct AllowlistEntry { pub tasks: Option>, } -fn serialize_path(path: &PathBuf, serializer: S) -> Result +fn serialize_path(path: &std::path::Path, serializer: S) -> Result where S: serde::Serializer, { diff --git a/tests/Dockerfile.builder b/tests/Dockerfile.builder index e39297f..db71009 100644 --- a/tests/Dockerfile.builder +++ b/tests/Dockerfile.builder @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.4 # --- Stage 1: Builder --- -FROM rust:alpine3.21 AS builder +FROM rust:1.91.1-alpine3.21 AS builder # Install build dependencies RUN apk add --no-cache \ diff --git a/tests/docker_unit/Dockerfile b/tests/docker_unit/Dockerfile index 73b0102..1a94203 100644 --- a/tests/docker_unit/Dockerfile +++ b/tests/docker_unit/Dockerfile @@ -2,7 +2,7 @@ FROM dela-builder AS builder # Test environment -FROM rust:alpine3.21 +FROM rust:1.91.1-alpine3.21 # Install build dependencies RUN apk add --no-cache \