diff --git a/src/conversions.rs b/src/conversions.rs index 1aebb78..e2074e3 100644 --- a/src/conversions.rs +++ b/src/conversions.rs @@ -8,14 +8,14 @@ pub fn lat_lon_to_cellid(lat: f64, lon: f64) -> Result { let latlng = LatLng::from_degrees(lat, lon); // We don't want to entertain any invalid latitudes or longitudes - if latlng.normalized() != latlng { - Err(Error::InvalidLatLng) - } else { + if latlng.normalized() == latlng { Ok(Into::::into(latlng)) + } else { + Err(Error::InvalidLatLng) } } -pub(crate) fn extract_binary(number: u64, range: RangeInclusive) -> u64 { +pub fn extract_binary(number: u64, range: RangeInclusive) -> u64 { (number >> range.start()) & (u64::MAX >> (63 - (range.end() - range.start()))) } diff --git a/src/lib.rs b/src/lib.rs index e083cab..0531ca0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,28 +27,46 @@ pub const V0_MAX_NUMBER: u32 = 1024; /// The minimum number value for V0 pub const V0_MIN_NUMBER: u32 = 1; +// Any encoding or decoding error #[derive(Error, Debug, Eq, PartialEq)] pub enum Error { + /// The given word is not in the wordlist #[error("Word does not exist: {0}")] WordDoesNotExist(String), - #[error("Invalid version: {0}")] - InvalidVersion(u8), - #[error("Unimplemented version")] - UnimplementedVersion(Version), + /// The encoded version bit is not supported by this version of the crate + #[error("Unimplemented version: {0}")] + UnimplementedVersion(u8), + /// The number component in the address is too large #[error("Number out of range")] NumberOutOfRange(Number), - #[error("Invalid encoding")] - InvalidEncoding, + /// The string to decode is not ascii + #[error("The string given is not ascii")] + NonAscii, + /// The number of components given to decode is incorrect #[error("Wrong number of components")] WrongComponentCount, + /// No number component was found + /// + /// See [`Algorithm::from_str`] for the possible position of the number component #[error("No number component")] NoNumberComponent, + /// The empty string was given to decode #[error("Empty string given")] Empty, + /// The latitude given is not in the valid range + /// + /// This is the case when any of these are true: + /// * `lat == f64::NAN` + /// * `lon == f64::NAN` + /// * `lat < -90` + /// * `lat > 90` + /// * `lon < -180` + /// * `lon > 180` #[error("Invalid latitude or longitude")] InvalidLatLng, } +#[allow(clippy::doc_markdown)] /// An encoded this_algorithm address #[derive(Debug, Clone, PartialEq, Eq)] pub struct Address<'a> { @@ -62,14 +80,6 @@ pub enum Version { V0, } -// pub trait Alg { -// type Err; - -// fn to_cellid(&self) -> Result; - -// fn from_cellid(); -// } - impl FromStr for Address<'_> { type Err = Error; @@ -77,9 +87,15 @@ impl FromStr for Address<'_> { /// /// * `0000 WORD0 WORD1 WORD2` (regular) /// * `WORD2 WORD1 WORD0 0000` (reversed) + /// + /// # Errors + /// + /// This function will return an error if the passed data does not correctly decode. + /// + /// See [`enum@Error`] for possible errors fn from_str(s: &str) -> Result { if !s.is_ascii() { - return Err(Error::InvalidEncoding); + return Err(Error::NonAscii); } let s = s.trim().to_ascii_uppercase(); @@ -109,12 +125,19 @@ impl FromStr for Address<'_> { components.into_iter().skip(1).collect::>() }; - Address::from_components(number, other_components) + Address::from_components(number, &other_components) } } impl Address<'_> { /// Converts a latitude and longitude into an encoded address + /// + /// # Errors + /// + /// This will return [`Error::InvalidLatLng`] when the latitude or longitude are not valid + /// + /// See [`Error::InvalidLatLng`] for the valid values + pub fn from_lat_lon(lat: f64, lon: f64) -> Result { let cellid = conversions::lat_lon_to_cellid(lat, lon)?; @@ -122,11 +145,13 @@ impl Address<'_> { } /// Constructs an address from the given cellid + #[must_use] pub fn from_cellid(cellid: CellID) -> Self { v0::UnpackedCellID::from(cellid).into() } /// Constructs an address from a u64 representation of a cellid + #[must_use] pub fn from_cellid_u64(cellid_u64: u64) -> Self { v0::UnpackedCellID::from(CellID(cellid_u64)).into() } @@ -137,7 +162,7 @@ impl Address<'_> { // } // Parses an address v0 given the number and word components - fn parse_v0(number: Number, word_components: Vec<&str>) -> Result { + fn parse_v0(number: Number, word_components: &[&str]) -> Result { if !(V0_MIN_NUMBER..=V0_MAX_NUMBER).contains(&number) { return Err(Error::NumberOutOfRange(number)); } @@ -150,7 +175,9 @@ impl Address<'_> { let words = TryInto::<[&'static Word; 3]>::try_into( word_components .iter() - .map(|w| words::get_word(w).ok_or_else(|| Error::WordDoesNotExist(w.to_string()))) + .map(|w| { + words::get_word(w).ok_or_else(|| Error::WordDoesNotExist((*w).to_string())) + }) .collect::, Error>>()?, ) // This unwrap is okay because we just checked to make sure the number of word components was 3 @@ -163,41 +190,22 @@ impl Address<'_> { }) } - /// Builds an `Address` from a number and all word components - fn from_components(number: Number, other_components: Vec<&str>) -> Result { + fn from_components(number: Number, other_components: &[&str]) -> Result { match extract_version(number)? { Version::V0 => Self::parse_v0(number, other_components), } } + /// Get the address as a [`CellID`] value + #[must_use] pub fn as_cell_id(&self) -> CellID { match self.version { - Version::V0 => { - UnpackedCellID::from(self).into() - // let ten_bits = 0b1111_1111_11; - // let thirteen_bits = 0b1111_1111_1111_1; - // let mut ret = 0b0; - - // // Add words in reverse order - // for word in self.words.iter().rev() { - // ret = (ret << 13) | (word.number as u64 & thirteen_bits); - // } - - // // Add the number - // ret = (ret << 10) | (self.number as u64 & ten_bits); - - // // Add the final bit - // ret = (ret << 1) | 0b1; - - // // Shift the whole id left by the number of unused levels * 2 - // ret = ret << ((s2::cellid::MAX_LEVEL - CELLID_LEVEL as u64) * 2); - - // CellID(ret) - } + Version::V0 => UnpackedCellID::from(self).into(), } } // TODO: Test this + #[must_use] pub fn to_lat_lon(&self) -> (f64, f64) { let lat_lng = LatLng::from(self.as_cell_id()); (lat_lng.lat.deg(), lat_lng.lng.deg()) @@ -208,10 +216,10 @@ impl Address<'_> { /// /// The version number is set by the two bits 11 and 12 // TODO: impl TryFrom ? -fn extract_version(number: Number) -> Result { +const fn extract_version(number: Number) -> Result { match ((number >> 10) & 0b11) as u8 { 0 => Ok(Version::V0), - v => Err(Error::InvalidVersion(v)), + v => Err(Error::UnimplementedVersion(v)), } } @@ -299,41 +307,41 @@ mod tests { fn test_parse_v0() { // Regular case assert_eq!( - Address::parse_v0(1000, vec!["apple", "orange", "grape"]), + Address::parse_v0(1000, &["apple", "orange", "grape"]), Ok(addr![1000, apple, orange, grape]) ); // Number is on the edge assert_eq!( - Address::parse_v0(1, vec!["apple", "orange", "grape"]), + Address::parse_v0(1, &["apple", "orange", "grape"]), Ok(addr![1, apple, orange, grape]) ); assert_eq!( - Address::parse_v0(1024, vec!["apple", "orange", "grape"]), + Address::parse_v0(1024, &["apple", "orange", "grape"]), Ok(addr![1024, apple, orange, grape]) ); assert_eq!( - Address::parse_v0(V0_MAX_NUMBER, vec!["apple", "orange", "grape"]), + Address::parse_v0(V0_MAX_NUMBER, &["apple", "orange", "grape"]), Ok(addr![V0_MAX_NUMBER, apple, orange, grape]) ); assert_eq!( - Address::parse_v0(V0_MIN_NUMBER, vec!["apple", "orange", "grape"]), + Address::parse_v0(V0_MIN_NUMBER, &["apple", "orange", "grape"]), Ok(addr![V0_MIN_NUMBER, apple, orange, grape]) ); // Word not found - assert!(Address::parse_v0(1000, vec!["ASDF", "orange", "grape"]).is_err()); + assert!(Address::parse_v0(1000, &["ASDF", "orange", "grape"]).is_err()); // Too few words - assert!(Address::parse_v0(1000, vec!["apple", "orange"]).is_err()); - assert!(Address::parse_v0(1000, vec!["apple"]).is_err()); - assert!(Address::parse_v0(1000, vec![]).is_err()); + assert!(Address::parse_v0(1000, &["apple", "orange"]).is_err()); + assert!(Address::parse_v0(1000, &["apple"]).is_err()); + assert!(Address::parse_v0(1000, &[]).is_err()); // Too many words - assert!(Address::parse_v0(1000, vec!["apple", "orange", "grape", "banana"]).is_err()); + assert!(Address::parse_v0(1000, &["apple", "orange", "grape", "banana"]).is_err()); // Number too large or small - assert!(Address::parse_v0(10_000, vec!["apple", "orange", "grape"]).is_err()); - assert!(Address::parse_v0(0, vec!["apple", "orange", "grape"]).is_err()); + assert!(Address::parse_v0(10_000, &["apple", "orange", "grape"]).is_err()); + assert!(Address::parse_v0(0, &["apple", "orange", "grape"]).is_err()); } } diff --git a/src/v0.rs b/src/v0.rs index 14b81fb..a2136ce 100644 --- a/src/v0.rs +++ b/src/v0.rs @@ -36,7 +36,7 @@ impl From for Address<'_> { let word2 = words::NUMBER_TO_WORDS[unpacked_cell_id.word2_bits as usize][0]; Self { - number: unpacked_cell_id.number_bits as u32, + number: u32::from(unpacked_cell_id.number_bits), words: [word0, word1, word2], version: Version::V0, } @@ -47,36 +47,33 @@ impl From<&Address<'_>> for UnpackedCellID { fn from(value: &Address) -> Self { Self { number_bits: value.number as u16, - word0_bits: value.words[0].number as u16, - word1_bits: value.words[1].number as u16, - word2_bits: value.words[2].number as u16, + word0_bits: value.words[0].number, + word1_bits: value.words[1].number, + word2_bits: value.words[2].number, } } } +#[allow(clippy::use_self)] impl From for u64 { fn from(value: UnpackedCellID) -> Self { let ten_bits = 0b1111_1111_11; - let thirteen_bits = 0b1111_1111_1111_1; + let thirteen_bits = 0b1_1111_1111_1111; let mut ret = 0b0; // Add words in reverse order - ret = (ret << 13) | (value.word2_bits as u64 & thirteen_bits); - ret = (ret << 13) | (value.word1_bits as u64 & thirteen_bits); - ret = (ret << 13) | (value.word0_bits as u64 & thirteen_bits); - - // for word in self.words.iter().rev() { - // ret = (ret << 13) | (word.number as u64 & thirteen_bits); - // } + ret = (ret << 13) | (u64::from(value.word2_bits) & thirteen_bits); + ret = (ret << 13) | (u64::from(value.word1_bits) & thirteen_bits); + ret = (ret << 13) | (u64::from(value.word0_bits) & thirteen_bits); // Add the number - ret = (ret << 10) | (value.number_bits as u64 & ten_bits); + ret = (ret << 10) | (u64::from(value.number_bits) & ten_bits); // Add the final bit ret = (ret << 1) | 0b1; // Shift the whole id left by the number of unused levels * 2 - ret = ret << ((s2::cellid::MAX_LEVEL - CELLID_LEVEL as u64) * 2); + ret <<= (s2::cellid::MAX_LEVEL - CELLID_LEVEL) * 2; ret } @@ -84,7 +81,7 @@ impl From for u64 { impl From for CellID { fn from(value: UnpackedCellID) -> Self { - CellID(value.into()) + Self(value.into()) } } diff --git a/tests/algorithm.rs b/tests/algorithm.rs index bf0108a..5b242f2 100644 --- a/tests/algorithm.rs +++ b/tests/algorithm.rs @@ -1,11 +1,24 @@ use common::test_events; use s2::cellid::CellID; +use this_algorithm::Error; use this_algorithm::{Address, CELLID_LEVEL}; #[macro_use] mod common; use common::CELLID_LEVEL_23_BITMASK; use common::CELLID_LEVEL_23_END_BIT; +#[test] +fn test_invalid_lat_lon() { + assert_eq!( + Address::from_lat_lon(1.0_f64, 400.0_f64), + Err(Error::InvalidLatLng) + ); + assert_eq!( + Address::from_lat_lon(1.0_f64, f64::NAN), + Err(Error::InvalidLatLng) + ); +} + #[test] fn test_cellid_translation() { for (idx, entry) in test_events().iter().enumerate() { @@ -44,7 +57,8 @@ fn test_encoding() { let addr = Address::from_lat_lon(entry.lat, entry.lon).unwrap(); eprintln!("({}, {}) => {addr}", entry.lat, entry.lon); } - assert!(false); + // TODO: + // assert!(false); } #[test] diff --git a/words/build.rs b/words/build.rs index 1126f68..3f817fc 100644 --- a/words/build.rs +++ b/words/build.rs @@ -59,7 +59,7 @@ fn write_word_map(mut file: impl Write, words: &[Word]) { writeln!( &mut file, r#"/// Mapping from all caps `&str` to `&'static Word` -pub static WORD_MAP: phf::Map<&'static str, &'static Word> = {};"#, +pub static WORD_MAP: phf::Map<&'static str, &Word> = {};"#, word_map.build() ) .unwrap(); @@ -76,7 +76,7 @@ fn write_number_to_words(mut file: impl Write, words: &[Word]) { &mut file, // "pub static NUMBER_TO_WORDS: &[&[usize]] = &[" r#"/// Mapping from each number to its associated word -pub static NUMBER_TO_WORDS: &[&[&'static Word]] = &["# +pub static NUMBER_TO_WORDS: &[&[&Word]] = &["# ) .unwrap();