Skip to content

refactor(semantic): always use SymbolFlags::Function for function id #7479

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

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
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 @@ -1316,11 +1316,7 @@ impl<'a> Function<'a> {

/// Returns `true` if this function uses overload signatures or `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 @@ -1349,6 +1345,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
3 changes: 0 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,6 @@ impl<'a> Symbol<'_, 'a> {
let Some(ref_node) = self.get_ref_relevant_node(reference) else {
return false;
};
if !matches!(ref_node.kind(), AstKind::CallExpression(_) | AstKind::NewExpression(_)) {
return false;
}

// Do the easy/fast path if possible. If we know its a class/fn from
// flags, that means it's declared within this file in an understandable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Variable 'a' is declared but never used.
⚠ eslint(no-unused-vars): Function 'a' is declared but never used.
╭─[no_unused_vars.tsx:1:29]
1 │ function f(){var x;function a(){x=42;}function b(){alert(x);}}
· ┬
· ╰── 'a' is declared here
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Variable 'b' is declared but never used.
⚠ eslint(no-unused-vars): Function 'b' is declared but never used.
╭─[no_unused_vars.tsx:1:48]
1 │ function f(){var x;function a(){x=42;}function b(){alert(x);}}
· ┬
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider removing this declaration.

⚠ eslint(no-unused-vars): Variable 'bar' is declared but never used.
⚠ eslint(no-unused-vars): Function 'bar' is declared but never used.
╭─[no_unused_vars.tsx:1:30]
1 │ const foo = () => { function bar() { } }
· ─┬─
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 @@ -141,6 +141,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 @@ -165,8 +175,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.scoping.create_symbol(
Expand All @@ -177,35 +188,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 @@ -215,7 +221,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.scoping.scope_flags_mut(current_scope_id);
let flags = builder.scoping.scope_flags_mut(builder.current_scope_id);
match prop.kind {
PropertyKind::Get => *flags |= ScopeFlags::GetAccessor,
PropertyKind::Set => *flags |= ScopeFlags::SetAccessor,
Expand Down
38 changes: 35 additions & 3 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use oxc_syntax::{

use crate::{
JSDocFinder, Semantic,
binder::Binder,
binder::{Binder, is_function_redeclared_not_allowed},
checker,
class::ClassTableBuilder,
diagnostics::redeclaration,
Expand Down Expand Up @@ -425,11 +425,43 @@ impl<'a> SemanticBuilder<'a> {
let symbol_id = self.scoping.get_binding(scope_id, name).or_else(|| {
self.hoisting_variables.get(&scope_id).and_then(|symbols| symbols.get(name).copied())
})?;
if report_error && self.scoping.symbol_flags(symbol_id).intersects(excludes) {
if report_error {
self.check_and_report_redeclaration(name, span, symbol_id, excludes);
}
Some(symbol_id)
}

/// Check if a symbol with the same name has already been declared but
/// it actually is not allowed to be redeclared.
fn check_and_report_redeclaration(
&self,
name: &str,
span: Span,
symbol_id: SymbolId,
excludes: SymbolFlags,
) {
let flags = self.scoping.symbol_flags(symbol_id);
let function_kind = if flags.is_function() {
self.nodes.kind(self.scoping.symbol_declaration(symbol_id)).as_function()
} else {
None
};

if (
flags.intersects(excludes)
// `function n() { let n; }`
// ^ is not a redeclaration
&& !function_kind.is_some_and(Function::is_expression))
// 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 &&
function_kind.is_some_and(|func| is_function_redeclared_not_allowed(func, self)))
{
let symbol_span = self.scoping.symbol_span(symbol_id);
self.error(redeclaration(name, symbol_span, span));
}
Some(symbol_id)
}

/// Declare an unresolved reference in the current scope.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/oxc_semantic/tests/main.rs
input_file: crates/oxc_semantic/tests/fixtures/oxc/ts/functions/return-type.ts
snapshot_kind: text
---
[
{
Expand Down Expand Up @@ -84,7 +85,7 @@ input_file: crates/oxc_semantic/tests/fixtures/oxc/ts/functions/return-type.ts
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "Foo",
"node": "Function(Foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declarati
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "f",
"node": "Function(f)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declarati
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "f",
"node": "Function(f)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/acce
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/clas
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/clas
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/meth
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/para
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/para
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/decorators/type
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "decorator",
"node": "Function(decorator)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/export/default1
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "f",
"node": "Function(f)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
"references": []
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
"node": "Program",
"symbols": [
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 0,
"name": "foo",
"node": "Function(foo)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/functions/funct
]
},
{
"flags": "SymbolFlags(BlockScopedVariable | Function)",
"flags": "SymbolFlags(Function)",
"id": 1,
"name": "foo",
"node": "Function(foo)",
Expand Down
Loading