Skip to content

Commit

Permalink
refactor(semantic): keep consistent SymbolFlags for function id
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Nov 26, 2024
1 parent 00a7372 commit 70d22c4
Show file tree
Hide file tree
Showing 12 changed files with 17,655 additions and 5,337 deletions.
13 changes: 8 additions & 5 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,11 +968,7 @@ impl<'a> Function<'a> {

/// `true` for overload signatures and `declare function` statements.
pub fn is_typescript_syntax(&self) -> bool {
matches!(
self.r#type,
FunctionType::TSDeclareFunction | FunctionType::TSEmptyBodyFunctionExpression
) || self.body.is_none()
|| self.declare
self.r#type.is_typescript_syntax() || self.body.is_none() || self.declare
}

/// `true` for function expressions
Expand Down Expand Up @@ -1001,6 +997,13 @@ impl<'a> Function<'a> {
}
}

impl FunctionType {
/// Returns `true` if it is a [`FunctionType::TSDeclareFunction`] or [`FunctionType::TSEmptyBodyFunctionExpression`].
pub fn is_typescript_syntax(&self) -> bool {
matches!(self, Self::TSDeclareFunction | Self::TSEmptyBodyFunctionExpression)
}
}

impl<'a> FormalParameters<'a> {
/// Number of parameters bound in this parameter list.
pub fn parameters_count(&self) -> usize {
Expand Down
58 changes: 32 additions & 26 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ fn function_as_var(flags: ScopeFlags, source_type: SourceType) -> bool {
flags.is_function() || (source_type.is_script() && flags.is_top())
}

/// Check if the function is not allowed to be redeclared.
pub fn is_function_redeclared_not_allowed(
func: &Function<'_>,
builder: &SemanticBuilder<'_>,
) -> bool {
let current_scope_flags = builder.current_scope_flags();
(current_scope_flags.is_strict_mode() || func.r#async || func.generator)
&& !function_as_var(current_scope_flags, builder.source_type)
}

/// 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() {
Expand All @@ -150,8 +160,9 @@ fn is_function_part_of_if_statement(function: &Function, builder: &SemanticBuild

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 self.r#type.is_typescript_syntax() {
return;
}
if let Some(ident) = &self.id {
if is_function_part_of_if_statement(self, builder) {
let symbol_id = builder.symbols.create_symbol(
Expand All @@ -162,35 +173,30 @@ impl<'a> Binder<'a> for Function<'a> {
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.

let (includes, excludes) =
if (scope_flags.is_strict_mode() || self.r#async || self.generator)
&& !function_as_var(scope_flags, builder.source_type)
{
(
SymbolFlags::Function | SymbolFlags::BlockScopedVariable,
SymbolFlags::BlockScopedVariableExcludes,
)
} else {
let excludes = if self.is_declaration() {
// The visitor is already inside the function scope,
// retrieve the parent scope for the function id to bind to.
if is_function_redeclared_not_allowed(self, builder) {
SymbolFlags::BlockScopedVariableExcludes
} else {
(
SymbolFlags::FunctionScopedVariable,
SymbolFlags::FunctionScopedVariableExcludes,
)
};
SymbolFlags::FunctionScopedVariableExcludes
}
} else if self.is_expression() {
// https://tc39.es/ecma262/#sec-runtime-semantics-instantiateordinaryfunctionexpression
// 5. Perform ! funcEnv.CreateImmutableBinding(name, false).
SymbolFlags::empty()
} else {
unreachable!(
"Currently we haven't create a symbol for typescript syntax function"
);
};

let symbol_id = builder.declare_symbol(ident.span, &ident.name, includes, excludes);
ident.symbol_id.set(Some(symbol_id));
} else if self.r#type == FunctionType::FunctionExpression {
// https://tc39.es/ecma262/#sec-runtime-semantics-instantiateordinaryfunctionexpression
// 5. Perform ! funcEnv.CreateImmutableBinding(name, false).
let symbol_id = builder.declare_symbol(
ident.span,
&ident.name,
SymbolFlags::Function,
SymbolFlags::empty(),
excludes,
);
ident.symbol_id.set(Some(symbol_id));
}
Expand All @@ -200,7 +206,7 @@ impl<'a> Binder<'a> for Function<'a> {
if let Some(AstKind::ObjectProperty(prop)) =
builder.nodes.parent_kind(builder.current_node_id)
{
let flags = builder.scope.get_flags_mut(current_scope_id);
let flags = builder.scope.get_flags_mut(builder.current_scope_id);
match prop.kind {
PropertyKind::Get => *flags |= ScopeFlags::GetAccessor,
PropertyKind::Set => *flags |= ScopeFlags::SetAccessor,
Expand Down
35 changes: 26 additions & 9 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use oxc_span::{Atom, CompactStr, SourceType, Span};
use oxc_syntax::module_record::ModuleRecord;

use crate::{
binder::Binder,
binder::{is_function_redeclared_not_allowed, Binder},
checker,
class::ClassTableBuilder,
diagnostics::redeclaration,
Expand Down Expand Up @@ -461,9 +461,23 @@ impl<'a> SemanticBuilder<'a> {
let symbol_id = self.scope.get_binding(scope_id, name).or_else(|| {
self.hoisting_variables.get(&scope_id).and_then(|symbols| symbols.get(name).copied())
})?;
if report_error && self.symbols.get_flags(symbol_id).intersects(excludes) {
let symbol_span = self.symbols.get_span(symbol_id);
self.error(redeclaration(name, symbol_span, span));
if report_error {
let flags = self.symbols.get_flags(symbol_id);
if flags.intersects(excludes)
// Needs to further check if the previous declaration is a function and the function
// is not allowed to be redeclared.
// For example: `async function goo() var goo;`
// ^^^ Redeclare
|| (excludes == SymbolFlags::FunctionScopedVariableExcludes
&& flags.is_function() // The previous declaration is a function
&& matches!( // Check the previous function whether it is allowed to be redeclared
self.nodes.kind(self.symbols.get_declaration(symbol_id)),
AstKind::Function(func) if is_function_redeclared_not_allowed(func, self)
))
{
let symbol_span = self.symbols.get_span(symbol_id);
self.error(redeclaration(name, symbol_span, span));
}
}
Some(symbol_id)
}
Expand Down Expand Up @@ -1628,11 +1642,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
&func.scope_id,
);

if func.is_expression() {
// We need to bind function expression in the function scope
func.bind(self);
}

if let Some(id) = &func.id {
self.visit_binding_identifier(id);
}
Expand Down Expand Up @@ -1679,6 +1688,14 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg */

if func.is_expression() {
// We need to bind the function expression in the function scope and place
// it at the end of the function body in order to prevent redeclaration errors,
// as the function body may contain variables with the same name as the function.
// For example: `function foo() { let foo = 1; }` is valid.
func.bind(self);
}

self.leave_scope();
self.leave_node(kind);
}
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_syntax/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,18 @@ bitflags! {

const BlockScoped = Self::BlockScopedVariable.bits() | Self::Enum.bits() | Self::Class.bits();

const Value = Self::Variable.bits() | Self::Class.bits() | Self::Enum.bits() | Self::EnumMember.bits() | Self::ValueModule.bits();
const Value = Self::Variable.bits() | Self::Class.bits() | Self::Function.bits() | Self::Enum.bits() | Self::EnumMember.bits() | Self::ValueModule.bits();
const Type = Self::Class.bits() | Self::Interface.bits() | Self::Enum.bits() | Self::EnumMember.bits() | Self::TypeParameter.bits() | Self::TypeAlias.bits();

/// Variables can be redeclared, but can not redeclare a block-scoped declaration with the
/// same name, or any other value that is not a variable, e.g. ValueModule or Class
const FunctionScopedVariableExcludes = Self::Value.bits() - Self::FunctionScopedVariable.bits();
const FunctionScopedVariableExcludes = Self::Value.bits() - Self::FunctionScopedVariable.bits() - Self::Function.bits();

/// Block-scoped declarations are not allowed to be re-declared
/// they can not merge with anything in the value space
const BlockScopedVariableExcludes = Self::Value.bits();

const ClassExcludes = (Self::Value.bits() | Self::TypeAlias.bits()) & !(Self::ValueModule.bits() | Self::Interface.bits() | Self::Function.bits());
const ClassExcludes = (Self::Value.bits() | Self::TypeAlias.bits()) & !(Self::ValueModule.bits() | Self::Interface.bits());
const ImportBindingExcludes = Self::Import.bits() | Self::TypeImport.bits();
// Type specific excludes
const TypeAliasExcludes = Self::Type.bits();
Expand Down
19 changes: 2 additions & 17 deletions tasks/coverage/snapshots/parser_typescript.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ commit: d85767ab
parser_typescript Summary:
AST Parsed : 6494/6503 (99.86%)
Positive Passed: 6483/6503 (99.69%)
Negative Passed: 1239/5747 (21.56%)
Negative Passed: 1238/5747 (21.54%)
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration10.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration11.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration13.ts
Expand Down Expand Up @@ -1014,6 +1014,7 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/identityForS
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/identityForSignaturesWithTypeParametersSwitched.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ifElseWithStatements1.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ignoredJsxAttributes.tsx
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/illegalSuperCallsInConstructor.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/implementArrayInterface.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/implementClausePrecedingExtends.ts
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/implementGenericWithMismatchedTypes.ts
Expand Down Expand Up @@ -8045,22 +8046,6 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
╰────
help: Try insert a semicolon here

× Super calls are not permitted outside constructors or in nested functions inside constructors.
╭─[typescript/tests/cases/compiler/illegalSuperCallsInConstructor.ts:12:17]
11 │ get foo() {
12 │ super();
· ───────
13 │ return 1;
╰────

× Super calls are not permitted outside constructors or in nested functions inside constructors.
╭─[typescript/tests/cases/compiler/illegalSuperCallsInConstructor.ts:16:17]
15 │ set foo(v: number) {
16 │ super();
· ───────
17 │ }
╰────

× Identifier `x` has already been declared
╭─[typescript/tests/cases/compiler/importAndVariableDeclarationConflict3.ts:5:8]
4 │
Expand Down
Loading

0 comments on commit 70d22c4

Please sign in to comment.