Skip to content

Commit

Permalink
refactor(semantic): change IndexMap to Vec for Bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Dec 19, 2024
1 parent 63a95e4 commit 212b437
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 89 deletions.
37 changes: 19 additions & 18 deletions crates/oxc_linter/src/rules/eslint/no_else_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ fn is_safe_from_name_collisions(
let parent_bindings = scopes.get_bindings(parent_scope_id);

if bindings.iter().any(|(name, symbol_id)| {
let Some((parent_name, parent_symbol_id)) = parent_bindings.get_key_value(name)
let Some((parent_name, parent_symbol_id)) =
parent_bindings.iter().find(|(n, _)| n == name)
else {
return false;
};
Expand Down Expand Up @@ -495,41 +496,41 @@ fn test() {
),
("function foo10() { if (foo) return bar; else (foo).bar(); }", None),
(
"function foo11() { if (foo) return bar
"function foo11() { if (foo) return bar
else { [1, 2, 3].map(foo) } }",
None,
),
(
"function foo12() { if (foo) return bar
else { baz() }
"function foo12() { if (foo) return bar
else { baz() }
[1, 2, 3].map(foo) }",
None,
),
(
"function foo13() { if (foo) return bar;
"function foo13() { if (foo) return bar;
else { [1, 2, 3].map(foo) } }",
None,
),
(
"function foo14() { if (foo) return bar
else { baz(); }
"function foo14() { if (foo) return bar
else { baz(); }
[1, 2, 3].map(foo) }",
None,
),
("function foo15() { if (foo) return bar; else { baz() } qaz() }", None),
(
"function foo16() { if (foo) return bar
"function foo16() { if (foo) return bar
else { baz() } qaz() }",
None,
),
(
"function foo17() { if (foo) return bar
else { baz() }
"function foo17() { if (foo) return bar
else { baz() }
qaz() }",
None,
),
(
"function foo18() { if (foo) return function() {}
"function foo18() { if (foo) return function() {}
else [1, 2, 3].map(bar) }",
None,
),
Expand Down Expand Up @@ -730,24 +731,24 @@ fn test() {
None,
),
(
"function foo13() { if (foo) return bar;
"function foo13() { if (foo) return bar;
else { [1, 2, 3].map(foo) } }",
"function foo13() { if (foo) return bar; [1, 2, 3].map(foo) }",
None,
),
(
"function foo14() { if (foo) return bar
else { baz(); }
"function foo14() { if (foo) return bar
else { baz(); }
[1, 2, 3].map(foo) }",
"function foo14() { if (foo) return bar\n baz();
"function foo14() { if (foo) return bar\n baz();
[1, 2, 3].map(foo) }",
None,
),
(
"function foo17() { if (foo) return bar
else { baz() }
"function foo17() { if (foo) return bar
else { baz() }
qaz() }",
"function foo17() { if (foo) return bar\n baz()
"function foo17() { if (foo) return bar\n baz()
qaz() }",
None,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl Rule for NoUnsafeDeclarationMerging {
match node.kind() {
AstKind::Class(decl) => {
if let Some(ident) = decl.id.as_ref() {
for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) {
for symbol_id in ctx.semantic().scopes().iter_bindings_in(node.scope_id()) {
if let AstKind::TSInterfaceDeclaration(scope_interface) =
get_symbol_kind(symbol_id, ctx)
{
Expand All @@ -53,7 +53,7 @@ impl Rule for NoUnsafeDeclarationMerging {
}
}
AstKind::TSInterfaceDeclaration(decl) => {
for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) {
for symbol_id in ctx.semantic().scopes().iter_bindings_in(node.scope_id()) {
if let AstKind::Class(scope_class) = get_symbol_kind(symbol_id, ctx) {
if let Some(scope_class_ident) = scope_class.id.as_ref() {
check_and_diagnostic(&decl.id, scope_class_ident, ctx);
Expand Down
20 changes: 10 additions & 10 deletions crates/oxc_linter/src/snapshots/eslint_no_else_return.snap
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ snapshot_kind: text

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo11() { if (foo) return bar
1 │ ╭─▶ function foo11() { if (foo) return bar
· │ ─────┬────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { [1, 2, 3].map(foo) } }
Expand All @@ -158,18 +158,18 @@ snapshot_kind: text

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo12() { if (foo) return bar
1 │ ╭─▶ function foo12() { if (foo) return bar
· │ ─────┬────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { baz() }
2 │ ├─▶ else { baz() }
· ╰──── Making this `else` block unnecessary.
3 │ [1, 2, 3].map(foo) }
╰────
help: Remove the `else` block, moving its contents outside of the `if` statement.

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo13() { if (foo) return bar;
1 │ ╭─▶ function foo13() { if (foo) return bar;
· │ ─────┬─────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { [1, 2, 3].map(foo) } }
Expand All @@ -179,10 +179,10 @@ snapshot_kind: text

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo14() { if (foo) return bar
1 │ ╭─▶ function foo14() { if (foo) return bar
· │ ─────┬────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { baz(); }
2 │ ├─▶ else { baz(); }
· ╰──── Making this `else` block unnecessary.
3 │ [1, 2, 3].map(foo) }
╰────
Expand All @@ -199,7 +199,7 @@ snapshot_kind: text

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo16() { if (foo) return bar
1 │ ╭─▶ function foo16() { if (foo) return bar
· │ ─────┬────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { baz() } qaz() }
Expand All @@ -209,18 +209,18 @@ snapshot_kind: text

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo17() { if (foo) return bar
1 │ ╭─▶ function foo17() { if (foo) return bar
· │ ─────┬────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else { baz() }
2 │ ├─▶ else { baz() }
· ╰──── Making this `else` block unnecessary.
3qaz() }
╰────
help: Remove the `else` block, moving its contents outside of the `if` statement.

eslint(no-else-return): Unnecessary 'else' after 'return'.
╭─[no_else_return.tsx:1:29]
1 │ ╭─▶ function foo18() { if (foo) return function() {}
1 │ ╭─▶ function foo18() { if (foo) return function() {}
· │ ──────────┬─────────
· │ ╰── This consequent block always returns,
2 │ ├─▶ else [1, 2, 3].map(bar) }
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl Mangler {

if !bindings.is_empty() {
// `bindings` are stored in order, traverse and increment slot
for symbol_id in bindings.values().copied() {
slots[symbol_id] = slot;
for (_, symbol_id) in bindings {
slots[*symbol_id] = slot;
slot += 1;
}
}
Expand Down Expand Up @@ -143,7 +143,7 @@ impl Mangler {
if !is_keyword(n)
&& !is_special_name(n)
&& !root_unresolved_references.contains_key(n)
&& (self.options.top_level || !root_bindings.contains_key(n))
&& (self.options.top_level || !root_bindings.iter().any(|(name, _)| name == n))
{
break name;
}
Expand Down
5 changes: 1 addition & 4 deletions crates/oxc_semantic/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ oxc_span = { workspace = true }
oxc_syntax = { workspace = true }

assert-unchecked = { workspace = true }
indexmap = { workspace = true }
itertools = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
Expand All @@ -40,10 +39,8 @@ tsify = { workspace = true, optional = true }
wasm-bindgen = { workspace = true, optional = true }

[dev-dependencies]
oxc_parser = { workspace = true }

indexmap = { workspace = true }
insta = { workspace = true, features = ["glob"] }
oxc_parser = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rustc-hash = { workspace = true }
serde_json = { workspace = true }
Expand Down
36 changes: 17 additions & 19 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,10 @@ use oxc_syntax::{
};

use crate::{
binder::Binder,
checker,
class::ClassTableBuilder,
diagnostics::redeclaration,
jsdoc::JSDocBuilder,
label::UnusedLabels,
node::AstNodes,
reference::Reference,
scope::{Bindings, ScopeTree},
stats::Stats,
symbol::SymbolTable,
unresolved_stack::UnresolvedReferencesStack,
JSDocFinder, Semantic,
binder::Binder, checker, class::ClassTableBuilder, diagnostics::redeclaration,
jsdoc::JSDocBuilder, label::UnusedLabels, node::AstNodes, reference::Reference,
scope::ScopeTree, stats::Stats, symbol::SymbolTable,
unresolved_stack::UnresolvedReferencesStack, JSDocFinder, Semantic,
};

macro_rules! control_flow {
Expand Down Expand Up @@ -474,7 +465,12 @@ impl<'a> SemanticBuilder<'a> {
self.current_scope_id,
self.current_node_id,
);
self.scope.get_bindings_mut(scope_id).insert(name, symbol_id);
let bindings_mut = self.scope.get_bindings_mut(scope_id);
if let Some((_, sid)) = bindings_mut.iter_mut().find(|(n, _)| *n == name) {
*sid = symbol_id;
} else {
bindings_mut.push((name, symbol_id));
};
symbol_id
}

Expand All @@ -489,7 +485,9 @@ impl<'a> SemanticBuilder<'a> {
// Try to resolve a reference.
// If unresolved, transfer it to parent scope's unresolved references.
let bindings = self.scope.get_bindings(self.current_scope_id);
if let Some(symbol_id) = bindings.get(name.as_str()).copied() {
if let Some((_, symbol_id)) = bindings.iter().find(|(n, _)| n.as_str() == name.as_str())
{
let symbol_id = *symbol_id;
let symbol_flags = self.symbols.get_flags(symbol_id);
references.retain(|&reference_id| {
let reference = &mut self.symbols.references[reference_id];
Expand Down Expand Up @@ -714,10 +712,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
if self.scope.get_flags(parent_scope_id).is_catch_clause() {
let parent_bindings = self.scope.get_bindings_mut(parent_scope_id);
if !parent_bindings.is_empty() {
let parent_bindings = parent_bindings.drain(..).collect::<Bindings>();
parent_bindings.values().for_each(|&symbol_id| {
self.symbols.set_scope_id(symbol_id, self.current_scope_id);
});
let parent_bindings = std::mem::take(parent_bindings);
for (_, symbol_id) in &parent_bindings {
self.symbols.set_scope_id(*symbol_id, self.current_scope_id);
}
*self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings;
}
}
Expand Down
Loading

0 comments on commit 212b437

Please sign in to comment.