diff --git a/src/apply.rs b/src/apply.rs index 406e96286f..e3202fb7e0 100644 --- a/src/apply.rs +++ b/src/apply.rs @@ -1,9 +1,6 @@ //! git_apply support //! see original: -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::new_without_default)] - use crate::{panic, raw, util::Binding, DiffDelta, DiffHunk}; use libc::c_int; use std::{ffi::c_void, mem}; @@ -146,11 +143,23 @@ impl<'cb> ApplyOptions<'cb> { } /// Pointer to a raw git_stash_apply_options + /// + /// # Safety + /// + /// The provided pointer must not be used to manipulate the options, and + /// must not outlive the [`ApplyOptions`] instance. pub unsafe fn raw(&mut self) -> *const raw::git_apply_options { &self.raw as *const _ } } +impl<'cb> Default for ApplyOptions<'cb> { + /// Creates a new set of empty options (zeroed). + fn default() -> Self { + Self::new() + } +} + #[cfg(test)] #[allow(clippy::needless_borrows_for_generic_args)] mod tests { diff --git a/src/blame.rs b/src/blame.rs index 3bd4aeace2..40870f6256 100644 --- a/src/blame.rs +++ b/src/blame.rs @@ -1,5 +1,3 @@ -#![allow(clippy::redundant_closure)] - use crate::util::{self, Binding}; use crate::{raw, signature, Error, ErrorClass, ErrorCode, Oid, Repository, Signature}; use libc::c_char; @@ -222,7 +220,7 @@ impl<'blame> BlameHunk<'blame> { /// `Ok(None)` may be returned if there is no summary. pub fn summary(&self) -> Result, Error> { match self.summary_bytes() { - Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()), + Some(sb) => str::from_utf8(sb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/blob.rs b/src/blob.rs index c73862ddd9..3d605739f0 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -1,5 +1,3 @@ -#![allow(clippy::io_other_error)] - use std::io; use std::marker; use std::mem; @@ -139,12 +137,12 @@ impl<'repo> io::Write for BlobWriter<'repo> { if let Some(f) = write_cb { let res = f(self.raw, buf.as_ptr() as *const _, buf.len()); if res < 0 { - Err(io::Error::new(io::ErrorKind::Other, "Write error")) + Err(io::Error::other("Write error")) } else { Ok(buf.len()) } } else { - Err(io::Error::new(io::ErrorKind::Other, "no write callback")) + Err(io::Error::other("no write callback")) } } fn flush(&mut self) -> io::Result<()> { diff --git a/src/branch.rs b/src/branch.rs index ffcccecfb6..1d5cf767c5 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -1,5 +1,3 @@ -#![allow(clippy::missing_safety_doc)] - use std::ffi::CString; use std::marker; use std::ptr; @@ -129,8 +127,11 @@ impl<'repo> Branch<'repo> { impl<'repo> Branches<'repo> { /// Creates a new iterator from the raw pointer given. /// + /// # Safety + /// /// This function is unsafe as it is not guaranteed that `raw` is a valid - /// pointer. + /// pointer. The caller must ensure that `raw` is a valid pointer and lives + /// as long as the [`Branches`] object. pub unsafe fn from_raw(raw: *mut raw::git_branch_iterator) -> Branches<'repo> { Branches { raw, diff --git a/src/buf.rs b/src/buf.rs index 754fd93ad5..904ee6e21d 100644 --- a/src/buf.rs +++ b/src/buf.rs @@ -1,5 +1,3 @@ -#![allow(clippy::explicit_auto_deref)] - use std::ops::{Deref, DerefMut}; use std::ptr; use std::slice; @@ -38,7 +36,7 @@ impl Buf { /// Attempt to view this buffer as a string slice. pub fn as_str(&self) -> Result<&str, Error> { - str::from_utf8(&**self).map_err(|e| e.into()) + str::from_utf8(self).map_err(|e| e.into()) } } @@ -78,6 +76,7 @@ impl Drop for Buf { } #[test] +#[allow(clippy::explicit_auto_deref)] fn empty_buf() { let mut buf = Buf::new(); let x: &[u8] = &*buf; diff --git a/src/build.rs b/src/build.rs index 0b27006718..09c6838251 100644 --- a/src/build.rs +++ b/src/build.rs @@ -1,8 +1,5 @@ //! Builder-pattern objects for configuration various git operations. -#![allow(clippy::manual_non_exhaustive)] -#![allow(clippy::missing_safety_doc)] - use libc::{c_char, c_int, c_uint, c_void, size_t}; use std::ffi::{CStr, CString}; use std::mem; @@ -128,6 +125,7 @@ impl<'cb> Default for RepoBuilder<'cb> { /// Options that can be passed to `RepoBuilder::clone_local`. #[derive(Clone, Copy)] +#[non_exhaustive] pub enum CloneLocal { /// Auto-detect (default) /// @@ -143,9 +141,6 @@ pub enum CloneLocal { /// Bypass the git-aware transport, but don't try to use hardlinks. NoLinks = raw::GIT_CLONE_LOCAL_NO_LINKS as isize, - - #[doc(hidden)] - __Nonexhaustive = 0xff, } impl<'cb> RepoBuilder<'cb> { @@ -593,8 +588,14 @@ impl<'cb> CheckoutBuilder<'cb> { /// Configure a raw checkout options based on this configuration. /// + /// # Safety + /// /// This method is unsafe as there is no guarantee that this structure will - /// outlive the provided checkout options. + /// outlive the provided checkout options. The caller must ensure that + /// `opts` does not outlive the [`CheckoutBuilder`], and also that for any + /// fields configured in the `CheckoutBuilder` that are provided to the + /// `opts` as pointers, those pointers are not used if the original + /// references held in the `CheckoutBuilder` are dropped. pub unsafe fn configure(&mut self, opts: &mut raw::git_checkout_options) { opts.version = raw::GIT_CHECKOUT_OPTIONS_VERSION; opts.disable_filters = self.disable_filters as c_int; diff --git a/src/call.rs b/src/call.rs index 210d5e3409..1a757b97ac 100644 --- a/src/call.rs +++ b/src/call.rs @@ -1,4 +1,3 @@ -#![allow(clippy::needless_lifetimes)] #![macro_use] use crate::Error; @@ -69,12 +68,12 @@ mod impls { *self as libc::c_int } } - impl<'a, T> Convert<*const T> for &'a T { + impl Convert<*const T> for &T { fn convert(&self) -> *const T { *self as *const T } } - impl<'a, T> Convert<*mut T> for &'a mut T { + impl Convert<*mut T> for &mut T { fn convert(&self) -> *mut T { &**self as *const T as *mut T } diff --git a/src/cherrypick.rs b/src/cherrypick.rs index 81e11e94a2..3496cf7d3c 100644 --- a/src/cherrypick.rs +++ b/src/cherrypick.rs @@ -1,5 +1,3 @@ -#![allow(clippy::new_without_default)] - use std::mem; use crate::build::CheckoutBuilder; @@ -78,3 +76,10 @@ impl<'cb> CherrypickOptions<'cb> { cherrypick_opts } } + +impl<'cb> Default for CherrypickOptions<'cb> { + /// Creates a default set of cherrypick options + fn default() -> Self { + Self::new() + } +} diff --git a/src/commit.rs b/src/commit.rs index 7acaaddbc3..fb1ef1363d 100644 --- a/src/commit.rs +++ b/src/commit.rs @@ -1,5 +1,3 @@ -#![allow(clippy::redundant_closure)] - use std::iter::FusedIterator; use std::marker; use std::mem; @@ -84,7 +82,7 @@ impl<'repo> Commit<'repo> { pub fn message_encoding(&self) -> Result, Error> { let bytes = unsafe { crate::opt_bytes(self, raw::git_commit_message_encoding(&*self.raw)) }; match bytes { - Some(b) => str::from_utf8(b).map(|s| Some(s)).map_err(|e| e.into()), + Some(b) => str::from_utf8(b).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -131,7 +129,7 @@ impl<'repo> Commit<'repo> { /// `Ok(None)` may be returned if there is no summary pub fn summary(&self) -> Result, Error> { match self.summary_bytes() { - Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()), + Some(sb) => str::from_utf8(sb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -155,7 +153,7 @@ impl<'repo> Commit<'repo> { /// `Ok(None)` may be returned if there is no body. pub fn body(&self) -> Result, Error> { match self.body_bytes() { - Some(sb) => str::from_utf8(sb).map(|s| Some(s)).map_err(|e| e.into()), + Some(sb) => str::from_utf8(sb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/config.rs b/src/config.rs index f4ab82dd64..189fc6deac 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,3 @@ -#![allow(clippy::should_implement_trait)] - use std::ffi::CString; use std::marker; use std::path::{Path, PathBuf}; @@ -601,6 +599,7 @@ impl<'cfg> ConfigEntries<'cfg> { /// Advances the iterator and returns the next value. /// /// Returns `None` when iteration is finished. + #[expect(clippy::should_implement_trait)] pub fn next(&mut self) -> Option, Error>> { let mut raw = ptr::null_mut(); drop(self.current.take()); diff --git a/src/cred.rs b/src/cred.rs index 999570fdaa..07f78ef658 100644 --- a/src/cred.rs +++ b/src/cred.rs @@ -1,10 +1,3 @@ -#![allow(clippy::manual_strip)] -#![allow(clippy::match_result_ok)] -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::needless_borrowed_reference)] -#![allow(clippy::needless_borrows_for_generic_args)] -#![allow(clippy::should_implement_trait)] - #[cfg(feature = "cred")] use log::{debug, trace}; use std::ffi::CString; @@ -39,6 +32,7 @@ pub struct CredentialHelper { impl Cred { /// Create a "default" credential usable for Negotiate mechanisms like NTLM /// or Kerberos authentication. + #[expect(clippy::should_implement_trait)] pub fn default() -> Result { crate::init(); let mut out = ptr::null_mut(); @@ -171,6 +165,11 @@ impl Cred { } /// Unwrap access to the underlying raw pointer, canceling the destructor + /// + /// # Safety + /// + /// The caller must assume responsibility for freeing the credential if + /// needed. See the drop implementation for guidance. pub unsafe fn unwrap(mut self) -> *mut raw::git_cred { mem::replace(&mut self.raw, ptr::null_mut()) } @@ -277,14 +276,14 @@ impl CredentialHelper { // Discover `useHttpPath` from `config` fn config_use_http_path(&mut self, config: &Config) { let mut use_http_path = false; - if let Some(value) = config.get_bool(&self.exact_key("useHttpPath")).ok() { + if let Ok(value) = config.get_bool(&self.exact_key("useHttpPath")) { use_http_path = value; } else if let Some(value) = self .url_key("useHttpPath") .and_then(|key| config.get_bool(&key).ok()) { use_http_path = value; - } else if let Some(value) = config.get_bool("credential.useHttpPath").ok() { + } else if let Ok(value) = config.get_bool("credential.useHttpPath") { use_http_path = value; } @@ -313,8 +312,8 @@ impl CredentialHelper { Some(s) => s, }; - if cmd.starts_with('!') { - self.commands.push(cmd[1..].to_string()); + if let Some(stripped) = cmd.strip_prefix('!') { + self.commands.push(stripped.to_string()); } else if is_absolute_path(cmd) { self.commands.push(cmd.to_string()); } else { @@ -328,7 +327,7 @@ impl CredentialHelper { fn url_key(&self, name: &str) -> Option { match (&self.host, &self.protocol) { - (&Some(ref host), &Some(ref protocol)) => { + (Some(host), Some(protocol)) => { Some(format!("credential.{}://{}.{}", protocol, host, name)) } _ => None, @@ -398,7 +397,7 @@ impl CredentialHelper { c.creation_flags(CREATE_NO_WINDOW); } c.arg("-c") - .arg(&format!("{} get", cmd)) + .arg(format!("{} get", cmd)) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); @@ -502,6 +501,7 @@ impl CredentialHelper { #[cfg(test)] #[cfg(feature = "cred")] +#[allow(clippy::needless_borrows_for_generic_args)] #[allow(clippy::unused_io_amount)] #[allow(clippy::useless_conversion)] mod test { diff --git a/src/diff.rs b/src/diff.rs index 6583f13e7b..d1eda4f4fc 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1,7 +1,3 @@ -#![allow(clippy::empty_docs)] -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::new_without_default)] - use libc::{c_char, c_int, c_void, size_t}; use std::ffi::CString; use std::iter::FusedIterator; @@ -956,6 +952,8 @@ impl DiffOptions { /// Acquire a pointer to the underlying raw options. /// + /// # Safety + /// /// This function is unsafe as the pointer is only valid so long as this /// structure is not moved, modified, or used elsewhere. pub unsafe fn raw(&mut self) -> *const raw::git_diff_options { @@ -1000,9 +998,9 @@ impl<'diff> ExactSizeIterator for Deltas<'diff> {} pub enum DiffLineType { /// These values will be sent to `git_diff_line_cb` along with the line Context, - /// + /// Line was added Addition, - /// + /// Line was removed Deletion, /// Both files have no LF at end ContextEOFNL, @@ -1013,7 +1011,7 @@ pub enum DiffLineType { /// The following values will only be sent to a `git_diff_line_cb` when /// the content of a diff is being formatted through `git_diff_print`. FileHeader, - /// + /// Line represents the header of a hunk, e.g. "@@ -1,2 +0,0 @@\n" HunkHeader, /// For "Binary files x and y differ" Binary, @@ -1529,6 +1527,11 @@ impl DiffFindOptions { // TODO: expose git_diff_similarity_metric /// Acquire a pointer to the underlying raw options. + /// + /// # Safety + /// + /// The provided pointer must not be used to manipulate the options, and + /// must not outlive the [`DiffFindOptions`] instance. pub unsafe fn raw(&mut self) -> *const raw::git_diff_find_options { &self.raw } @@ -1592,6 +1595,14 @@ impl DiffPatchidOptions { } } +impl Default for DiffPatchidOptions { + /// Creates a new set of patchid options, + /// initialized to the default values + fn default() -> Self { + Self::new() + } +} + #[cfg(test)] #[allow(clippy::assign_op_pattern)] #[allow(clippy::needless_borrows_for_generic_args)] diff --git a/src/email.rs b/src/email.rs index 14218b06d7..4f87e056f1 100644 --- a/src/email.rs +++ b/src/email.rs @@ -1,5 +1,3 @@ -#![allow(clippy::too_many_arguments)] - use std::ffi::CString; use std::{mem, ptr}; @@ -140,6 +138,7 @@ impl Email { } /// Create a diff for a commit in mbox format for sending via email. + #[expect(clippy::too_many_arguments)] pub fn from_diff( diff: &Diff<'_>, patch_idx: usize, diff --git a/src/error.rs b/src/error.rs index ba4d3afb8f..20f3c17dea 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,3 @@ -#![allow(clippy::should_implement_trait)] - use libc::c_int; use std::env::JoinPathsError; use std::error; @@ -77,6 +75,7 @@ impl Error { /// /// The error returned will have the code `GIT_ERROR` and the class /// `GIT_ERROR_NONE`. + #[expect(clippy::should_implement_trait)] pub fn from_str(s: &str) -> Error { Error { code: raw::GIT_ERROR as c_int, diff --git a/src/indexer.rs b/src/indexer.rs index 045d22cf90..ac65ca9bd6 100644 --- a/src/indexer.rs +++ b/src/indexer.rs @@ -1,5 +1,3 @@ -#![allow(clippy::io_other_error)] - use std::ffi::CStr; use std::path::Path; use std::{io, marker, mem, ptr}; @@ -223,7 +221,7 @@ impl io::Write for Indexer<'_> { let len = buf.len(); let res = unsafe { raw::git_indexer_append(self.raw, ptr, len, &mut self.progress) }; if res < 0 { - Err(io::Error::new(io::ErrorKind::Other, Error::last_error(res))) + Err(io::Error::other(Error::last_error(res))) } else { Ok(buf.len()) } diff --git a/src/lib.rs b/src/lib.rs index e44b4a39c3..628fc94daa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,8 +70,6 @@ #![deny(missing_docs)] #![warn(rust_2018_idioms)] #![cfg_attr(test, deny(warnings))] -#![allow(clippy::needless_lifetimes)] -#![allow(clippy::should_implement_trait)] #![allow(clippy::unnecessary_cast)] use bitflags::bitflags; @@ -906,7 +904,7 @@ fn openssl_env_init() { ))] fn openssl_env_init() {} -unsafe fn opt_bytes<'a, T>(_anchor: &'a T, c: *const libc::c_char) -> Option<&'a [u8]> { +unsafe fn opt_bytes(_anchor: &T, c: *const libc::c_char) -> Option<&[u8]> { if c.is_null() { None } else { @@ -955,6 +953,7 @@ impl ObjectType { } /// Convert a string object type representation to its object type. + #[expect(clippy::should_implement_trait)] pub fn from_str(s: &str) -> Option { let raw = unsafe { call!(raw::git_object_string2type(CString::new(s).unwrap())) }; ObjectType::from_raw(raw) diff --git a/src/merge.rs b/src/merge.rs index a95754d9a2..401d68ca5a 100644 --- a/src/merge.rs +++ b/src/merge.rs @@ -1,6 +1,3 @@ -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::redundant_closure)] - use libc::{c_char, c_uint, c_ushort, size_t}; use std::ffi::CString; use std::marker; @@ -185,6 +182,11 @@ impl MergeOptions { } /// Acquire a pointer to the underlying raw options. + /// + /// # Safety + /// + /// The provided pointer must not be used to manipulate the options, and + /// must not outlive the [`MergeOptions`] instance. pub unsafe fn raw(&self) -> *const raw::git_merge_options { &self.raw as *const _ } @@ -365,7 +367,7 @@ impl MergeFileResult { /// returns `Ok(None)` if a filename conflict would occur pub fn path(&self) -> Result, Error> { match self.path_bytes() { - Some(pb) => str::from_utf8(pb).map(|s| Some(s)).map_err(|e| e.into()), + Some(pb) => str::from_utf8(pb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/message.rs b/src/message.rs index cbd929fffd..ef094f761b 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1,7 +1,3 @@ -#![allow(clippy::len_without_is_empty)] -#![allow(clippy::needless_borrow)] -#![allow(clippy::redundant_closure)] - use core::ops::Range; use std::ffi::CStr; use std::ffi::CString; @@ -43,7 +39,7 @@ pub const DEFAULT_COMMENT_CHAR: Option = Some(b'#'); /// /// Use this function when you are dealing with a UTF-8-encoded message. pub fn message_trailers_strs(message: &str) -> Result { - _message_trailers(message.into_c_string()?).map(|res| MessageTrailersStrs(res)) + _message_trailers(message.into_c_string()?).map(MessageTrailersStrs) } /// Get the trailers for the given message. @@ -52,7 +48,7 @@ pub fn message_trailers_strs(message: &str) -> Result(message: S) -> Result { - _message_trailers(message.into_c_string()?).map(|res| MessageTrailersBytes(res)) + _message_trailers(message.into_c_string()?).map(MessageTrailersBytes) } fn _message_trailers(message: CString) -> Result { @@ -77,6 +73,10 @@ impl MessageTrailersStrs { pub fn len(&self) -> usize { self.0.len() } + /// Whether there are no trailers + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } /// Convert to the “bytes” variant. pub fn to_bytes(self) -> MessageTrailersBytes { MessageTrailersBytes(self.0) @@ -97,6 +97,10 @@ impl MessageTrailersBytes { pub fn len(&self) -> usize { self.0.len() } + /// Whether there are no trailers + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } } struct MessageTrailers { @@ -126,6 +130,10 @@ impl MessageTrailers { fn len(&self) -> usize { self.raw.count } + /// Whether there are no trailers + fn is_empty(&self) -> bool { + self.raw.count == 0 + } } impl Drop for MessageTrailers { @@ -166,7 +174,7 @@ impl<'pair> Iterator for MessageTrailersStrsIterator<'pair> { self.0 .range .next() - .map(|index| to_str_tuple(&self.0.trailers, index)) + .map(|index| to_str_tuple(self.0.trailers, index)) } fn size_hint(&self) -> (usize, Option) { @@ -187,12 +195,12 @@ impl DoubleEndedIterator for MessageTrailersStrsIterator<'_> { self.0 .range .next_back() - .map(|index| to_str_tuple(&self.0.trailers, index)) + .map(|index| to_str_tuple(self.0.trailers, index)) } } fn to_str_tuple(trailers: &MessageTrailers, index: usize) -> (&str, &str) { - let (rkey, rvalue) = to_raw_tuple(&trailers, index); + let (rkey, rvalue) = to_raw_tuple(trailers, index); let key = unsafe { CStr::from_ptr(rkey).to_str().unwrap() }; let value = unsafe { CStr::from_ptr(rvalue).to_str().unwrap() }; (key, value) @@ -208,7 +216,7 @@ impl<'pair> Iterator for MessageTrailersBytesIterator<'pair> { self.0 .range .next() - .map(|index| to_bytes_tuple(&self.0.trailers, index)) + .map(|index| to_bytes_tuple(self.0.trailers, index)) } fn size_hint(&self) -> (usize, Option) { @@ -229,12 +237,12 @@ impl DoubleEndedIterator for MessageTrailersBytesIterator<'_> { self.0 .range .next_back() - .map(|index| to_bytes_tuple(&self.0.trailers, index)) + .map(|index| to_bytes_tuple(self.0.trailers, index)) } } fn to_bytes_tuple(trailers: &MessageTrailers, index: usize) -> (&[u8], &[u8]) { - let (rkey, rvalue) = to_raw_tuple(&trailers, index); + let (rkey, rvalue) = to_raw_tuple(trailers, index); let key = unsafe { CStr::from_ptr(rkey).to_bytes() }; let value = unsafe { CStr::from_ptr(rvalue).to_bytes() }; (key, value) diff --git a/src/odb.rs b/src/odb.rs index d854093159..7916b737f8 100644 --- a/src/odb.rs +++ b/src/odb.rs @@ -1,8 +1,3 @@ -#![allow(clippy::io_other_error)] -#![allow(clippy::len_without_is_empty)] -#![allow(clippy::single_match)] -#![allow(clippy::should_implement_trait)] - use std::io; use std::marker; use std::ptr; @@ -337,6 +332,11 @@ impl<'a> OdbObject<'a> { unsafe { raw::git_odb_object_size(self.raw) } } + /// Check if the data is empty + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Get the object data. pub fn data(&self) -> &[u8] { let size = self.len(); @@ -388,7 +388,7 @@ impl<'repo> io::Read for OdbReader<'repo> { let len = buf.len(); let res = unsafe { raw::git_odb_stream_read(self.raw, ptr, len) }; if res < 0 { - Err(io::Error::new(io::ErrorKind::Other, "Read error")) + Err(io::Error::other("Read error")) } else { Ok(res as _) } @@ -447,7 +447,7 @@ impl<'repo> io::Write for OdbWriter<'repo> { let len = buf.len(); let res = unsafe { raw::git_odb_stream_write(self.raw, ptr, len) }; if res < 0 { - Err(io::Error::new(io::ErrorKind::Other, "Write error")) + Err(io::Error::other("Write error")) } else { Ok(buf.len()) } @@ -514,7 +514,7 @@ impl<'repo> io::Write for OdbPackwriter<'repo> { }; } if res < 0 { - Err(io::Error::new(io::ErrorKind::Other, "Write error")) + Err(io::Error::other("Write error")) } else { Ok(buf.len()) } @@ -528,9 +528,8 @@ impl<'repo> Drop for OdbPackwriter<'repo> { fn drop(&mut self) { unsafe { let writepack = &*self.raw; - match writepack.free { - Some(free) => free(self.raw), - None => (), + if let Some(free) = writepack.free { + free(self.raw); }; drop(Box::from_raw(self.progress_payload_ptr)); diff --git a/src/oid.rs b/src/oid.rs index 8bfca61180..01f2885756 100644 --- a/src/oid.rs +++ b/src/oid.rs @@ -92,6 +92,7 @@ impl Oid { /// /// Returns an error if the string is empty, is longer than 40 hex /// characters, or contains any non-hex characters. + #[expect(clippy::should_implement_trait)] pub fn from_str(s: &str) -> Result { Self::from_str_ext(s, ObjectFormat::Sha1) } diff --git a/src/oid_array.rs b/src/oid_array.rs index 87abef2974..dfdae33bfa 100644 --- a/src/oid_array.rs +++ b/src/oid_array.rs @@ -1,7 +1,5 @@ //! Bindings to libgit2's raw `git_oidarray` type -#![allow(clippy::extra_unused_lifetimes)] - use std::ops::Deref; use crate::oid::Oid; @@ -41,7 +39,7 @@ impl Binding for OidArray { } } -impl<'repo> std::fmt::Debug for OidArray { +impl std::fmt::Debug for OidArray { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { f.debug_tuple("OidArray").field(&self.deref()).finish() } diff --git a/src/opts.rs b/src/opts.rs index facf5896e5..bac124a076 100644 --- a/src/opts.rs +++ b/src/opts.rs @@ -1,7 +1,5 @@ //! Bindings to libgit2's git_libgit2_opts function. -#![allow(clippy::missing_safety_doc)] - use std::ffi::CString; use std::ptr; @@ -19,6 +17,8 @@ use crate::{raw, Buf, ConfigLevel, Error, IntoCString, ObjectType}; /// Use magic path `$PATH` to include the old value of the path /// (if you want to prepend or append, for instance). /// +/// # Safety +/// /// This function is unsafe as it mutates the global state but cannot guarantee /// thread-safety. It needs to be externally synchronized with calls to access /// the global state. @@ -41,6 +41,8 @@ where /// `level` must be one of [`ConfigLevel::System`], [`ConfigLevel::Global`], /// [`ConfigLevel::XDG`], [`ConfigLevel::ProgramData`]. /// +/// # Safety +/// /// This function is unsafe as it mutates the global state but cannot guarantee /// thread-safety. It needs to be externally synchronized with calls to access /// the global state. @@ -59,6 +61,8 @@ pub unsafe fn reset_search_path(level: ConfigLevel) -> Result<(), Error> { /// `level` must be one of [`ConfigLevel::System`], [`ConfigLevel::Global`], /// [`ConfigLevel::XDG`], [`ConfigLevel::ProgramData`]. /// +/// # Safety +/// /// This function is unsafe as it mutates the global state but cannot guarantee /// thread-safety. It needs to be externally synchronized with calls to access /// the global state. @@ -242,6 +246,11 @@ where /// Set whether or not to verify ownership before performing a repository. /// Enabled by default, but disabling this can lead to code execution vulnerabilities. +/// +/// # Safety +/// +/// This function is modifying a C global without synchronization, so it is not +/// thread safe, and should only be called before any thread is spawned. pub unsafe fn set_verify_owner_validation(enabled: bool) -> Result<(), Error> { crate::init(); let error = raw::git_libgit2_opts( @@ -256,6 +265,11 @@ pub unsafe fn set_verify_owner_validation(enabled: bool) -> Result<(), Error> { /// Set the SSL certificate-authority location to `file`. `file` is the location /// of a file containing several certificates concatenated together. +/// +/// # Safety +/// +/// This function is modifying a C global without synchronization, so it is not +/// thread safe, and should only be called before any thread is spawned. pub unsafe fn set_ssl_cert_file

(file: P) -> Result<(), Error> where P: IntoCString, @@ -275,6 +289,11 @@ where /// Set the SSL certificate-authority location to `path`. `path` is the location /// of a directory holding several certificates, one per file. +/// +/// # Safety +/// +/// This function is modifying a C global without synchronization, so it is not +/// thread safe, and should only be called before any thread is spawned. pub unsafe fn set_ssl_cert_dir

(path: P) -> Result<(), Error> where P: IntoCString, diff --git a/src/packbuilder.rs b/src/packbuilder.rs index 8d4c3279d2..89dd84d377 100644 --- a/src/packbuilder.rs +++ b/src/packbuilder.rs @@ -1,5 +1,3 @@ -#![allow(clippy::redundant_closure)] - use libc::{c_int, c_uint, c_void, size_t}; use std::marker; use std::path::Path; @@ -204,7 +202,7 @@ impl<'repo> PackBuilder<'repo> { /// Returns `Ok(None)` if the packfile has not been written. pub fn name(&self) -> Result, Error> { match self.name_bytes() { - Some(nb) => str::from_utf8(nb).map(|s| Some(s)).map_err(|e| e.into()), + Some(nb) => str::from_utf8(nb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/rebase.rs b/src/rebase.rs index 1020812110..986ee8c530 100644 --- a/src/rebase.rs +++ b/src/rebase.rs @@ -1,7 +1,3 @@ -#![allow(clippy::len_without_is_empty)] -#![allow(clippy::needless_option_take)] -#![allow(clippy::redundant_closure)] - use std::ffi::CString; use std::{marker, mem, ptr, str}; @@ -87,7 +83,7 @@ impl<'cb> RebaseOptions<'cb> { /// Acquire a pointer to the underlying raw options. pub fn raw(&mut self) -> *const raw::git_rebase_options { - if let Some(opts) = self.merge_options.as_mut().take() { + if let Some(opts) = self.merge_options.as_mut() { unsafe { ptr::copy_nonoverlapping(opts.raw(), &mut self.raw.merge_options, 1); } @@ -118,12 +114,17 @@ impl<'repo> Rebase<'repo> { unsafe { raw::git_rebase_operation_entrycount(self.raw) } } + /// Checks if ther are no rebase operations that are to be applied. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Gets the original `HEAD` ref name for merge rebases. pub fn orig_head_name(&self) -> Result, Error> { let name_bytes = unsafe { crate::opt_bytes(self, raw::git_rebase_orig_head_name(self.raw)) }; match name_bytes { - Some(nb) => str::from_utf8(nb).map(|s| Some(s)).map_err(|e| e.into()), + Some(nb) => str::from_utf8(nb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -325,7 +326,7 @@ impl<'rebase> RebaseOperation<'rebase> { pub fn exec(&self) -> Result, Error> { let exec_bytes = unsafe { crate::opt_bytes(self, (*self.raw).exec) }; match exec_bytes { - Some(eb) => str::from_utf8(eb).map(|s| Some(s)).map_err(|e| e.into()), + Some(eb) => str::from_utf8(eb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/reference.rs b/src/reference.rs index f69f9b6b10..bd414d5b01 100644 --- a/src/reference.rs +++ b/src/reference.rs @@ -1,5 +1,3 @@ -#![allow(clippy::redundant_closure)] - use std::cmp::Ordering; use std::ffi::CString; use std::marker; @@ -262,7 +260,7 @@ impl<'repo> Reference<'repo> { /// May return `Ok(None)` if the reference is not symbolic. pub fn symbolic_target(&self) -> Result, Error> { match self.symbolic_target_bytes() { - Some(stb) => str::from_utf8(stb).map(|s| Some(s)).map_err(|e| e.into()), + Some(stb) => str::from_utf8(stb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/reflog.rs b/src/reflog.rs index 69d971e7af..b0f91e1526 100644 --- a/src/reflog.rs +++ b/src/reflog.rs @@ -1,5 +1,3 @@ -#![allow(clippy::redundant_closure)] - use libc::size_t; use std::iter::FusedIterator; use std::marker; @@ -141,7 +139,7 @@ impl<'reflog> ReflogEntry<'reflog> { /// Get the log message. pub fn message(&self) -> Result, Error> { match self.message_bytes() { - Some(mb) => str::from_utf8(mb).map(|s| Some(s)).map_err(|e| e.into()), + Some(mb) => str::from_utf8(mb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/remote.rs b/src/remote.rs index 377e66a068..3693960579 100644 --- a/src/remote.rs +++ b/src/remote.rs @@ -1,8 +1,3 @@ -#![allow(clippy::derivable_impls)] -#![allow(clippy::empty_line_after_doc_comments)] -#![allow(clippy::redundant_closure)] -#![allow(clippy::unwrap_or_default)] - use raw::git_strarray; use std::iter::FusedIterator; use std::marker; @@ -82,11 +77,13 @@ pub struct RemoteConnection<'repo, 'connection, 'cb> { /// /// By default, git will follow a redirect on the initial request /// (`/info/refs`), but not subsequent requests. +#[derive(Default)] pub enum RemoteRedirect { /// Do not follow any off-site redirects at any stage of the fetch or push. None, /// Allow off-site redirects only upon the initial request. This is the /// default. + #[default] Initial, /// Allow redirects at any stage in the fetch or push. All, @@ -135,7 +132,7 @@ impl<'repo> Remote<'repo> { /// Returns `Ok(None)` if this remote has not yet been named. pub fn name(&self) -> Result, Error> { match self.name_bytes() { - Some(nb) => str::from_utf8(nb).map(|s| Some(s)).map_err(|e| e.into()), + Some(nb) => str::from_utf8(nb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -162,7 +159,7 @@ impl<'repo> Remote<'repo> { /// Returns `Ok(None)` if no special url for pushing is set. pub fn pushurl(&self) -> Result, Error> { match self.pushurl_bytes() { - Some(pb) => str::from_utf8(pb).map(|s| Some(s)).map_err(|e| e.into()), + Some(pb) => str::from_utf8(pb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -225,8 +222,8 @@ impl<'repo> Remote<'repo> { cb: Option>, proxy_options: Option>, ) -> Result, Error> { - let cb = Box::new(cb.unwrap_or_else(RemoteCallbacks::new)); - let proxy_options = proxy_options.unwrap_or_else(ProxyOptions::new); + let cb = Box::new(cb.unwrap_or_default()); + let proxy_options = proxy_options.unwrap_or_default(); unsafe { try_call!(raw::git_remote_connect( self.raw, @@ -431,7 +428,7 @@ impl<'repo> Remote<'repo> { /// Prune tracking refs that are no longer present on remote pub fn prune(&mut self, callbacks: Option>) -> Result<(), Error> { - let cbs = Box::new(callbacks.unwrap_or_else(RemoteCallbacks::new)); + let cbs = Box::new(callbacks.unwrap_or_default()); unsafe { try_call!(raw::git_remote_prune(self.raw, &cbs.raw())); } @@ -593,7 +590,6 @@ impl<'cb> FetchOptions<'cb> { /// Set fetch depth, a value less or equal to 0 is interpreted as pull /// everything (effectively the same as not declaring a limit depth). - // FIXME(blyxyas): We currently don't have a test for shallow functions // because libgit2 doesn't support local shallow clones. // https://github.com/rust-lang/git2-rs/pull/979#issuecomment-1716299900 @@ -813,12 +809,6 @@ impl<'repo, 'connection, 'cb> Drop for RemoteConnection<'repo, 'connection, 'cb> } } -impl Default for RemoteRedirect { - fn default() -> Self { - RemoteRedirect::Initial - } -} - impl RemoteRedirect { fn raw(&self) -> raw::git_remote_redirect_t { match self { diff --git a/src/remote_callbacks.rs b/src/remote_callbacks.rs index 1d988bb7f7..a10926a55d 100644 --- a/src/remote_callbacks.rs +++ b/src/remote_callbacks.rs @@ -1,5 +1,3 @@ -#![allow(clippy::doc_overindented_list_items)] - use libc::{c_char, c_int, c_uint, c_void, size_t}; use std::ffi::CStr; use std::mem; @@ -35,7 +33,7 @@ pub struct RemoteCallbacks<'a> { /// /// * `url` - the resource for which the credentials are required. /// * `username_from_url` - the username that was embedded in the URL, or `None` -/// if it was not included. +/// if it was not included. /// * `allowed_types` - a bitmask stating which cred types are OK to return. pub type Credentials<'a> = dyn FnMut(&str, Option<&str>, CredentialType) -> Result + 'a; diff --git a/src/repo.rs b/src/repo.rs index e7af75b5c8..18b7a952d8 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -1,11 +1,3 @@ -#![allow(clippy::explicit_auto_deref)] -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::needless_borrow)] -#![allow(clippy::new_without_default)] -#![allow(clippy::redundant_closure)] -#![allow(clippy::too_many_arguments)] -#![allow(clippy::zero_ptr)] - use libc::{c_char, c_int, c_uint, c_void, size_t}; use std::env; use std::ffi::{CStr, CString, OsStr}; @@ -94,7 +86,7 @@ extern "C" fn fetchhead_foreach_cb( let oid = Binding::from_raw(oid); let is_merge = is_merge == 1; - callback(&ref_name, remote_url, &oid, is_merge) + callback(ref_name, remote_url, &oid, is_merge) }; if res { @@ -269,7 +261,7 @@ impl Repository { ptr::null() )); } - Repository::open(util::bytes2path(&*buf)) + Repository::open(util::bytes2path(&buf)) } /// Attempt to find the path to a git repo for a given path @@ -296,7 +288,7 @@ impl Repository { )); } - Ok(util::bytes2path(&*buf).to_path_buf()) + Ok(util::bytes2path(&buf).to_path_buf()) } /// Creates a new repository in the specified folder. @@ -552,7 +544,7 @@ impl Repository { /// If there is no namespace, Ok(None) is returned. pub fn namespace(&self) -> Result, Error> { match self.namespace_bytes() { - Some(nb) => str::from_utf8(nb).map(|s| Some(s)).map_err(|e| e.into()), + Some(nb) => str::from_utf8(nb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -2815,6 +2807,7 @@ impl Repository { /// like binary data, the `DiffFile` binary attribute will be set to 1 and no call to /// the `hunk_cb` nor `line_cb` will be made (unless you set the `force_text` /// option). + #[expect(clippy::too_many_arguments)] pub fn diff_blobs( &self, old_blob: Option<&Blob<'_>>, @@ -3327,7 +3320,7 @@ impl Repository { let raw_opts = options.map(|o| o.raw()); let ptr_raw_opts = match raw_opts.as_ref() { Some(v) => v, - None => 0 as *const _, + None => std::ptr::null(), }; unsafe { try_call!(raw::git_revert(self.raw(), commit.raw(), ptr_raw_opts)); @@ -3635,8 +3628,15 @@ impl RepositoryInitOptions { /// Creates a set of raw init options to be used with /// `git_repository_init_ext`. /// + /// # Safety + /// /// This method is unsafe as the returned value may have pointers to the - /// interior of this structure. + /// interior of this structure. The caller must ensure that the returned + /// raw instance does not outlive the [`RepositoryInitOptions`], and also + /// that for any fields configured in the `RepositoryInitOptions` that are + /// provided in the raw structure as pointers, those pointers are not used + /// if the original references held in the `RepositoryInitOptions` are + /// dropped. pub unsafe fn raw(&self) -> raw::git_repository_init_options { let mut opts = mem::zeroed(); assert_eq!( @@ -3661,9 +3661,19 @@ impl RepositoryInitOptions { } } +impl Default for RepositoryInitOptions { + /// Creates a default set of initialization options. + /// + /// See [`RepositoryInitOptions::new()`] for more details. + fn default() -> Self { + Self::new() + } +} + #[cfg(test)] #[allow(clippy::assertions_on_constants)] #[allow(clippy::bool_assert_comparison)] +#[allow(clippy::needless_borrow)] #[allow(clippy::needless_borrows_for_generic_args)] mod tests { use crate::build::CheckoutBuilder; diff --git a/src/revert.rs b/src/revert.rs index 78bfe5d7cc..c09ed9a260 100644 --- a/src/revert.rs +++ b/src/revert.rs @@ -1,5 +1,3 @@ -#![allow(clippy::new_without_default)] - use std::mem; use crate::build::CheckoutBuilder; @@ -69,3 +67,10 @@ impl<'cb> RevertOptions<'cb> { } } } + +impl<'cb> Default for RevertOptions<'cb> { + /// Creates a default set of revert options + fn default() -> Self { + Self::new() + } +} diff --git a/src/stash.rs b/src/stash.rs index 74f375e568..09edcc88e6 100644 --- a/src/stash.rs +++ b/src/stash.rs @@ -1,5 +1,3 @@ -#![allow(clippy::missing_safety_doc)] - use crate::build::CheckoutBuilder; use crate::util::{self, Binding}; use crate::{panic, raw, IntoCString, Oid, Signature, StashApplyProgress, StashFlags}; @@ -56,6 +54,8 @@ impl<'a> StashSaveOptions<'a> { /// Acquire a pointer to the underlying raw options. /// + /// # Safety + /// /// This function is unsafe as the pointer is only valid so long as this /// structure is not moved, modified, or used elsewhere. pub unsafe fn raw(&mut self) -> *const raw::git_stash_save_options { diff --git a/src/status.rs b/src/status.rs index 5da7253fb2..fd0b8353b0 100644 --- a/src/status.rs +++ b/src/status.rs @@ -1,5 +1,3 @@ -#![allow(clippy::missing_safety_doc)] - use libc::{c_char, c_uint, size_t}; use std::ffi::CString; use std::iter::FusedIterator; @@ -227,6 +225,8 @@ impl StatusOptions { /// Get a pointer to the inner list of status options. /// + /// # Safety + /// /// This function is unsafe as the returned structure has interior pointers /// and may no longer be valid if these options continue to be mutated. pub unsafe fn raw(&mut self) -> *const raw::git_status_options { diff --git a/src/string_array.rs b/src/string_array.rs index 7ac04cf70d..a53024b7d1 100644 --- a/src/string_array.rs +++ b/src/string_array.rs @@ -1,7 +1,5 @@ //! Bindings to libgit2's raw `git_strarray` type -#![allow(clippy::redundant_closure)] - use std::iter::FusedIterator; use std::ops::Range; use std::str; @@ -35,7 +33,7 @@ impl StringArray { /// Returns Ok(None) if i is out of bounds. pub fn get(&self, i: usize) -> Result, Error> { match self.get_bytes(i) { - Some(gb) => str::from_utf8(gb).map(|s| Some(s)).map_err(|e| e.into()), + Some(gb) => str::from_utf8(gb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/submodule.rs b/src/submodule.rs index 878a74fa85..087df2755a 100644 --- a/src/submodule.rs +++ b/src/submodule.rs @@ -1,7 +1,3 @@ -#![allow(clippy::empty_line_after_doc_comments)] -#![allow(clippy::let_and_return)] -#![allow(clippy::redundant_closure)] - use std::marker; use std::mem; use std::os::raw::c_int; @@ -27,7 +23,7 @@ impl<'repo> Submodule<'repo> { /// Returns `Ok(None)` if the branch is not yet available. pub fn branch(&self) -> Result, Error> { match self.branch_bytes() { - Some(bb) => str::from_utf8(bb).map(|s| Some(s)).map_err(|e| e.into()), + Some(bb) => str::from_utf8(bb).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -63,7 +59,7 @@ impl<'repo> Submodule<'repo> { /// Returns `Ok(None)` if the URL isn't present pub fn url(&self) -> Result, Error> { match self.opt_url_bytes() { - Some(oub) => str::from_utf8(oub).map(|s| Some(s)).map_err(|e| e.into()), + Some(oub) => str::from_utf8(oub).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -150,7 +146,7 @@ impl<'repo> Submodule<'repo> { /// /// This function can be called to init and set up a submodule repository /// from a submodule in preparation to clone it from its remote. - + /// /// use_gitlink: Should the workdir contain a gitlink to the repo in /// .git/modules vs. repo directly in workdir. pub fn repo_init(&mut self, use_gitlink: bool) -> Result { @@ -297,13 +293,12 @@ impl<'cb> SubmoduleUpdateOptions<'cb> { raw::git_checkout_init_options(&mut checkout_opts, raw::GIT_CHECKOUT_OPTIONS_VERSION); assert_eq!(0, init_res); self.checkout_builder.configure(&mut checkout_opts); - let opts = raw::git_submodule_update_options { + raw::git_submodule_update_options { version: raw::GIT_SUBMODULE_UPDATE_OPTIONS_VERSION, checkout_opts, fetch_opts: self.fetch_opts.raw(), allow_fetch: self.allow_fetch as c_int, - }; - opts + } } /// Set checkout options. diff --git a/src/tag.rs b/src/tag.rs index 1198b47ba2..c15bb3fb79 100644 --- a/src/tag.rs +++ b/src/tag.rs @@ -1,6 +1,3 @@ -#![allow(clippy::from_over_into)] -#![allow(clippy::redundant_closure)] - use std::ffi::CString; use std::marker; use std::mem; @@ -45,7 +42,7 @@ impl<'repo> Tag<'repo> { /// Returns Ok(None) if there is no message pub fn message(&self) -> Result, Error> { match self.message_bytes() { - Some(mb) => str::from_utf8(mb).map(|s| Some(s)).map_err(|e| e.into()), + Some(mb) => str::from_utf8(mb).map(Some).map_err(|e| e.into()), None => Ok(None), } } diff --git a/src/transport.rs b/src/transport.rs index a105f71bd6..031803a7b3 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -1,8 +1,5 @@ //! Interfaces for adding custom transports to libgit2 -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::missing_transmute_annotations)] - use libc::{c_char, c_int, c_uint, c_void, size_t}; use std::ffi::{CStr, CString}; use std::io; @@ -106,6 +103,8 @@ struct RawSmartSubtransportStream { /// Add a custom transport definition, to be used in addition to the built-in /// set of transports that come with libgit2. /// +/// # Safety +/// /// This function is unsafe as it needs to be externally synchronized with calls /// to creation of other transports. pub unsafe fn register(prefix: &str, factory: F) -> Result<(), Error> @@ -264,7 +263,10 @@ extern "C" fn subtransport_action( Ok(s) => s, Err(e) => return e.raw_set_git_error(), }; - *stream = mem::transmute(Box::new(RawSmartSubtransportStream { + *stream = mem::transmute::< + Box, + *mut raw::git_smart_subtransport_stream, + >(Box::new(RawSmartSubtransportStream { raw: raw::git_smart_subtransport_stream { subtransport: raw_transport, read: Some(stream_read), @@ -303,7 +305,7 @@ extern "C" fn subtransport_close(transport: *mut raw::git_smart_subtransport) -> // object. extern "C" fn subtransport_free(transport: *mut raw::git_smart_subtransport) { let _ = panic::wrap(|| unsafe { - mem::transmute::<_, Box>(transport); + mem::transmute::<*mut raw::git_smart_subtransport, Box>(transport); }); } @@ -367,7 +369,9 @@ unsafe fn set_err_io(e: &io::Error) { // object. extern "C" fn stream_free(stream: *mut raw::git_smart_subtransport_stream) { let _ = panic::wrap(|| unsafe { - mem::transmute::<_, Box>(stream); + mem::transmute::<*mut raw::git_smart_subtransport_stream, Box>( + stream, + ); }); } diff --git a/src/tree.rs b/src/tree.rs index 6b72390032..841d2f10a6 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -1,6 +1,3 @@ -#![allow(clippy::from_over_into)] -#![allow(clippy::manual_unwrap_or)] - use libc::{c_char, c_int, c_void}; use std::cmp::Ordering; use std::ffi::{CStr, CString}; @@ -58,20 +55,18 @@ pub enum TreeWalkResult { Abort = raw::GIT_EUSER, } -impl Into for TreeWalkResult { - fn into(self) -> i32 { - self as i32 +impl From for i32 { + fn from(val: TreeWalkResult) -> i32 { + val as i32 } } -impl Into for TreeWalkMode { - #[cfg(target_env = "msvc")] - fn into(self) -> raw::git_treewalk_mode { - self as i32 - } - #[cfg(not(target_env = "msvc"))] - fn into(self) -> raw::git_treewalk_mode { - self as u32 +// On Windows raw::git_treewalk_mode is the same as i32 and the `From` above +// works for raw::git_treewalk_mode as well +#[cfg(not(target_env = "msvc"))] +impl From for raw::git_treewalk_mode { + fn from(val: TreeWalkResult) -> raw::git_treewalk_mode { + val as u32 } } @@ -212,7 +207,7 @@ extern "C" fn treewalk_cb>( entry: *const raw::git_tree_entry, payload: *mut c_void, ) -> c_int { - match panic::wrap(|| unsafe { + panic::wrap(|| unsafe { let root = match CStr::from_ptr(root).to_str() { Ok(value) => value, _ => return -1, @@ -221,10 +216,8 @@ extern "C" fn treewalk_cb>( let payload = &mut *(payload as *mut TreeWalkCbData<'_, T>); let callback = &mut payload.callback; callback(root, &entry).into() - }) { - Some(value) => value, - None => -1, - } + }) + .unwrap_or(-1) } impl<'repo> Binding for Tree<'repo> { diff --git a/src/util.rs b/src/util.rs index 5460d1e727..8cd1d5b167 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,8 +1,3 @@ -#![allow(clippy::missing_safety_doc)] -#![allow(clippy::needless_lifetimes)] -#![allow(clippy::needless_range_loop)] -#![allow(clippy::needless_return)] - use libc::{c_char, c_int, size_t}; use std::cmp::Ordering; use std::ffi::{CString, OsStr, OsString}; @@ -34,6 +29,10 @@ pub trait Binding: Sized { type Raw; /// Build a git2 struct from its [Binding::Raw] value. + /// + /// # Safety + /// + /// The raw value (which is expected to be a pointer) must be valid. unsafe fn from_raw(raw: Self::Raw) -> Self; /// Access the [Binding::Raw] value for a struct. @@ -46,6 +45,11 @@ pub trait Binding: Sized { /// /// If the input parameter is null, then the funtion returns None. Otherwise, it /// calls [Binding::from_raw]. + /// + /// # Safety + /// + /// If the input parameter is not null, it must satisfy the safety + /// requirements of the [`Binding::from_raw()`] method. unsafe fn from_raw_opt(raw: T) -> Option where T: Copy + IsNull, @@ -121,13 +125,13 @@ pub trait IntoCString { fn into_c_string(self) -> Result; } -impl<'a, T: IntoCString + Clone> IntoCString for &'a T { +impl IntoCString for &T { fn into_c_string(self) -> Result { self.clone().into_c_string() } } -impl<'a> IntoCString for &'a str { +impl IntoCString for &str { fn into_c_string(self) -> Result { Ok(CString::new(self)?) } @@ -145,7 +149,7 @@ impl IntoCString for CString { } } -impl<'a> IntoCString for &'a Path { +impl IntoCString for &Path { fn into_c_string(self) -> Result { let s: &OsStr = self.as_ref(); s.into_c_string() @@ -159,7 +163,7 @@ impl IntoCString for PathBuf { } } -impl<'a> IntoCString for &'a OsStr { +impl IntoCString for &OsStr { fn into_c_string(self) -> Result { self.to_os_string().into_c_string() } @@ -183,7 +187,7 @@ impl IntoCString for OsString { } } -impl<'a> IntoCString for &'a [u8] { +impl IntoCString for &[u8] { fn into_c_string(self) -> Result { Ok(CString::new(self)?) } @@ -239,12 +243,10 @@ pub fn path_to_repo_path(path: &Path) -> Result { #[cfg(windows)] { match path.to_str() { - None => { - return Err(Error::from_str( - "only valid unicode paths are accepted on windows", - )) - } - Some(s) => return fixup_windows_path(s), + None => Err(Error::from_str( + "only valid unicode paths are accepted on windows", + )), + Some(s) => fixup_windows_path(s), } } #[cfg(not(windows))] @@ -260,9 +262,9 @@ pub fn cstring_to_repo_path(path: T) -> Result { #[cfg(windows)] fn fixup_windows_path>>(path: P) -> Result { let mut bytes: Vec = path.into(); - for i in 0..bytes.len() { - if bytes[i] == b'\\' { - bytes[i] = b'/'; + for byte in &mut bytes { + if *byte == b'\\' { + *byte = b'/'; } } Ok(CString::new(bytes)?) diff --git a/src/worktree.rs b/src/worktree.rs index b99731c4e1..e568e6d182 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -1,6 +1,3 @@ -#![allow(clippy::new_without_default)] -#![allow(clippy::redundant_closure)] - use crate::buf::Buf; use crate::reference::Reference; use crate::repo::Repository; @@ -62,7 +59,7 @@ impl Worktree { pub fn name(&self) -> Result, Error> { let opt_bytes = unsafe { crate::opt_bytes(self, raw::git_worktree_name(self.raw)) }; match opt_bytes { - Some(ob) => str::from_utf8(ob).map(|s| Some(s)).map_err(|e| e.into()), + Some(ob) => str::from_utf8(ob).map(Some).map_err(|e| e.into()), None => Ok(None), } } @@ -193,6 +190,15 @@ impl<'a> WorktreeAddOptions<'a> { } } +impl<'a> Default for WorktreeAddOptions<'a> { + /// Creates a default set of add options. + /// + /// See [`WorktreeAddOptions::new()`] for more details. + fn default() -> Self { + Self::new() + } +} + impl WorktreePruneOptions { /// Creates a default set of pruning options /// @@ -249,6 +255,15 @@ impl WorktreePruneOptions { } } +impl Default for WorktreePruneOptions { + /// Creates a default set of pruning options + /// + /// See [`WorktreePruneOptions::new()`] for more details. + fn default() -> Self { + Self::new() + } +} + impl Binding for Worktree { type Raw = *mut raw::git_worktree; unsafe fn from_raw(ptr: *mut raw::git_worktree) -> Worktree {