diff --git a/gix-hash/src/change_id.rs b/gix-hash/src/change_id.rs index 9872c0659cd..4a3084e5b86 100644 --- a/gix-hash/src/change_id.rs +++ b/gix-hash/src/change_id.rs @@ -87,12 +87,29 @@ impl std::fmt::Display for ChangeId { } } +impl ChangeId { + fn eq_str(&self, other: &str) -> bool { + self.to_reverse_hex().eq_str(other) + } +} + +impl_partial_eq_str!(ChangeId); + impl ReverseHexDisplay<'_> { pub(crate) fn new(inner: &oid, hex_len: usize) -> ReverseHexDisplay<'_> { ReverseHexDisplay { inner, hex_len } } + + fn eq_str(&self, other: &str) -> bool { + let mut buf = Kind::hex_buf(); + let reverse_hex = encode_reverse_hex(self.inner, &mut buf); + reverse_hex[..self.hex_len.min(reverse_hex.len())] == *other + } } +// Keep this directional as truncated displays aren't uniquely identified by their text. +impl_partial_eq_str_one_way!(ReverseHexDisplay<'_>); + impl std::fmt::Display for ReverseHexDisplay<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut buf = Kind::hex_buf(); diff --git a/gix-hash/src/lib.rs b/gix-hash/src/lib.rs index 9b3dc02fada..112b8aa7ed0 100644 --- a/gix-hash/src/lib.rs +++ b/gix-hash/src/lib.rs @@ -31,6 +31,52 @@ #[cfg(all(not(feature = "sha1"), not(feature = "sha256")))] compile_error!("Please set either the `sha1` or the `sha256` feature flag"); +macro_rules! impl_partial_eq_str_one_way { + ($type:ty) => { + impl PartialEq for $type { + fn eq(&self, other: &str) -> bool { + self.eq_str(other) + } + } + + impl PartialEq<&str> for $type { + fn eq(&self, other: &&str) -> bool { + self.eq_str(other) + } + } + + impl PartialEq for $type { + fn eq(&self, other: &String) -> bool { + self.eq_str(other) + } + } + }; +} + +macro_rules! impl_partial_eq_str { + ($type:ty) => { + impl_partial_eq_str_one_way!($type); + + impl PartialEq<$type> for str { + fn eq(&self, other: &$type) -> bool { + other.eq_str(self) + } + } + + impl PartialEq<$type> for &str { + fn eq(&self, other: &$type) -> bool { + other.eq_str(self) + } + } + + impl PartialEq<$type> for String { + fn eq(&self, other: &$type) -> bool { + other.eq_str(self) + } + } + }; +} + #[path = "oid.rs"] mod borrowed; pub use borrowed::{Error, oid}; diff --git a/gix-hash/src/object_id.rs b/gix-hash/src/object_id.rs index c81ba2cacd4..227d65a8f9f 100644 --- a/gix-hash/src/object_id.rs +++ b/gix-hash/src/object_id.rs @@ -343,6 +343,14 @@ impl std::fmt::Display for ObjectId { } } +impl ObjectId { + fn eq_str(&self, other: &str) -> bool { + self.as_ref().eq_str(other) + } +} + +impl_partial_eq_str!(ObjectId); + impl PartialEq<&oid> for ObjectId { fn eq(&self, other: &&oid) -> bool { self.as_ref() == *other diff --git a/gix-hash/src/oid.rs b/gix-hash/src/oid.rs index de048e67568..1c0603612ed 100644 --- a/gix-hash/src/oid.rs +++ b/gix-hash/src/oid.rs @@ -55,6 +55,17 @@ impl std::fmt::Display for HexDisplay<'_> { } } +impl HexDisplay<'_> { + pub(crate) fn eq_str(&self, other: &str) -> bool { + let mut hex = Kind::hex_buf(); + let hex = self.inner.hex_to_buf(hex.as_mut()); + hex[..self.hex_len.min(hex.len())] == *other + } +} + +// Keep this directional as truncated displays aren't uniquely identified by their text. +impl_partial_eq_str_one_way!(HexDisplay<'_>); + impl std::fmt::Debug for oid { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( @@ -177,6 +188,10 @@ impl oid { out.write_all(&hex[..hex_len]) } + pub(crate) fn eq_str(&self, other: &str) -> bool { + self.to_hex().eq_str(other) + } + /// Returns `true` if this hash consists of all null bytes. #[inline] #[doc(alias = "is_zero", alias = "git2")] @@ -311,6 +326,20 @@ impl PartialEq for &oid { } } +impl PartialEq for &oid { + fn eq(&self, other: &String) -> bool { + self.eq_str(other) + } +} + +impl PartialEq<&oid> for String { + fn eq(&self, other: &&oid) -> bool { + other.eq_str(self) + } +} + +impl_partial_eq_str!(oid); + /// Manually created from a version that uses a slice, and we forcefully try to convert it into a borrowed array of the desired size /// Could be improved by fitting this into serde. /// Unfortunately the `serde::Deserialize` derive wouldn't work for borrowed arrays. diff --git a/gix-hash/src/prefix.rs b/gix-hash/src/prefix.rs index f23b499aa4f..62b03ef1fb7 100644 --- a/gix-hash/src/prefix.rs +++ b/gix-hash/src/prefix.rs @@ -185,6 +185,15 @@ impl std::fmt::Display for Prefix { } } +impl Prefix { + fn eq_str(&self, other: &str) -> bool { + self.bytes.to_hex_with_len(self.hex_len).eq_str(other) + } +} + +// Keep this directional as the hash kind and unused suffix aren't uniquely identified by the displayed prefix. +impl_partial_eq_str_one_way!(Prefix); + impl From for Prefix { fn from(oid: ObjectId) -> Self { Prefix { diff --git a/gix-hash/tests/hash/comparisons.rs b/gix-hash/tests/hash/comparisons.rs new file mode 100644 index 00000000000..6e04cbb41ba --- /dev/null +++ b/gix-hash/tests/hash/comparisons.rs @@ -0,0 +1,121 @@ +use gix_hash::{ChangeId, Prefix}; + +use crate::hex_to_id; + +macro_rules! assert_text_eq { + ($value:expr, $text:expr) => {{ + let value = $value; + let text = $text; + let owned = text.to_owned(); + assert!(value == text, "the value compares with a string literal"); + assert!(text == value, "string-literal comparison is symmetric"); + assert!(&value == text, "a borrowed value compares with str"); + assert!(text == &value, "str comparison with a borrowed value is symmetric"); + assert!(value == owned, "the value compares with String"); + assert!(owned == value, "String comparison is symmetric"); + }}; +} + +macro_rules! assert_text_ne { + ($value:expr, $text:expr) => {{ + let value = $value; + let text = $text; + assert!(value != text, "different text does not compare equal"); + assert!(text != value, "inequality is symmetric"); + }}; +} + +macro_rules! assert_text_eq_one_way { + ($value:expr, $text:expr) => {{ + let value = $value; + let text = $text; + let owned = text.to_owned(); + assert!(value == text, "the value compares with a string literal"); + assert!(&value == text, "a borrowed value compares with str"); + assert!(value == owned, "the value compares with String"); + }}; +} + +macro_rules! assert_text_ne_one_way { + ($value:expr, $text:expr) => {{ + let value = $value; + let text = $text; + assert!(value != text, "different text does not match the value"); + }}; +} + +fn compare_all(object_hex: &str, reverse_hex: &str) { + let id = hex_to_id(object_hex); + let object_hex_upper = object_hex.to_ascii_uppercase(); + assert_text_eq!(id, object_hex); + assert_text_ne!(id, object_hex_upper.as_str()); + assert_text_ne!(id, &object_hex[..object_hex.len() - 1]); + let invalid_object_hex = format!("g{}", &object_hex[1..]); + assert_text_ne!(id, invalid_object_hex.as_str()); + + let borrowed = id.as_ref(); + assert!(borrowed == object_hex, "oid compares with str"); + assert!(object_hex == borrowed, "str comparison with oid is symmetric"); + assert!( + borrowed != object_hex_upper, + "oid only matches canonical lowercase text" + ); + assert!(object_hex_upper != borrowed, "non-canonical comparison is symmetric"); + assert!( + borrowed != &object_hex[..object_hex.len() - 1], + "oid requires the exact length" + ); + assert!(borrowed != invalid_object_hex.as_str(), "oid rejects invalid hex"); + + let change_id = ChangeId::from(id); + let reverse_hex_upper = reverse_hex.to_ascii_uppercase(); + assert_text_eq!(change_id, reverse_hex); + assert_text_ne!(change_id, reverse_hex_upper.as_str()); + assert_text_ne!(change_id, &reverse_hex[..reverse_hex.len() - 1]); + let invalid_reverse_hex = format!("j{}", &reverse_hex[1..]); + assert_text_ne!(change_id, invalid_reverse_hex.as_str()); + + let prefix_len = 17; + let prefix = Prefix::new(&id, prefix_len).expect("the requested prefix length is valid"); + let object_prefix = &object_hex[..prefix_len]; + let object_prefix_upper = object_prefix.to_ascii_uppercase(); + assert_text_eq_one_way!(prefix, object_prefix); + assert_text_ne_one_way!(prefix, object_prefix_upper.as_str()); + assert_text_ne_one_way!(prefix, &object_hex[..prefix_len + 1]); + assert_text_ne_one_way!(prefix, "abcdefg"); + + assert_text_eq_one_way!(id.to_hex_with_len(prefix_len), object_prefix); + assert_text_ne_one_way!(id.to_hex_with_len(prefix_len), object_prefix_upper.as_str()); + assert_text_ne_one_way!(id.to_hex_with_len(prefix_len), &object_hex[..prefix_len + 1]); + assert_text_ne_one_way!(id.to_hex_with_len(prefix_len), "abcdefg"); + + let reverse_prefix = &reverse_hex[..prefix_len]; + let reverse_prefix_upper = reverse_prefix.to_ascii_uppercase(); + assert_text_eq_one_way!(change_id.to_reverse_hex_with_len(prefix_len), reverse_prefix); + assert_text_ne_one_way!( + change_id.to_reverse_hex_with_len(prefix_len), + reverse_prefix_upper.as_str() + ); + assert_text_ne_one_way!( + change_id.to_reverse_hex_with_len(prefix_len), + &reverse_hex[..prefix_len + 1] + ); + assert_text_ne_one_way!(change_id.to_reverse_hex_with_len(prefix_len), "abcdefg"); +} + +#[test] +fn compares_sha1_with_text() { + compare_all( + "0123456789abcdef0123456789abcdef01234567", + "zyxwvutsrqponmlkzyxwvutsrqponmlkzyxwvuts", + ); +} + +#[test] +#[cfg(feature = "sha256")] +fn compares_sha256_with_text() { + compare_all( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + "zyxwvutsrqponmlkzyxwvutsrqponmlkzyxwvutsrqponmlkzyxwvutsrqponmlk", + ); +} diff --git a/gix-hash/tests/hash/main.rs b/gix-hash/tests/hash/main.rs index c4bf6356e60..16228cdf344 100644 --- a/gix-hash/tests/hash/main.rs +++ b/gix-hash/tests/hash/main.rs @@ -1,6 +1,7 @@ use gix_hash::ObjectId; mod change_id; +mod comparisons; mod hasher; mod kind; mod object_id; diff --git a/gix-merge/tests/merge/blob/platform.rs b/gix-merge/tests/merge/blob/platform.rs index 4f7e7da19e1..9653192be21 100644 --- a/gix-merge/tests/merge/blob/platform.rs +++ b/gix-merge/tests/merge/blob/platform.rs @@ -221,7 +221,7 @@ theirs }) .unwrap() .unwrap(), - hex_to_id("424860eef4edb9f5a2dacbbd6dc8c2d2e7645035"), + "424860eef4edb9f5a2dacbbd6dc8c2d2e7645035", "there is no need to write a buffer here, it just returns one of our inputs" ); diff --git a/gix-object/tests/object/commit/from_bytes.rs b/gix-object/tests/object/commit/from_bytes.rs index c3f2e003def..2ac9bda60a6 100644 --- a/gix-object/tests/object/commit/from_bytes.rs +++ b/gix-object/tests/object/commit/from_bytes.rs @@ -6,7 +6,7 @@ use smallvec::SmallVec; use crate::{ commit::{LONG_MESSAGE, MERGE_TAG, SIGNATURE}, - fixture_name, fixture_oid, hex_to_id, linus_signature, + fixture_name, fixture_oid, linus_signature, }; #[test] @@ -104,7 +104,7 @@ committer Name 1312735823 +0518 message"; let commit = CommitRef::from_bytes(input, gix_hash::Kind::Sha1)?; assert_eq!(commit.tree, b"7989DFB2EC2F41914611A22FB30BBC2B3849DF9A".as_bstr()); - assert_eq!(commit.tree(), hex_to_id("7989dfb2ec2f41914611a22fb30bbc2b3849df9a")); + assert_eq!(commit.tree(), "7989dfb2ec2f41914611a22fb30bbc2b3849df9a"); Ok(()) } diff --git a/gix-object/tests/object/commit/iter.rs b/gix-object/tests/object/commit/iter.rs index 2f10c5ba1e3..cf68a376380 100644 --- a/gix-object/tests/object/commit/iter.rs +++ b/gix-object/tests/object/commit/iter.rs @@ -120,7 +120,7 @@ fn signed_singleline() -> crate::Result { CommitRefIter::from_bytes(&fixture_name("commit", "signed-singleline.txt"), gix_hash::Kind::Sha1) .parent_ids() .collect::>(), - vec![hex_to_id("09d8d3a12e161a7f6afb522dbe8900a9c09bce06")] + vec!["09d8d3a12e161a7f6afb522dbe8900a9c09bce06"] ); Ok(()) } @@ -166,8 +166,8 @@ fn mergetag() -> crate::Result { assert_eq!( iter.parent_ids().collect::>(), vec![ - hex_to_id("44ebe016df3aad96e3be8f95ec52397728dd7701"), - hex_to_id("8d485da0ddee79d0e6713405694253d401e41b93") + "44ebe016df3aad96e3be8f95ec52397728dd7701", + "8d485da0ddee79d0e6713405694253d401e41b93" ] ); assert_eq!(iter.message().ok(), Some(LONG_MESSAGE.into())); diff --git a/gix-object/tests/object/commit/mod.rs b/gix-object/tests/object/commit/mod.rs index 2eef0d7ddfd..d2dcda4d236 100644 --- a/gix-object/tests/object/commit/mod.rs +++ b/gix-object/tests/object/commit/mod.rs @@ -156,13 +156,13 @@ mod method { use gix_object::CommitRef; use pretty_assertions::assert_eq; - use crate::{fixture_name, hex_to_id, signature}; + use crate::{fixture_name, signature}; #[test] fn tree() -> crate::Result { let fixture = fixture_name("commit", "unsigned.txt"); let commit = CommitRef::from_bytes(&fixture, gix_hash::Kind::Sha1)?; - assert_eq!(commit.tree(), hex_to_id("1b2dfb4ac5e42080b682fc676e9738c94ce6d54d")); + assert_eq!(commit.tree(), "1b2dfb4ac5e42080b682fc676e9738c94ce6d54d"); assert_eq!(commit.tree, "1b2dfb4ac5e42080b682fc676e9738c94ce6d54d"); Ok(()) } diff --git a/gix-object/tests/object/tag/mod.rs b/gix-object/tests/object/tag/mod.rs index 4b4b1010620..cf3596ab21e 100644 --- a/gix-object/tests/object/tag/mod.rs +++ b/gix-object/tests/object/tag/mod.rs @@ -1,4 +1,4 @@ -use crate::{hex_to_id, signature}; +use crate::signature; use gix_object::{Kind, TagRef, TagRefIter, bstr::ByteSlice}; use crate::fixture_name; @@ -189,7 +189,7 @@ sha256-tag-signature fn target() -> crate::Result { let fixture = fixture_name("tag", "signed.txt"); let tag_ref = TagRef::from_bytes(&fixture, gix_hash::Kind::Sha1)?; - assert_eq!(tag_ref.target(), hex_to_id("ffa700b4aca13b80cb6b98a078e7c96804f8e0ec")); + assert_eq!(tag_ref.target(), "ffa700b4aca13b80cb6b98a078e7c96804f8e0ec"); assert_eq!(tag_ref.target, "ffa700b4aca13b80cb6b98a078e7c96804f8e0ec".as_bytes()); let gix_object::Tag { @@ -200,7 +200,7 @@ fn target() -> crate::Result { message, signature, } = tag_ref.into_owned()?; - assert_eq!(target.to_string(), tag_ref.target); + assert_eq!(target, tag_ref.target.to_str()?); assert_eq!(target_kind, tag_ref.target_kind); assert_eq!(name, tag_ref.name); let expected_tagger = tag_ref.tagger()?.map(Into::into); @@ -362,10 +362,7 @@ tag uppercase-target message"; let tag = TagRef::from_bytes(input, gix_hash::Kind::Sha1)?; assert_eq!(tag.target, b"FFA700B4ACA13B80CB6B98A078E7C96804F8E0EC".as_bstr()); - assert_eq!( - tag.target(), - crate::hex_to_id("ffa700b4aca13b80cb6b98a078e7c96804f8e0ec") - ); + assert_eq!(tag.target(), "ffa700b4aca13b80cb6b98a078e7c96804f8e0ec"); Ok(()) } diff --git a/gix-pack/tests/pack/data/file.rs b/gix-pack/tests/pack/data/file.rs index 0bfbbbe0b70..6929497dc58 100644 --- a/gix-pack/tests/pack/data/file.rs +++ b/gix-pack/tests/pack/data/file.rs @@ -11,12 +11,12 @@ mod method { use gix_features::progress; - use crate::{SMALL_PACK, data::file::pack_at, hex_to_id, pack_from_memory_at}; + use crate::{SMALL_PACK, data::file::pack_at, pack_from_memory_at}; #[test] fn checksum() { let p = pack_at(SMALL_PACK); - assert_eq!(p.checksum(), hex_to_id("0f3ea84cd1bba10c2a03d736a460635082833e59")); + assert_eq!(p.checksum(), "0f3ea84cd1bba10c2a03d736a460635082833e59"); } #[test] diff --git a/gix-pack/tests/pack/index.rs b/gix-pack/tests/pack/index.rs index 4ef6e070175..6cba25ac96d 100644 --- a/gix-pack/tests/pack/index.rs +++ b/gix-pack/tests/pack/index.rs @@ -6,8 +6,7 @@ use gix_object::{self as object}; use gix_odb::pack; use crate::{ - INDEX_V1, PACK_FOR_INDEX_V1, SMALL_PACK, SMALL_PACK_INDEX, fixture_path, hex_to_id, leaked_fixture_bytes, - pack_from_memory_at, + INDEX_V1, PACK_FOR_INDEX_V1, SMALL_PACK, SMALL_PACK_INDEX, fixture_path, leaked_fixture_bytes, pack_from_memory_at, }; fn memory_backed_index(at: &str) -> gix_pack::index::File<&'static [u8]> { @@ -624,8 +623,8 @@ fn iter() -> Result<(), Box> { .map(|o| (o.actual_index_checksum, o.pack_traverse_statistics))?, (idx.index_checksum(), None) ); - assert_eq!(idx.index_checksum(), hex_to_id(index_checksum)); - assert_eq!(idx.pack_checksum(), hex_to_id(pack_checksum)); + assert_eq!(idx.index_checksum(), *index_checksum); + assert_eq!(idx.pack_checksum(), *pack_checksum); assert_eq!(idx.iter().count(), *num_objects as usize); } Ok(()) diff --git a/gix-pack/tests/pack/multi_index/write.rs b/gix-pack/tests/pack/multi_index/write.rs index 0b0a48bedc5..5bac137a20a 100644 --- a/gix-pack/tests/pack/multi_index/write.rs +++ b/gix-pack/tests/pack/multi_index/write.rs @@ -6,8 +6,6 @@ use std::{ use gix_features::progress; use gix_testtools::fixture_path; -use crate::hex_to_id; - /// Writes a multi-index from the static SHA-1 pack indices, with pinned SHA-1 expectations. /// The SHA-256 counterpart lives in [`from_a_hash_parameterized_pack`] below. #[test] @@ -18,7 +16,7 @@ fn from_paths() -> crate::Result { assert_eq!( written.outcome.multi_index_checksum, - hex_to_id("d34d327039a3554f8a644b29e07b903fa71ef269") + "d34d327039a3554f8a644b29e07b903fa71ef269" ); assert_eq!(written.file.num_indices(), 3); diff --git a/gix-packetline/tests/packetline/read/sideband.rs b/gix-packetline/tests/packetline/read/sideband.rs index 2c17a843cbd..c472eb54bd0 100644 --- a/gix-packetline/tests/packetline/read/sideband.rs +++ b/gix-packetline/tests/packetline/read/sideband.rs @@ -82,10 +82,7 @@ async fn read_pack_with_progress_extraction() -> crate::Result { drop(pack_entries); assert_eq!( - last.trailer - .expect("trailer to exist on last entry") - .to_hex() - .to_string(), + last.trailer.expect("trailer to exist on last entry"), "150a1045f04dc0fc2dbf72313699fda696bf4126" ); assert_eq!( diff --git a/gix-protocol/tests/protocol/fetch/response.rs b/gix-protocol/tests/protocol/fetch/response.rs index 3d29c937752..159b8d1367b 100644 --- a/gix-protocol/tests/protocol/fetch/response.rs +++ b/gix-protocol/tests/protocol/fetch/response.rs @@ -353,8 +353,7 @@ mod v2 { assert_eq!(bytes_read, 1643, "should be able to read the whole pack"); assert_eq!(&buf[..4], b"PACK"); assert_eq!( - gix_hash::ObjectId::from_bytes_or_panic(&buf[buf.len() - gix_hash::Kind::Sha1.len_in_bytes()..]) - .to_string(), + gix_hash::ObjectId::from_bytes_or_panic(&buf[buf.len() - gix_hash::Kind::Sha1.len_in_bytes()..]), "f34c9be7e0c3ef2c3ed7c62cc7791dbf6dc5ec9a" ); Ok(()) diff --git a/gix-ref/src/compare.rs b/gix-ref/src/compare.rs new file mode 100644 index 00000000000..9224fec1500 --- /dev/null +++ b/gix-ref/src/compare.rs @@ -0,0 +1,126 @@ +use crate::{ + FullName, FullNameRef, Namespace, PartialName, PartialNameRef, Reference, + bstr::{BStr, BString, ByteSlice}, + file, packed, +}; + +macro_rules! impl_partial_eq { + ($left_type:ty, $right_type:ty, $left:ident => $left_bytes:expr, $right:ident => $right_bytes:expr) => { + impl PartialEq<$right_type> for $left_type { + fn eq(&self, other: &$right_type) -> bool { + let $left = self; + let $right = other; + $left_bytes == $right_bytes + } + } + }; +} + +macro_rules! impl_partial_eq_pair { + ($left_type:ty, $right_type:ty, $left:ident => $left_bytes:expr, $right:ident => $right_bytes:expr) => { + impl_partial_eq!($left_type, $right_type, $left => $left_bytes, $right => $right_bytes); + impl_partial_eq!($right_type, $left_type, $right => $right_bytes, $left => $left_bytes); + }; +} + +macro_rules! impl_partial_eq_bytes { + ($type:ty, $value:ident => $bytes:expr) => { + impl_partial_eq_pair!($type, str, $value => $bytes.as_bytes(), other => other.as_bytes()); + impl_partial_eq_pair!($type, &str, $value => $bytes.as_bytes(), other => other.as_bytes()); + impl_partial_eq_pair!($type, String, $value => $bytes.as_bytes(), other => other.as_bytes()); + impl_partial_eq_pair!($type, BStr, $value => $bytes.as_bytes(), other => other.as_bytes()); + impl_partial_eq_pair!($type, &BStr, $value => $bytes.as_bytes(), other => other.as_bytes()); + impl_partial_eq_pair!($type, BString, $value => $bytes.as_bytes(), other => other.as_bytes()); + }; +} + +macro_rules! impl_partial_eq_reference { + ($type:ty, $value:ident => $name:expr) => { + impl_partial_eq!($type, str, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!($type, &str, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!($type, String, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!($type, BStr, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!($type, &BStr, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!($type, BString, $value => $name.as_bytes(), other => other.as_bytes()); + impl_partial_eq!( + $type, + FullName, + $value => $name.as_bytes(), + other => other.as_bstr().as_bytes() + ); + impl_partial_eq!( + $type, + FullNameRef, + $value => $name.as_bytes(), + other => other.as_bstr().as_bytes() + ); + impl_partial_eq!( + $type, + &FullNameRef, + $value => $name.as_bytes(), + other => other.as_bstr().as_bytes() + ); + }; +} + +impl_partial_eq_bytes!(FullName, value => value.as_bstr()); +impl_partial_eq_bytes!(FullNameRef, value => value.as_bstr()); +impl_partial_eq_bytes!(PartialName, value => value.as_ref().as_bstr()); +impl_partial_eq_bytes!(PartialNameRef, value => value.as_bstr()); +impl_partial_eq_bytes!(Namespace, value => value.as_bstr()); + +impl_partial_eq_pair!( + &FullNameRef, + String, + name => name.as_bstr().as_bytes(), + text => text.as_bytes() +); +impl_partial_eq_pair!( + &FullNameRef, + BString, + name => name.as_bstr().as_bytes(), + text => text.as_bytes() +); +impl_partial_eq_pair!( + &PartialNameRef, + String, + name => name.as_bstr().as_bytes(), + text => text.as_bytes() +); +impl_partial_eq_pair!( + &PartialNameRef, + BString, + name => name.as_bstr().as_bytes(), + text => text.as_bytes() +); + +impl_partial_eq_pair!( + FullName, + FullNameRef, + owned => owned.as_bstr().as_bytes(), + borrowed => borrowed.as_bstr().as_bytes() +); +impl_partial_eq_pair!( + FullName, + &FullNameRef, + owned => owned.as_bstr().as_bytes(), + borrowed => borrowed.as_bstr().as_bytes() +); +impl_partial_eq_pair!( + PartialName, + PartialNameRef, + owned => owned.as_ref().as_bstr().as_bytes(), + borrowed => borrowed.as_bstr().as_bytes() +); +impl_partial_eq_pair!( + PartialName, + &PartialNameRef, + owned => owned.as_ref().as_bstr().as_bytes(), + borrowed => borrowed.as_bstr().as_bytes() +); + +// Keep these comparisons one-way: same-type reference equality is structural, so reverse +// implementations would let references with different targets form a non-transitive chain through their name. +impl_partial_eq_reference!(Reference, value => value.name.as_bstr()); +impl_partial_eq_reference!(file::loose::Reference, value => value.name.as_bstr()); +impl_partial_eq_reference!(packed::Reference<'_>, value => value.name.as_bstr()); diff --git a/gix-ref/src/lib.rs b/gix-ref/src/lib.rs index 2f29aefcb4d..046d23d8f42 100644 --- a/gix-ref/src/lib.rs +++ b/gix-ref/src/lib.rs @@ -31,6 +31,7 @@ use gix_object::bstr::{BStr, BString}; mod store_impl; pub use store_impl::{file, packed}; +mod compare; mod fullname; /// pub mod name; diff --git a/gix-ref/src/store/file/log/line.rs b/gix-ref/src/store/file/log/line.rs index cfd8b72f3ae..43fa2b53a1f 100644 --- a/gix-ref/src/store/file/log/line.rs +++ b/gix-ref/src/store/file/log/line.rs @@ -162,11 +162,6 @@ pub mod decode { mod test_decode { use super::*; - /// Convert a hexadecimal hash into its corresponding `ObjectId` or _panic_. - fn hex_to_oid(hex: &str) -> gix_hash::ObjectId { - gix_hash::ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex") - } - fn with_newline(mut v: Vec) -> Vec { v.push(b'\n'); v @@ -229,11 +224,8 @@ pub mod decode { message: b"pull --ff-only: Fast-forward".as_bstr(), }; assert_eq!(res, actual); - assert_eq!( - actual.previous_oid(), - hex_to_oid("a5828ae6b52137b913b978e16cd2334482eb4c1f") - ); - assert_eq!(actual.new_oid(), hex_to_oid("89b43f80a514aee58b662ad606e6352e03eaeee4")); + assert_eq!(actual.previous_oid(), "a5828ae6b52137b913b978e16cd2334482eb4c1f"); + assert_eq!(actual.new_oid(), "89b43f80a514aee58b662ad606e6352e03eaeee4"); } } diff --git a/gix-ref/src/store/packed/decode/tests.rs b/gix-ref/src/store/packed/decode/tests.rs index d0c67673481..086b583b0d2 100644 --- a/gix-ref/src/store/packed/decode/tests.rs +++ b/gix-ref/src/store/packed/decode/tests.rs @@ -9,11 +9,6 @@ mod reference { const HASH_KIND: gix_hash::Kind = gix_hash::Kind::Sha1; - /// Convert a hexadecimal hash into its corresponding `ObjectId` or _panic_. - fn hex_to_id(hex: &str) -> gix_hash::ObjectId { - gix_hash::ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex") - } - #[test] fn invalid() { let mut input = b"# what looks like a comment".as_slice(); @@ -41,8 +36,8 @@ mod reference { assert!(input.is_empty(), "exhausted"); assert_eq!(parsed.name, FullNameRef::new_unchecked("refs/heads/uppercase".into())); - assert_eq!(parsed.target(), hex_to_id("d53c4b0f91f1b29769c9430f2d1c0bcab1170c75")); - assert_eq!(parsed.object(), hex_to_id("e9cdc958e7ce2290e2d7958cdb5aa9323ef35d37")); + assert_eq!(parsed.target(), "d53c4b0f91f1b29769c9430f2d1c0bcab1170c75"); + assert_eq!(parsed.object(), "e9cdc958e7ce2290e2d7958cdb5aa9323ef35d37"); Ok(()) } @@ -71,8 +66,8 @@ mod reference { object: Some("e9cdc958e7ce2290e2d7958cdb5aa9323ef35d37".into()) } ); - assert_eq!(parsed.target(), hex_to_id("d53c4b0f91f1b29769c9430f2d1c0bcab1170c75")); - assert_eq!(parsed.object(), hex_to_id("e9cdc958e7ce2290e2d7958cdb5aa9323ef35d37")); + assert_eq!(parsed.target(), "d53c4b0f91f1b29769c9430f2d1c0bcab1170c75"); + assert_eq!(parsed.object(), "e9cdc958e7ce2290e2d7958cdb5aa9323ef35d37"); let parsed = decode::reference(&mut input, HASH_KIND).unwrap(); assert!(input.is_empty(), "exhausted"); diff --git a/gix-ref/tests/refs/equality.rs b/gix-ref/tests/refs/equality.rs new file mode 100644 index 00000000000..48dc0d519d1 --- /dev/null +++ b/gix-ref/tests/refs/equality.rs @@ -0,0 +1,174 @@ +use gix_ref::{ + FullName, FullNameRef, PartialName, Target, + bstr::{BStr, BString, ByteSlice}, +}; + +macro_rules! assert_natural_equality { + ($value:ident, $matching:literal, $different:literal) => {{ + let matching = $matching; + let matching_string = matching.to_owned(); + let matching_bstr = matching.as_bytes().as_bstr(); + let matching_bstring: BString = matching.as_bytes().into(); + + assert_eq!($value, matching, "the value matches str"); + assert_eq!(matching, $value, "str comparison is symmetric"); + assert_eq!($value, matching_string, "the value matches String"); + assert_eq!(matching_string, $value, "String comparison is symmetric"); + assert_eq!($value, matching_bstr, "the value matches BStr"); + assert_eq!(matching_bstr, $value, "BStr comparison is symmetric"); + assert_eq!($value, matching_bstring, "the value matches BString"); + assert_eq!(matching_bstring, $value, "BString comparison is symmetric"); + + let different = $different; + let different_string = different.to_owned(); + let different_bstr = different.as_bytes().as_bstr(); + let different_bstring: BString = different.as_bytes().into(); + + assert_ne!($value, different, "the value differs from str"); + assert_ne!(different, $value, "str inequality is symmetric"); + assert_ne!($value, different_string, "the value differs from String"); + assert_ne!(different_string, $value, "String inequality is symmetric"); + assert_ne!($value, different_bstr, "the value differs from BStr"); + assert_ne!(different_bstr, $value, "BStr inequality is symmetric"); + assert_ne!($value, different_bstring, "the value differs from BString"); + assert_ne!(different_bstring, $value, "BString inequality is symmetric"); + }}; +} + +macro_rules! assert_binary_equality { + ($value:ident, $matching:expr, $different:expr) => {{ + let matching: &BStr = $matching; + let matching_owned = matching.to_owned(); + assert_eq!($value, matching, "the value matches non-UTF-8 BStr"); + assert_eq!(matching, $value, "non-UTF-8 BStr comparison is symmetric"); + assert_eq!($value, matching_owned, "the value matches non-UTF-8 BString"); + assert_eq!(matching_owned, $value, "non-UTF-8 BString comparison is symmetric"); + + let different: &BStr = $different; + let different_owned = different.to_owned(); + assert_ne!($value, different, "the value differs from BStr"); + assert_ne!(different, $value, "BStr inequality is symmetric"); + assert_ne!($value, different_owned, "the value differs from BString"); + assert_ne!(different_owned, $value, "BString inequality is symmetric"); + }}; +} + +macro_rules! assert_reference_name_equality { + ($reference:ident, $name:ident) => {{ + let name_ref: &FullNameRef = $name.as_ref(); + assert_eq!($reference, $name, "the reference matches its owned name"); + assert_eq!($reference, name_ref, "the reference matches its borrowed name"); + + let matching = "refs/heads/main"; + assert_eq!($reference, matching, "the reference matches str"); + assert_eq!($reference, matching.to_owned(), "the reference matches String"); + assert_eq!($reference, matching.as_bytes().as_bstr(), "the reference matches BStr"); + assert_eq!($reference, BString::from(matching), "the reference matches BString"); + + let different = "refs/heads/other"; + assert_ne!($reference, different, "the reference differs from str"); + assert_ne!($reference, different.to_owned(), "the reference differs from String"); + assert_ne!( + $reference, + different.as_bytes().as_bstr(), + "the reference differs from BStr" + ); + assert_ne!( + $reference, + BString::from(different), + "the reference differs from BString" + ); + }}; +} + +#[test] +fn name_types_compare_with_text_and_byte_strings() -> gix_testtools::Result { + let full = FullName::try_from("refs/heads/main")?; + let full_ref: &FullNameRef = full.as_ref(); + assert_eq!(full, full_ref, "owned and borrowed full names match"); + assert_eq!(full_ref, full, "full-name comparison is symmetric"); + assert_natural_equality!(full, "refs/heads/main", "refs/heads/other"); + assert_natural_equality!(full_ref, "refs/heads/main", "refs/heads/other"); + + let partial = PartialName::try_from("heads/main")?; + let partial_ref = partial.as_ref(); + assert_eq!(partial, partial_ref, "owned and borrowed partial names match"); + assert_eq!(partial_ref, partial, "partial-name comparison is symmetric"); + assert_natural_equality!(partial, "heads/main", "heads/other"); + assert_natural_equality!(partial_ref, "heads/main", "heads/other"); + + let namespace = gix_ref::namespace::expand("foo")?; + assert_natural_equality!(namespace, "refs/namespaces/foo/", "refs/namespaces/bar/"); + Ok(()) +} + +#[test] +fn names_compare_as_exact_bytes() -> gix_testtools::Result { + let full_bytes = b"refs/heads/\xff".as_bstr(); + let full = FullName::try_from(full_bytes)?; + let full_ref: &FullNameRef = full.as_ref(); + assert_binary_equality!(full, full_bytes, b"refs/heads/other".as_bstr()); + assert_binary_equality!(full_ref, full_bytes, b"refs/heads/other".as_bstr()); + + let partial_bytes = b"heads/\xff".as_bstr(); + let partial = PartialName::try_from(partial_bytes.to_owned())?; + let partial_ref = partial.as_ref(); + assert_binary_equality!(partial, partial_bytes, b"heads/other".as_bstr()); + assert_binary_equality!(partial_ref, partial_bytes, b"heads/other".as_bstr()); + + let namespace = gix_ref::namespace::expand(b"\xff".as_bstr())?; + assert_binary_equality!( + namespace, + b"refs/namespaces/\xff/".as_bstr(), + b"refs/namespaces/other/".as_bstr() + ); + Ok(()) +} + +#[test] +fn references_compare_by_name_without_changing_structural_equality() -> gix_testtools::Result { + let name = FullName::try_from("refs/heads/main")?; + let raw = gix_ref::Reference { + name: name.clone(), + target: Target::Symbolic(FullName::try_from("refs/heads/target-a")?), + peeled: None, + }; + let raw_with_other_target = gix_ref::Reference { + name: name.clone(), + target: Target::Symbolic(FullName::try_from("refs/heads/target-b")?), + peeled: None, + }; + assert_ne!(raw, raw_with_other_target, "structural equality still includes targets"); + assert_reference_name_equality!(raw, name); + + let loose = gix_ref::file::loose::Reference { + name: name.clone(), + target: Target::Symbolic(FullName::try_from("refs/heads/target-a")?), + }; + let loose_with_other_target = gix_ref::file::loose::Reference { + name: name.clone(), + target: Target::Symbolic(FullName::try_from("refs/heads/target-b")?), + }; + assert_ne!( + loose, loose_with_other_target, + "loose-reference structural equality still includes targets" + ); + assert_reference_name_equality!(loose, name); + + let packed = gix_ref::packed::Reference { + name: name.as_ref(), + target: b"0000000000000000000000000000000000000000".as_bstr(), + object: None, + }; + let packed_with_other_target = gix_ref::packed::Reference { + name: name.as_ref(), + target: b"1111111111111111111111111111111111111111".as_bstr(), + object: None, + }; + assert_ne!( + packed, packed_with_other_target, + "packed-reference structural equality still includes targets" + ); + assert_reference_name_equality!(packed, name); + Ok(()) +} diff --git a/gix-ref/tests/refs/file/log.rs b/gix-ref/tests/refs/file/log.rs index 268743d3b7e..da170882f88 100644 --- a/gix-ref/tests/refs/file/log.rs +++ b/gix-ref/tests/refs/file/log.rs @@ -109,7 +109,7 @@ mod iter { mod with_buffer_big_enough_for_largest_line { use gix_ref::log::Line; - use crate::{file::log::iter::reflog, sha1_hex_to_id}; + use crate::file::log::iter::reflog; #[test] fn single_line() -> crate::Result { @@ -129,8 +129,8 @@ mod iter { signature: _, message, } = iter.next().expect("a single line")?; - assert_eq!(previous_oid, sha1_hex_to_id("0000000000000000000000000000000000000000")); - assert_eq!(new_oid, sha1_hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03")); + assert_eq!(previous_oid, "0000000000000000000000000000000000000000"); + assert_eq!(new_oid, "134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(message, "commit (initial): c1"); assert!(iter.next().is_none(), "iterator depleted"); } @@ -157,8 +157,8 @@ mod iter { signature: _, message, } = iter.next().expect("a single line")?; - assert_eq!(previous_oid, sha1_hex_to_id("0000000000000000000000000000000000000000")); - assert_eq!(new_oid, sha1_hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03")); + assert_eq!(previous_oid, "0000000000000000000000000000000000000000"); + assert_eq!(new_oid, "134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(message, "commit (initial): c1"); let Line { previous_oid, @@ -167,8 +167,8 @@ mod iter { message, } = iter.next().expect("a single line")?; assert_eq!(message, "commit (initial): c2"); - assert_eq!(previous_oid, sha1_hex_to_id("1000000000000000000000000000000000000000")); - assert_eq!(new_oid, sha1_hex_to_id("234385f6d781b7e97062102c6a483440bfda2a03")); + assert_eq!(previous_oid, "1000000000000000000000000000000000000000"); + assert_eq!(new_oid, "234385f6d781b7e97062102c6a483440bfda2a03"); assert!(iter.next().is_none(), "iterator depleted"); } } diff --git a/gix-ref/tests/refs/file/reference.rs b/gix-ref/tests/refs/file/reference.rs index f33c4d112cf..fc594923ee3 100644 --- a/gix-ref/tests/refs/file/reference.rs +++ b/gix-ref/tests/refs/file/reference.rs @@ -136,7 +136,7 @@ mod peel { let commit = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(r.peel_to_id(&store, &EmptyCommit)?, commit); - assert_eq!(r.name.as_bstr(), "refs/remotes/origin/multi-link-target3"); + assert_eq!(r, "refs/remotes/origin/multi-link-target3"); let mut r: Reference = store.find_loose("dt1")?.into(); assert_eq!( @@ -189,13 +189,13 @@ mod peel { let store = file::store()?; let mut r: Reference = store.find_loose("loop-a")?.into(); assert_eq!(r.kind(), gix_ref::Kind::Symbolic, "there is something to peel"); - assert_eq!(r.name.as_bstr(), "refs/loop-a"); + assert_eq!(r, "refs/loop-a"); assert!(matches!( r.peel_to_id(&store, &gix_object::find::Never).unwrap_err(), gix_ref::peel::to_id::Error::FollowToObject(gix_ref::peel::to_object::Error::Cycle { .. }) )); - assert_eq!(r.name.as_bstr(), "refs/loop-a", "the ref is not changed on error"); + assert_eq!(r, "refs/loop-a", "the ref is not changed on error"); let mut r: Reference = store.find_loose("loop-a")?.into(); let err = r @@ -334,7 +334,10 @@ mod parse { assert_eq!(reference.kind(), gix_ref::Kind::Object); let target_id = reference.target.to_ref().try_id().expect("non-symbolic").to_owned(); assert_eq!(target_id.kind(), gix_hash::Kind::Sha256); - assert_eq!(target_id, gix_hash::ObjectId::from_hex(input).unwrap()); + assert_eq!( + target_id, + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + ); } } } diff --git a/gix-ref/tests/refs/file/store/find.rs b/gix-ref/tests/refs/file/store/find.rs index 9040d97b1fe..d5aa13f4277 100644 --- a/gix-ref/tests/refs/file/store/find.rs +++ b/gix-ref/tests/refs/file/store/find.rs @@ -12,8 +12,8 @@ mod existing { assert_eq!(store.is_pristine("refs/heads/main".try_into()?), Some(false)); let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); let r = store.find("main")?; + assert_eq!(r, "refs/heads/main"); assert_eq!(r.target.into_id(), c1); - assert_eq!(r.name.as_bstr(), "refs/heads/main"); let r = store .find("A") .unwrap_or_else(|_| panic!("{fixture}: should find capitalized refs")); @@ -113,7 +113,7 @@ mod loose { fn capitalized_branch() -> crate::Result { let store = store()?; assert_eq!( - store.find("A")?.name.as_bstr(), + store.find("A")?, "refs/heads/A", "capitalized loose refs can be found fine" ); @@ -126,7 +126,7 @@ mod loose { for (partial_name, expected_path) in &[("main", Some("refs/heads/main")), ("does-not-exist", None)] { let reference = store.find_loose(*partial_name); match expected_path { - Some(expected_path) => assert_eq!(reference?.name.as_bstr(), expected_path), + Some(expected_path) => assert_eq!(reference?, *expected_path), None => match reference { Ok(_) => panic!("Expected error"), Err(gix_ref::file::find::existing::Error::NotFound { name }) => { @@ -170,7 +170,7 @@ mod loose { ("refs/heads/main", "refs/heads/main", gix_ref::Kind::Object), ] { let reference = store.try_find_loose(*partial_name)?.expect("exists"); - assert_eq!(reference.name.as_bstr(), expected_path); + assert_eq!(reference, *expected_path); assert_eq!(reference.target.to_ref().kind(), *expected_ref_kind); } Ok(()) diff --git a/gix-ref/tests/refs/file/store/iter.rs b/gix-ref/tests/refs/file/store/iter.rs index 7067fb48699..9ec88495795 100644 --- a/gix-ref/tests/refs/file/store/iter.rs +++ b/gix-ref/tests/refs/file/store/iter.rs @@ -35,18 +35,11 @@ mod with_namespace { "refs/namespaces/bar/refs/remotes/origin/multi-link-target3", "refs/namespaces/bar/refs/tags/multi-link-target2", ]; - assert_eq!( - namespaced_refs - .iter() - .map(gix_ref::FullName::as_bstr) - .collect::>(), - expected_namespaced_refs - ); + assert_eq!(namespaced_refs, expected_namespaced_refs); assert_eq!( store .loose_iter_prefixed(ns_two.as_bstr().try_into().unwrap())? .map(Result::unwrap) - .map(|r| r.name.into_inner()) .collect::>(), [ "refs/namespaces/bar/refs/heads/multi-link-target1", @@ -60,20 +53,14 @@ mod with_namespace { .expect("present") .iter_prefixed(ns_two.as_bstr().to_owned())? .map(Result::unwrap) - .map(|r| r.name.to_owned().into_inner()) .collect::>(), ["refs/namespaces/bar/refs/remotes/origin/multi-link-target3"] ); for fullname in namespaced_refs { let reference = store.find(fullname.as_bstr())?; + assert_eq!(reference, fullname, "it finds namespaced items by fully qualified name"); assert_eq!( - reference.name, fullname, - "it finds namespaced items by fully qualified name" - ); - assert_eq!( - store - .find(fullname.as_bstr().splitn_str(2, b"/").nth(1).expect("name").as_bstr())? - .name, + store.find(fullname.as_bstr().splitn_str(2, b"/").nth(1).expect("name").as_bstr())?, fullname, "it will find namespaced items just by their shortened (but not shortest) name" ); @@ -121,7 +108,7 @@ mod with_namespace { |r: gix_ref::Reference| if r.name.as_bstr().starts_with_str("refs/namespaces") { None } else { - Some(r.name.as_bstr().to_owned()) + Some(r) } ) .collect::>(), @@ -159,14 +146,11 @@ mod with_namespace { .map(Result::unwrap) .map(|r: gix_ref::Reference| r.name) .collect::>(); - assert_eq!( - ref_names.iter().map(gix_ref::FullName::as_bstr).collect::>(), - expected_refs - ); + assert_eq!(ref_names, expected_refs); for fullname in ref_names { assert_eq!( - ns_store.find(fullname.as_bstr())?.name, + ns_store.find(fullname.as_bstr())?, fullname, "it finds namespaced items by fully qualified name, excluding namespace" ); @@ -177,9 +161,7 @@ mod with_namespace { "it won't find namespaced items by their store-relative name with namespace" ); assert_eq!( - ns_store - .find(fullname.as_bstr().splitn_str(2, b"/").nth(1).expect("name").as_bstr())? - .name, + ns_store.find(fullname.as_bstr().splitn_str(2, b"/").nth(1).expect("name").as_bstr())?, fullname, "it finds partial names within the namespace" ); @@ -191,11 +173,7 @@ mod with_namespace { "packed refs have no namespace support at all" ); assert_eq!( - ns_store - .loose_iter()? - .map(Result::unwrap) - .map(|r| r.name.into_inner()) - .collect::>(), + ns_store.loose_iter()?.map(Result::unwrap).collect::>(), [ "refs/heads/multi-link-target1", "refs/multi-link", @@ -207,11 +185,7 @@ mod with_namespace { { let prev = ns_store.namespace.take(); assert_eq!( - ns_store - .loose_iter()? - .map(Result::unwrap) - .map(|r| r.name.into_inner()) - .collect::>(), + ns_store.loose_iter()?.map(Result::unwrap).collect::>(), [ "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/multi-link", @@ -227,12 +201,7 @@ mod with_namespace { ns_store.namespace = ns_one.into(); assert_eq!( - ns_store - .iter()? - .all()? - .map(Result::unwrap) - .map(|r: gix_ref::Reference| r.name.into_inner()) - .collect::>(), + ns_store.iter()?.all()?.map(Result::unwrap).collect::>(), vec!["refs/d1", "refs/remotes/origin/HEAD", "refs/remotes/origin/main"], ); Ok(()) @@ -260,11 +229,7 @@ fn packed_file_iter() -> crate::Result { fn pseudo_refs_iter() -> crate::Result { let store = store_at("make_pseudo_ref_repository.sh")?; - let actual = store - .iter_pseudo()? - .map(Result::unwrap) - .map(|r: gix_ref::Reference| r.name.as_bstr().to_string()) - .collect::>(); + let actual = store.iter_pseudo()?.map(Result::unwrap).collect::>(); assert_eq!(actual, ["FETCH_HEAD", "HEAD", "JIRI_HEAD"]); Ok(()) @@ -295,10 +260,7 @@ fn loose_iter_with_broken_refs() -> crate::Result { actual[first_error].as_ref().expect_err("unparsable ref").to_string(), msg ); - let ref_paths: Vec<_> = actual - .drain(..first_error) - .filter_map(|e| e.ok().map(|e| e.name.into_inner())) - .collect(); + let ref_paths: Vec<_> = actual.drain(..first_error).filter_map(Result::ok).collect(); assert_eq!( ref_paths, @@ -335,23 +297,17 @@ fn loose_iter_with_prefix() -> crate::Result { let actual = store()? .loose_iter_prefixed(prefix_with_slash.try_into().unwrap())? .collect::, _>>() - .expect("no broken ref in this subset") - .into_iter() - .map(|e| e.name.into_inner()) - .collect::>(); + .expect("no broken ref in this subset"); assert_eq!( actual, - vec![ + [ "refs/heads/A", "refs/heads/d1", "refs/heads/dt1", "refs/heads/main", "refs/heads/multi-link-target1", - ] - .into_iter() - .map(String::from) - .collect::>(), + ], "all paths are as expected" ); Ok(()) @@ -363,23 +319,17 @@ fn loose_iter_with_partial_prefix_dir() -> crate::Result { let actual = store()? .loose_iter_prefixed(prefix_without_slash.try_into().unwrap())? .collect::, _>>() - .expect("no broken ref in this subset") - .into_iter() - .map(|e| e.name.into_inner()) - .collect::>(); + .expect("no broken ref in this subset"); assert_eq!( actual, - vec![ + [ "refs/heads/A", "refs/heads/d1", "refs/heads/dt1", "refs/heads/main", "refs/heads/multi-link-target1", - ] - .into_iter() - .map(String::from) - .collect::>(), + ], "all paths are as expected" ); Ok(()) @@ -390,19 +340,9 @@ fn loose_iter_with_partial_prefix() -> crate::Result { let actual = store()? .loose_iter_prefixed(b"refs/heads/d".as_bstr().try_into().unwrap())? .collect::, _>>() - .expect("no broken ref in this subset") - .into_iter() - .map(|e| e.name.into_inner()) - .collect::>(); + .expect("no broken ref in this subset"); - assert_eq!( - actual, - vec!["refs/heads/d1", "refs/heads/dt1"] - .into_iter() - .map(String::from) - .collect::>(), - "all paths are as expected" - ); + assert_eq!(actual, ["refs/heads/d1", "refs/heads/dt1"], "all paths are as expected"); Ok(()) } diff --git a/gix-ref/tests/refs/file/store/mod.rs b/gix-ref/tests/refs/file/store/mod.rs index a01bff45410..1f58c056e67 100644 --- a/gix-ref/tests/refs/file/store/mod.rs +++ b/gix-ref/tests/refs/file/store/mod.rs @@ -44,7 +44,7 @@ fn precompose_unicode_journey() -> crate::Result { .commit(committer().to_ref(&mut buf))?; let r = store_decomposed.iter()?.all()?.next().expect("created one ref")?; - assert_eq!(r.name.as_bstr(), decomposed_ref, "no transformation happens by default"); + assert_eq!(r, decomposed_ref, "no transformation happens by default"); // For some reason, `tmpfs` can qualify as decomposing, but it then doesn't really work the same as expected here // to the point where the precomposed iteration can't find a single reference. @@ -67,18 +67,17 @@ fn precompose_unicode_journey() -> crate::Result { let precomposed_ref = format!("refs/heads/{precomposed_a}"); let r = store_precomposed.iter()?.all()?.next().expect("created one ref")?; assert_eq!( - r.name.as_bstr(), - precomposed_ref, + r, precomposed_ref, "it transforms all refs it sees to precomposed format, in order to unify it with what we store in packed-refs (precomposed)" ); assert_eq!( - store_precomposed.find(precomposed_a)?.name.as_bstr(), + store_precomposed.find(precomposed_a)?, precomposed_ref, "can find as precomposed, even though on disk is decomposed it is decomposed" ); assert_eq!( - store_precomposed.find(decomposed_a)?.name.as_bstr(), + store_precomposed.find(decomposed_a)?, decomposed_ref, "can find as decomposed, and it keeps it as is to not violate expectations of the returned name being equal to the input (when comparing as bytes)" ); @@ -90,8 +89,7 @@ fn precompose_unicode_journey() -> crate::Result { .prepare(Some(create_at(&decomposed_ref)), Fail::Immediately, Fail::Immediately)? .commit(committer().to_ref(&mut buf))?; assert_eq!( - edits[0].name.as_bstr(), - decomposed_ref, + edits[0].name, decomposed_ref, "it doesn't alter the composition style to allow input and output to remain unchanged" ); @@ -196,7 +194,7 @@ fn precompose_unicode_journey() -> crate::Result { )? .commit(committer().to_ref(&mut buf))?; assert_eq!( - edits[0].change.new_value().unwrap().try_name().unwrap().as_bstr(), + edits[0].change.new_value().unwrap().try_name().unwrap(), decomposed_ref, "the composition doesn't change, it matches the original edit" ); @@ -206,8 +204,7 @@ fn precompose_unicode_journey() -> crate::Result { .expect("exists") .target .try_name() - .unwrap() - .as_bstr(), + .unwrap(), decomposed_ref, "on disk it's stored in the original format as well" ); diff --git a/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs b/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs index 65ac6fb5c8e..2b365d10084 100644 --- a/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs +++ b/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs @@ -421,7 +421,7 @@ fn symbolic_reference_writes_reflog_if_previous_value_is_set() -> crate::Result .commit(committer().to_ref(&mut TimeBuf::default()))?; assert_eq!(edits.len(), 1, "no split was performed"); let head = store.find_loose(&edits[0].name)?; - assert_eq!(head.name.as_bstr(), "refs/heads/symbolic"); + assert_eq!(head, "refs/heads/symbolic"); assert_eq!(head.kind(), gix_ref::Kind::Symbolic); assert_eq!( head.target.to_ref().try_name().map(gix_ref::FullNameRef::as_bstr), @@ -622,7 +622,7 @@ fn symbolic_head_missing_referent_then_update_referent() -> crate::Result { ); let head = store.find_loose(&edits[0].name)?; - assert_eq!(head.name.as_bstr(), "HEAD"); + assert_eq!(head, "HEAD"); assert_eq!(head.kind(), gix_ref::Kind::Symbolic); assert_eq!( std::fs::read_to_string(store.git_dir().join("HEAD"))?, diff --git a/gix-ref/tests/refs/fullname.rs b/gix-ref/tests/refs/fullname.rs index 8f7e1d1d2de..fcfc36c4153 100644 --- a/gix-ref/tests/refs/fullname.rs +++ b/gix-ref/tests/refs/fullname.rs @@ -119,8 +119,7 @@ fn shorten_and_category() { let (cat, short_name) = cat_and_short_name.expect("we know it's set"); let actual = cat.to_full_name(short_name).expect("valid input = valid output"); assert_eq!( - actual.as_ref().as_bstr(), - input, + actual, input, "{input}: {cat:?}:{short_name}: categories and short-names can round-trip" ); } @@ -147,15 +146,13 @@ fn shorten_and_category() { #[test] fn to_full_name() -> gix_testtools::Result { assert_eq!( - Category::LocalBranch.to_full_name("refs/heads/full")?.as_bstr(), + Category::LocalBranch.to_full_name("refs/heads/full")?, "refs/heads/full", "prefixes aren't duplicated" ); assert_eq!( - Category::LocalBranch - .to_full_name("refs/remotes/origin/other")? - .as_bstr(), + Category::LocalBranch.to_full_name("refs/remotes/origin/other")?, "refs/heads/refs/remotes/origin/other", "full names with a different category will be prefixed, to support 'main-worktree' special cases" ); @@ -166,12 +163,12 @@ fn to_full_name() -> gix_testtools::Result { #[test] fn local_branch_head_is_representable_as_full_ref_name() -> gix_testtools::Result { assert_eq!( - Category::LocalBranch.to_full_name("HEAD")?.as_bstr(), + Category::LocalBranch.to_full_name("HEAD")?, "refs/heads/HEAD", "generic full-name construction accepts names that are invalid only in branch-specific contexts" ); assert_eq!( - Category::LocalBranch.to_full_name("refs/heads/HEAD")?.as_bstr(), + Category::LocalBranch.to_full_name("refs/heads/HEAD")?, "refs/heads/HEAD", "fully qualified names keep their category prefix de-duplicated" ); @@ -182,21 +179,14 @@ fn local_branch_head_is_representable_as_full_ref_name() -> gix_testtools::Resul fn prefix_with_namespace_and_stripping() { let ns = gix_ref::namespace::expand("foo").unwrap(); let mut name: gix_ref::FullName = "refs/heads/main".try_into().unwrap(); + assert_eq!(name.prefix_namespace(&ns), "refs/namespaces/foo/refs/heads/main"); assert_eq!( - name.prefix_namespace(&ns).as_bstr(), - "refs/namespaces/foo/refs/heads/main" - ); - assert_eq!( - name.prefix_namespace(&ns).as_bstr(), + name.prefix_namespace(&ns), "refs/namespaces/foo/refs/heads/main", "idempotent prefixing" ); - assert_eq!(name.strip_namespace(&ns).as_bstr(), "refs/heads/main"); - assert_eq!( - name.strip_namespace(&ns).as_bstr(), - "refs/heads/main", - "idempotent stripping" - ); + assert_eq!(name.strip_namespace(&ns), "refs/heads/main"); + assert_eq!(name.strip_namespace(&ns), "refs/heads/main", "idempotent stripping"); } #[test] diff --git a/gix-ref/tests/refs/main.rs b/gix-ref/tests/refs/main.rs index a130ea1429e..c723d12e57d 100644 --- a/gix-ref/tests/refs/main.rs +++ b/gix-ref/tests/refs/main.rs @@ -63,6 +63,7 @@ fn translate_sha1_to_fixture_sha256(hex: &str) -> String { pub use gix_testtools::Result; +mod equality; mod file; mod fullname; mod partialname { @@ -71,7 +72,7 @@ mod partialname { #[test] fn join() -> crate::Result { let pn = PartialName::try_from("no-trailing-slash")?; - assert_eq!(pn.join("name".into())?.as_ref().as_bstr(), "no-trailing-slash/name"); + assert_eq!(pn.join("name".into())?, "no-trailing-slash/name"); let err = PartialName::try_from("trailing-slash/").unwrap_err(); assert!( diff --git a/gix-ref/tests/refs/namespace.rs b/gix-ref/tests/refs/namespace.rs index b6ecd5aaa68..ccd78c3712a 100644 --- a/gix-ref/tests/refs/namespace.rs +++ b/gix-ref/tests/refs/namespace.rs @@ -17,16 +17,13 @@ fn into_namespaced_prefix() { mod expand { #[test] fn components_end_with_trailing_slash_to_help_with_prefix_stripping() { - assert_eq!( - gix_ref::namespace::expand("foo").unwrap().as_bstr(), - "refs/namespaces/foo/" - ); + assert_eq!(gix_ref::namespace::expand("foo").unwrap(), "refs/namespaces/foo/"); } #[test] fn each_component_expands_to_the_namespace_prefix_individually() { assert_eq!( - gix_ref::namespace::expand("foo/bar").unwrap().as_bstr(), + gix_ref::namespace::expand("foo/bar").unwrap(), "refs/namespaces/foo/refs/namespaces/bar/" ); } diff --git a/gix-ref/tests/refs/packed/find.rs b/gix-ref/tests/refs/packed/find.rs index 48bd7493ccf..a7158c15282 100644 --- a/gix-ref/tests/refs/packed/find.rs +++ b/gix-ref/tests/refs/packed/find.rs @@ -23,7 +23,7 @@ fn capitalized_branch() -> crate::Result { let packed_refs = store.open_packed_buffer()?.expect("packed-refs exist"); assert_eq!( - packed_refs.find("A")?.name.as_bstr(), + packed_refs.find("A")?, "refs/heads/A", "fully capitalized refs aren't just considered pseudorefs" ); @@ -102,7 +102,7 @@ fn partial_name_to_full_name_conversion_rules_are_applied() -> crate::Result { let packed = store.open_packed_buffer()?.expect("packed-refs exists"); assert_eq!( - store.find_loose("origin")?.name.as_bstr(), + store.find_loose("origin")?, "refs/remotes/origin/HEAD", "a special that only applies to loose refs" ); @@ -110,37 +110,33 @@ fn partial_name_to_full_name_conversion_rules_are_applied() -> crate::Result { packed.try_find("origin")?.is_none(), "packed refs don't have this special case as they don't store HEADs or symrefs" ); - assert_eq!( - store.find_loose("HEAD")?.name.as_bstr(), - "HEAD", - "HEAD can be found in loose stores" - ); + assert_eq!(store.find_loose("HEAD")?, "HEAD", "HEAD can be found in loose stores"); assert!( packed.try_find("HEAD")?.is_none(), "packed refs definitely don't contain HEAD" ); assert_eq!( - packed.try_find("head-or-tag")?.expect("present").name.as_bstr(), + packed.try_find("head-or-tag")?.expect("present"), "refs/tags/head-or-tag", "it finds tags first" ); assert_eq!( - packed.try_find("heads/head-or-tag")?.expect("present").name.as_bstr(), + packed.try_find("heads/head-or-tag")?.expect("present"), "refs/heads/head-or-tag", "it finds heads when disambiguated" ); assert_eq!( - packed.try_find("main")?.expect("present").name.as_bstr(), + packed.try_find("main")?.expect("present"), "refs/heads/main", "it finds local heads before remote ones" ); assert_eq!( - packed.try_find("origin/main")?.expect("present").name.as_bstr(), + packed.try_find("origin/main")?.expect("present"), "refs/remotes/origin/main", "it finds remote heads when disambiguated" ); assert_eq!( - packed.try_find("remotes/origin/main")?.expect("present").name.as_bstr(), + packed.try_find("remotes/origin/main")?.expect("present"), "refs/remotes/origin/main", "more specification is possible, too" ); diff --git a/gix-ref/tests/refs/packed/iter.rs b/gix-ref/tests/refs/packed/iter.rs index 6db275d684e..1c1b58d6e85 100644 --- a/gix-ref/tests/refs/packed/iter.rs +++ b/gix-ref/tests/refs/packed/iter.rs @@ -1,4 +1,3 @@ -use gix_object::bstr::ByteSlice; use gix_ref::packed; use crate::file::{store_at, store_with_packed_refs}; @@ -30,52 +29,39 @@ fn iter_prefix() -> crate::Result { assert_eq!( packed .iter_prefixed("refs/heads/".into())? - .map(|r| r.map(|r| r.name.as_bstr())) .collect::, _>>()?, - vec![ - "refs/heads/A".as_bytes().as_bstr(), - "refs/heads/d1".into(), - "refs/heads/dt1".into(), - "refs/heads/main".into() - ] + vec!["refs/heads/A", "refs/heads/d1", "refs/heads/dt1", "refs/heads/main"] ); assert_eq!( packed .iter_prefixed("refs/heads/d".into())? - .map(|r| r.map(|r| r.name.as_bstr())) .collect::, _>>()?, - vec!["refs/heads/d1".as_bytes().as_bstr(), "refs/heads/dt1".into(),], + vec!["refs/heads/d1", "refs/heads/dt1"], "partial prefixes are fine, they don't have to resemble or be a directory" ); assert_eq!( packed .iter_prefixed("refs/remotes/".into())? - .map(|r| r.map(|r| r.name.as_bstr())) .collect::, _>>()?, - vec![ - "refs/remotes/origin/main".as_bytes().as_bstr(), - "refs/remotes/origin/multi-link-target3".into(), - ] + vec!["refs/remotes/origin/main", "refs/remotes/origin/multi-link-target3",] ); let last_ref_in_file = "refs/tags/t1"; assert_eq!( packed .iter_prefixed(last_ref_in_file.into())? - .map(|r| r.map(|r| r.name.as_bstr())) .collect::, _>>()?, - vec![last_ref_in_file.as_bytes().as_bstr()], + vec![last_ref_in_file], "prefixes which are a ref also work, this one is the last of the file" ); let first_ref_in_file = "refs/d1"; assert_eq!( packed .iter_prefixed(first_ref_in_file.into())? - .map(|r| r.map(|r| r.name.as_bstr())) .collect::, _>>()?, - vec![first_ref_in_file.as_bytes().as_bstr()], + vec![first_ref_in_file], "prefixes which are a ref also work, and this one at the beginning of the file" ); Ok(()) diff --git a/gix-ref/tests/refs/reference.rs b/gix-ref/tests/refs/reference.rs index 9662462818d..f6e5ff1b895 100644 --- a/gix-ref/tests/refs/reference.rs +++ b/gix-ref/tests/refs/reference.rs @@ -17,9 +17,9 @@ fn strip_namespace() { peeled: None, }; r.strip_namespace(&ns); - assert_eq!(r.name.as_bstr(), "refs/heads/main", "name is stripped"); + assert_eq!(r, "refs/heads/main", "name is stripped"); assert!( - matches!(r.target, Target::Symbolic(n) if n.as_bstr() == "refs/tags/foo"), + matches!(r.target, Target::Symbolic(n) if n == "refs/tags/foo"), "and the symbolic target as well" ); } diff --git a/gix-ref/tests/refs/transaction.rs b/gix-ref/tests/refs/transaction.rs index c15e44672ca..5ecaca08eba 100644 --- a/gix-ref/tests/refs/transaction.rs +++ b/gix-ref/tests/refs/transaction.rs @@ -107,10 +107,7 @@ mod refedit_ext { fn find<'a>(edits: &'a [RefEdit], name: &str) -> &'a RefEdit { let name: &FullNameRef = name.try_into().unwrap(); - edits - .iter() - .find(|e| e.name.as_bstr() == name.as_bstr()) - .expect("always available") + edits.iter().find(|e| e.name == name).expect("always available") } #[test] diff --git a/gix/src/head/mod.rs b/gix/src/head/mod.rs index ef867bd3062..da8027d361b 100644 --- a/gix/src/head/mod.rs +++ b/gix/src/head/mod.rs @@ -55,7 +55,7 @@ impl<'repo> Head<'repo> { /// # let repo = doctest::open_repo(doctest::basic_repo_dir()?)?; /// let head = repo.head()?; /// - /// assert_eq!(head.referent_name().expect("branch head").as_bstr(), "refs/heads/main"); + /// assert_eq!(head.referent_name().expect("branch head"), "refs/heads/main"); /// # Ok(()) } /// ``` pub fn referent_name(&self) -> Option<&FullNameRef> { @@ -111,7 +111,7 @@ impl<'repo> Head<'repo> { /// # let repo = doctest::open_repo(doctest::basic_repo_dir()?)?; /// let branch = repo.head()?.try_into_referent().expect("symbolic head"); /// - /// assert_eq!(branch.name().as_bstr(), "refs/heads/main"); + /// assert_eq!(branch, "refs/heads/main"); /// # Ok(()) } /// ``` pub fn try_into_referent(self) -> Option> { diff --git a/gix/src/id.rs b/gix/src/id.rs index e3eba8ee38d..90a9bb86566 100644 --- a/gix/src/id.rs +++ b/gix/src/id.rs @@ -116,6 +116,22 @@ mod impls { use crate::{Id, Object, ObjectDetached}; + macro_rules! impl_partial_eq_text { + ($text:ty) => { + impl PartialEq<$text> for Id<'_> { + fn eq(&self, other: &$text) -> bool { + self.inner.eq(other) + } + } + + impl<'repo> PartialEq> for $text { + fn eq(&self, other: &Id<'repo>) -> bool { + self.eq(&other.inner) + } + } + }; + } + // Eq, Hash, Ord, PartialOrd, impl std::hash::Hash for Id<'_> { @@ -148,6 +164,10 @@ mod impls { } } + impl_partial_eq_text!(str); + impl_partial_eq_text!(&str); + impl_partial_eq_text!(String); + impl PartialEq for Id<'_> { fn eq(&self, other: &oid) -> bool { self.inner == other diff --git a/gix/src/reference/mod.rs b/gix/src/reference/mod.rs index ff8847a8c30..6c334357111 100644 --- a/gix/src/reference/mod.rs +++ b/gix/src/reference/mod.rs @@ -68,6 +68,35 @@ impl std::fmt::Debug for Reference<'_> { } } +mod impls { + use gix_ref::{ + FullName, FullNameRef, + bstr::{BStr, BString}, + }; + + use crate::Reference; + + macro_rules! impl_partial_eq { + ($other:ty) => { + impl PartialEq<$other> for Reference<'_> { + fn eq(&self, other: &$other) -> bool { + self.inner.eq(other) + } + } + }; + } + + impl_partial_eq!(str); + impl_partial_eq!(&str); + impl_partial_eq!(String); + impl_partial_eq!(BStr); + impl_partial_eq!(&BStr); + impl_partial_eq!(BString); + impl_partial_eq!(FullName); + impl_partial_eq!(FullNameRef); + impl_partial_eq!(&FullNameRef); +} + impl<'repo> Reference<'repo> { pub(crate) fn from_ref(reference: gix_ref::Reference, repo: &'repo crate::Repository) -> Self { Reference { inner: reference, repo } @@ -266,7 +295,7 @@ impl<'repo> Reference<'repo> { /// let head = repo.find_reference("HEAD")?; /// let branch = head.follow().expect("symbolic")?; /// - /// assert_eq!(branch.name().as_bstr(), "refs/heads/main"); + /// assert_eq!(branch, "refs/heads/main"); /// # Ok(()) } /// ``` pub fn follow(&self) -> Option, gix_ref::file::find::existing::Error>> { diff --git a/gix/src/remote/connection/fetch/update_refs/tests.rs b/gix/src/remote/connection/fetch/update_refs/tests.rs index ad89a5a2407..55b3ac8e73b 100644 --- a/gix/src/remote/connection/fetch/update_refs/tests.rs +++ b/gix/src/remote/connection/fetch/update_refs/tests.rs @@ -752,8 +752,7 @@ mod update { _ => unreachable!("only updates"), } assert_eq!( - edit.name.as_bstr(), - "refs/heads/HEAD", + edit.name, "refs/heads/HEAD", "it's not possible to refer to the local HEAD with refspecs" ); } @@ -811,7 +810,7 @@ mod update { } _ => unreachable!("only updates"), } - assert_eq!(edit.name.as_bstr(), "refs/remotes/origin/new-HEAD"); + assert_eq!(edit.name, "refs/remotes/origin/new-HEAD"); } #[test] diff --git a/gix/src/repository/reference.rs b/gix/src/repository/reference.rs index b0c170b98b4..afd1303fecf 100644 --- a/gix/src/repository/reference.rs +++ b/gix/src/repository/reference.rs @@ -179,7 +179,7 @@ impl crate::Repository { /// # let repo = doctest::open_repo(doctest::basic_repo_dir()?)?; /// let head = repo.head()?; /// - /// assert_eq!(head.referent_name().expect("born").as_bstr(), "refs/heads/main"); + /// assert_eq!(head.referent_name().expect("born"), "refs/heads/main"); /// assert!(!head.is_detached()); /// assert!(!head.is_unborn()); /// # Ok(()) } @@ -316,7 +316,7 @@ impl crate::Repository { /// # let repo = doctest::open_repo(doctest::basic_repo_dir()?)?; /// let mut reference = repo.find_reference("main")?; /// - /// assert_eq!(reference.name().as_bstr(), "refs/heads/main"); + /// assert_eq!(reference, "refs/heads/main"); /// assert_eq!(reference.peel_to_commit()?.message()?.title, "c2\n"); /// # Ok(()) } /// ``` @@ -351,7 +351,6 @@ impl crate::Repository { /// let branches = repo /// .references()? /// .local_branches()? - /// .map(|reference| reference.map(|reference| reference.name().as_bstr().to_string())) /// .collect::, _>>()?; /// /// assert_eq!(branches, vec!["refs/heads/main".to_owned()]); diff --git a/gix/tests/gix/clone.rs b/gix/tests/gix/clone.rs index 90c47d91ef6..e86251a6a67 100644 --- a/gix/tests/gix/clone.rs +++ b/gix/tests/gix/clone.rs @@ -241,16 +241,14 @@ mod blocking_io { let checked_out_ref = repo.head_ref()?.expect("head points to ref"); assert_eq!( - checked_out_ref.name().as_bstr(), - "refs/heads/b", + checked_out_ref, "refs/heads/b", "branches win over same-named tags, matching git clone --branch" ); assert_eq!( checked_out_ref .remote_ref_name(gix::remote::Direction::Fetch) .transpose()? - .unwrap() - .as_bstr(), + .unwrap(), "refs/heads/b", "branch merge configuration records the chosen branch" ); @@ -633,8 +631,8 @@ mod blocking_io { "the object pointed to by HEAD was fetched as well" ); assert_eq!( - referent.name().as_bstr(), - remote_repo.head_name()?.expect("symbolic").as_bstr(), + referent, + remote_repo.head_name()?.expect("symbolic"), "local clone always adopts the name of the remote" ); @@ -649,8 +647,7 @@ mod blocking_io { ); assert_eq!( repo.branch_remote_ref_name(ref_name, gix::remote::Direction::Fetch) - .expect("present")? - .as_bstr(), + .expect("present")?, "refs/heads/main" ); @@ -897,8 +894,7 @@ mod blocking_io { let checked_out_ref = repo.head_ref()?.expect("head points to ref"); let remote_ref_name = format!("refs/heads/{ref_to_checkout}"); assert_eq!( - checked_out_ref.name().as_bstr(), - remote_ref_name, + checked_out_ref, remote_ref_name, "it's possible to checkout anything with that name, but here we have an ordinary branch" ); @@ -906,8 +902,7 @@ mod blocking_io { checked_out_ref .remote_ref_name(gix::remote::Direction::Fetch) .transpose()? - .unwrap() - .as_bstr(), + .unwrap(), remote_ref_name, "the merge configuration is using the given name" ); @@ -1154,11 +1149,7 @@ mod blocking_io { } let checked_out_ref = repo.head_ref()?.expect("head points to ref"); - assert_eq!( - checked_out_ref.name().as_bstr(), - remote_ref_name, - "it also works with tags" - ); + assert_eq!(checked_out_ref, remote_ref_name, "it also works with tags"); assert_eq!( checked_out_ref @@ -1212,13 +1203,13 @@ mod blocking_io { .is_some_and(|cap| cap.supports("unborn").unwrap_or(false)); if supports_unborn { assert_eq!( - head.referent_name().expect("present").as_bstr(), + head.referent_name().expect("present"), "refs/heads/special", "we pick up the name as present on the server, not the one we default to" ); } else { assert_eq!( - head.referent_name().expect("present").as_bstr(), + head.referent_name().expect("present"), "refs/heads/main", "we simply keep our own post-init HEAD which defaults to the branch name we configured locally" ); diff --git a/gix/tests/gix/id.rs b/gix/tests/gix/id.rs index 5ac7b1d2f99..85b31d471b6 100644 --- a/gix/tests/gix/id.rs +++ b/gix/tests/gix/id.rs @@ -69,6 +69,21 @@ fn display_and_debug() -> crate::Result { Ok(()) } +#[test] +fn compares_with_text() -> crate::Result { + let repo = crate::basic_repo()?; + let id = repo.head_id()?; + let text = id.to_string(); + + assert_eq!(id, text.as_str(), "an attached ID matches str"); + assert_eq!(text.as_str(), id, "str comparison is symmetric"); + assert_eq!(id, text, "an attached ID matches String"); + assert_eq!(text, id, "String comparison is symmetric"); + assert_ne!(id, "not an object ID", "invalid text does not match an attached ID"); + assert_ne!("not an object ID", id, "invalid-text comparison is symmetric"); + Ok(()) +} + mod ancestors { use crate::util::hex_to_id; diff --git a/gix/tests/gix/init.rs b/gix/tests/gix/init.rs index 7df4d64ffb1..9fbe994d1ad 100644 --- a/gix/tests/gix/init.rs +++ b/gix/tests/gix/init.rs @@ -70,10 +70,7 @@ mod non_bare { ]), )? .into(); - assert_eq!( - repo.head()?.referent_name().expect("name").as_bstr(), - "refs/heads/special" - ); + assert_eq!(repo.head()?.referent_name().expect("name"), "refs/heads/special"); Ok(()) } @@ -91,10 +88,7 @@ mod non_bare { ]), )? .into(); - assert_eq!( - repo.head()?.referent_name().expect("name").as_bstr(), - "refs/heads/special" - ); + assert_eq!(repo.head()?.referent_name().expect("name"), "refs/heads/special"); assert_eq!( repo.is_pristine(), Some(true), diff --git a/gix/tests/gix/reference/mod.rs b/gix/tests/gix/reference/mod.rs index 4c629f8d253..c650828e3bc 100644 --- a/gix/tests/gix/reference/mod.rs +++ b/gix/tests/gix/reference/mod.rs @@ -1,5 +1,41 @@ use gix::remote::Direction; +#[test] +fn compares_with_name_representations() -> crate::Result { + use gix::{ + bstr::{BString, ByteSlice}, + refs::{FullName, FullNameRef, Target}, + }; + + let repo = crate::basic_repo()?; + let reference = repo.find_reference("main")?; + let text = "refs/heads/main"; + let string = text.to_owned(); + let bytes = text.as_bytes().as_bstr(); + let byte_string: BString = text.into(); + let name = FullName::try_from(text)?; + let name_ref: &FullNameRef = name.as_ref(); + + assert_eq!(reference, text, "an attached reference matches str"); + assert_eq!(reference, string, "an attached reference matches String"); + assert_eq!(reference, bytes, "an attached reference matches BStr"); + assert_eq!(reference, byte_string, "an attached reference matches BString"); + assert_eq!(reference, name, "an attached reference matches FullName"); + assert_eq!(reference, name_ref, "an attached reference matches FullNameRef"); + + let mut other_target = reference.clone(); + other_target.inner.target = Target::Symbolic(FullName::try_from("refs/heads/other")?); + assert_eq!( + other_target, text, + "reference-name equality is independent of the target" + ); + assert_ne!( + other_target, "refs/heads/other", + "a target name does not compare as the reference name" + ); + Ok(()) +} + mod log { #[test] @@ -58,7 +94,7 @@ mod find { let repo = repo()?; let mut packed_tag_ref = repo.try_find_reference("dt1")?.expect("tag to exist"); let expected: &FullNameRef = "refs/tags/dt1".try_into()?; - assert_eq!(packed_tag_ref.name(), expected); + assert_eq!(packed_tag_ref, expected); assert_eq!( packed_tag_ref.inner.target, @@ -78,11 +114,11 @@ mod find { let mut symbolic_ref = repo.find_reference("multi-link-target1")?; let expected: &FullNameRef = "refs/heads/multi-link-target1".try_into()?; - assert_eq!(symbolic_ref.name(), expected); + assert_eq!(symbolic_ref, expected); assert_eq!(symbolic_ref.peel_to_id()?, the_commit); let expected: &FullNameRef = "refs/remotes/origin/multi-link-target3".try_into()?; - assert_eq!(symbolic_ref.name(), expected, "it follows symbolic refs, too"); + assert_eq!(symbolic_ref, expected, "it follows symbolic refs, too"); assert_eq!(symbolic_ref.into_fully_peeled_id()?, the_commit, "idempotency"); let mut tag_ref = repo.find_reference("dt3")?; diff --git a/gix/tests/gix/remote/fetch.rs b/gix/tests/gix/remote/fetch.rs index 2b83d52456f..f0ff121d91f 100644 --- a/gix/tests/gix/remote/fetch.rs +++ b/gix/tests/gix/remote/fetch.rs @@ -824,13 +824,13 @@ mod blocking_and_async_io { assert_eq!(update_refs.edits.len(), 2); let edit = &update_refs.edits[0]; - assert_eq!(edit.name.as_bstr(), "refs/remotes/changes-on-top-of-origin/main"); + assert_eq!(edit.name, "refs/remotes/changes-on-top-of-origin/main"); assert!( edit.change.new_value().expect("no deletion").try_id().is_some(), "a simple peeled ref" ); let edit = &update_refs.edits[1]; - assert_eq!(edit.name.as_bstr(), "refs/remotes/changes-on-top-of-origin/symbolic"); + assert_eq!(edit.name, "refs/remotes/changes-on-top-of-origin/symbolic"); match version.unwrap_or_default() { gix::protocol::transport::Protocol::V2 => { assert!( @@ -896,7 +896,7 @@ mod blocking_and_async_io { assert_eq!(id, mapping.remote.as_id().expect("no unborn")); } gix_ref::TargetRef::Symbolic(target) => { - assert_eq!(target.as_bstr(), mapping.remote.as_target().expect("no direct ref")); + assert_eq!(target, mapping.remote.as_target().expect("no direct ref")); } } assert!( @@ -914,7 +914,7 @@ mod blocking_and_async_io { ); } gix_ref::TargetRef::Symbolic(target) => { - assert_eq!(target.as_bstr(), mapping.remote.as_target().expect("no direct ref")); + assert_eq!(target, mapping.remote.as_target().expect("no direct ref")); } } } diff --git a/gix/tests/gix/repository/config/remote.rs b/gix/tests/gix/repository/config/remote.rs index f4e73859896..1eb18f449e0 100644 --- a/gix/tests/gix/repository/config/remote.rs +++ b/gix/tests/gix/repository/config/remote.rs @@ -103,14 +103,13 @@ mod branch_remote { assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/main".try_into()?, remote::Direction::Fetch) .expect("Remote Merge ref exists") - .expect("Remote Merge ref is valid") - .as_bstr(), + .expect("Remote Merge ref is valid"), "refs/remotes/remote_repo/main" ); let (upstream, remote_name) = repo .upstream_branch_and_remote_for_tracking_branch("refs/remotes/remote_repo/main".try_into()?)? .expect("mapping exists"); - assert_eq!(upstream.as_bstr(), "refs/heads/main"); + assert_eq!(upstream, "refs/heads/main"); assert_eq!( remote_name.name().expect("non-anonymous remote").as_bstr(), "remote_repo" @@ -140,8 +139,7 @@ mod branch_remote { assert_eq!( repo.branch_remote_ref_name("refs/heads/broken".try_into()?, remote::Direction::Fetch) .expect("Remote Merge ref exists") - .expect("merge ref is turned into a full-name") - .as_bstr(), + .expect("merge ref is turned into a full-name"), "refs/heads/not_a_valid_merge_ref", "short names are simply turned into branch names - this doesn't always work, but sometimes." ); @@ -158,8 +156,7 @@ mod branch_remote { assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/broken".try_into()?, remote::Direction::Fetch) .expect("no error") - .expect("valid result") - .as_bstr(), + .expect("valid result"), "refs/remotes/remote_repo/not_a_valid_merge_ref", "the merge ref is broken, but we turned it into a full ref name from which everything else was derived", ); @@ -179,7 +176,7 @@ mod branch_remote { )? .expect("mapping exists"); assert_eq!(remote.name().expect("named remote").as_bstr(), expected_remote_name); - assert_eq!(upstream.as_bstr(), "refs/heads/main"); + assert_eq!(upstream, "refs/heads/main"); } let err = repo .upstream_branch_and_remote_for_tracking_branch("refs/remotes/with/two/slashes/main".try_into()?) @@ -195,8 +192,7 @@ mod branch_remote { .expect("mapping exists"); assert_eq!(remote.name().expect("non-anonymous remote").as_bstr(), "with/two"); assert_eq!( - upstream.as_bstr(), - "refs/heads/special", + upstream, "refs/heads/special", "it finds a single mapping even though there are two refspecs" ); Ok(()) @@ -217,8 +213,7 @@ mod branch_remote { for direction in [remote::Direction::Fetch, remote::Direction::Push] { assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/main".try_into()?, direction) - .expect("exists")? - .as_bstr(), + .expect("exists")?, "refs/remotes/remote_repo/main", "this is a 'simple' mapping of an existing branch, using push.default=simple and the default refspec" ); @@ -247,15 +242,13 @@ mod branch_remote { assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/main".try_into()?, remote::Direction::Push) - .expect("exists")? - .as_bstr(), + .expect("exists")?, "refs/remotes/origin/remapped-main", "the first matching push-spec maps the branch to another head, then it's mapped with fetch-specs" ); assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/main".try_into()?, remote::Direction::Fetch) - .expect("exists")? - .as_bstr(), + .expect("exists")?, "refs/remotes/origin/main", "push.simple is set (or the default), hence it's a one-one mapping along with the standard refspec" ); @@ -278,15 +271,13 @@ mod branch_remote { assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/feature".try_into()?, remote::Direction::Push) - .expect("exists")? - .as_bstr(), + .expect("exists")?, "refs/remotes/origin/remapped-feature", "this branch is mapped with push-specs, then it's mapped with fetch-specs as well" ); assert_eq!( repo.branch_remote_tracking_ref_name("refs/heads/feature".try_into()?, remote::Direction::Fetch) - .expect("exists")? - .as_bstr(), + .expect("exists")?, "refs/remotes/origin/main", "remapping by branch.feature.merge=main, then mapped by refspec" ); diff --git a/gix/tests/gix/repository/mod.rs b/gix/tests/gix/repository/mod.rs index f8f7a3fee9f..a9a5857fe1e 100644 --- a/gix/tests/gix/repository/mod.rs +++ b/gix/tests/gix/repository/mod.rs @@ -30,22 +30,19 @@ mod worktree; #[cfg(feature = "revision")] mod revision { - use crate::util::hex_to_id_sha1_only; - #[test] fn date() -> crate::Result { let repo = crate::named_repo("make_rev_parse_repo.sh")?; let actual = repo .rev_parse_single("old@{20 years ago}") .expect("it returns the oldest possible rev when overshooting"); - assert_eq!(actual, hex_to_id_sha1_only("be2f093f0588eaeb71e1eff7451b18c2a9b1d765")); + assert_eq!(actual, "be2f093f0588eaeb71e1eff7451b18c2a9b1d765"); let actual = repo .rev_parse_single("old@{1732184844}") .expect("it finds something in the middle"); assert_eq!( - actual, - hex_to_id_sha1_only("b29405fe9147a3a366c4048fbe295ea04de40fa6"), + actual, "b29405fe9147a3a366c4048fbe295ea04de40fa6", "It also figures out that we don't mean an index, but a date" ); Ok(()) diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index f1ea1c7b80b..4093a2e31a9 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -649,7 +649,7 @@ mod tag { message, gix_ref::transaction::PreviousValue::MustNotExist, )?; - assert_eq!(tag_ref.name().as_bstr(), "refs/tags/v1.0.0"); + assert_eq!(tag_ref, "refs/tags/v1.0.0"); assert_ne!(tag_ref.id(), current_head_id, "it points to the tag object"); let tag = tag_ref.id().object()?; let tag = tag.try_to_tag_ref()?; @@ -725,10 +725,7 @@ mod commit { gix_hash::Kind::Sha256 => expected_sha256, _ => unreachable!(), }; - assert_eq!( - actual, - gix_hash::ObjectId::from_hex(expected_hash.as_bytes()).expect("valid sha1") - ); + assert_eq!(actual, expected_hash); } #[test] @@ -778,7 +775,7 @@ mod commit { ); let head = repo.head()?.try_into_referent().expect("born"); - assert_eq!(head.name().as_bstr(), "refs/heads/main", "'main' is the default name"); + assert_eq!(head, "refs/heads/main", "'main' is the default name"); assert_eq!( head.log_iter() .rev()? @@ -894,8 +891,7 @@ fn new_commit_as() -> crate::Result { _ => unreachable!(), }; assert_eq!( - commit.id, - gix_hash::ObjectId::from_hex(expected_hex.as_bytes()).expect("valid object id"), + commit.id, expected_hex, "The commit-id is stable as the author/committer is controlled" ); diff --git a/gix/tests/gix/repository/reference.rs b/gix/tests/gix/repository/reference.rs index f7d4be8c1df..521385b8509 100644 --- a/gix/tests/gix/repository/reference.rs +++ b/gix/tests/gix/repository/reference.rs @@ -33,11 +33,7 @@ mod set_namespace { )?; assert_eq!( - repo.references()? - .all()? - .filter_map(Result::ok) - .map(|r| r.name().as_bstr().to_owned()) - .collect::>(), + repo.references()?.all()?.filter_map(Result::ok).collect::>(), vec!["refs/heads/new-branch", "refs/tags/new-tag"], "namespaced references appear like normal ones" ); @@ -46,25 +42,24 @@ mod set_namespace { repo.references()? .prefixed("refs/tags/")? .filter_map(Result::ok) - .map(|r| r.name().as_bstr().to_owned()) .collect::>(), vec!["refs/tags/new-tag"], "namespaced references appear like normal ones" ); let fully_qualified_tag_name = "refs/tags/new-tag"; assert_eq!( - repo.find_reference(fully_qualified_tag_name)?.name().as_bstr(), + repo.find_reference(fully_qualified_tag_name)?, fully_qualified_tag_name, "fully qualified (yet namespaced) names work" ); assert_eq!( - repo.find_reference("new-tag")?.name().as_bstr(), + repo.find_reference("new-tag")?, fully_qualified_tag_name, "namespaces are transparent" ); let previous_ns = repo.clear_namespace().expect("namespace set"); - assert_eq!(previous_ns.as_bstr(), "refs/namespaces/foo/"); + assert_eq!(previous_ns, "refs/namespaces/foo/"); assert!(repo.clear_namespace().is_none(), "it doesn't invent namespaces"); assert_eq!( @@ -103,11 +98,7 @@ mod iter_references { fn all() -> crate::Result { let repo = repo()?; assert_eq!( - repo.references()? - .all()? - .filter_map(Result::ok) - .map(|r| r.name().as_bstr().to_owned()) - .collect::>(), + repo.references()?.all()?.filter_map(Result::ok).collect::>(), vec![ "refs/d1", "refs/heads/d1", @@ -207,7 +198,7 @@ mod iter_references { .filter_map(Result::ok) .max_by_key(|tag| tag.name().shorten().to_owned()) .ok_or(std::io::Error::other("latest tag not found"))?; - assert_eq!(actual.name().as_bstr(), "refs/tags/t1"); + assert_eq!(actual, "refs/tags/t1"); Ok(()) } } @@ -231,7 +222,7 @@ mod head { } _ => panic!("unexpected head kind"), } - assert_eq!(head.referent_name().expect("born").as_bstr(), "refs/heads/main"); + assert_eq!(head.referent_name().expect("born"), "refs/heads/main"); assert!(!head.is_detached()); Ok(()) } diff --git a/gix/tests/gix/repository/worktree.rs b/gix/tests/gix/repository/worktree.rs index 4b388c8bdb8..32e1d2a4d0f 100644 --- a/gix/tests/gix/repository/worktree.rs +++ b/gix/tests/gix/repository/worktree.rs @@ -409,7 +409,7 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { ); assert_eq!(main_repo.head_id().unwrap(), expected_main.peeled); assert_eq!( - main_repo.head_name().unwrap().expect("no detached head").as_bstr(), + main_repo.head_name().unwrap().expect("no detached head"), expected_main.branch.unwrap() ); let worktree = main_repo.worktree().expect("not bare"); diff --git a/gix/tests/gix/revision/spec/from_bytes/mod.rs b/gix/tests/gix/revision/spec/from_bytes/mod.rs index 433e690facc..869fd126345 100644 --- a/gix/tests/gix/revision/spec/from_bytes/mod.rs +++ b/gix/tests/gix/revision/spec/from_bytes/mod.rs @@ -24,10 +24,7 @@ mod sibling_branch { for op in ["upstream", "push"] { for branch in ["", "main"] { let actual = parse_spec(format!("{branch}@{{{op}}}"), &repo)?; - assert_eq!( - actual.first_reference().expect("set").name.as_bstr(), - "refs/remotes/origin/main" - ); + assert_eq!(actual.first_reference().expect("set"), "refs/remotes/origin/main"); assert_eq!(actual.second_reference(), None); assert_eq!( actual.single().expect("just one"), diff --git a/gix/tests/gix/revision/spec/from_bytes/reflog.rs b/gix/tests/gix/revision/spec/from_bytes/reflog.rs index f0eb111d688..876a3cd1b7e 100644 --- a/gix/tests/gix/revision/spec/from_bytes/reflog.rs +++ b/gix/tests/gix/revision/spec/from_bytes/reflog.rs @@ -17,7 +17,7 @@ fn nth_prior_checkout() { ("@{-5}", "refs/heads/h"), ] { let parsed = parse_spec(spec, &repo).unwrap_or_else(|_| panic!("{spec} to be parsed successfully")); - assert_eq!(parsed.first_reference().expect("present").name.as_bstr(), prior_branch); + assert_eq!(parsed.first_reference().expect("present"), prior_branch); assert_eq!(parsed.second_reference(), None); } @@ -71,7 +71,7 @@ fn by_index() { Spec::from_id(hex_to_id_sha1_only("55e825ebe8fd2ff78cad3826afb696b96b576a7e").attach(repo)) ); assert_eq!( - spec.first_reference().expect("set").name.as_bstr(), + spec.first_reference().expect("set"), "refs/heads/main", "it sets the reference name even if it is implied" );