From e6854b1e30e6237f99b3e9dd45fa7dd4bcc4ac74 Mon Sep 17 00:00:00 2001
From: raskad <32105367+raskad@users.noreply.github.com>
Date: Fri, 29 Sep 2023 21:11:40 +0200
Subject: [PATCH] Apply review

---
 boa_engine/src/property/mod.rs       | 31 +++++++++++++---------------
 boa_engine/src/property/nonmaxu32.rs |  4 ++--
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/boa_engine/src/property/mod.rs b/boa_engine/src/property/mod.rs
index 78ea271b47f..45475fdff55 100644
--- a/boa_engine/src/property/mod.rs
+++ b/boa_engine/src/property/mod.rs
@@ -603,7 +603,7 @@ pub enum PropertyKey {
 }
 
 /// Utility function for parsing [`PropertyKey`].
-fn parse_u32_index<I, T>(mut input: I) -> Option<u32>
+fn parse_u32_index<I, T>(mut input: I) -> Option<NonMaxU32>
 where
     I: Iterator<Item = T> + ExactSizeIterator + FusedIterator,
     T: Into<u16>,
@@ -635,7 +635,8 @@ where
     let byte = input.next()?.into();
     if byte == CHAR_ZERO {
         if len == 1 {
-            return Some(0);
+            // SAFETY: `0` is not `u32::MAX`.
+            return unsafe { Some(NonMaxU32::new_unchecked(0)) };
         }
 
         // String "012345" is not a valid index.
@@ -644,19 +645,23 @@ where
 
     let mut result = to_digit(byte)?;
 
-    // If the len is equal to max chars, then we need to do checked opterations,
+    // If the len is equal to max chars, then we need to do checked operations,
     // in case of overflows. If less use unchecked versions.
     if len == MAX_CHAR_COUNT {
         for c in input {
             result = result.checked_mul(10)?.checked_add(to_digit(c.into())?)?;
         }
+
+        NonMaxU32::new(result)
     } else {
         for c in input {
             result = result * 10 + to_digit(c.into())?;
         }
-    }
 
-    Some(result)
+        // SAFETY: `result` cannot be `u32::MAX`,
+        //         because the length of the input is smaller than `MAX_CHAR_COUNT`.
+        unsafe { Some(NonMaxU32::new_unchecked(result)) }
+    }
 }
 
 impl From<&[u16]> for PropertyKey {
@@ -675,36 +680,28 @@ impl From<&[u16]> for PropertyKey {
 impl From<JsString> for PropertyKey {
     #[inline]
     fn from(string: JsString) -> Self {
-        parse_u32_index(string.as_slice().iter().copied())
-            .and_then(NonMaxU32::new)
-            .map_or(Self::String(string), Self::Index)
+        parse_u32_index(string.as_slice().iter().copied()).map_or(Self::String(string), Self::Index)
     }
 }
 
 impl From<&str> for PropertyKey {
     #[inline]
     fn from(string: &str) -> Self {
-        parse_u32_index(string.bytes())
-            .and_then(NonMaxU32::new)
-            .map_or(Self::String(string.into()), Self::Index)
+        parse_u32_index(string.bytes()).map_or(Self::String(string.into()), Self::Index)
     }
 }
 
 impl From<String> for PropertyKey {
     #[inline]
     fn from(string: String) -> Self {
-        parse_u32_index(string.bytes())
-            .and_then(NonMaxU32::new)
-            .map_or(Self::String(string.into()), Self::Index)
+        parse_u32_index(string.bytes()).map_or(Self::String(string.into()), Self::Index)
     }
 }
 
 impl From<Box<str>> for PropertyKey {
     #[inline]
     fn from(string: Box<str>) -> Self {
-        parse_u32_index(string.bytes())
-            .and_then(NonMaxU32::new)
-            .map_or(Self::String(string.as_ref().into()), Self::Index)
+        parse_u32_index(string.bytes()).map_or(Self::String(string.as_ref().into()), Self::Index)
     }
 }
 
diff --git a/boa_engine/src/property/nonmaxu32.rs b/boa_engine/src/property/nonmaxu32.rs
index de0ffc78068..b9ebe3d8665 100644
--- a/boa_engine/src/property/nonmaxu32.rs
+++ b/boa_engine/src/property/nonmaxu32.rs
@@ -15,7 +15,7 @@ impl NonMaxU32 {
     #[must_use]
     pub const unsafe fn new_unchecked(inner: u32) -> Self {
         // SAFETY: The caller must ensure that `inner` is not `u32::MAX`.
-        let inner = unsafe { NonZeroU32::new_unchecked(inner + 1) };
+        let inner = unsafe { NonZeroU32::new_unchecked(inner.wrapping_add(1)) };
 
         Self { inner }
     }
@@ -34,6 +34,6 @@ impl NonMaxU32 {
     /// Returns the value as a primitive type.
     #[must_use]
     pub const fn get(&self) -> u32 {
-        self.inner.get() - 1
+        self.inner.get().wrapping_sub(1)
     }
 }