Skip to content

Commit

Permalink
fix(semantic): bind SymbolId to function name in `if (foo) function…
Browse files Browse the repository at this point in the history
… id() {}` (#5673)
  • Loading branch information
Boshen committed Sep 10, 2024
1 parent 067f9b5 commit f9e3a41
Show file tree
Hide file tree
Showing 4 changed files with 806 additions and 686 deletions.
45 changes: 36 additions & 9 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ use oxc_ast::{
};
use oxc_span::{GetSpan, SourceType};

use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder};
use crate::{
scope::{ScopeFlags, ScopeId},
symbol::SymbolFlags,
SemanticBuilder,
};

pub(crate) trait Binder<'a> {
#[allow(unused_variables)]
Expand Down Expand Up @@ -132,19 +136,42 @@ fn function_as_var(flags: ScopeFlags, source_type: SourceType) -> bool {
flags.is_function() || (source_type.is_script() && flags.is_top())
}

/// Check for Annex B `if (foo) function a() {} else function b() {}`
fn is_function_part_of_if_statement(function: &Function, builder: &SemanticBuilder) -> bool {
if builder.current_scope_flags().is_strict_mode() {
return false;
}
let Some(AstKind::IfStatement(stmt)) = builder.nodes.parent_kind(builder.current_node_id)
else {
return false;
};
if let Statement::FunctionDeclaration(func) = &stmt.consequent {
if func.span == function.span {
return true;
}
}
if let Some(Statement::FunctionDeclaration(func)) = &stmt.alternate {
if func.span == function.span {
return true;
}
}
false
}

impl<'a> Binder<'a> for Function<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let current_scope_id = builder.current_scope_id;
let scope_flags = builder.current_scope_flags();
if let Some(ident) = &self.id {
if !scope_flags.is_strict_mode()
&& matches!(
builder.nodes.parent_kind(builder.current_node_id),
Some(AstKind::IfStatement(_))
)
{
// Do not declare in if single statements,
// if (false) function f() {} else function g() { }
if is_function_part_of_if_statement(self, builder) {
let symbol_id = builder.symbols.create_symbol(
ident.span,
ident.name.clone().into(),
SymbolFlags::Function,
ScopeId::new(u32::MAX - 1), // Not bound to any scope.
builder.current_node_id,
);
ident.symbol_id.set(Some(symbol_id));
} else if self.r#type == FunctionType::FunctionDeclaration {
// The visitor is already inside the function scope,
// retrieve the parent scope for the function id to bind to.
Expand Down
16 changes: 0 additions & 16 deletions crates/oxc_semantic/src/post_transform_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,22 +648,6 @@ impl<'a, 'e> Visit<'a> for SemanticIdsCollector<'e> {
walk::walk_declaration(self, it);
}

fn visit_if_statement(&mut self, stmt: &IfStatement<'a>) {
// skip `if (function foo() {}) {}`
if !matches!(stmt.test, Expression::FunctionExpression(_)) {
self.visit_expression(&stmt.test);
}
// skip `if (true) function foo() {} else function bar() {}`
if !stmt.consequent.is_declaration() {
self.visit_statement(&stmt.consequent);
}
if let Some(alternate) = &stmt.alternate {
if !alternate.is_declaration() {
self.visit_statement(alternate);
}
}
}

fn visit_ts_type(&mut self, _it: &TSType<'a>) {
/* noop */
}
Expand Down
33 changes: 13 additions & 20 deletions tasks/coverage/semantic_babel.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ commit: 3bcfee23

semantic_babel Summary:
AST Parsed : 2101/2101 (100.00%)
Positive Passed: 1737/2101 (82.67%)
Positive Passed: 1739/2101 (82.77%)
tasks/coverage/babel/packages/babel-parser/test/fixtures/annex-b/enabled/3.3-function-in-if-body/input.js
semantic error: Scope children mismatch:
after transform: ScopeId(0): [ScopeId(1), ScopeId(2)]
rebuilt : ScopeId(0): [ScopeId(1), ScopeId(2)]
semantic error: Symbol scope ID mismatch:
after transform: SymbolId(0): ScopeId(4294967294)
rebuilt : SymbolId(0): ScopeId(4294967294)
Symbol scope ID mismatch:
after transform: SymbolId(1): ScopeId(4294967294)
rebuilt : SymbolId(1): ScopeId(4294967294)

tasks/coverage/babel/packages/babel-parser/test/fixtures/comments/interpreter-directive/interpreter-directive-import/input.js
semantic error: Bindings mismatch:
Expand All @@ -23,14 +26,9 @@ semantic error: Unexpected new.target expression
Unexpected new.target expression

tasks/coverage/babel/packages/babel-parser/test/fixtures/core/scope/dupl-bind-catch-hang-func/input.js
semantic error: Scope children mismatch:
after transform: ScopeId(3): [ScopeId(4)]
rebuilt : ScopeId(3): [ScopeId(4)]

tasks/coverage/babel/packages/babel-parser/test/fixtures/core/uncategorised/230/input.js
semantic error: Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]
semantic error: Symbol scope ID mismatch:
after transform: SymbolId(1): ScopeId(4294967294)
rebuilt : SymbolId(1): ScopeId(4294967294)

tasks/coverage/babel/packages/babel-parser/test/fixtures/core/uncategorised/327/input.js
semantic error: A 'return' statement can only be used within a function body.
Expand Down Expand Up @@ -138,15 +136,10 @@ semantic error: Bindings mismatch:
after transform: ScopeId(0): ["nil"]
rebuilt : ScopeId(0): []

tasks/coverage/babel/packages/babel-parser/test/fixtures/esprima/statement-if/migrated_0002/input.js
semantic error: Binding symbols mismatch:
after transform: ScopeId(0): [SymbolId(0)]
rebuilt : ScopeId(0): [SymbolId(0)]

tasks/coverage/babel/packages/babel-parser/test/fixtures/esprima/statement-if/migrated_0003/input.js
semantic error: Scope children mismatch:
after transform: ScopeId(0): [ScopeId(1)]
rebuilt : ScopeId(0): [ScopeId(1)]
semantic error: Symbol scope ID mismatch:
after transform: SymbolId(0): ScopeId(4294967294)
rebuilt : SymbolId(0): ScopeId(4294967294)

tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/arrow-function/arrow-function-with-newline/input.ts
semantic error: Unresolved references mismatch:
Expand Down
Loading

0 comments on commit f9e3a41

Please sign in to comment.