From f2b8d824994c2e26374d198d49b78949ed0c347b Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Thu, 22 Aug 2024 05:57:55 +0000 Subject: [PATCH] refactor(semantic)!: `ScopeTree::get_child_ids` + `get_child_ids_mut` return value not `Option` (#5058) Previously `ScopeTree::get_child_ids` and `ScopeTree::get_child_ids_mut` returned an `Option`. Instead return the unwrapped value, in line with other methods e.g. `get_flags`. --- .../src/post_transform_checker.rs | 5 ++--- crates/oxc_semantic/src/scope.rs | 20 ++++++------------- .../oxc_semantic/tests/integration/scopes.rs | 2 +- crates/oxc_semantic/tests/main.rs | 9 ++++----- crates/oxc_traverse/src/context/scoping.rs | 6 ++---- crates/oxc_wasm/src/lib.rs | 8 +++----- 6 files changed, 18 insertions(+), 32 deletions(-) diff --git a/crates/oxc_semantic/src/post_transform_checker.rs b/crates/oxc_semantic/src/post_transform_checker.rs index 59e6e82c67b80..86f72557e0a02 100644 --- a/crates/oxc_semantic/src/post_transform_checker.rs +++ b/crates/oxc_semantic/src/post_transform_checker.rs @@ -401,9 +401,8 @@ impl<'s> PostTransformChecker<'s> { } // Check children match - let child_ids = self.get_pair(scope_ids, |data, scope_id| { - data.scopes.get_child_ids(scope_id).cloned().unwrap_or_default() - }); + let child_ids = self + .get_pair(scope_ids, |data, scope_id| data.scopes.get_child_ids(scope_id).to_vec()); let is_match = child_ids.after_transform.len() == child_ids.rebuilt.len() && { let mut child_ids_after_transform = child_ids .after_transform diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index 59aba91554cee..746cb5db0466c 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -95,24 +95,16 @@ impl ScopeTree { list.into_iter() } - /// Get the child scopes of a scope. - /// - /// Will return [`None`] if no scope exists, which should never happen if - /// you obtained `scope_id` through valid means. Scopes with no children - /// return [`Some`] empty [`Vec`]. + /// Get the child scopes of a scope #[inline] - pub fn get_child_ids(&self, scope_id: ScopeId) -> Option<&Vec> { - self.child_ids.get(scope_id) + pub fn get_child_ids(&self, scope_id: ScopeId) -> &[ScopeId] { + &self.child_ids[scope_id] } - /// Get a mutable reference to a scope's children. - /// - /// Will return [`None`] if no scope exists, which should never happen if - /// you obtained `scope_id` through valid means. Scopes with no children - /// return [`Some`] empty [`Vec`]. + /// Get a mutable reference to a scope's children #[inline] - pub fn get_child_ids_mut(&mut self, scope_id: ScopeId) -> Option<&mut Vec> { - self.child_ids.get_mut(scope_id) + pub fn get_child_ids_mut(&mut self, scope_id: ScopeId) -> &mut Vec { + &mut self.child_ids[scope_id] } pub fn descendants_from_root(&self) -> impl Iterator + '_ { diff --git a/crates/oxc_semantic/tests/integration/scopes.rs b/crates/oxc_semantic/tests/integration/scopes.rs index 36188103cf739..dd181268a0953 100644 --- a/crates/oxc_semantic/tests/integration/scopes.rs +++ b/crates/oxc_semantic/tests/integration/scopes.rs @@ -27,7 +27,7 @@ fn test_only_program() { // children assert_eq!(scopes.descendants(root).count(), 0); - assert!(scopes.get_child_ids(root).unwrap().is_empty()); + assert!(scopes.get_child_ids(root).is_empty()); } #[test] diff --git a/crates/oxc_semantic/tests/main.rs b/crates/oxc_semantic/tests/main.rs index 4fb1cb2f41ada..203c2d76f9e28 100644 --- a/crates/oxc_semantic/tests/main.rs +++ b/crates/oxc_semantic/tests/main.rs @@ -16,11 +16,10 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator } let flags = semantic.scopes().get_flags(scope_id); result.push('{'); - if let Some(child_ids) = semantic.scopes().get_child_ids(scope_id) { - result.push_str("\"children\":"); - result.push_str(&get_scope_snapshot(semantic, child_ids.iter().copied())); - result.push(','); - } + let child_ids = semantic.scopes().get_child_ids(scope_id); + result.push_str("\"children\":"); + result.push_str(&get_scope_snapshot(semantic, child_ids.iter().copied())); + result.push(','); result.push_str(format!("\"flags\": \"{flags:?}\",").as_str()); result.push_str(format!("\"id\": {},", scope_id.index()).as_str()); result.push_str( diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs index c3b1a8e3729ef..57c338d8e2de9 100644 --- a/crates/oxc_traverse/src/context/scoping.rs +++ b/crates/oxc_traverse/src/context/scoping.rs @@ -122,10 +122,8 @@ impl TraverseScoping { fn insert_scope_below(&mut self, child_scope_ids: &[ScopeId], flags: ScopeFlags) -> ScopeId { // Remove these scopes from parent's children - if let Some(current_child_scope_ids) = self.scopes.get_child_ids_mut(self.current_scope_id) - { - current_child_scope_ids.retain(|scope_id| !child_scope_ids.contains(scope_id)); - } + let current_child_scope_ids = self.scopes.get_child_ids_mut(self.current_scope_id); + current_child_scope_ids.retain(|scope_id| !child_scope_ids.contains(scope_id)); // Create new scope as child of parent let new_scope_id = self.create_child_scope_of_current(flags); diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index c170de787c350..3e7b349f85643 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -309,7 +309,7 @@ impl Oxc { semantic: &Semantic, scope_text: &mut String, depth: usize, - scope_ids: &Vec, + scope_ids: &[ScopeId], ) { let space = " ".repeat(depth * 2); @@ -332,15 +332,13 @@ impl Oxc { scope_text.push_str(&format!("\n{binding_space}}}\n")); } - if let Some(next_scope_ids) = next_scope_ids { - write_scope_text(semantic, scope_text, depth + 1, next_scope_ids); - } + write_scope_text(semantic, scope_text, depth + 1, next_scope_ids); scope_text.push_str(&format!("{space}}}\n")); } } let mut scope_text = String::default(); - write_scope_text(semantic, &mut scope_text, 0, &vec![semantic.scopes().root_scope_id()]); + write_scope_text(semantic, &mut scope_text, 0, &[semantic.scopes().root_scope_id()]); scope_text }