Skip to content

Commit

Permalink
refactor(semantic): update the order of visit_function and Visit
Browse files Browse the repository at this point in the history
…fields in the builder to be consistent (#4248)

Same as #4195
  • Loading branch information
Dunqing committed Jul 14, 2024
1 parent 8bfeabf commit ace4f1f
Show file tree
Hide file tree
Showing 14 changed files with 145 additions and 141 deletions.
4 changes: 2 additions & 2 deletions crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ pub struct BindingRestElement<'a> {
#[scope(
// TODO: `ScopeFlags::Function` is not correct if this is a `MethodDefinition`
flags(flags.unwrap_or(ScopeFlags::empty()) | ScopeFlags::Function),
strict_if(self.body.as_ref().is_some_and(|body| body.has_use_strict_directive())),
strict_if(self.is_strict()),
)]
#[derive(Debug)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
Expand Down Expand Up @@ -1470,8 +1470,8 @@ pub struct Function<'a> {
/// ```
pub this_param: Option<TSThisParameter<'a>>,
pub params: Box<'a, FormalParameters<'a>>,
pub body: Option<Box<'a, FunctionBody<'a>>>,
pub return_type: Option<Box<'a, TSTypeAnnotation<'a>>>,
pub body: Option<Box<'a, FunctionBody<'a>>>,
pub scope_id: Cell<Option<ScopeId>>,
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/ast_builder_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ impl<'a> AstBuilder<'a> {
Option::<TSTypeParameterDeclaration>::None,
None,
params,
body,
Option::<TSTypeAnnotation>::None,
body,
))
}

Expand Down
50 changes: 25 additions & 25 deletions crates/oxc_ast/src/generated/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,14 +554,14 @@ impl<'a> AstBuilder<'a> {
type_parameters: T1,
this_param: Option<TSThisParameter<'a>>,
params: T2,
body: T3,
return_type: T4,
return_type: T3,
body: T4,
) -> Expression<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterDeclaration<'a>>>>,
T2: IntoIn<'a, Box<'a, FormalParameters<'a>>>,
T3: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T3: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
{
Expression::FunctionExpression(self.alloc(self.function(
r#type,
Expand All @@ -573,8 +573,8 @@ impl<'a> AstBuilder<'a> {
type_parameters,
this_param,
params,
body,
return_type,
body,
)))
}

Expand Down Expand Up @@ -2689,14 +2689,14 @@ impl<'a> AstBuilder<'a> {
type_parameters: T1,
this_param: Option<TSThisParameter<'a>>,
params: T2,
body: T3,
return_type: T4,
return_type: T3,
body: T4,
) -> Declaration<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterDeclaration<'a>>>>,
T2: IntoIn<'a, Box<'a, FormalParameters<'a>>>,
T3: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T3: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
{
Declaration::FunctionDeclaration(self.alloc(self.function(
r#type,
Expand All @@ -2708,8 +2708,8 @@ impl<'a> AstBuilder<'a> {
type_parameters,
this_param,
params,
body,
return_type,
body,
)))
}

Expand Down Expand Up @@ -3720,14 +3720,14 @@ impl<'a> AstBuilder<'a> {
type_parameters: T1,
this_param: Option<TSThisParameter<'a>>,
params: T2,
body: T3,
return_type: T4,
return_type: T3,
body: T4,
) -> Function<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterDeclaration<'a>>>>,
T2: IntoIn<'a, Box<'a, FormalParameters<'a>>>,
T3: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T3: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
{
Function {
r#type,
Expand All @@ -3739,8 +3739,8 @@ impl<'a> AstBuilder<'a> {
type_parameters: type_parameters.into_in(self.allocator),
this_param,
params: params.into_in(self.allocator),
body: body.into_in(self.allocator),
return_type: return_type.into_in(self.allocator),
body: body.into_in(self.allocator),
scope_id: Default::default(),
}
}
Expand All @@ -3757,14 +3757,14 @@ impl<'a> AstBuilder<'a> {
type_parameters: T1,
this_param: Option<TSThisParameter<'a>>,
params: T2,
body: T3,
return_type: T4,
return_type: T3,
body: T4,
) -> Box<'a, Function<'a>>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterDeclaration<'a>>>>,
T2: IntoIn<'a, Box<'a, FormalParameters<'a>>>,
T3: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T3: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
{
self.function(
r#type,
Expand All @@ -3776,8 +3776,8 @@ impl<'a> AstBuilder<'a> {
type_parameters,
this_param,
params,
body,
return_type,
body,
)
.into_in(self.allocator)
}
Expand Down Expand Up @@ -4901,14 +4901,14 @@ impl<'a> AstBuilder<'a> {
type_parameters: T1,
this_param: Option<TSThisParameter<'a>>,
params: T2,
body: T3,
return_type: T4,
return_type: T3,
body: T4,
) -> ExportDefaultDeclarationKind<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterDeclaration<'a>>>>,
T2: IntoIn<'a, Box<'a, FormalParameters<'a>>>,
T3: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T3: IntoIn<'a, Option<Box<'a, TSTypeAnnotation<'a>>>>,
T4: IntoIn<'a, Option<Box<'a, FunctionBody<'a>>>>,
{
ExportDefaultDeclarationKind::FunctionDeclaration(self.alloc(self.function(
r#type,
Expand All @@ -4920,8 +4920,8 @@ impl<'a> AstBuilder<'a> {
type_parameters,
this_param,
params,
body,
return_type,
body,
)))
}

Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_ast/src/generated/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3038,7 +3038,7 @@ pub mod walk {
visitor.enter_scope(
{
let mut flags = flags.unwrap_or(ScopeFlags::empty()) | ScopeFlags::Function;
if it.body.as_ref().is_some_and(|body| body.has_use_strict_directive()) {
if it.is_strict() {
flags |= ScopeFlags::StrictMode;
}
flags
Expand All @@ -3055,12 +3055,12 @@ pub mod walk {
visitor.visit_ts_this_parameter(this_param);
}
visitor.visit_formal_parameters(&it.params);
if let Some(body) = &it.body {
visitor.visit_function_body(body);
}
if let Some(return_type) = &it.return_type {
visitor.visit_ts_type_annotation(return_type);
}
if let Some(body) = &it.body {
visitor.visit_function_body(body);
}
visitor.leave_scope();
visitor.leave_node(kind);
}
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_ast/src/generated/visit_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3171,7 +3171,7 @@ pub mod walk_mut {
visitor.enter_scope(
{
let mut flags = flags.unwrap_or(ScopeFlags::empty()) | ScopeFlags::Function;
if it.body.as_ref().is_some_and(|body| body.has_use_strict_directive()) {
if it.is_strict() {
flags |= ScopeFlags::StrictMode;
}
flags
Expand All @@ -3188,12 +3188,12 @@ pub mod walk_mut {
visitor.visit_ts_this_parameter(this_param);
}
visitor.visit_formal_parameters(&mut it.params);
if let Some(body) = &mut it.body {
visitor.visit_function_body(body);
}
if let Some(return_type) = &mut it.return_type {
visitor.visit_ts_type_annotation(return_type);
}
if let Some(body) = &mut it.body {
visitor.visit_function_body(body);
}
visitor.leave_scope();
visitor.leave_node(kind);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ impl<'a> IsolatedDeclarations<'a> {
self.ast.copy(&function.type_parameters),
self.ast.copy(&function.this_param),
params,
Option::<FunctionBody>::None,
return_type,
Option::<FunctionBody>::None,
);

self.ast.class_element_method_definition(
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ impl<'a> IsolatedDeclarations<'a> {
self.ast.copy(&func.type_parameters),
self.ast.copy(&func.this_param),
params,
Option::<FunctionBody>::None,
return_type,
Option::<FunctionBody>::None,
))
}
}
Expand Down
16 changes: 8 additions & 8 deletions crates/oxc_linter/src/snapshots/ban_types.snap
Original file line number Diff line number Diff line change
Expand Up @@ -209,27 +209,27 @@ source: crates/oxc_linter/src/tester.rs
help: The `Function` type accepts any function-like value

⚠ typescript-eslint(ban-types): Do not use "String" as a type. Use "string" instead
╭─[ban_types.tsx:6:24]
╭─[ban_types.tsx:5:24]
4 │
5 │ arg(): Array<String> {
6const foo: String = 1 as String;
· ──────
7}
6 const foo: String = 1 as String;
╰────

typescript-eslint(ban-types): Do not use "String" as a type. Use "string" instead
╭─[ban_types.tsx:6:38]
╭─[ban_types.tsx:6:24]
5arg(): Array<String> {
6const foo: String = 1 as String;
· ──────
· ──────
7}
╰────

⚠ typescript-eslint(ban-types): Do not use "String" as a type. Use "string" instead
╭─[ban_types.tsx:5:24]
4 │
╭─[ban_types.tsx:6:38]
5 │ arg(): Array<String> {
· ──────
6const foo: String = 1 as String;
· ──────
7}
╰────

⚠ typescript-eslint(ban-types): Don't use `Function` as a type
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_parser/src/js/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ impl<'a> ParserImpl<'a> {
type_parameters,
this_param,
params,
body,
return_type,
body,
))
}

Expand Down
18 changes: 5 additions & 13 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ fn function_as_var(flags: ScopeFlags, source_type: SourceType) -> bool {
impl<'a> Binder 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 !builder.current_scope_flags().is_strict_mode()
if !scope_flags.is_strict_mode()
&& matches!(
builder.nodes.parent_kind(builder.current_node_id),
Some(AstKind::IfStatement(_))
Expand All @@ -118,12 +119,10 @@ impl<'a> Binder for Function<'a> {
} 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 parent_scope_id = builder.scope.get_parent_id(current_scope_id).unwrap();
let parent_flags = builder.scope.get_flags(parent_scope_id);

let (includes, excludes) =
if (parent_flags.is_strict_mode() || self.r#async || self.generator)
&& !function_as_var(parent_flags, builder.source_type)
if (scope_flags.is_strict_mode() || self.r#async || self.generator)
&& !function_as_var(scope_flags, builder.source_type)
{
(
SymbolFlags::Function | SymbolFlags::BlockScopedVariable,
Expand All @@ -136,13 +135,7 @@ impl<'a> Binder for Function<'a> {
)
};

let symbol_id = builder.declare_symbol_on_scope(
ident.span,
&ident.name,
parent_scope_id,
includes,
excludes,
);
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
Expand All @@ -158,7 +151,6 @@ impl<'a> Binder for Function<'a> {
}

// bind scope flags: Constructor | GetAccessor | SetAccessor
debug_assert!(builder.current_scope_flags().contains(ScopeFlags::Function));
if let Some(kind) = builder.nodes.parent_kind(builder.current_node_id) {
match kind {
AstKind::MethodDefinition(def) => {
Expand Down
Loading

0 comments on commit ace4f1f

Please sign in to comment.