Moved util from game module to crate level to add functions more global, outside of game.

Removed all `as` conversions lints and fixed code using them.

src/game.rs

@@ -18,7 +18,6 @@
 pub mod bytes;
 pub mod card;
 pub mod deck;
-mod util;
 pub mod validation;
 
 // Exports

src/game/card/digimon.rs

@@ -1,13 +1,15 @@
 #![doc(include = "digimon.md")]
 
 // Imports
-use crate::game::{
-	card::property::{self, ArrowColor, CrossMoveEffect, Effect, EffectCondition, Level, Move, Speciality},
+use crate::{
+	game::{
+		card::property::{self, ArrowColor, CrossMoveEffect, Effect, EffectCondition, Level, Move, Speciality},
+		Bytes,
+	},
 	util::{
 		array_split, array_split_mut,
 		null_ascii_string::{self, NullAsciiString},
 	},
-	Bytes,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/card/digivolve.rs

@@ -1,12 +1,12 @@
 #![doc(include = "digivolve.md")]
 
 // Imports
-use crate::game::{
+use crate::{
+	game::Bytes,
 	util::{
 		array_split, array_split_mut,
 		null_ascii_string::{self, NullAsciiString},
 	},
-	Bytes,
 };
 
 /// A digivolve card

src/game/card/item.rs

@@ -1,13 +1,15 @@
 #![doc(include = "item.md")]
 
 // Imports
-use crate::game::{
-	card::property::{self, ArrowColor, Effect, EffectCondition},
+use crate::{
+	game::{
+		card::property::{self, ArrowColor, Effect, EffectCondition},
+		Bytes,
+	},
 	util::{
 		array_split, array_split_mut,
 		null_ascii_string::{self, NullAsciiString},
 	},
-	Bytes,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/card/property/effect.rs

@@ -1,10 +1,12 @@
 #![doc(include = "effect.md")]
 
 // Imports
-use crate::game::{
-	card::property::{self, AttackType, DigimonProperty, EffectOperation, PlayerType, Slot},
+use crate::{
+	game::{
+		card::property::{self, AttackType, DigimonProperty, EffectOperation, PlayerType, Slot},
+		Bytes,
+	},
 	util::{array_split, array_split_mut},
-	Bytes,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/card/property/effect_condition.rs

@@ -1,10 +1,12 @@
 #![doc(include = "effect_condition.md")]
 
 // Imports
-use crate::game::{
-	card::property::{self, DigimonProperty, EffectConditionOperation},
+use crate::{
+	game::{
+		card::property::{self, DigimonProperty, EffectConditionOperation},
+		Bytes,
+	},
 	util::{array_split, array_split_mut},
-	Bytes,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/card/property/moves.rs

@@ -5,12 +5,12 @@
 mod test;
 
 // Imports
-use crate::game::{
+use crate::{
+	game::{Bytes, Validatable, Validation},
 	util::{
 		array_split, array_split_mut,
 		null_ascii_string::{self, NullAsciiString},
 	},
-	Bytes, Validatable, Validation,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/card/table.rs

@@ -8,13 +8,16 @@ use crate::{
 			property::{self, CardType},
 			Digimon, Digivolve, Item,
 		},
-		util::array_split_mut,
 		Bytes,
 	},
 	io::{address::Data, GameFile},
+	util::array_split_mut,
 };
 use byteorder::{ByteOrder, LittleEndian};
-use std::io::{Read, Seek, Write};
+use std::{
+	convert::TryInto,
+	io::{Read, Seek, Write},
+};
 
 /// The table storing all cards
 #[derive(Debug)]
@@ -291,9 +294,9 @@ impl Table {
 		}
 
 		// Then check the number of each card
-		let digimon_cards = LittleEndian::read_u16(&header_bytes[0x4..0x6]) as usize;
-		let item_cards = header_bytes[0x6] as usize;
-		let digivolve_cards = header_bytes[0x7] as usize;
+		let digimon_cards: usize = LittleEndian::read_u16(&header_bytes[0x4..0x6]).into();
+		let item_cards: usize = header_bytes[0x6].into();
+		let digivolve_cards: usize = header_bytes[0x7].into();
 		log::debug!("[Table Header] Found {} digimon cards", digimon_cards);
 		log::debug!("[Table Header] Found {} item cards", item_cards);
 		log::debug!("[Table Header] Found {} digivolve cards", digivolve_cards);
@@ -409,13 +412,15 @@ impl Table {
 			LittleEndian::write_u32(bytes.magic, Self::HEADER_MAGIC);
 
 			// Write card lens
-			use std::convert::TryInto;
 			log::debug!("[Table Header] Writing {} digimon cards", self.digimons.len());
 			log::debug!("[Table Header] Writing {} item cards", self.items.len());
 			log::debug!("[Table Header] Writing {} digivolve cards", self.digivolves.len());
-			LittleEndian::write_u16(bytes.digimons_len, self.digimons.len().try_into().expect("Too many digimons"));
-			*bytes.items_len = self.items.len().try_into().expect("Too many items");
-			*bytes.digivolves_len = self.digivolves.len().try_into().expect("Too many digivolves");
+			LittleEndian::write_u16(
+				bytes.digimons_len,
+				self.digimons.len().try_into().expect("Number of digimon cards exceeded `u16`"),
+			);
+			*bytes.items_len = self.items.len().try_into().expect("Number of item cards exceeded `u8`");
+			*bytes.digivolves_len = self.digivolves.len().try_into().expect("Number of digivolve cards exceeded `u8`");
 		}
 
 		file.write_all(&header_bytes).map_err(SerializeError::WriteHeader)?;
@@ -435,7 +440,7 @@ impl Table {
 			);
 
 			// Write the header
-			LittleEndian::write_u16(bytes.header_id, cur_id as u16);
+			LittleEndian::write_u16(bytes.header_id, cur_id.try_into().expect("Card ID exceeded `u16`"));
 			CardType::Digimon.to_bytes(bytes.header_type)?;
 
 			// Write the digimon
@@ -464,7 +469,7 @@ impl Table {
 			);
 
 			// Write the header
-			LittleEndian::write_u16(bytes.header_id, cur_id as u16);
+			LittleEndian::write_u16(bytes.header_id, cur_id.try_into().expect("Card ID exceeded `u16`"));
 			CardType::Item.to_bytes(bytes.header_type)?;
 
 			// Write the item
@@ -492,7 +497,7 @@ impl Table {
 			);
 
 			// Write the header
-			LittleEndian::write_u16(bytes.header_id, cur_id as u16);
+			LittleEndian::write_u16(bytes.header_id, cur_id.try_into().expect("Card ID exceeded `u16`"));
 			CardType::Digivolve.to_bytes(bytes.header_type)?;
 
 			// Write the digivolve

src/game/deck/deck.rs

@@ -1,12 +1,12 @@
 //! Decks
 
 // Imports
-use crate::game::{
+use crate::{
+	game::Bytes,
 	util::{
 		array_split, array_split_mut,
 		null_ascii_string::{self, NullAsciiString},
 	},
-	Bytes,
 };
 use byteorder::{ByteOrder, LittleEndian};
 

src/game/util.rs

@@ -1,14 +0,0 @@
-//! Utility macros and functions
-//!
-//! This modules is used for miscellaneous macros, functions that have
-//! not been moved to a more permanent location.
-//!
-//! All items in this module will eventually be deprecated and moved
-//! somewhere else, but this change might take some time.
-
-// Modules
-pub mod array_split;
-pub mod null_ascii_string;
-
-// Exports
-pub use array_split::{array_split, array_split_mut};

src/io/address/data.rs

@@ -1,7 +1,10 @@
 //! File data-only addresses
 
 // Imports
-use crate::io::address::Real;
+use crate::{
+	io::address::Real,
+	util::{abs_diff, signed_offset},
+};
 
 /// A type for defining addresses on the data parts of `.bin` file.
 ///
@@ -49,11 +52,7 @@ impl std::ops::Add<i64> for Data {
 	type Output = Self;
 
 	fn add(self, offset: i64) -> Self {
-		if offset > 0 {
-			self + (offset as u64)
-		} else {
-			self - (-offset as u64)
-		}
+		Self::from(signed_offset(self.0, offset))
 	}
 }
 
@@ -101,7 +100,7 @@ impl std::ops::Sub<Data> for Data {
 	type Output = i64;
 
 	fn sub(self, address: Self) -> i64 {
-		self.0 as i64 - address.0 as i64
+		abs_diff(self.0, address.0)
 	}
 }
 

src/io/address/real.rs

@@ -1,5 +1,8 @@
 //! File real addresses
 
+// Imports
+use crate::util::{abs_diff, signed_offset};
+
 /// A type for defining addresses on the `.bin` file.
 ///
 /// All real addresses will depict the actual position
@@ -68,7 +71,7 @@ impl std::ops::Add<i64> for Real {
 	type Output = Self;
 
 	fn add(self, offset: i64) -> Self {
-		Self::from((u64::from(self) as i64 + offset) as u64)
+		Self::from(signed_offset(self.into(), offset))
 	}
 }
 
@@ -84,7 +87,7 @@ impl std::ops::Sub<Real> for Real {
 	type Output = i64;
 
 	fn sub(self, address: Self) -> i64 {
-		u64::from(self) as i64 - u64::from(address) as i64
+		abs_diff(u64::from(self), u64::from(address))
 	}
 }
 

src/io/game_file.rs

@@ -4,7 +4,10 @@
 
 // Imports
 use crate::io::address::{Data as DataAddress, Real as RealAddress, RealToDataError};
-use std::io::{Read, Seek, Write};
+use std::{
+	convert::TryInto,
+	io::{Read, Seek, Write},
+};
 
 /// A type that abstracts over a the game reader.
 ///
@@ -76,7 +79,9 @@ impl<R: Read + Write + Seek> Read for GameFile<R> {
 			if cur_real_address == data_section_end {
 				// Seek ahead by skipping the footer and next header
 				self.reader.seek(std::io::SeekFrom::Current(
-					(RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE) as i64,
+					(RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE)
+						.try_into()
+						.expect("Sector offset didn't fit into `u64`"),
 				))?;
 
 				// And restart this loop
@@ -95,7 +100,10 @@ impl<R: Read + Write + Seek> Read for GameFile<R> {
 			);
 
 			// Check how many bytes we can read
-			let mut bytes_to_read = (data_section_end - cur_real_address) as usize;
+			// Note: Cannot overflow, max is `2048`, so an error means `cur_real_address` was past the data section end
+			let mut bytes_to_read = (data_section_end - cur_real_address)
+				.try_into()
+				.expect("Current address is past end of data");
 
 			// If we were to read more bytes than the buffer has, read less
 			if bytes_to_read > buf.len() {
@@ -144,7 +152,9 @@ impl<R: Read + Write + Seek> Write for GameFile<R> {
 			if cur_real_address == data_section_end {
 				// Seek ahead by skipping the footer and next header
 				self.reader.seek(std::io::SeekFrom::Current(
-					(RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE) as i64,
+					(RealAddress::FOOTER_BYTE_SIZE + RealAddress::HEADER_BYTE_SIZE)
+						.try_into()
+						.expect("Sector offset didn't fit into `u64`"),
 				))?;
 
 				// And restart this loop
@@ -162,8 +172,10 @@ impl<R: Read + Write + Seek> Write for GameFile<R> {
 				cur_real_address.offset()
 			);
 
-			// Check how many bytes we can write
-			let mut bytes_to_write = (data_section_end - cur_real_address) as usize;
+			// Check how many bytes we can write, up to the buffer's len
+			let mut bytes_to_write = (data_section_end - cur_real_address)
+				.try_into()
+				.expect("Current address is past end of data");
 
 			// If we were to write more bytes than the buffer has, write less
 			if bytes_to_write > buf.len() {

src/lib.rs

@@ -46,17 +46,16 @@
 #![allow(clippy::missing_inline_in_public_items)]
 // We prefer tail returns where possible, as they help with code readability in most cases.
 #![allow(clippy::implicit_return)]
-// Very useful for arguments such as `arg: impl Into<U>`, then used
-// with `let arg = arg.into()`. As well as just going from `Option<T>`
-// to `T` without needing to change their names.
+// We're fine with shadowing, as long as the variable is used for the same purpose.
+// Hence why `clippy::shadow_unrelated` isn't allowed.
 #![allow(clippy::shadow_reuse, clippy::shadow_same)]
-// We use `.expect("...")` when we either know we cannot panic or it
-// is the safest alternative, as proceeding would corrupt the program state.
+// We use `.expect("...")` when we either know it cannot panic or if
+// it is the safest alternative.
 #![allow(clippy::expect_used)]
-// Like-wise with `.expect()`, we use `unreachable!` when we know a branch
+// Like-wise with `.expect("...")`, we use `unreachable!` / `todo!` when we know a branch
 // if unreachable, and if it ever does get reached, panicking would be the
 // safest option
-#![allow(clippy::unreachable)]
+#![allow(clippy::unreachable, clippy::todo)]
 // We find it more important to be able to copy paste literals such as `0xabcd1234` than
 // being able to read them, which does not provide many benefits
 #![allow(clippy::unreadable_literal, clippy::unseparated_literal_suffix)]
@@ -64,7 +63,8 @@
 #![allow(clippy::multiple_inherent_impl)]
 // Many operations we need to repeat, and to keep symmetry
 #![allow(clippy::identity_op)]
-// We only introduce items before their first usage, which sometimes is half-way through the code
+// We only introduce items before their first usage, which sometimes is half-way through the code.
+// We make sure that we only use the item after introduced, however.
 #![allow(clippy::items_after_statements)]
 // Useful for when they either change a lot with new variants / data,
 // or for symmetry purposes
@@ -73,13 +73,12 @@
 // will have it's own error type ideally, so any errors are explicit
 // by the type, without needing a section for them
 #![allow(clippy::missing_errors_doc)]
-// Incomplete code should be tagged as `todo`. In future versions of the library,
-// this lint may be removed, as incomplete code should not lie on a master branch.
-#![allow(clippy::todo)]
 // Although we generally try to avoid this, this can happen due to our module organization.
 // In the future, this lint should be removed globally and only enabled for modules which
 // actually require the use of it.
 #![allow(clippy::module_inception, clippy::module_name_repetitions)]
+// Banning arithmetic is too strict for this project
+#![allow(clippy::integer_arithmetic)]
 // False positives:
 // TODO: Remove them in the future once they are no longer triggered.
 // We only slice arrays, which are verified at compile time. This
@@ -87,19 +86,11 @@
 #![allow(clippy::indexing_slicing)]
 // We don't have any `unsafe` impl for types that derive `Deserialize`.
 #![allow(clippy::unsafe_derive_deserialize)]
-// Banning arithmetic is too strict for this project
-#![allow(clippy::integer_arithmetic)]
-// TODO: Remove once fixed
-#![allow(
-	clippy::as_conversions,
-	clippy::cast_possible_wrap,
-	clippy::cast_sign_loss,
-	clippy::cast_possible_truncation
-)]
 
 // Modules
 pub mod game;
 pub mod io;
+mod util;
 
 // Exports
 pub use game::{Bytes, CardTable, Deck, DeckTable, Digimon, Digivolve, Item, Validatable, Validation};

src/util.rs

@@ -0,0 +1,56 @@
+//! Utility macros and functions
+//!
+//! This modules is used for miscellaneous macros, functions that have
+//! not been moved to a more permanent location.
+//!
+//! All items in this module will eventually be deprecated and moved
+//! somewhere else, but this change might take some time.
+
+// Modules
+pub mod array_split;
+pub mod null_ascii_string;
+
+// Exports
+pub use array_split::{array_split, array_split_mut};
+
+
+/// Returns the absolute different between `a` and `b`, `a - b` as a `i64`.
+///
+/// If the result would not fit into a `i64`, a panic occurs.
+#[allow(clippy::as_conversions)] // We check every operation
+#[allow(clippy::panic)] // Rust panics on failed arithmetic operations by default
+pub fn abs_diff(a: u64, b: u64) -> i64 {
+	let diff = if a > b { a - b } else { b - a };
+
+	if diff > i64::MAX as u64 {
+		panic!("Overflow when computing signed distance between `u64`");
+	}
+
+	#[allow(clippy::cast_possible_wrap)] // We've verified, `diff` is less than `i64::MAX`
+	if a > b {
+		diff as i64
+	} else {
+		-(diff as i64)
+	}
+}
+
+/// Adds a `i64` to a `u64`, performing `a + b`.
+///
+/// This function panics if the result would overflow or underflow
+#[allow(clippy::as_conversions)] // We check every operation
+#[allow(clippy::panic)] // Rust panics on failed arithmetic operations by default
+#[allow(clippy::cast_sign_loss)] // We've verify it's positive
+pub fn signed_offset(a: u64, b: i64) -> u64 {
+	// If `b` is positive, check for overflows. Else check for underflows
+	if b > 0 {
+		// Note: Cast is safe, as a positive `i64` fits into a `u64`.
+		a.checked_add(b as u64).expect("Overflow evaluating `u64 + i64`")
+	} else {
+		// Note: On `i64::MIN`, `-b` would overflow
+		if b == i64::MIN || a < (-b) as u64 {
+			panic!("Underflow evaluating `u64 + i64`");
+		} else {
+			a - ((-b) as u64)
+		}
+	}
+}