Fix clippy pedantic warnings across workspace (#94)

Author: lukidoescode

.changeset/changesets/mightily-harmonic-canvasback.md

@@ -0,0 +1,4 @@
+---
+changeset-operations: major
+---
+AddOperation::execute() now takes &AddInput instead of owned AddInput.

.changeset/changesets/nobly-handsome-onager.md

@@ -0,0 +1,5 @@
+---
+changeset-project: none
+cargo-changeset: none
+---
+Internal refactoring to resolve clippy pedantic warnings with no user-visible changes.

Cargo.toml

@@ -12,9 +12,6 @@ authors = ["Lukas Wagner <appdev.lukaswagner@gmail.com>"]
 
 [workspace.lints.clippy]
 pedantic = { level = "warn", priority = -1 }
-needless_pass_by_value = "allow"             # needed for Bevy systems
-type_complexity = "allow"                    # Some bevy types are just complex. We do not care about that. Clippy's type complexity metric is too strict
-struct_excessive_bools = "allow"             # CLI input structs map directly to command-line flags
 unwrap_used = "deny"
 
 [workspace.metadata]

crates/cargo-changeset/src/commands/add.rs

@@ -34,11 +34,11 @@ pub(super) fn run(args: AddArgs, start_path: &Path) -> Result<()> {
     let result = if is_interactive() {
         let interaction_provider = TerminalInteractionProvider::new(args.editor);
         let operation = AddOperation::new(project_provider, changeset_writer, interaction_provider);
-        operation.execute(start_path, input)?
+        operation.execute(start_path, &input)?
     } else {
         let interaction_provider = NonInteractiveProvider;
         let operation = AddOperation::new(project_provider, changeset_writer, interaction_provider);
-        operation.execute(start_path, input)?
+        operation.execute(start_path, &input)?
     };
 
     match result {

crates/changeset-operations/src/mocks.rs

@@ -170,7 +170,7 @@ impl DependencyGraphProvider for MockProjectProvider {
             .map(|p| p.name().clone())
             .collect();
         Ok(WorkspaceDependencyGraph::from_edges(
-            member_names,
+            &member_names,
             &self.dependency_edges,
         ))
     }
@@ -414,18 +414,28 @@ impl ChangesetWriter for MockChangesetWriter {
     }
 }
 
+#[derive(Clone, Copy)]
+enum TagFailMode {
+    Never,
+    Always,
+    OnNth(usize),
+}
+
+struct MockFailureFlags {
+    commit: bool,
+    stage_files: bool,
+    is_clean: bool,
+    create_tag: TagFailMode,
+}
+
 struct MockGitState {
     staged_files: Vec<PathBuf>,
     commits: Vec<String>,
     tags_created: Vec<(String, String)>,
     deleted_files: Vec<PathBuf>,
     deleted_tags: Vec<String>,
     reset_count: usize,
-    fail_on_commit: bool,
-    fail_on_create_tag: bool,
-    fail_on_create_tag_nth: Option<usize>,
-    fail_on_stage_files: bool,
-    fail_on_is_clean: bool,
+    failure_flags: MockFailureFlags,
 }
 
 pub struct MockGitProvider {
@@ -453,11 +463,12 @@ impl MockGitProvider {
                 deleted_files: Vec::new(),
                 deleted_tags: Vec::new(),
                 reset_count: 0,
-                fail_on_commit: false,
-                fail_on_create_tag: false,
-                fail_on_create_tag_nth: None,
-                fail_on_stage_files: false,
-                fail_on_is_clean: false,
+                failure_flags: MockFailureFlags {
+                    commit: false,
+                    stage_files: false,
+                    is_clean: false,
+                    create_tag: TagFailMode::Never,
+                },
             }),
         }
     }
@@ -539,29 +550,47 @@ impl MockGitProvider {
     }
 
     pub fn set_fail_on_commit(&self, fail: bool) {
-        self.state.lock().expect("lock poisoned").fail_on_commit = fail;
+        self.state
+            .lock()
+            .expect("lock poisoned")
+            .failure_flags
+            .commit = fail;
     }
 
     pub fn set_fail_on_create_tag(&self, fail: bool) {
-        self.state.lock().expect("lock poisoned").fail_on_create_tag = fail;
+        self.state
+            .lock()
+            .expect("lock poisoned")
+            .failure_flags
+            .create_tag = if fail {
+            TagFailMode::Always
+        } else {
+            TagFailMode::Never
+        };
     }
 
     pub fn set_fail_on_create_tag_nth(&self, n: usize) {
         self.state
             .lock()
             .expect("lock poisoned")
-            .fail_on_create_tag_nth = Some(n);
+            .failure_flags
+            .create_tag = TagFailMode::OnNth(n);
     }
 
     pub fn set_fail_on_stage_files(&self, fail: bool) {
         self.state
             .lock()
             .expect("lock poisoned")
-            .fail_on_stage_files = fail;
+            .failure_flags
+            .stage_files = fail;
     }
 
     pub fn set_fail_on_is_clean(&self, fail: bool) {
-        self.state.lock().expect("lock poisoned").fail_on_is_clean = fail;
+        self.state
+            .lock()
+            .expect("lock poisoned")
+            .failure_flags
+            .is_clean = fail;
     }
 }
 
@@ -590,7 +619,13 @@ impl GitWorkdirDiffProvider for MockGitProvider {
 
 impl GitStatusProvider for MockGitProvider {
     fn is_working_tree_clean(&self, _project_root: &Path) -> Result<bool> {
-        if self.state.lock().expect("lock poisoned").fail_on_is_clean {
+        if self
+            .state
+            .lock()
+            .expect("lock poisoned")
+            .failure_flags
+            .is_clean
+        {
             return Err(crate::OperationError::Io(std::io::Error::other(
                 "mock is_working_tree_clean failure",
             )));
@@ -610,7 +645,7 @@ impl GitStatusProvider for MockGitProvider {
 impl GitStagingProvider for MockGitProvider {
     fn stage_files(&self, _project_root: &Path, paths: &[&Path]) -> Result<()> {
         let mut state = self.state.lock().expect("lock poisoned");
-        if state.fail_on_stage_files {
+        if state.failure_flags.stage_files {
             return Err(crate::OperationError::Io(std::io::Error::other(
                 "mock stage files failure",
             )));
@@ -634,7 +669,7 @@ impl GitStagingProvider for MockGitProvider {
 impl GitCommitProvider for MockGitProvider {
     fn commit(&self, _project_root: &Path, message: &str) -> Result<CommitInfo> {
         let mut state = self.state.lock().expect("lock poisoned");
-        if state.fail_on_commit {
+        if state.failure_flags.commit {
             return Err(crate::OperationError::Io(std::io::Error::other(
                 "mock commit failure",
             )));
@@ -655,21 +690,16 @@ impl GitCommitProvider for MockGitProvider {
 impl GitTagProvider for MockGitProvider {
     fn create_tag(&self, _project_root: &Path, tag_name: &str, message: &str) -> Result<TagInfo> {
         let mut state = self.state.lock().expect("lock poisoned");
-        if state.fail_on_create_tag {
+        let should_fail = match state.failure_flags.create_tag {
+            TagFailMode::Never => false,
+            TagFailMode::Always => true,
+            TagFailMode::OnNth(n) => state.tags_created.len() == n,
+        };
+        if should_fail {
             return Err(crate::OperationError::Io(std::io::Error::other(
                 "mock create tag failure",
             )));
         }
-
-        let current_count = state.tags_created.len();
-        if let Some(n) = state.fail_on_create_tag_nth {
-            if current_count == n {
-                return Err(crate::OperationError::Io(std::io::Error::other(
-                    "mock create tag failure (nth)",
-                )));
-            }
-        }
-
         state
             .tags_created
             .push((tag_name.to_string(), message.to_string()));

crates/changeset-operations/src/operations/add.rs

@@ -68,7 +68,7 @@ where
     ///
     /// Returns an error if the project cannot be discovered, has no packages, or
     /// if the changeset cannot be written.
-    pub fn execute(&self, start_path: &Path, input: AddInput) -> Result<AddResult> {
+    pub fn execute(&self, start_path: &Path, input: &AddInput) -> Result<AddResult> {
         let project = self.project_provider.discover_project(start_path)?;
 
         if project.packages().is_empty() {
@@ -104,7 +104,7 @@ where
         });
 
         let packages =
-            match self.select_packages(project.packages(), &input, display_labels.as_deref())? {
+            match self.select_packages(project.packages(), input, display_labels.as_deref())? {
                 Some(packages) if packages.is_empty() => return Ok(AddResult::NoPackages),
                 Some(packages) => packages,
                 None => return Ok(AddResult::Cancelled),
@@ -120,15 +120,15 @@ where
             Vec::new()
         };
 
-        let Some(releases) = self.collect_releases(&packages, &input)? else {
+        let Some(releases) = self.collect_releases(&packages, input)? else {
             return Ok(AddResult::Cancelled);
         };
 
-        let Some(category) = self.select_category(&input)? else {
+        let Some(category) = self.select_category(input)? else {
             return Ok(AddResult::Cancelled);
         };
 
-        let Some(desc) = self.get_description(&input)? else {
+        let Some(desc) = self.get_description(input)? else {
             return Ok(AddResult::Cancelled);
         };
 
@@ -354,7 +354,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation failed with valid single-package input");
 
         match result {
@@ -393,7 +393,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation failed with valid multi-package input");
 
         match result {
@@ -420,7 +420,7 @@ mod tests {
         let operation = AddOperation::new(project_provider, writer, interaction);
 
         let result = operation
-            .execute(Path::new("/any"), AddInput::default())
+            .execute(Path::new("/any"), &AddInput::default())
             .expect("AddOperation should not fail when interaction is cancelled");
 
         assert!(matches!(result, AddResult::Cancelled));
@@ -441,7 +441,7 @@ mod tests {
         let operation = AddOperation::new(project_provider, writer, interaction);
 
         let result = operation
-            .execute(Path::new("/any"), AddInput::default())
+            .execute(Path::new("/any"), &AddInput::default())
             .expect("AddOperation should not fail when bump selection is cancelled");
 
         assert!(matches!(result, AddResult::Cancelled));
@@ -462,7 +462,7 @@ mod tests {
             ..Default::default()
         };
 
-        let result = operation.execute(Path::new("/any"), input);
+        let result = operation.execute(Path::new("/any"), &input);
 
         assert!(result.is_err());
         let err = result.expect_err("AddOperation should fail for unknown package");
@@ -484,7 +484,7 @@ mod tests {
             ..Default::default()
         };
 
-        let result = operation.execute(Path::new("/any"), input);
+        let result = operation.execute(Path::new("/any"), &input);
 
         assert!(result.is_err());
         let err = result.expect_err("AddOperation should fail for empty description");
@@ -507,7 +507,7 @@ mod tests {
         let operation = AddOperation::new(project_provider, writer, interaction);
 
         let result = operation
-            .execute(Path::new("/any"), AddInput::default())
+            .execute(Path::new("/any"), &AddInput::default())
             .expect("AddOperation failed with interactive workspace selection");
 
         match result {
@@ -538,7 +538,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation failed for single-package auto-selection");
 
         match result {
@@ -568,7 +568,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation failed with explicit category");
 
         match result {
@@ -595,7 +595,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation failed to create changeset file");
 
         match result {
@@ -621,7 +621,7 @@ mod tests {
             ..Default::default()
         };
 
-        let result = operation.execute(Path::new("/any"), input);
+        let result = operation.execute(Path::new("/any"), &input);
 
         assert!(result.is_err());
         let err = result.expect_err("AddOperation should fail for none bump without description");
@@ -644,7 +644,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed for none bump with description");
 
         match result {
@@ -677,7 +677,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed with interactive description for none bump");
 
         match result {
@@ -710,7 +710,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed for mixed none/patch bumps with description");
 
         match result {
@@ -756,7 +756,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed with exclude_dependents");
 
         match result {
@@ -791,7 +791,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed and report uncovered dependents");
 
         match result {
@@ -823,7 +823,7 @@ mod tests {
         };
 
         let result = operation
-            .execute(Path::new("/any"), input)
+            .execute(Path::new("/any"), &input)
             .expect("AddOperation should succeed for single-package project");
 
         match result {