Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(semantic)!: ScopeTree::get_child_ids + get_child_ids_mut return value not Option #5058

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions crates/oxc_semantic/src/post_transform_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 6 additions & 14 deletions crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ScopeId>> {
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<ScopeId>> {
self.child_ids.get_mut(scope_id)
pub fn get_child_ids_mut(&mut self, scope_id: ScopeId) -> &mut Vec<ScopeId> {
&mut self.child_ids[scope_id]
}

pub fn descendants_from_root(&self) -> impl Iterator<Item = ScopeId> + '_ {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/tests/integration/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
9 changes: 4 additions & 5 deletions crates/oxc_semantic/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ fn get_scope_snapshot(semantic: &Semantic, scopes: impl Iterator<Item = ScopeId>
}
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(
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 3 additions & 5 deletions crates/oxc_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl Oxc {
semantic: &Semantic,
scope_text: &mut String,
depth: usize,
scope_ids: &Vec<ScopeId>,
scope_ids: &[ScopeId],
) {
let space = " ".repeat(depth * 2);

Expand All @@ -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
}

Expand Down