Skip to content

Commit

Permalink
fix(semantic): transform checker handle conditional scopes (#5040)
Browse files Browse the repository at this point in the history
Some scopes are conditional e.g. `ForStatement` only gets a scope when initializer has a binding (`for (let i = 0; ...)` vs `for (i = 0; ...)`).

Make transform compare this between post-transform and fresh semantics.
  • Loading branch information
overlookmotel committed Aug 21, 2024
1 parent 586e15c commit 863b9cb
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 29 deletions.
65 changes: 41 additions & 24 deletions crates/oxc_semantic/src/post_transform_checker.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{cell::Cell, collections::HashSet, mem};
use std::{cell::Cell, mem};

use oxc_allocator::{Allocator, CloneIn};
#[allow(clippy::wildcard_imports)]
Expand Down Expand Up @@ -87,28 +87,47 @@ impl PostTransformChecker {
}

// Check whether bindings are the same for scopes in the same visitation order.
for (prev_scope_id, cur_scope_id) in
for (&prev_scope_id, &cur_scope_id) in
self.collect_after_transform.scope_ids.iter().zip(current_collect.scope_ids.iter())
{
let mut prev_bindings =
previous_scopes.get_bindings(*prev_scope_id).keys().cloned().collect::<Vec<_>>();
prev_bindings.sort_unstable();
let mut current_bindings =
current_scopes.get_bindings(*cur_scope_id).keys().cloned().collect::<Vec<_>>();
current_bindings.sort_unstable();

if prev_bindings.iter().collect::<HashSet<&CompactStr>>()
!= current_bindings.iter().collect::<HashSet<&CompactStr>>()
{
let message = format!(
"
Bindings mismatch:
previous {prev_scope_id:?}: {prev_bindings:?}
current {cur_scope_id:?}: {current_bindings:?}
"
);
self.errors.push(OxcDiagnostic::error(message.trim().to_string()));
fn get_sorted_bindings(scopes: &ScopeTree, scope_id: ScopeId) -> Vec<CompactStr> {
let mut bindings =
scopes.get_bindings(scope_id).keys().cloned().collect::<Vec<_>>();
bindings.sort_unstable();
bindings
}

let (previous, current) = match (prev_scope_id, cur_scope_id) {
(None, None) => continue,
(Some(prev_scope_id), Some(cur_scope_id)) => {
let prev_bindings = get_sorted_bindings(previous_scopes, prev_scope_id);
let current_bindings = get_sorted_bindings(current_scopes, cur_scope_id);
if prev_bindings == current_bindings {
continue;
}
(
format!("{prev_scope_id:?}: {prev_bindings:?}"),
format!("{cur_scope_id:?}: {current_bindings:?}"),
)
}
(Some(prev_scope_id), None) => {
let prev_bindings = get_sorted_bindings(previous_scopes, prev_scope_id);
(format!("{prev_scope_id:?}: {prev_bindings:?}"), "No scope".to_string())
}
(None, Some(cur_scope_id)) => {
let current_bindings = get_sorted_bindings(current_scopes, cur_scope_id);
("No scope".to_string(), format!("{cur_scope_id:?}: {current_bindings:?}"))
}
};

let message = format!(
"
Bindings mismatch:
previous {previous}
current {current}
"
);
self.errors.push(OxcDiagnostic::error(message.trim().to_string()));
}
}

Expand Down Expand Up @@ -209,7 +228,7 @@ current {cur_reference_id:?}: {cur_symbol_name:?}

#[derive(Default)]
pub struct SemanticCollector {
scope_ids: Vec<ScopeId>,
scope_ids: Vec<Option<ScopeId>>,
symbol_ids: Vec<SymbolId>,
reference_ids: Vec<ReferenceId>,

Expand All @@ -218,9 +237,7 @@ pub struct SemanticCollector {

impl<'a> Visit<'a> for SemanticCollector {
fn enter_scope(&mut self, _flags: ScopeFlags, scope_id: &Cell<Option<ScopeId>>) {
if let Some(scope_id) = scope_id.get() {
self.scope_ids.push(scope_id);
}
self.scope_ids.push(scope_id.get());
}

fn visit_identifier_reference(&mut self, ident: &IdentifierReference<'a>) {
Expand Down
24 changes: 22 additions & 2 deletions tasks/transform_conformance/babel.snap.md
Original file line number Diff line number Diff line change
Expand Up @@ -1647,11 +1647,31 @@ preset-env: unknown field `shippedProposals`, expected `targets` or `bugfixes`

# babel-plugin-transform-optional-catch-binding (2/4)
* optional-catch-bindings/try-catch-block-no-binding/input.js
x Scopes mismatch after transform
x Bindings mismatch:
| previous ScopeId(0): ["_unused"]
| current ScopeId(0): []

x Bindings mismatch:
| previous No scope
| current ScopeId(2): []

x Bindings mismatch:
| previous ScopeId(2): []
| current ScopeId(3): ["_unused"]


* optional-catch-bindings/try-catch-finally-no-binding/input.js
x Scopes mismatch after transform
x Bindings mismatch:
| previous ScopeId(0): ["_unused"]
| current ScopeId(0): []

x Bindings mismatch:
| previous No scope
| current ScopeId(2): []

x Bindings mismatch:
| previous ScopeId(2): []
| current ScopeId(3): ["_unused"]



Expand Down
24 changes: 21 additions & 3 deletions tasks/transform_conformance/oxc.snap.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ Passed: 9/35

# babel-plugin-transform-optional-catch-binding (0/1)
* try-catch-shadow/input.js
x Scopes mismatch after transform
x Bindings mismatch:
| previous ScopeId(0): ["_unused", "_unused2"]
| current ScopeId(0): ["_unused"]

x Bindings mismatch:
| previous No scope
| current ScopeId(2): []

x Bindings mismatch:
| previous ScopeId(2): []
| current ScopeId(3): ["_unused2"]



Expand Down Expand Up @@ -218,7 +228,9 @@ Passed: 9/35


* refresh/generates-valid-signature-for-exotic-ways-to-call-hooks/input.jsx
x Scopes mismatch after transform
x Bindings mismatch:
| previous No scope
| current ScopeId(3): []

x Reference mismatch:
| previous ReferenceId(17): Some("_s2")
Expand All @@ -234,7 +246,13 @@ Passed: 9/35


* refresh/includes-custom-hooks-into-the-signatures/input.jsx
x Scopes mismatch after transform
x Bindings mismatch:
| previous No scope
| current ScopeId(2): []

x Bindings mismatch:
| previous No scope
| current ScopeId(6): []

x Reference mismatch:
| previous ReferenceId(10): Some("_s")
Expand Down

0 comments on commit 863b9cb

Please sign in to comment.