Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/apply.rs
Copy link
Copy Markdown
Member

@weihanglo weihanglo May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of thoughts:

  • What is our MSRV? Edition 2021 is 1.56 I remembered, and lint reason was introduced along with #[expect] which is 1.81.
  • This adds lots of visual noise. Not sure if there are other good way to improve this, like, should we just suppress for all?
  • We don't have clippy in CI yet. Should probably add one in a separate PR.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is our MSRV? Edition 2021 is 1.56 I remembered, and lint reason was introduced along with #[expect] which is 1.81.

I guess I had assumed that CI tested against the MSRV

This adds lots of visual noise. Not sure if there are other good way to improve this, like, should we just suppress for all?

Some of the places that it caught things were indeed unneeded casts that I removed, so I think it is useful to have

We don't have clippy in CI yet. Should probably add one in a separate PR.

I wanted to get it passing first, see my comment above that

This is the first of a few PRs to get clippy passing, and then I'll send a PR to add it to CI

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the places that it caught things were indeed unneeded casts that I removed, so I think it is useful to have

Indeed, though I would still argue that we probably don't want allow everywhere to keep the code slimmer. Alternatively, we might want to run this under Windows so we know where we need it where we dont.

Copy link
Copy Markdown
Member

@weihanglo weihanglo May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1238 (comment)

This is the first of a few PRs to get clippy passing, and then I'll send a PR to add it to CI

The other way to do this. We integrate CI and allow the most noisy ones first, and then fix up and remove the allow as we go one by one. Though I think we should figure out MSRV first as we have yet set any.

View changes since the review

Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ impl<'cb> ApplyOptions<'cb> {
}

fn flag(&mut self, opt: raw::git_apply_flags_t, val: bool) -> &mut Self {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
let opt = opt as u32;
if val {
self.raw.flags |= opt;
Expand Down
2 changes: 1 addition & 1 deletion src/blame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<'blame> BlameHunk<'blame> {

/// Returns number of lines in this hunk.
pub fn lines_in_hunk(&self) -> usize {
unsafe { (*self.raw).lines_in_hunk as usize }
unsafe { (*self.raw).lines_in_hunk }
}

/// Get the short "summary" of the git commit message for the hunk.
Expand Down
4 changes: 2 additions & 2 deletions src/buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Deref for Buf {
if self.raw.ptr.is_null() {
return &[];
}
unsafe { slice::from_raw_parts(self.raw.ptr as *const u8, self.raw.size as usize) }
unsafe { slice::from_raw_parts(self.raw.ptr as *const u8, self.raw.size) }
}
}

Expand All @@ -56,7 +56,7 @@ impl DerefMut for Buf {
if self.raw.ptr.is_null() {
return &mut [];
}
unsafe { slice::from_raw_parts_mut(self.raw.ptr as *mut u8, self.raw.size as usize) }
unsafe { slice::from_raw_parts_mut(self.raw.ptr as *mut u8, self.raw.size) }
}
}

Expand Down
26 changes: 25 additions & 1 deletion src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,10 @@ impl<'cb> CheckoutBuilder<'cb> {
ancestor_label: None,
our_label: None,
their_label: None,
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
checkout_opts: raw::GIT_CHECKOUT_SAFE as u32,
progress: None,
notify: None,
Expand All @@ -347,6 +351,10 @@ impl<'cb> CheckoutBuilder<'cb> {

/// Indicate that this checkout should perform a dry run by checking for
/// conflicts but not make any actual changes.
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
pub fn dry_run(&mut self) -> &mut CheckoutBuilder<'cb> {
self.checkout_opts &= !((1 << 4) - 1);
self.checkout_opts |= raw::GIT_CHECKOUT_NONE as u32;
Expand All @@ -355,6 +363,10 @@ impl<'cb> CheckoutBuilder<'cb> {

/// Take any action necessary to get the working directory to match the
/// target including potentially discarding modified files.
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
pub fn force(&mut self) -> &mut CheckoutBuilder<'cb> {
self.checkout_opts &= !((1 << 4) - 1);
self.checkout_opts |= raw::GIT_CHECKOUT_FORCE as u32;
Expand All @@ -365,13 +377,21 @@ impl<'cb> CheckoutBuilder<'cb> {
/// files to be created but not overwriting existing files or changes.
///
/// This is the default.
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
pub fn safe(&mut self) -> &mut CheckoutBuilder<'cb> {
self.checkout_opts &= !((1 << 4) - 1);
self.checkout_opts |= raw::GIT_CHECKOUT_SAFE as u32;
self
}

fn flag(&mut self, bit: raw::git_checkout_strategy_t, on: bool) -> &mut CheckoutBuilder<'cb> {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
if on {
self.checkout_opts |= bit as u32;
} else {
Expand Down Expand Up @@ -645,7 +665,7 @@ extern "C" fn progress_cb(
} else {
Some(util::bytes2path(CStr::from_ptr(path).to_bytes()))
};
callback(path, completed as usize, total as usize)
callback(path, completed, total)
});
}

Expand Down Expand Up @@ -688,6 +708,10 @@ extern "C" fn notify_cb(
Some(DiffFile::from_raw(workdir))
};

#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
let why = CheckoutNotificationType::from_bits_truncate(why as u32);
let keep_going = callback(why, path, baseline, target, workdir);
if keep_going {
Expand Down
16 changes: 14 additions & 2 deletions src/cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ impl<'a> CertHostkey<'a> {
/// Returns the md5 hash of the hostkey, if available.
pub fn hash_md5(&self) -> Option<&[u8; 16]> {
unsafe {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_MD5 as u32 == 0 {
None
} else {
Expand All @@ -117,6 +121,10 @@ impl<'a> CertHostkey<'a> {
/// Returns the SHA-1 hash of the hostkey, if available.
pub fn hash_sha1(&self) -> Option<&[u8; 20]> {
unsafe {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_SHA1 as u32 == 0 {
None
} else {
Expand All @@ -128,6 +136,10 @@ impl<'a> CertHostkey<'a> {
/// Returns the SHA-256 hash of the hostkey, if available.
pub fn hash_sha256(&self) -> Option<&[u8; 32]> {
unsafe {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
if (*self.raw).kind as u32 & raw::GIT_CERT_SSH_SHA256 as u32 == 0 {
None
} else {
Expand All @@ -144,7 +156,7 @@ impl<'a> CertHostkey<'a> {
}
Some(slice::from_raw_parts(
(*self.raw).hostkey as *const u8,
(*self.raw).hostkey_len as usize,
(*self.raw).hostkey_len,
))
}
}
Expand Down Expand Up @@ -173,7 +185,7 @@ impl<'a> CertHostkey<'a> {
impl<'a> CertX509<'a> {
/// Return the X.509 certificate data as a byte slice
pub fn data(&self) -> &[u8] {
unsafe { slice::from_raw_parts((*self.raw).data as *const u8, (*self.raw).len as usize) }
unsafe { slice::from_raw_parts((*self.raw).data as *const u8, (*self.raw).len) }
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,10 @@ impl<'cfg> ConfigEntry<'cfg> {

/// Depth of includes where this variable was found
pub fn include_depth(&self) -> u32 {
unsafe { (*self.raw).include_depth as u32 }
#[allow(clippy::unnecessary_cast, reason = "c_uint is not always u32")]
unsafe {
(*self.raw).include_depth as u32
}
}
}

Expand Down
55 changes: 38 additions & 17 deletions src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,27 +591,51 @@ impl<'a> DiffFile<'a> {

/// Returns the size of this entry, in bytes
pub fn size(&self) -> u64 {
unsafe { (*self.raw).size as u64 }
unsafe { (*self.raw).size }
}

/// Returns `true` if file(s) are treated as binary data.
pub fn is_binary(&self) -> bool {
unsafe { (*self.raw).flags & raw::GIT_DIFF_FLAG_BINARY as u32 != 0 }
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
unsafe {
(*self.raw).flags & raw::GIT_DIFF_FLAG_BINARY as u32 != 0
}
}

/// Returns `true` if file(s) are treated as text data.
pub fn is_not_binary(&self) -> bool {
unsafe { (*self.raw).flags & raw::GIT_DIFF_FLAG_NOT_BINARY as u32 != 0 }
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
unsafe {
(*self.raw).flags & raw::GIT_DIFF_FLAG_NOT_BINARY as u32 != 0
}
}

/// Returns `true` if `id` value is known correct.
pub fn is_valid_id(&self) -> bool {
unsafe { (*self.raw).flags & raw::GIT_DIFF_FLAG_VALID_ID as u32 != 0 }
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
unsafe {
(*self.raw).flags & raw::GIT_DIFF_FLAG_VALID_ID as u32 != 0
}
}

/// Returns `true` if file exists at this side of the delta.
pub fn exists(&self) -> bool {
unsafe { (*self.raw).flags & raw::GIT_DIFF_FLAG_EXISTS as u32 != 0 }
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
unsafe {
(*self.raw).flags & raw::GIT_DIFF_FLAG_EXISTS as u32 != 0
}
}

/// Returns file mode.
Expand Down Expand Up @@ -680,6 +704,10 @@ impl DiffOptions {
}

fn flag(&mut self, opt: raw::git_diff_option_t, val: bool) -> &mut DiffOptions {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
let opt = opt as u32;
if val {
self.raw.flags |= opt;
Expand Down Expand Up @@ -1041,17 +1069,12 @@ impl<'a> DiffLine<'a> {

/// Offset in the original file to the content
pub fn content_offset(&self) -> i64 {
unsafe { (*self.raw).content_offset as i64 }
unsafe { (*self.raw).content_offset }
}

/// Content of this line as bytes.
pub fn content(&self) -> &'a [u8] {
unsafe {
slice::from_raw_parts(
(*self.raw).content as *const u8,
(*self.raw).content_len as usize,
)
}
unsafe { slice::from_raw_parts((*self.raw).content as *const u8, (*self.raw).content_len) }
}

/// origin of this `DiffLine`.
Expand Down Expand Up @@ -1143,7 +1166,7 @@ impl<'a> DiffHunk<'a> {
unsafe {
slice::from_raw_parts(
(*self.raw).header.as_ptr() as *const u8,
(*self.raw).header_len as usize,
(*self.raw).header_len,
)
}
}
Expand Down Expand Up @@ -1275,14 +1298,12 @@ impl<'a> DiffBinaryFile<'a> {

/// The binary data, deflated
pub fn data(&self) -> &[u8] {
unsafe {
slice::from_raw_parts((*self.raw).data as *const u8, (*self.raw).datalen as usize)
}
unsafe { slice::from_raw_parts((*self.raw).data as *const u8, (*self.raw).datalen) }
}

/// The length of the binary data after inflation
pub fn inflated_len(&self) -> usize {
unsafe { (*self.raw).inflatedlen as usize }
unsafe { (*self.raw).inflatedlen }
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/email.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ impl Default for EmailCreateOptions {
// Defaults options created in corresponding to `GIT_EMAIL_CREATE_OPTIONS_INIT`
let default_options = raw::git_email_create_options {
version: raw::GIT_EMAIL_CREATE_OPTIONS_VERSION,
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
flags: raw::GIT_EMAIL_CREATE_DEFAULT as u32,
diff_opts: unsafe { mem::zeroed() },
diff_find_opts: unsafe { mem::zeroed() },
Expand Down Expand Up @@ -51,6 +55,10 @@ impl EmailCreateOptions {
}

fn flag(&mut self, opt: raw::git_email_create_flags_t, val: bool) -> &mut Self {
#[allow(
clippy::unnecessary_cast,
reason = "u32 unless compiling for msvc target env"
)]
let opt = opt as u32;
if val {
self.raw.flags |= opt;
Expand Down
5 changes: 2 additions & 3 deletions src/indexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a> Progress<'a> {
}
/// Size of the packfile received up to now
pub fn received_bytes(&self) -> usize {
unsafe { (*self.raw()).received_bytes as usize }
unsafe { (*self.raw()).received_bytes }
}

/// Convert this to an owned version of `Progress`.
Expand Down Expand Up @@ -175,8 +175,7 @@ impl<'a> Indexer<'a> {
where
F: FnMut(Progress<'_>) -> bool + 'a,
{
let progress_payload =
unsafe { &mut *(self.progress_payload_ptr as *mut OdbPackwriterCb<'_>) };
let progress_payload = unsafe { &mut *self.progress_payload_ptr };
progress_payload.cb = Some(Box::new(cb) as Box<IndexerProgress<'a>>);

self
Expand Down
Loading