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 f40c689
Show file tree
Hide file tree
Showing 63 changed files with 17,741 additions and 5,461 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
12 changes: 11 additions & 1 deletion crates/oxc_linter/src/rules/eslint/no_setter_return.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use oxc_ast::AstKind;
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::ScopeFlags;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};
Expand Down Expand Up @@ -41,7 +42,16 @@ impl Rule for NoSetterReturn {
let AstKind::ReturnStatement(stmt) = node.kind() else {
return;
};
if stmt.argument.is_some() && ctx.scopes().get_flags(node.scope_id()).is_set_accessor() {

if stmt.argument.is_some() {
for scope_id in ctx.scopes().ancestors(node.scope_id()) {
let flags = ctx.scopes().get_flags(scope_id);
if flags.is_set_accessor() {
break;
} else if flags.intersects(ScopeFlags::Function | ScopeFlags::Top) {
return;
}
}
ctx.diagnostic(no_setter_return_diagnostic(stmt.span));
}
}
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 @@ -614,9 +614,6 @@ impl<'s, 'a> Symbol<'s, '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
49 changes: 24 additions & 25 deletions crates/oxc_linter/src/snapshots/no_shadow_restricted_names.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/oxc_linter/src/tester.rs
snapshot_kind: text
---
eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:10]
Expand All @@ -24,16 +23,16 @@ snapshot_kind: text
help: Shadowing of global properties 'NaN'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:40]
╭─[no_shadow_restricted_names.tsx:1:44]
1function NaN(NaN) { var NaN; !function NaN(NaN) { try {} catch(NaN) {} }; }
· ───
· ───
╰────
help: Shadowing of global properties 'NaN'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:44]
╭─[no_shadow_restricted_names.tsx:1:40]
1function NaN(NaN) { var NaN; !function NaN(NaN) { try {} catch(NaN) {} }; }
· ───
· ───
╰────
help: Shadowing of global properties 'NaN'.

Expand All @@ -59,16 +58,16 @@ snapshot_kind: text
help: Shadowing of global properties 'undefined'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:43]
╭─[no_shadow_restricted_names.tsx:1:53]
1function undefined(undefined) { !function undefined(undefined) { try {} catch(undefined) {} }; }
· ─────────
· ─────────
╰────
help: Shadowing of global properties 'undefined'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:53]
╭─[no_shadow_restricted_names.tsx:1:43]
1function undefined(undefined) { !function undefined(undefined) { try {} catch(undefined) {} }; }
· ─────────
· ─────────
╰────
help: Shadowing of global properties 'undefined'.

Expand Down Expand Up @@ -101,16 +100,16 @@ snapshot_kind: text
help: Shadowing of global properties 'Infinity'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:55]
╭─[no_shadow_restricted_names.tsx:1:64]
1function Infinity(Infinity) { var Infinity; !function Infinity(Infinity) { try {} catch(Infinity) {} }; }
· ────────
· ────────
╰────
help: Shadowing of global properties 'Infinity'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:64]
╭─[no_shadow_restricted_names.tsx:1:55]
1function Infinity(Infinity) { var Infinity; !function Infinity(Infinity) { try {} catch(Infinity) {} }; }
· ────────
· ────────
╰────
help: Shadowing of global properties 'Infinity'.

Expand Down Expand Up @@ -143,16 +142,16 @@ snapshot_kind: text
help: Shadowing of global properties 'arguments'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:58]
╭─[no_shadow_restricted_names.tsx:1:68]
1function arguments(arguments) { var arguments; !function arguments(arguments) { try {} catch(arguments) {} }; }
· ─────────
· ─────────
╰────
help: Shadowing of global properties 'arguments'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:68]
╭─[no_shadow_restricted_names.tsx:1:58]
1function arguments(arguments) { var arguments; !function arguments(arguments) { try {} catch(arguments) {} }; }
· ─────────
· ─────────
╰────
help: Shadowing of global properties 'arguments'.

Expand Down Expand Up @@ -185,16 +184,16 @@ snapshot_kind: text
help: Shadowing of global properties 'eval'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:43]
╭─[no_shadow_restricted_names.tsx:1:48]
1function eval(eval) { var eval; !function eval(eval) { try {} catch(eval) {} }; }
· ────
· ────
╰────
help: Shadowing of global properties 'eval'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:48]
╭─[no_shadow_restricted_names.tsx:1:43]
1function eval(eval) { var eval; !function eval(eval) { try {} catch(eval) {} }; }
· ────
· ────
╰────
help: Shadowing of global properties 'eval'.

Expand Down Expand Up @@ -227,16 +226,16 @@ snapshot_kind: text
help: Shadowing of global properties 'eval'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:44]
╭─[no_shadow_restricted_names.tsx:1:49]
1var eval = (eval) => { var eval; !function eval(eval) { try {} catch(eval) {} }; }
· ────
· ────
╰────
help: Shadowing of global properties 'eval'.

eslint(no-shadow-restricted-names): Shadowing of global properties such as 'undefined' is not allowed.
╭─[no_shadow_restricted_names.tsx:1:49]
╭─[no_shadow_restricted_names.tsx:1:44]
1var eval = (eval) => { var eval; !function eval(eval) { try {} catch(eval) {} }; }
· ────
· ────
╰────
help: Shadowing of global properties 'eval'.

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/snapshots/[email protected]
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]
1function 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]
1function f(){var x;function a(){x=42;}function b(){alert(x);}}
· ┬
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_linter/src/snapshots/[email protected]
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/oxc_linter/src/tester.rs
snapshot_kind: text
---
eslint(no-unused-vars): Function 'foo' is declared but never used.
╭─[no_unused_vars.tsx:1:10]
Expand All @@ -18,7 +17,7 @@ snapshot_kind: text
╰────
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]
1const 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 @@ -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
Loading

0 comments on commit f40c689

Please sign in to comment.