Fix Clippy warnings and run Clippy in CI

.github/workflows/rust.yml

@@ -19,7 +19,7 @@ jobs:
       - name: Install Rust toolchain
         uses: dtolnay/rust-toolchain@stable
         with:
-          components: rustfmt
+          components: rustfmt, clippy
 
       - name: Cache dependencies
         uses: actions/cache@v5
@@ -33,5 +33,8 @@ jobs:
       - name: Check formatting
         run: cargo fmt --all -- --check
 
+      - name: Run clippy
+        run: cargo clippy --all-targets --all-features -- -D warnings
+
       - name: Run tests
         run: cargo test --verbose

src/allowlist.rs

@@ -68,11 +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) {
+        if let AllowScope::Deny = entry.scope
+            && path_matches(&task.file_path, &entry.path, true) {
                 return Ok((false, true));
             }
-        }
     }
 
     // Second pass: Check for allow entries
@@ -89,13 +88,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) {
+                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 => {
                 // Already handled deny in first pass, skip Once

src/commands/init.rs

@@ -70,12 +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() {
+    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
     let mut file = fs::OpenOptions::new()

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<String> = 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()

src/commands/mcp.rs

@@ -169,12 +169,11 @@ fn merge_dela_into_toml(existing: &str) -> Result<String, String> {
 /// 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() {
+    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
     if config_path.exists() {

src/mcp/allowlist.rs

@@ -35,11 +35,10 @@ impl McpAllowlistEvaluator {
     pub fn is_task_allowed(&self, task: &Task) -> Result<bool, String> {
         // 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) {
+            if let AllowScope::Deny = entry.scope
+                && self.path_matches(&task.file_path, &entry.path, true) {
                     return Ok(false);
                 }
-            }
         }
 
         // Second pass: Check for allow entries
@@ -56,13 +55,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) {
+                    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 => {
                     // Already handled deny in first pass, skip Once (not applicable for MCP)

src/mcp/dto.rs

@@ -86,17 +86,13 @@ impl TaskDto {
 
 /// Parameters for the list_tasks MCP tool
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
+#[derive(Default)]
 pub struct ListTasksArgs {
     /// Optional runner filter - if provided, only return tasks for this runner
     /// Examples: "make", "npm", "gradle", "poetry"
     pub runner: Option<String>,
 }
 
-impl Default for ListTasksArgs {
-    fn default() -> Self {
-        Self { runner: None }
-    }
-}
 
 #[cfg(test)]
 mod tests {

src/mcp/server.rs

@@ -317,11 +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 {
+            if let Some(entry) = cache.as_ref()
+                && entry.cached_at.elapsed() < self.task_cache_ttl {
                     return entry.discovered.clone();
                 }
-            }
         }
 
         let discovered = task_discovery::discover_tasks(&self.root);
@@ -372,7 +371,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 +400,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 +742,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 +1065,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 +1154,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 +1216,7 @@ impl DelaMcpServer {
         };
 
         Ok(CallToolResult::success(vec![
-            Content::json(&serde_json::json!({
+            Content::json(serde_json::json!({
                 "pid": args.pid,
                 "status": status,
                 "message": message,

src/parsers/parse_cmake.rs

@@ -39,6 +39,9 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result<Vec<Task>, 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,14 +55,10 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result<Vec<Task>, 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) {
+        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 {
             name: target_name.to_string(),

src/parsers/parse_github_actions.rs

@@ -29,14 +29,14 @@ fn parse_workflow_string(content: &str, file_path: &Path) -> Result<Vec<Task>, 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()),
     };

src/parsers/parse_gradle.rs

@@ -131,45 +131,39 @@ fn extract_custom_tasks(
 fn extract_task_description(content: &str, task_name: &str) -> Option<String> {
     // 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) {
+    ))
+        && 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) {
+    ))
+        && 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
     let kotlin_pattern = format!(
         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) {
+    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())
 }

src/parsers/parse_makefile.rs

@@ -11,7 +11,7 @@ pub fn parse(path: &Path) -> Result<Vec<Task>, String> {
 
     // Special case for the test_discover_tasks_with_invalid_makefile test
     if content.contains("<hello>not a make file</hello>") {
-        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

src/parsers/parse_package_json.rs

@@ -20,8 +20,8 @@ pub fn parse(path: &PathBuf) -> Result<Vec<Task>, String> {
 
     let mut tasks = Vec::new();
 
-    if let Some(scripts) = json.get("scripts") {
-        if let Some(scripts_obj) = scripts.as_object() {
+    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(),
@@ -35,7 +35,6 @@ pub fn parse(path: &PathBuf) -> Result<Vec<Task>, String> {
                 });
             }
         }
-    }
 
     Ok(tasks)
 }

src/parsers/parse_pom_xml.rs

@@ -84,8 +84,8 @@ fn add_profile_tasks(tasks: &mut Vec<Task>, root: Node, file_path: &Path) -> Res
 /// Add tasks from Maven plugins
 fn add_plugin_tasks(tasks: &mut Vec<Task>, root: Node, file_path: &Path) -> Result<(), String> {
     // Find <build> section and then <plugins>
-    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")) {
+    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
@@ -139,7 +139,6 @@ fn add_plugin_tasks(tasks: &mut Vec<Task>, root: Node, file_path: &Path) -> Resu
                 }
             }
         }
-    }
 
     Ok(())
 }