Merge branch 'auto-clippy'

Makefile

@@ -1,3 +1,11 @@
+# Rejected for now, and why
+#	-D clippy::default-trait-access - sometimes makes imports necessary, just for a default value. It's good for more explicit typing though.
+#   -D clippy::range-plus-one - useful, but caused too many false positives as we use range types directly quite a lot
+CLIPPY_FLAGS = \
+	-D clippy::uninlined_format_args \
+	-D clippy::unnested-or-patterns \
+	-D clippy::explicit-iter-loop \
+	-D clippy::map-unwrap-or
 
 help:  ## Display this help
 	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n"} /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
@@ -51,10 +59,16 @@ doc: ## Run cargo doc on all crates
 	RUSTDOCFLAGS="-D warnings" cargo doc --features=max,lean,small --all --no-deps
 
 clippy: ## Run cargo clippy on all crates
-	cargo clippy --all --tests --examples
-	cargo clippy --all --no-default-features --features small
-	cargo clippy --all --no-default-features --features max-pure
-	cargo clippy --all --no-default-features --features lean-async --tests
+	cargo clippy --all --tests --examples -- $(CLIPPY_FLAGS)
+	cargo clippy --all --no-default-features --features small -- $(CLIPPY_FLAGS)
+	cargo clippy --all --no-default-features --features max-pure -- $(CLIPPY_FLAGS)
+	cargo clippy --all --no-default-features --features lean-async --tests -- $(CLIPPY_FLAGS)
+
+fix-clippy: ## Run cargo clippy on all crates, fixing what can be fixed
+	cargo clippy --fix --all --tests --examples -- $(CLIPPY_FLAGS)
+	cargo clippy --fix --allow-dirty --all --no-default-features --features small -- $(CLIPPY_FLAGS)
+	cargo clippy --fix --allow-dirty --all --no-default-features --features max-pure -- $(CLIPPY_FLAGS)
+	cargo clippy --fix --allow-dirty --all --no-default-features --features lean-async --tests -- $(CLIPPY_FLAGS)
 
 check-msrv: ## run cargo msrv to validate the current msrv requirements, similar to what CI does
 	cd gix && cargo check --package gix --no-default-features --features async-network-client,max-performance
@@ -342,4 +356,3 @@ fmt: ## run nightly rustfmt for its extra features, but check that it won't upse
 
 try-publish-all: ## Dry-run publish all crates in the currently set version if they are not published yet.
 	(cd cargo-smart-release && cargo build --bin cargo-smart-release) && cargo-smart-release/target/debug/cargo-smart-release smart-release gitoxide
-

cargo-smart-release/src/changelog/parse.rs

@@ -79,13 +79,10 @@ impl ChangeLog {
             }),
         }
 
-        let insert_sorted_at_pos = sections
-            .first()
-            .map(|s| match s {
-                Section::Verbatim { .. } => 1,
-                Section::Release { .. } => 0,
-            })
-            .unwrap_or(0);
+        let insert_sorted_at_pos = sections.first().map_or(0, |s| match s {
+            Section::Verbatim { .. } => 1,
+            Section::Release { .. } => 0,
+        });
         let mut non_release_sections = Vec::new();
         let mut release_sections = Vec::new();
         for section in sections {
@@ -362,7 +359,7 @@ fn parse_id_fallback_to_user_message(
                     .messages
                     .push(section::segment::conventional::Message::Generated {
                         id,
-                        title: lines.next().map(|l| l.trim()).unwrap_or("").to_owned(),
+                        title: lines.next().map_or("", |l| l.trim()).to_owned(),
                         body: lines
                             .map(|l| {
                                 match l
@@ -437,10 +434,12 @@ fn track_unknown_event(unknown_event: Event<'_>, unknown: &mut String) {
         | Event::Code(text)
         | Event::Text(text)
         | Event::FootnoteReference(text)
-        | Event::Start(Tag::FootnoteDefinition(text))
-        | Event::Start(Tag::CodeBlock(pulldown_cmark::CodeBlockKind::Fenced(text)))
-        | Event::Start(Tag::Link(_, text, _))
-        | Event::Start(Tag::Image(_, text, _)) => unknown.push_str(text.as_ref()),
+        | Event::Start(
+            Tag::FootnoteDefinition(text)
+            | Tag::CodeBlock(pulldown_cmark::CodeBlockKind::Fenced(text))
+            | Tag::Link(_, text, _)
+            | Tag::Image(_, text, _),
+        ) => unknown.push_str(text.as_ref()),
         _ => {}
     }
 }
@@ -518,7 +517,7 @@ fn headline<'a, E: ParseError<&'a str> + FromExternalError<&'a str, ()>>(i: &'a
         ),
         |((hashes, (prefix, version)), date)| Headline {
             level: hashes.len(),
-            version_prefix: prefix.map(ToOwned::to_owned).unwrap_or_else(String::new),
+            version_prefix: prefix.map_or_else(String::new, ToOwned::to_owned),
             version,
             date,
         },

cargo-smart-release/src/changelog/section/from_history.rs

@@ -130,16 +130,17 @@ impl Section {
             }
         }
 
-        let version = crate::git::try_strip_tag_path(segment.head.name.as_ref())
-            .map(|tag_name| {
+        let version = crate::git::try_strip_tag_path(segment.head.name.as_ref()).map_or_else(
+            || changelog::Version::Unreleased,
+            |tag_name| {
                 let package_name =
                     (!is_top_level_package(&package.manifest_path, repo)).then_some(package.name.as_str());
                 changelog::Version::Semantic(
                     utils::parse_possibly_prefixed_tag_version(package_name, tag_name)
                         .expect("here we always have a valid version as it passed a filter when creating it"),
                 )
-            })
-            .unwrap_or_else(|| changelog::Version::Unreleased);
+            },
+        );
         let date = match version {
             changelog::Version::Unreleased => None,
             changelog::Version::Semantic(_) => Some(date_time),

cargo-smart-release/src/changelog/write.rs

@@ -51,11 +51,12 @@ impl From<gix::Url> for RepositoryUrl {
 
 impl RepositoryUrl {
     pub fn is_github(&self) -> bool {
-        self.inner.host().map(|h| h == "github.com").unwrap_or(false)
+        self.inner.host().map_or(false, |h| h == "github.com")
     }
 
     fn cleaned_path(&self) -> String {
         let path = self.inner.path.to_str_lossy().into_owned();
+        #[allow(clippy::map_unwrap_or)]
         let path = path.strip_suffix(".git").map(ToOwned::to_owned).unwrap_or(path);
         if !path.starts_with('/') {
             format!("/{path}")

cargo-smart-release/src/command/changelog.rs

@@ -66,11 +66,9 @@ pub fn changelog(opts: Options, crates: Vec<String>) -> anyhow::Result<()> {
     let linkables = if dry_run || no_links {
         Linkables::AsText
     } else {
-        crate::git::remote_url(&ctx.repo)?
-            .map(|url| Linkables::AsLinks {
-                repository_url: url.into(),
-            })
-            .unwrap_or(Linkables::AsText)
+        git::remote_url(&ctx.repo)?.map_or(Linkables::AsText, |url| Linkables::AsLinks {
+            repository_url: url.into(),
+        })
     };
     let mut num_crates = 0;
     for (idx, package) in crates.iter().enumerate() {

cargo-smart-release/src/command/release/manifest.rs

@@ -485,7 +485,7 @@ fn gather_changelog_data<'meta>(
                 capitalize_commit,
             )?;
             lock.with_mut(|file| file.write_all(write_buf.as_bytes()))?;
-            *made_change |= previous_content.map(|previous| write_buf != previous).unwrap_or(true);
+            *made_change |= previous_content.map_or(true, |previous| write_buf != previous);
             pending_changelogs.push((publishee, log_init_state.is_modified(), lock));
             release_section_by_publishee.insert(publishee.name.as_str(), log.take_recent_release_section());
         }
@@ -534,7 +534,7 @@ fn set_version_and_update_package_dependency(
                     let force_update = conservative_pre_release_version_handling
                         && version::is_pre_release(new_version) // setting the lower bound unnecessarily can be harmful
                         // don't claim to be conservative if this is necessary anyway
-                        && req_as_version(&version_req).map(|req_version|!version::rhs_is_breaking_bump_for_lhs(&req_version, new_version)).unwrap_or(false);
+                        && req_as_version(&version_req).map_or(false, |req_version|!version::rhs_is_breaking_bump_for_lhs(&req_version, new_version));
                     if !version_req.matches(new_version) || force_update {
                         if !version_req_unset_or_default(&version_req) {
                             bail!(

cargo-smart-release/src/command/release/mod.rs

@@ -36,11 +36,9 @@ impl Context {
     ) -> anyhow::Result<Self> {
         let base = crate::Context::new(crate_names, changelog, bump, bump_dependencies)?;
         let changelog_links = if changelog_links {
-            crate::git::remote_url(&base.repo)?
-                .map(|url| Linkables::AsLinks {
-                    repository_url: url.into(),
-                })
-                .unwrap_or(Linkables::AsText)
+            crate::git::remote_url(&base.repo)?.map_or(Linkables::AsText, |url| Linkables::AsLinks {
+                repository_url: url.into(),
+            })
         } else {
             Linkables::AsText
         };
@@ -281,10 +279,10 @@ fn present_and_validate_dependencies(
                         kind,
                         dep.package.name,
                         dep.package.version,
-                        bump.latest_release
-                            .as_ref()
-                            .map(|latest_release| format!(" to succeed latest released version {latest_release}"))
-                            .unwrap_or_else(|| ", creating a new release 🎉".into()),
+                        bump.latest_release.as_ref().map_or_else(
+                            || ", creating a new release 🎉".into(),
+                            |latest_release| format!(" to succeed latest released version {latest_release}")
+                        ),
                         bump.desired_release
                     );
                 };
@@ -455,7 +453,7 @@ fn perform_release(ctx: &Context, options: Options, crates: &[Dependency<'_>]) -
         }
     }
 
-    publish_err.map(Err).unwrap_or(Ok(()))
+    publish_err.map_or(Ok(()), Err)
 }
 
 fn wait_for_release(

cargo-smart-release/src/commit/message.rs

@@ -25,10 +25,7 @@ mod additions {
                     .or_else(|| part_to_left[p..].chars().next().map(|c| p + c.len_utf8()))
             })
             .unwrap_or(start);
-        let new_end = s[end..]
-            .find(|c: char| !c.is_whitespace())
-            .map(|p| p + end)
-            .unwrap_or(end);
+        let new_end = s[end..].find(|c: char| !c.is_whitespace()).map_or(end, |p| p + end);
         s.replace_range(
             new_start..new_end,
             if new_end != end && new_start != start { " " } else { "" },
@@ -84,18 +81,8 @@ mod additions {
 
 impl From<&'_ str> for Message {
     fn from(m: &str) -> Self {
-        let (title, kind, body, breaking, breaking_description) = git_conventional::Commit::parse(m)
-            .map(|c: git_conventional::Commit<'_>| {
-                (
-                    c.description().into(),
-                    Some(c.type_()),
-                    c.body().map(Into::into),
-                    c.breaking(),
-                    c.breaking_description()
-                        .and_then(|d| if d == c.description() { None } else { Some(d) }),
-                )
-            })
-            .unwrap_or_else(|_| {
+        let (title, kind, body, breaking, breaking_description) = git_conventional::Commit::parse(m).map_or_else(
+            |_| {
                 let m = gix::objs::commit::MessageRef::from_bytes(m.as_bytes());
                 (
                     m.summary().as_ref().to_string().into(),
@@ -104,7 +91,18 @@ impl From<&'_ str> for Message {
                     false,
                     None,
                 )
-            });
+            },
+            |c: git_conventional::Commit<'_>| {
+                (
+                    c.description().into(),
+                    Some(c.type_()),
+                    c.body().map(Into::into),
+                    c.breaking(),
+                    c.breaking_description()
+                        .and_then(|d| if d == c.description() { None } else { Some(d) }),
+                )
+            },
+        );
         let (title, additions) = additions::strip(title);
         Message {
             title: title.into_owned(),

cargo-smart-release/src/context.rs

@@ -77,9 +77,10 @@ fn fill_in_root_crate_if_needed(crate_names: Vec<String>) -> anyhow::Result<Vec<
             .to_str()
             .expect("directory is UTF8 representable");
         let crate_name = if manifest.is_file() {
-            cargo_toml::Manifest::from_path(manifest)
-                .map(|manifest| manifest.package.map_or(dir_name.to_owned(), |p| p.name))
-                .unwrap_or_else(|_| dir_name.to_owned())
+            cargo_toml::Manifest::from_path(manifest).map_or_else(
+                |_| dir_name.to_owned(),
+                |manifest| manifest.package.map_or(dir_name.to_owned(), |p| p.name),
+            )
         } else {
             dir_name.to_owned()
         };

cargo-smart-release/src/git/history.rs

@@ -132,20 +132,8 @@ pub fn crate_ref_segments<'h>(
     };
 
     let dir = ctx.repo_relative_path(package);
-    let mut filter = dir
-        .map(|dir| {
-            let mut components = dir.components().collect::<Vec<_>>();
-            match components.len() {
-                0 => unreachable!("BUG: it's None if empty"),
-                1 => Filter::Fast {
-                    name: components.pop().map(component_to_bytes).expect("exactly one").into(),
-                },
-                _ => Filter::Slow {
-                    components: components.into_iter().map(component_to_bytes).collect(),
-                },
-            }
-        })
-        .unwrap_or_else(|| {
+    let mut filter = dir.map_or_else(
+        || {
             if ctx.meta.workspace_members.len() == 1 {
                 Filter::None
             } else {
@@ -158,9 +146,22 @@ pub fn crate_ref_segments<'h>(
                     name: Cow::Borrowed(b"src"),
                 }
             }
-        });
+        },
+        |dir| {
+            let mut components = dir.components().collect::<Vec<_>>();
+            match components.len() {
+                0 => unreachable!("BUG: it's None if empty"),
+                1 => Filter::Fast {
+                    name: components.pop().map(component_to_bytes).expect("exactly one").into(),
+                },
+                _ => Filter::Slow {
+                    components: components.into_iter().map(component_to_bytes).collect(),
+                },
+            }
+        },
+    );
 
-    for item in history.items.iter() {
+    for item in &history.items {
         match tags_by_commit.remove(&item.id) {
             None => add_item_if_package_changed(ctx, &mut segment, &mut filter, item, &history.data_by_tree_id)?,
             Some(next_ref) => {
@@ -249,7 +250,7 @@ fn add_item_if_package_changed<'a>(
                     }
                     history.push(item)
                 }
-                (None, Some(_)) | (None, None) => {}
+                (None, _) => {}
             };
         }
         Filter::Slow { ref components } => {
@@ -269,7 +270,7 @@ fn add_item_if_package_changed<'a>(
                     }
                 }
                 (Some(_), None) => history.push(item),
-                (None, Some(_)) | (None, None) => {}
+                (None, _) => {}
             };
         }
     };

cargo-smart-release/src/utils.rs

@@ -60,10 +60,7 @@ pub fn is_top_level_package(manifest_path: &Utf8Path, repo: &gix::Repository) ->
 }
 
 pub fn version_req_unset_or_default(req: &VersionReq) -> bool {
-    req.comparators
-        .last()
-        .map(|comp| comp.op == semver::Op::Caret)
-        .unwrap_or(true)
+    req.comparators.last().map_or(true, |comp| comp.op == semver::Op::Caret)
 }
 
 pub fn package_eq_dependency_ignore_dev_without_version(package: &Package, dependency: &Dependency) -> bool {

cargo-smart-release/src/version.rs

@@ -116,7 +116,7 @@ pub(crate) fn bump_package_with_spec(
             } else if unreleased
                 .history
                 .iter()
-                .any(|item| item.message.kind.map(|kind| kind == "feat").unwrap_or(false))
+                .any(|item| item.message.kind.map_or(false, |kind| kind == "feat"))
             {
                 let is_breaking = if is_pre_release(&v) {
                     bump_major_minor_patch(&mut v, Patch)

gitoxide-core/src/hours/mod.rs

@@ -412,7 +412,7 @@ where
         .expect("at least one commit at this point");
     if show_pii {
         results_by_hours.sort_by(|a, b| a.hours.partial_cmp(&b.hours).unwrap_or(std::cmp::Ordering::Equal));
-        for entry in results_by_hours.iter() {
+        for entry in &results_by_hours {
             entry.write_to(
                 total_hours,
                 file_stats.then_some(total_files),

gitoxide-core/src/pack/explode.rs

@@ -157,7 +157,7 @@ pub fn pack_or_pack_index(
         )
     })?;
 
-    if !object_path.as_ref().map(|p| p.as_ref().is_dir()).unwrap_or(true) {
+    if !object_path.as_ref().map_or(true, |p| p.as_ref().is_dir()) {
         return Err(anyhow!(
             "The object directory at '{}' is inaccessible",
             object_path
@@ -167,16 +167,16 @@ pub fn pack_or_pack_index(
         ));
     }
 
-    let algorithm = object_path
-        .as_ref()
-        .map(|_| pack::index::traverse::Algorithm::Lookup)
-        .unwrap_or_else(|| {
+    let algorithm = object_path.as_ref().map_or_else(
+        || {
             if sink_compress {
                 pack::index::traverse::Algorithm::Lookup
             } else {
                 pack::index::traverse::Algorithm::DeltaTreeLookup
             }
-        });
+        },
+        |_| pack::index::traverse::Algorithm::Lookup,
+    );
 
     let pack::index::traverse::Outcome { progress, .. } = bundle
         .index