Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
- fix doc tests and enable them
- move all assertions onto integration test level and out of the module
- module documentation with credits section.
- use rustdoc feature to link other methods
- renamed functions to be more obvious
  • Loading branch information
Byron committed Feb 15, 2024
1 parent 3a5a229 commit dbc5f0f
Show file tree
Hide file tree
Showing 9 changed files with 132 additions and 127 deletions.
8 changes: 4 additions & 4 deletions gix-actor/src/signature/decode.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
pub(crate) mod function {
use bstr::ByteSlice;
use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
use gix_utils::btoi::btoi;
use gix_utils::btoi::to_signed;
use winnow::{
combinator::{alt, separated_pair, terminated},
error::{AddContext, ParserError, StrContext},
Expand All @@ -23,18 +23,18 @@ pub(crate) mod function {
b" ",
(
terminated(take_until(0.., SPACE), take(1usize))
.verify_map(|v| btoi::<SecondsSinceUnixEpoch>(v).ok())
.verify_map(|v| to_signed::<SecondsSinceUnixEpoch>(v).ok())
.context(StrContext::Expected("<timestamp>".into())),
alt((
take_while(1.., b'-').map(|_| Sign::Minus),
take_while(1.., b'+').map(|_| Sign::Plus),
))
.context(StrContext::Expected("+|-".into())),
take_while(2, AsChar::is_dec_digit)
.verify_map(|v| btoi::<OffsetInSeconds>(v).ok())
.verify_map(|v| to_signed::<OffsetInSeconds>(v).ok())
.context(StrContext::Expected("HH".into())),
take_while(1..=2, AsChar::is_dec_digit)
.verify_map(|v| btoi::<OffsetInSeconds>(v).ok())
.verify_map(|v| to_signed::<OffsetInSeconds>(v).ok())
.context(StrContext::Expected("MM".into())),
)
.map(|(time, sign, hours, minutes)| {
Expand Down
4 changes: 2 additions & 2 deletions gix-index/src/extension/tree/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ fn one_recursive(data: &[u8], hash_len: usize) -> Option<(Tree, &[u8])> {
let (path, data) = split_at_byte_exclusive(data, 0)?;

let (entry_count, data) = split_at_byte_exclusive(data, b' ')?;
let num_entries: i32 = gix_utils::btoi::btoi(entry_count).ok()?;
let num_entries: i32 = gix_utils::btoi::to_signed(entry_count).ok()?;

let (subtree_count, data) = split_at_byte_exclusive(data, b'\n')?;
let subtree_count: usize = gix_utils::btoi::btou(subtree_count).ok()?;
let subtree_count: usize = gix_utils::btoi::to_unsigned(subtree_count).ok()?;

let (id, mut data) = if num_entries >= 0 {
let (hash, data) = split_at_pos(data, hash_len)?;
Expand Down
2 changes: 1 addition & 1 deletion gix-object/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ pub mod decode {
message: "Did not find 0 byte in header",
})?;
let size_bytes = &input[kind_end + 1..size_end];
let size = gix_utils::btoi::btoi(size_bytes).map_err(|source| ParseIntegerError {
let size = gix_utils::btoi::to_signed(size_bytes).map_err(|source| ParseIntegerError {
source,
message: "Object size in header could not be parsed",
number: size_bytes.into(),
Expand Down
2 changes: 1 addition & 1 deletion gix-protocol/src/remote_progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'a> RemoteProgress<'a> {

fn parse_number(i: &mut &[u8]) -> PResult<usize, ()> {
take_till(0.., |c: u8| !c.is_ascii_digit())
.try_map(gix_utils::btoi::btoi)
.try_map(gix_utils::btoi::to_signed)
.parse_next(i)
}

Expand Down
4 changes: 2 additions & 2 deletions gix-quote/src/ansi_c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ pub fn undo(input: &BStr) -> Result<(Cow<'_, BStr>, usize), undo::Error> {
})?
.read_exact(&mut buf[1..])
.expect("impossible to fail as numbers match");
let byte =
gix_utils::btoi::btou_radix(&buf, 8).map_err(|e| undo::Error::new(e, original))?;
let byte = gix_utils::btoi::to_unsigned_with_radix(&buf, 8)
.map_err(|e| undo::Error::new(e, original))?;
out.push(byte);
input = &input[2..];
consumed += 2;
Expand Down
2 changes: 1 addition & 1 deletion gix-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ rust-version = "1.65"
include = ["src/**/*", "LICENSE-*"]

[lib]
doctest = false
doctest = true

[features]
bstr = ["dep:bstr"]
Expand Down
203 changes: 87 additions & 116 deletions gix-utils/src/btoi.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
// ported from https://github.com/niklasf/rust-btoi version 0.4.3
// see https://github.com/Byron/gitoxide/issues/729#issuecomment-1941515655
/// A module with utilities to turn byte slices with decimal numbers back into their
/// binary representation.
///
/// ### Credits
///
/// This module was ported from <https://github.com/niklasf/rust-btoi> version 0.4.3
/// see <https://github.com/Byron/gitoxide/issues/729> for how it came to be in order
/// to save 2.2 seconds of per-core compile time by not compiling the `num-traits` crate
/// anymore.
///
/// Licensed with compatible licenses [MIT] and [Apache]
///
/// [MIT]: https://github.com/niklasf/rust-btoi/blob/master/LICENSE-MIT
/// [Apache]: https://github.com/niklasf/rust-btoi/blob/master/LICENSE-APACHE

/// An error that can occur when parsing an integer.
///
Expand Down Expand Up @@ -42,52 +54,6 @@ impl std::error::Error for ParseIntegerError {
self.desc()
}
}
/// minimal subset of traits used by btoi_radix and btou_radix
pub trait MinNumTraits: Sized + Copy {
///
fn from_u32(n: u32) -> Option<Self>;
///
fn zero() -> Self;
///
fn checked_mul(self, v: Self) -> Option<Self>;
///
fn checked_add(self, v: Self) -> Option<Self>;
///
fn checked_sub(self, v: Self) -> Option<Self>;
}

macro_rules! min_num_traits {
($t : ty, from_u32 => $from_u32 : expr) => {
impl MinNumTraits for $t {
fn from_u32(n: u32) -> Option<$t> {
#[allow(clippy::redundant_closure_call)]
$from_u32(n)
}

fn zero() -> Self {
0
}

fn checked_mul(self, v: $t) -> Option<$t> {
<$t>::checked_mul(self, v)
}

fn checked_add(self, v: $t) -> Option<$t> {
<$t>::checked_add(self, v)
}

fn checked_sub(self, v: $t) -> Option<$t> {
<$t>::checked_sub(self, v)
}
}
};
}

min_num_traits!(i32, from_u32 => |n: u32| n.try_into().ok());
min_num_traits!(i64, from_u32 => |n: u32| Some(n.into()));
min_num_traits!(u64, from_u32 => |n: u32| Some(n.into()));
min_num_traits!(u8, from_u32 => |n: u32| n.try_into().ok());
min_num_traits!(usize, from_u32 => |n: u32| n.try_into().ok());

/// Converts a byte slice to an integer. Signs are not allowed.
///
Expand All @@ -107,23 +73,14 @@ min_num_traits!(usize, from_u32 => |n: u32| n.try_into().ok());
/// # Examples
///
/// ```
/// # use btoi::btou;
/// assert_eq!(Ok(12345), btou(b"12345"));
/// assert!(btou::<u8>(b"+1").is_err()); // only btoi allows signs
/// assert!(btou::<u8>(b"256").is_err()); // overflow
/// # use gix_utils::btoi::to_unsigned;
/// assert_eq!(Ok(12345), to_unsigned(b"12345"));
/// assert!(to_unsigned::<u8>(b"+1").is_err()); // only btoi allows signs
/// assert!(to_unsigned::<u8>(b"256").is_err()); // overflow
/// ```
///
/// [`ParseIntegerError`]: struct.ParseIntegerError.html
#[track_caller]
pub fn btou<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
btou_radix(bytes, 10)
}

#[test]
fn btou_assert() {
assert_eq!(Ok(12345), btou(b"12345"));
assert!(btou::<u8>(b"+1").is_err()); // only btoi allows signs
assert!(btou::<u8>(b"256").is_err()); // overflow
pub fn to_unsigned<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
to_unsigned_with_radix(bytes, 10)
}

/// Converts a byte slice in a given base to an integer. Signs are not allowed.
Expand All @@ -145,13 +102,11 @@ fn btou_assert() {
/// # Examples
///
/// ```
/// # use btoi::btou_radix;
/// assert_eq!(Ok(255), btou_radix(b"ff", 16));
/// assert_eq!(Ok(42), btou_radix(b"101010", 2));
/// # use gix_utils::btoi::to_unsigned_with_radix;
/// assert_eq!(Ok(255), to_unsigned_with_radix(b"ff", 16));
/// assert_eq!(Ok(42), to_unsigned_with_radix(b"101010", 2));
/// ```
///
/// [`ParseIntegerError`]: struct.ParseIntegerError.html
pub fn btou_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
pub fn to_unsigned_with_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
assert!(
(2..=36).contains(&radix),
"radix must lie in the range 2..=36, found {radix}"
Expand Down Expand Up @@ -195,15 +150,9 @@ pub fn btou_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseI
Ok(result)
}

#[test]
fn btou_radix_assert() {
assert_eq!(Ok(255), btou_radix(b"ff", 16));
assert_eq!(Ok(42), btou_radix(b"101010", 2));
}

/// Converts a byte slice to an integer.
///
/// Like [`btou`], but numbers may optionally start with a sign (`-` or `+`).
/// Like [`to_unsigned`], but numbers may optionally start with a sign (`-` or `+`).
///
/// # Errors
///
Expand All @@ -222,38 +171,23 @@ fn btou_radix_assert() {
/// # Examples
///
/// ```
/// # use btoi::btoi;
/// assert_eq!(Ok(123), btoi(b"123"));
/// assert_eq!(Ok(123), btoi(b"+123"));
/// assert_eq!(Ok(-123), btoi(b"-123"));
/// # use gix_utils::btoi::to_signed;
/// assert_eq!(Ok(123), to_signed(b"123"));
/// assert_eq!(Ok(123), to_signed(b"+123"));
/// assert_eq!(Ok(-123), to_signed(b"-123"));
///
/// assert!(btoi::<i16>(b"123456789").is_err()); // overflow
/// assert!(btoi::<u32>(b"-1").is_err()); // underflow
/// assert!(to_signed::<u8>(b"123456789").is_err()); // overflow
/// assert!(to_signed::<u8>(b"-1").is_err()); // underflow
///
/// assert!(btoi::<i32>(b" 42").is_err()); // leading space
/// assert!(to_signed::<i32>(b" 42").is_err()); // leading space
/// ```
///
/// [`btou`]: fn.btou.html
/// [`ParseIntegerError`]: struct.ParseIntegerError.html
pub fn btoi<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
btoi_radix(bytes, 10)
}

#[test]
fn btoi_assert() {
assert_eq!(Ok(123), btoi(b"123"));
assert_eq!(Ok(123), btoi(b"+123"));
assert_eq!(Ok(-123), btoi(b"-123"));

assert!(btoi::<u8>(b"123456789").is_err()); // overflow
assert!(btoi::<u64>(b"-1").is_err()); // underflow

assert!(btoi::<i32>(b" 42").is_err()); // leading space
pub fn to_signed<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
to_signed_with_radix(bytes, 10)
}

/// Converts a byte slice in a given base to an integer.
///
/// Like [`btou_radix`], but numbers may optionally start with a sign
/// Like [`to_unsigned_with_radix`], but numbers may optionally start with a sign
/// (`-` or `+`).
///
/// # Errors
Expand All @@ -275,15 +209,12 @@ fn btoi_assert() {
/// # Examples
///
/// ```
/// # use btoi::btoi_radix;
/// assert_eq!(Ok(10), btoi_radix(b"a", 16));
/// assert_eq!(Ok(10), btoi_radix(b"+a", 16));
/// assert_eq!(Ok(-42), btoi_radix(b"-101010", 2));
/// # use gix_utils::btoi::to_signed_with_radix;
/// assert_eq!(Ok(10), to_signed_with_radix(b"a", 16));
/// assert_eq!(Ok(10), to_signed_with_radix(b"+a", 16));
/// assert_eq!(Ok(-42), to_signed_with_radix(b"-101010", 2));
/// ```
///
/// [`btou_radix`]: fn.btou_radix.html
/// [`ParseIntegerError`]: struct.ParseIntegerError.html
fn btoi_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
pub fn to_signed_with_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
assert!(
(2..=36).contains(&radix),
"radix must lie in the range 2..=36, found {radix}"
Expand All @@ -296,9 +227,9 @@ fn btoi_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseInteg
}

let digits = match bytes[0] {
b'+' => return btou_radix(&bytes[1..], radix),
b'+' => return to_unsigned_with_radix(&bytes[1..], radix),
b'-' => &bytes[1..],
_ => return btou_radix(bytes, radix),
_ => return to_unsigned_with_radix(bytes, radix),
};

if digits.is_empty() {
Expand Down Expand Up @@ -337,9 +268,49 @@ fn btoi_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseInteg
Ok(result)
}

#[test]
fn btoi_radix_assert() {
assert_eq!(Ok(10), btoi_radix(b"a", 16));
assert_eq!(Ok(10), btoi_radix(b"+a", 16));
assert_eq!(Ok(-42), btoi_radix(b"-101010", 2));
/// minimal subset of traits used by btoi_radix and btou_radix
pub trait MinNumTraits: Sized + Copy {
///
fn from_u32(n: u32) -> Option<Self>;
///
fn zero() -> Self;
///
fn checked_mul(self, v: Self) -> Option<Self>;
///
fn checked_add(self, v: Self) -> Option<Self>;
///
fn checked_sub(self, v: Self) -> Option<Self>;
}

macro_rules! min_num_traits {
($t : ty, from_u32 => $from_u32 : expr) => {
impl MinNumTraits for $t {
fn from_u32(n: u32) -> Option<$t> {
#[allow(clippy::redundant_closure_call)]
$from_u32(n)
}

fn zero() -> Self {
0
}

fn checked_mul(self, v: $t) -> Option<$t> {
<$t>::checked_mul(self, v)
}

fn checked_add(self, v: $t) -> Option<$t> {
<$t>::checked_add(self, v)
}

fn checked_sub(self, v: $t) -> Option<$t> {
<$t>::checked_sub(self, v)
}
}
};
}

min_num_traits!(i32, from_u32 => |n: u32| n.try_into().ok());
min_num_traits!(i64, from_u32 => |n: u32| Some(n.into()));
min_num_traits!(u64, from_u32 => |n: u32| Some(n.into()));
min_num_traits!(u8, from_u32 => |n: u32| n.try_into().ok());
min_num_traits!(usize, from_u32 => |n: u32| n.try_into().ok());
33 changes: 33 additions & 0 deletions gix-utils/tests/btoi/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use gix_utils::btoi::{to_signed, to_signed_with_radix, to_unsigned, to_unsigned_with_radix};

#[test]
fn binary_to_unsigned() {
assert_eq!(Ok(12345), to_unsigned(b"12345"));
assert!(to_unsigned::<u8>(b"+1").is_err()); // only btoi allows signs
assert!(to_unsigned::<u8>(b"256").is_err()); // overflow
}

#[test]
fn binary_to_unsigned_radix() {
assert_eq!(Ok(255), to_unsigned_with_radix(b"ff", 16));
assert_eq!(Ok(42), to_unsigned_with_radix(b"101010", 2));
}

#[test]
fn binary_to_integer_radix() {
assert_eq!(Ok(10), to_signed_with_radix(b"a", 16));
assert_eq!(Ok(10), to_signed_with_radix(b"+a", 16));
assert_eq!(Ok(-42), to_signed_with_radix(b"-101010", 2));
}

#[test]
fn binary_to_integer() {
assert_eq!(Ok(123), to_signed(b"123"));
assert_eq!(Ok(123), to_signed(b"+123"));
assert_eq!(Ok(-123), to_signed(b"-123"));

assert!(to_signed::<u8>(b"123456789").is_err()); // overflow
assert!(to_signed::<u64>(b"-1").is_err()); // underflow

assert!(to_signed::<i32>(b" 42").is_err()); // leading space
}
1 change: 1 addition & 0 deletions gix-utils/tests/utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod backoff;
mod btoi;
mod buffers;
mod str;

0 comments on commit dbc5f0f

Please sign in to comment.