Fix Clippy warnings and run Clippy in CI

.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

.github/workflows/rust.yml

@@ -14,24 +14,25 @@ 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
             ~/.cargo/git
             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

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"]

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

rust-toolchain.toml

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "1.91.1"
+components = ["rustfmt", "clippy"]

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 => {

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());

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()
@@ -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<String, Vec<&Task>> = 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

src/commands/mcp.rs

@@ -169,11 +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() {
-            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

src/mcp/allowlist.rs

@@ -35,10 +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) {
-                    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();