From 03bc041ddf09619f0737dd2543275f8c97eb206c Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Sun, 6 Oct 2024 23:26:26 +0000
Subject: [PATCH] refactor(syntax): remove some unsafe code creating IDs
 (#6324)

---
 crates/oxc_syntax/src/node.rs      | 11 +++++------
 crates/oxc_syntax/src/reference.rs |  2 +-
 crates/oxc_syntax/src/scope.rs     | 11 +++++------
 crates/oxc_syntax/src/symbol.rs    |  2 +-
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/crates/oxc_syntax/src/node.rs b/crates/oxc_syntax/src/node.rs
index 2cee717fb9c28..937a1a9715d12 100644
--- a/crates/oxc_syntax/src/node.rs
+++ b/crates/oxc_syntax/src/node.rs
@@ -16,11 +16,10 @@ impl NodeId {
     /// # Panics
     /// Panics if `idx` is `u32::MAX`.
     pub const fn new(idx: u32) -> Self {
-        // We could use `NonMaxU32::new(idx).unwrap()` but `Option::unwrap` is not a const function
-        // and we want this function to be
-        assert!(idx != u32::MAX);
-        // SAFETY: We have checked that `idx` is not `u32::MAX`
-        unsafe { Self::new_unchecked(idx) }
+        if let Some(idx) = NonMaxU32::new(idx) {
+            return Self(idx);
+        }
+        panic!();
     }
 
     /// Create `NodeId` from `u32` unchecked.
@@ -38,7 +37,7 @@ impl Idx for NodeId {
     #[allow(clippy::cast_possible_truncation)]
     fn from_usize(idx: usize) -> Self {
         assert!(idx < u32::MAX as usize);
-        // SAFETY: We just checked `idx` is valid for `NonMaxU32`
+        // SAFETY: We just checked `idx` is a legal value for `NonMaxU32`
         Self(unsafe { NonMaxU32::new_unchecked(idx as u32) })
     }
 
diff --git a/crates/oxc_syntax/src/reference.rs b/crates/oxc_syntax/src/reference.rs
index bf4c267e55a63..ea7f19fc96230 100644
--- a/crates/oxc_syntax/src/reference.rs
+++ b/crates/oxc_syntax/src/reference.rs
@@ -12,7 +12,7 @@ impl Idx for ReferenceId {
     #[allow(clippy::cast_possible_truncation)]
     fn from_usize(idx: usize) -> Self {
         assert!(idx < u32::MAX as usize);
-        // SAFETY: We just checked `idx` is valid for `NonMaxU32`
+        // SAFETY: We just checked `idx` is a legal value for `NonMaxU32`
         Self(unsafe { NonMaxU32::new_unchecked(idx as u32) })
     }
 
diff --git a/crates/oxc_syntax/src/scope.rs b/crates/oxc_syntax/src/scope.rs
index d5ca05c436453..258053c581699 100644
--- a/crates/oxc_syntax/src/scope.rs
+++ b/crates/oxc_syntax/src/scope.rs
@@ -13,11 +13,10 @@ impl ScopeId {
     /// # Panics
     /// Panics if `idx` is `u32::MAX`.
     pub const fn new(idx: u32) -> Self {
-        // We could use `NonMaxU32::new(idx).unwrap()` but `Option::unwrap` is not a const function
-        // and we want this function to be
-        assert!(idx != u32::MAX);
-        // SAFETY: We have checked that `idx` is not `u32::MAX`
-        unsafe { Self::new_unchecked(idx) }
+        if let Some(idx) = NonMaxU32::new(idx) {
+            return Self(idx);
+        }
+        panic!();
     }
 
     /// Create `ScopeId` from `u32` unchecked.
@@ -35,7 +34,7 @@ impl Idx for ScopeId {
     #[allow(clippy::cast_possible_truncation)]
     fn from_usize(idx: usize) -> Self {
         assert!(idx < u32::MAX as usize);
-        // SAFETY: We just checked `idx` is valid for `NonMaxU32`
+        // SAFETY: We just checked `idx` is a legal value for `NonMaxU32`
         Self(unsafe { NonMaxU32::new_unchecked(idx as u32) })
     }
 
diff --git a/crates/oxc_syntax/src/symbol.rs b/crates/oxc_syntax/src/symbol.rs
index 818446a86432b..0fa05c88a0631 100644
--- a/crates/oxc_syntax/src/symbol.rs
+++ b/crates/oxc_syntax/src/symbol.rs
@@ -11,7 +11,7 @@ impl Idx for SymbolId {
     #[allow(clippy::cast_possible_truncation)]
     fn from_usize(idx: usize) -> Self {
         assert!(idx < u32::MAX as usize);
-        // SAFETY: We just checked `idx` is valid for `NonMaxU32`
+        // SAFETY: We just checked `idx` is a legal value for `NonMaxU32`
         Self(unsafe { NonMaxU32::new_unchecked(idx as u32) })
     }