Skip to content
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

refactor(ast)!: Change order of fields in CallExpression #4859

Merged
merged 9 commits into from
Aug 20, 2024
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,9 +606,9 @@ pub struct PrivateFieldExpression<'a> {
pub struct CallExpression<'a> {
#[serde(flatten)]
pub span: Span,
pub arguments: Vec<'a, Argument<'a>>,
pub callee: Expression<'a>,
pub type_parameters: Option<Box<'a, TSTypeParameterInstantiation<'a>>>,
pub arguments: Vec<'a, Argument<'a>>,
pub optional: bool, // for optional chaining
}

Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_ast/src/generated/assert_layouts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ const _: () = {
assert!(size_of::<CallExpression>() == 72usize);
assert!(align_of::<CallExpression>() == 8usize);
assert!(offset_of!(CallExpression, span) == 0usize);
assert!(offset_of!(CallExpression, arguments) == 8usize);
assert!(offset_of!(CallExpression, callee) == 40usize);
assert!(offset_of!(CallExpression, type_parameters) == 56usize);
assert!(offset_of!(CallExpression, callee) == 8usize);
assert!(offset_of!(CallExpression, type_parameters) == 24usize);
assert!(offset_of!(CallExpression, arguments) == 32usize);
assert!(offset_of!(CallExpression, optional) == 64usize);

assert!(size_of::<NewExpression>() == 64usize);
Expand Down Expand Up @@ -1583,9 +1583,9 @@ const _: () = {
assert!(size_of::<CallExpression>() == 40usize);
assert!(align_of::<CallExpression>() == 4usize);
assert!(offset_of!(CallExpression, span) == 0usize);
assert!(offset_of!(CallExpression, arguments) == 8usize);
assert!(offset_of!(CallExpression, callee) == 24usize);
assert!(offset_of!(CallExpression, type_parameters) == 32usize);
assert!(offset_of!(CallExpression, callee) == 8usize);
assert!(offset_of!(CallExpression, type_parameters) == 16usize);
assert!(offset_of!(CallExpression, arguments) == 20usize);
assert!(offset_of!(CallExpression, optional) == 36usize);

assert!(size_of::<NewExpression>() == 36usize);
Expand Down
24 changes: 12 additions & 12 deletions crates/oxc_ast/src/generated/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,27 +679,27 @@ impl<'a> AstBuilder<'a> {
///
/// ## Parameters
/// - span: The [`Span`] covering this node
/// - arguments
/// - callee
/// - type_parameters
/// - arguments
/// - optional
#[inline]
pub fn expression_call<T1>(
self,
span: Span,
arguments: Vec<'a, Argument<'a>>,
callee: Expression<'a>,
type_parameters: T1,
arguments: Vec<'a, Argument<'a>>,
optional: bool,
) -> Expression<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterInstantiation<'a>>>>,
{
Expression::CallExpression(self.alloc(self.call_expression(
span,
arguments,
callee,
type_parameters,
arguments,
optional,
)))
}
Expand Down Expand Up @@ -2294,27 +2294,27 @@ impl<'a> AstBuilder<'a> {
///
/// ## Parameters
/// - span: The [`Span`] covering this node
/// - arguments
/// - callee
/// - type_parameters
/// - arguments
/// - optional
#[inline]
pub fn call_expression<T1>(
self,
span: Span,
arguments: Vec<'a, Argument<'a>>,
callee: Expression<'a>,
type_parameters: T1,
arguments: Vec<'a, Argument<'a>>,
optional: bool,
) -> CallExpression<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterInstantiation<'a>>>>,
{
CallExpression {
span,
arguments,
callee,
type_parameters: type_parameters.into_in(self.allocator),
arguments,
optional,
}
}
Expand All @@ -2325,24 +2325,24 @@ impl<'a> AstBuilder<'a> {
///
/// ## Parameters
/// - span: The [`Span`] covering this node
/// - arguments
/// - callee
/// - type_parameters
/// - arguments
/// - optional
#[inline]
pub fn alloc_call_expression<T1>(
self,
span: Span,
arguments: Vec<'a, Argument<'a>>,
callee: Expression<'a>,
type_parameters: T1,
arguments: Vec<'a, Argument<'a>>,
optional: bool,
) -> Box<'a, CallExpression<'a>>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterInstantiation<'a>>>>,
{
Box::new_in(
self.call_expression(span, arguments, callee, type_parameters, optional),
self.call_expression(span, callee, type_parameters, arguments, optional),
self.allocator,
)
}
Expand Down Expand Up @@ -3496,27 +3496,27 @@ impl<'a> AstBuilder<'a> {
///
/// ## Parameters
/// - span: The [`Span`] covering this node
/// - arguments
/// - callee
/// - type_parameters
/// - arguments
/// - optional
#[inline]
pub fn chain_element_call_expression<T1>(
self,
span: Span,
arguments: Vec<'a, Argument<'a>>,
callee: Expression<'a>,
type_parameters: T1,
arguments: Vec<'a, Argument<'a>>,
optional: bool,
) -> ChainElement<'a>
where
T1: IntoIn<'a, Option<Box<'a, TSTypeParameterInstantiation<'a>>>>,
{
ChainElement::CallExpression(self.alloc(self.call_expression(
span,
arguments,
callee,
type_parameters,
arguments,
optional,
)))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/generated/derive_clone_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,9 @@ impl<'old_alloc, 'new_alloc> CloneIn<'new_alloc> for CallExpression<'old_alloc>
fn clone_in(&self, allocator: &'new_alloc Allocator) -> Self::Cloned {
CallExpression {
span: self.span.clone_in(allocator),
arguments: self.arguments.clone_in(allocator),
callee: self.callee.clone_in(allocator),
type_parameters: self.type_parameters.clone_in(allocator),
arguments: self.arguments.clone_in(allocator),
optional: self.optional.clone_in(allocator),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/generated/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2900,11 +2900,11 @@ pub mod walk {
pub fn walk_call_expression<'a, V: Visit<'a>>(visitor: &mut V, it: &CallExpression<'a>) {
let kind = AstKind::CallExpression(visitor.alloc(it));
visitor.enter_node(kind);
visitor.visit_arguments(&it.arguments);
visitor.visit_expression(&it.callee);
if let Some(type_parameters) = &it.type_parameters {
visitor.visit_ts_type_parameter_instantiation(type_parameters);
}
visitor.visit_arguments(&it.arguments);
visitor.leave_node(kind);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/generated/visit_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3024,11 +3024,11 @@ pub mod walk_mut {
pub fn walk_call_expression<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut CallExpression<'a>) {
let kind = AstType::CallExpression;
visitor.enter_node(kind);
visitor.visit_arguments(&mut it.arguments);
visitor.visit_expression(&mut it.callee);
if let Some(type_parameters) = &mut it.type_parameters {
visitor.visit_ts_type_parameter_instantiation(type_parameters);
}
visitor.visit_arguments(&mut it.arguments);
visitor.leave_node(kind);
}

Expand Down
38 changes: 32 additions & 6 deletions crates/oxc_linter/src/rules/eslint/no_this_before_super.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::{HashMap, HashSet};

use oxc_ast::{
ast::{Expression, MethodDefinitionKind},
ast::{Argument, Expression, MethodDefinitionKind},
AstKind,
};
use oxc_cfg::{
Expand Down Expand Up @@ -73,11 +73,13 @@ impl Rule for NoThisBeforeSuper {
AstKind::Super(_) => {
let basic_block_id = node.cfg_id();
if let Some(parent) = semantic.nodes().parent_node(node.id()) {
if let AstKind::CallExpression(_) = parent.kind() {
// Note: we don't need to worry about also having invalid
// usage in the same callexpression, because arguments are visited
// before the callee in generating the semantic nodes.
basic_blocks_with_super_called.insert(basic_block_id);
if let AstKind::CallExpression(call_expr) = parent.kind() {
let has_this_or_super_in_args =
Self::contains_this_or_super_in_args(&call_expr.arguments);

if !has_this_or_super_in_args {
basic_blocks_with_super_called.insert(basic_block_id);
}
}
}
if !basic_blocks_with_super_called.contains(&basic_block_id) {
Expand Down Expand Up @@ -246,6 +248,27 @@ impl NoThisBeforeSuper {
}),
})
}

fn contains_this_or_super(arg: &Argument) -> bool {
match arg {
Argument::Super(_) | Argument::ThisExpression(_) => true,
Argument::CallExpression(call_expr) => {
matches!(&call_expr.callee, Expression::Super(_) | Expression::ThisExpression(_))
|| matches!(&call_expr.callee,
Expression::StaticMemberExpression(static_member) if
matches!(static_member.object, Expression::Super(_) | Expression::ThisExpression(_)))
|| Self::contains_this_or_super_in_args(&call_expr.arguments)
}
Argument::StaticMemberExpression(call_expr) => {
matches!(&call_expr.object, Expression::Super(_) | Expression::ThisExpression(_))
}
_ => false,
}
}

fn contains_this_or_super_in_args(args: &[Argument]) -> bool {
args.iter().any(|arg| Self::contains_this_or_super(arg))
}
}

#[test]
Expand Down Expand Up @@ -382,8 +405,11 @@ fn test() {
("class A extends B { constructor() { this.c(); super(); } }", None),
("class A extends B { constructor() { super.c(); super(); } }", None),
// disallows `this`/`super` in arguments of `super()`.
("class A extends B { constructor() { super(this); } }", None),
("class A extends B { constructor() { super(this.c); } }", None),
("class A extends B { constructor() { super(a(b(this.c))); } }", None),
("class A extends B { constructor() { super(this.c()); } }", None),
("class A extends B { constructor() { super(super.c); } }", None),
("class A extends B { constructor() { super(super.c()); } }", None),
// // even if is nested, reports correctly.
(
Expand Down
41 changes: 20 additions & 21 deletions crates/oxc_linter/src/snapshots/no_conditional_expect.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/oxc_linter/src/tester.rs
assertion_line: 216
---
⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
╭─[no_conditional_expect.tsx:3:34]
Expand Down Expand Up @@ -327,12 +326,12 @@ assertion_line: 216
help: Avoid calling `expect` conditionally`

⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
╭─[no_conditional_expect.tsx:9:37]
8 │ .then(() => { throw new Error('oh noes!'); })
9 │ .catch(error => expect(error).toBeInstanceOf(Error));
· ──────
10});
╰────
╭─[no_conditional_expect.tsx:5:37]
4 │ .then(() => { throw new Error('oh noes!'); })
5 │ .catch(error => expect(error).toBeInstanceOf(Error))
· ──────
6 .then(() => { throw new Error('oh noes!'); })
╰────
help: Avoid calling `expect` conditionally`

⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
Expand All @@ -345,20 +344,20 @@ assertion_line: 216
help: Avoid calling `expect` conditionally`

⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
╭─[no_conditional_expect.tsx:5:37]
4 │ .then(() => { throw new Error('oh noes!'); })
5 │ .catch(error => expect(error).toBeInstanceOf(Error))
· ──────
6 .then(() => { throw new Error('oh noes!'); })
╰────
╭─[no_conditional_expect.tsx:9:37]
8 │ .then(() => { throw new Error('oh noes!'); })
9 │ .catch(error => expect(error).toBeInstanceOf(Error));
· ──────
10});
╰────
help: Avoid calling `expect` conditionally`

⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
╭─[no_conditional_expect.tsx:6:32]
5 .catch(error => expect(error).toBeInstanceOf(Error))
6 │ .catch(error => expect(error).toBeInstanceOf(Error));
╭─[no_conditional_expect.tsx:4:32]
3await Promise.resolve()
4 │ .catch(error => expect(error).toBeInstanceOf(Error))
· ──────
7});
5 .catch(error => expect(error).toBeInstanceOf(Error))
╰────
help: Avoid calling `expect` conditionally`

Expand All @@ -372,11 +371,11 @@ assertion_line: 216
help: Avoid calling `expect` conditionally`

⚠ eslint-plugin-vitest(no-conditional-expect): Unexpected conditional expect
╭─[no_conditional_expect.tsx:4:32]
3 │ await Promise.resolve()
4 │ .catch(error => expect(error).toBeInstanceOf(Error))
· ──────
╭─[no_conditional_expect.tsx:6:32]
5 │ .catch(error => expect(error).toBeInstanceOf(Error))
6 │ .catch(error => expect(error).toBeInstanceOf(Error));
· ──────
7 │ });
╰────
help: Avoid calling `expect` conditionally`

Expand Down
Loading
Loading