diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index 48e5d6d0ce352..b98cd035dd5b1 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -430,7 +430,7 @@ impl<'a> From> for ArrayExpressionElement<'a> { fn from(argument: Argument<'a>) -> Self { match argument { Argument::SpreadElement(spread) => Self::SpreadElement(spread), - match_expression!(Argument) => Self::from(argument.into_expression()), + _ => Self::from(argument.into_expression()), } } } diff --git a/crates/oxc_ast/src/ast_kind_impl.rs b/crates/oxc_ast/src/ast_kind_impl.rs index d56ef32ec254f..9601ee7211a78 100644 --- a/crates/oxc_ast/src/ast_kind_impl.rs +++ b/crates/oxc_ast/src/ast_kind_impl.rs @@ -122,6 +122,54 @@ impl<'a> AstKind<'a> { matches!(self, Self::Function(_) | Self::ArrowFunctionExpression(_)) } + /// Check if this CallExpression or NewExpression has an argument with the given span + /// + /// This is useful for determining if a node is an argument to a call expression + /// when traversing the AST, particularly after the removal of `AstKind::Argument`. + /// + /// # Examples + /// + /// ```rust,ignore + /// // Check if a node is an argument to its parent call expression + /// if parent.has_argument_with_span(node.span()) { + /// // This node is an argument + /// } + /// ``` + #[inline] + pub fn has_argument_with_span(&self, span: oxc_span::Span) -> bool { + match self { + Self::CallExpression(call) => call.arguments.iter().any(|arg| arg.span() == span), + Self::NewExpression(new_expr) => { + new_expr.arguments.iter().any(|arg| arg.span() == span) + } + _ => false, + } + } + + /// Check if this CallExpression or NewExpression has the given span as its callee + /// + /// This is useful for determining if a node is the callee of a call expression + /// when traversing the AST. + /// + /// # Examples + /// + /// ```rust,ignore + /// // Detect eval() calls + /// if let AstKind::IdentifierReference(ident) = node.kind() { + /// if parent.is_callee_with_span(ident.span) && ident.name == "eval" { + /// // This is an eval() call + /// } + /// } + /// ``` + #[inline] + pub fn is_callee_with_span(&self, span: oxc_span::Span) -> bool { + match self { + Self::CallExpression(call) => call.callee.span() == span, + Self::NewExpression(new_expr) => new_expr.callee.span() == span, + _ => false, + } + } + /// Get the name of an identifier node /// /// Returns the identifier name if this is any kind of identifier node, @@ -415,7 +463,6 @@ impl AstKind<'_> { Self::ObjectProperty(p) => { format!("ObjectProperty({})", p.key.name().unwrap_or(COMPUTED)).into() } - Self::Argument(_) => "Argument".into(), Self::ArrayAssignmentTarget(_) => "ArrayAssignmentTarget".into(), Self::ObjectAssignmentTarget(_) => "ObjectAssignmentTarget".into(), Self::AssignmentTargetWithDefault(_) => "AssignmentTargetWithDefault".into(), @@ -789,3 +836,49 @@ impl GetAddress for PropertyKeyKind<'_> { } } } +#[cfg(test)] +mod tests { + use super::*; + use oxc_span::Span; + + // Note: These tests verify the logic of the methods. + // Integration tests using real parsed AST are in the linter crate. + + #[test] + fn test_has_argument_with_span_returns_false_for_non_call_expressions() { + // Test that non-CallExpression/NewExpression AstKinds always return false + let test_span = Span::new(0, 5); + + let num_lit = NumericLiteral { + span: test_span, + value: 42.0, + raw: None, + base: oxc_syntax::number::NumberBase::Decimal, + }; + let num_kind = AstKind::NumericLiteral(&num_lit); + assert!(!num_kind.has_argument_with_span(test_span)); + + let bool_lit = BooleanLiteral { span: test_span, value: true }; + let bool_kind = AstKind::BooleanLiteral(&bool_lit); + assert!(!bool_kind.has_argument_with_span(test_span)); + } + + #[test] + fn test_is_callee_with_span_returns_false_for_non_call_expressions() { + // Test that non-CallExpression/NewExpression AstKinds always return false + let test_span = Span::new(0, 5); + + let num_lit = NumericLiteral { + span: test_span, + value: 42.0, + raw: None, + base: oxc_syntax::number::NumberBase::Decimal, + }; + let num_kind = AstKind::NumericLiteral(&num_lit); + assert!(!num_kind.is_callee_with_span(test_span)); + + let bool_lit = BooleanLiteral { span: test_span, value: true }; + let bool_kind = AstKind::BooleanLiteral(&bool_lit); + assert!(!bool_kind.is_callee_with_span(test_span)); + } +} diff --git a/crates/oxc_ast/src/generated/ast_kind.rs b/crates/oxc_ast/src/generated/ast_kind.rs index 748a081e36e96..2c66b27c07574 100644 --- a/crates/oxc_ast/src/generated/ast_kind.rs +++ b/crates/oxc_ast/src/generated/ast_kind.rs @@ -12,7 +12,7 @@ use oxc_span::{GetSpan, Span}; use crate::ast::*; /// The largest integer value that can be mapped to an `AstType`/`AstKind` enum variant. -pub const AST_TYPE_MAX: u8 = 186; +pub const AST_TYPE_MAX: u8 = 185; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[repr(u8)] @@ -37,173 +37,172 @@ pub enum AstType { NewExpression = 17, MetaProperty = 18, SpreadElement = 19, - Argument = 20, - UpdateExpression = 21, - UnaryExpression = 22, - BinaryExpression = 23, - PrivateInExpression = 24, - LogicalExpression = 25, - ConditionalExpression = 26, - AssignmentExpression = 27, - ArrayAssignmentTarget = 28, - ObjectAssignmentTarget = 29, - AssignmentTargetRest = 30, - AssignmentTargetWithDefault = 31, - AssignmentTargetPropertyIdentifier = 32, - AssignmentTargetPropertyProperty = 33, - SequenceExpression = 34, - Super = 35, - AwaitExpression = 36, - ChainExpression = 37, - ParenthesizedExpression = 38, - Directive = 39, - Hashbang = 40, - BlockStatement = 41, - VariableDeclaration = 42, - VariableDeclarator = 43, - EmptyStatement = 44, - ExpressionStatement = 45, - IfStatement = 46, - DoWhileStatement = 47, - WhileStatement = 48, - ForStatement = 49, - ForInStatement = 50, - ForOfStatement = 51, - ContinueStatement = 52, - BreakStatement = 53, - ReturnStatement = 54, - WithStatement = 55, - SwitchStatement = 56, - SwitchCase = 57, - LabeledStatement = 58, - ThrowStatement = 59, - TryStatement = 60, - CatchClause = 61, - CatchParameter = 62, - DebuggerStatement = 63, - AssignmentPattern = 64, - ObjectPattern = 65, - BindingProperty = 66, - ArrayPattern = 67, - BindingRestElement = 68, - Function = 69, - FormalParameters = 70, - FormalParameter = 71, - FunctionBody = 72, - ArrowFunctionExpression = 73, - YieldExpression = 74, - Class = 75, - ClassBody = 76, - MethodDefinition = 77, - PropertyDefinition = 78, - PrivateIdentifier = 79, - StaticBlock = 80, - AccessorProperty = 81, - ImportExpression = 82, - ImportDeclaration = 83, - ImportSpecifier = 84, - ImportDefaultSpecifier = 85, - ImportNamespaceSpecifier = 86, - WithClause = 87, - ImportAttribute = 88, - ExportNamedDeclaration = 89, - ExportDefaultDeclaration = 90, - ExportAllDeclaration = 91, - ExportSpecifier = 92, - V8IntrinsicExpression = 93, - BooleanLiteral = 94, - NullLiteral = 95, - NumericLiteral = 96, - StringLiteral = 97, - BigIntLiteral = 98, - RegExpLiteral = 99, - JSXElement = 100, - JSXOpeningElement = 101, - JSXClosingElement = 102, - JSXFragment = 103, - JSXOpeningFragment = 104, - JSXClosingFragment = 105, - JSXNamespacedName = 106, - JSXMemberExpression = 107, - JSXExpressionContainer = 108, - JSXEmptyExpression = 109, - JSXAttribute = 110, - JSXSpreadAttribute = 111, - JSXIdentifier = 112, - JSXSpreadChild = 113, - JSXText = 114, - TSThisParameter = 115, - TSEnumDeclaration = 116, - TSEnumBody = 117, - TSEnumMember = 118, - TSTypeAnnotation = 119, - TSLiteralType = 120, - TSConditionalType = 121, - TSUnionType = 122, - TSIntersectionType = 123, - TSParenthesizedType = 124, - TSTypeOperator = 125, - TSArrayType = 126, - TSIndexedAccessType = 127, - TSTupleType = 128, - TSNamedTupleMember = 129, - TSOptionalType = 130, - TSRestType = 131, - TSAnyKeyword = 132, - TSStringKeyword = 133, - TSBooleanKeyword = 134, - TSNumberKeyword = 135, - TSNeverKeyword = 136, - TSIntrinsicKeyword = 137, - TSUnknownKeyword = 138, - TSNullKeyword = 139, - TSUndefinedKeyword = 140, - TSVoidKeyword = 141, - TSSymbolKeyword = 142, - TSThisType = 143, - TSObjectKeyword = 144, - TSBigIntKeyword = 145, - TSTypeReference = 146, - TSQualifiedName = 147, - TSTypeParameterInstantiation = 148, - TSTypeParameter = 149, - TSTypeParameterDeclaration = 150, - TSTypeAliasDeclaration = 151, - TSClassImplements = 152, - TSInterfaceDeclaration = 153, - TSInterfaceBody = 154, - TSPropertySignature = 155, - TSIndexSignature = 156, - TSCallSignatureDeclaration = 157, - TSMethodSignature = 158, - TSConstructSignatureDeclaration = 159, - TSIndexSignatureName = 160, - TSInterfaceHeritage = 161, - TSTypePredicate = 162, - TSModuleDeclaration = 163, - TSModuleBlock = 164, - TSTypeLiteral = 165, - TSInferType = 166, - TSTypeQuery = 167, - TSImportType = 168, - TSImportTypeQualifiedName = 169, - TSFunctionType = 170, - TSConstructorType = 171, - TSMappedType = 172, - TSTemplateLiteralType = 173, - TSAsExpression = 174, - TSSatisfiesExpression = 175, - TSTypeAssertion = 176, - TSImportEqualsDeclaration = 177, - TSExternalModuleReference = 178, - TSNonNullExpression = 179, - Decorator = 180, - TSExportAssignment = 181, - TSNamespaceExportDeclaration = 182, - TSInstantiationExpression = 183, - JSDocNullableType = 184, - JSDocNonNullableType = 185, - JSDocUnknownType = 186, + UpdateExpression = 20, + UnaryExpression = 21, + BinaryExpression = 22, + PrivateInExpression = 23, + LogicalExpression = 24, + ConditionalExpression = 25, + AssignmentExpression = 26, + ArrayAssignmentTarget = 27, + ObjectAssignmentTarget = 28, + AssignmentTargetRest = 29, + AssignmentTargetWithDefault = 30, + AssignmentTargetPropertyIdentifier = 31, + AssignmentTargetPropertyProperty = 32, + SequenceExpression = 33, + Super = 34, + AwaitExpression = 35, + ChainExpression = 36, + ParenthesizedExpression = 37, + Directive = 38, + Hashbang = 39, + BlockStatement = 40, + VariableDeclaration = 41, + VariableDeclarator = 42, + EmptyStatement = 43, + ExpressionStatement = 44, + IfStatement = 45, + DoWhileStatement = 46, + WhileStatement = 47, + ForStatement = 48, + ForInStatement = 49, + ForOfStatement = 50, + ContinueStatement = 51, + BreakStatement = 52, + ReturnStatement = 53, + WithStatement = 54, + SwitchStatement = 55, + SwitchCase = 56, + LabeledStatement = 57, + ThrowStatement = 58, + TryStatement = 59, + CatchClause = 60, + CatchParameter = 61, + DebuggerStatement = 62, + AssignmentPattern = 63, + ObjectPattern = 64, + BindingProperty = 65, + ArrayPattern = 66, + BindingRestElement = 67, + Function = 68, + FormalParameters = 69, + FormalParameter = 70, + FunctionBody = 71, + ArrowFunctionExpression = 72, + YieldExpression = 73, + Class = 74, + ClassBody = 75, + MethodDefinition = 76, + PropertyDefinition = 77, + PrivateIdentifier = 78, + StaticBlock = 79, + AccessorProperty = 80, + ImportExpression = 81, + ImportDeclaration = 82, + ImportSpecifier = 83, + ImportDefaultSpecifier = 84, + ImportNamespaceSpecifier = 85, + WithClause = 86, + ImportAttribute = 87, + ExportNamedDeclaration = 88, + ExportDefaultDeclaration = 89, + ExportAllDeclaration = 90, + ExportSpecifier = 91, + V8IntrinsicExpression = 92, + BooleanLiteral = 93, + NullLiteral = 94, + NumericLiteral = 95, + StringLiteral = 96, + BigIntLiteral = 97, + RegExpLiteral = 98, + JSXElement = 99, + JSXOpeningElement = 100, + JSXClosingElement = 101, + JSXFragment = 102, + JSXOpeningFragment = 103, + JSXClosingFragment = 104, + JSXNamespacedName = 105, + JSXMemberExpression = 106, + JSXExpressionContainer = 107, + JSXEmptyExpression = 108, + JSXAttribute = 109, + JSXSpreadAttribute = 110, + JSXIdentifier = 111, + JSXSpreadChild = 112, + JSXText = 113, + TSThisParameter = 114, + TSEnumDeclaration = 115, + TSEnumBody = 116, + TSEnumMember = 117, + TSTypeAnnotation = 118, + TSLiteralType = 119, + TSConditionalType = 120, + TSUnionType = 121, + TSIntersectionType = 122, + TSParenthesizedType = 123, + TSTypeOperator = 124, + TSArrayType = 125, + TSIndexedAccessType = 126, + TSTupleType = 127, + TSNamedTupleMember = 128, + TSOptionalType = 129, + TSRestType = 130, + TSAnyKeyword = 131, + TSStringKeyword = 132, + TSBooleanKeyword = 133, + TSNumberKeyword = 134, + TSNeverKeyword = 135, + TSIntrinsicKeyword = 136, + TSUnknownKeyword = 137, + TSNullKeyword = 138, + TSUndefinedKeyword = 139, + TSVoidKeyword = 140, + TSSymbolKeyword = 141, + TSThisType = 142, + TSObjectKeyword = 143, + TSBigIntKeyword = 144, + TSTypeReference = 145, + TSQualifiedName = 146, + TSTypeParameterInstantiation = 147, + TSTypeParameter = 148, + TSTypeParameterDeclaration = 149, + TSTypeAliasDeclaration = 150, + TSClassImplements = 151, + TSInterfaceDeclaration = 152, + TSInterfaceBody = 153, + TSPropertySignature = 154, + TSIndexSignature = 155, + TSCallSignatureDeclaration = 156, + TSMethodSignature = 157, + TSConstructSignatureDeclaration = 158, + TSIndexSignatureName = 159, + TSInterfaceHeritage = 160, + TSTypePredicate = 161, + TSModuleDeclaration = 162, + TSModuleBlock = 163, + TSTypeLiteral = 164, + TSInferType = 165, + TSTypeQuery = 166, + TSImportType = 167, + TSImportTypeQualifiedName = 168, + TSFunctionType = 169, + TSConstructorType = 170, + TSMappedType = 171, + TSTemplateLiteralType = 172, + TSAsExpression = 173, + TSSatisfiesExpression = 174, + TSTypeAssertion = 175, + TSImportEqualsDeclaration = 176, + TSExternalModuleReference = 177, + TSNonNullExpression = 178, + Decorator = 179, + TSExportAssignment = 180, + TSNamespaceExportDeclaration = 181, + TSInstantiationExpression = 182, + JSDocNullableType = 183, + JSDocNonNullableType = 184, + JSDocUnknownType = 185, } /// Untyped AST Node Kind @@ -232,7 +231,6 @@ pub enum AstKind<'a> { NewExpression(&'a NewExpression<'a>) = AstType::NewExpression as u8, MetaProperty(&'a MetaProperty<'a>) = AstType::MetaProperty as u8, SpreadElement(&'a SpreadElement<'a>) = AstType::SpreadElement as u8, - Argument(&'a Argument<'a>) = AstType::Argument as u8, UpdateExpression(&'a UpdateExpression<'a>) = AstType::UpdateExpression as u8, UnaryExpression(&'a UnaryExpression<'a>) = AstType::UnaryExpression as u8, BinaryExpression(&'a BinaryExpression<'a>) = AstType::BinaryExpression as u8, @@ -452,7 +450,6 @@ impl GetSpan for AstKind<'_> { Self::NewExpression(it) => it.span(), Self::MetaProperty(it) => it.span(), Self::SpreadElement(it) => it.span(), - Self::Argument(it) => it.span(), Self::UpdateExpression(it) => it.span(), Self::UnaryExpression(it) => it.span(), Self::BinaryExpression(it) => it.span(), @@ -646,7 +643,6 @@ impl GetAddress for AstKind<'_> { Self::NewExpression(it) => Address::from_ref(it), Self::MetaProperty(it) => Address::from_ref(it), Self::SpreadElement(it) => Address::from_ref(it), - Self::Argument(it) => it.address(), Self::UpdateExpression(it) => Address::from_ref(it), Self::UnaryExpression(it) => Address::from_ref(it), Self::BinaryExpression(it) => Address::from_ref(it), @@ -918,11 +914,6 @@ impl<'a> AstKind<'a> { if let Self::SpreadElement(v) = self { Some(v) } else { None } } - #[inline] - pub fn as_argument(self) -> Option<&'a Argument<'a>> { - if let Self::Argument(v) = self { Some(v) } else { None } - } - #[inline] pub fn as_update_expression(self) -> Option<&'a UpdateExpression<'a>> { if let Self::UpdateExpression(v) = self { Some(v) } else { None } diff --git a/crates/oxc_ast_visit/src/generated/visit.rs b/crates/oxc_ast_visit/src/generated/visit.rs index c85076294c15a..505ac2f601c19 100644 --- a/crates/oxc_ast_visit/src/generated/visit.rs +++ b/crates/oxc_ast_visit/src/generated/visit.rs @@ -1678,13 +1678,11 @@ pub mod walk { #[inline] pub fn walk_argument<'a, V: Visit<'a>>(visitor: &mut V, it: &Argument<'a>) { - let kind = AstKind::Argument(visitor.alloc(it)); - visitor.enter_node(kind); + // No `AstKind` for this type match it { Argument::SpreadElement(it) => visitor.visit_spread_element(it), match_expression!(Argument) => visitor.visit_expression(it.to_expression()), } - visitor.leave_node(kind); } #[inline] @@ -4259,7 +4257,14 @@ pub mod walk { #[inline] pub fn walk_arguments<'a, V: Visit<'a>>(visitor: &mut V, it: &Vec<'a, Argument<'a>>) { for el in it { - visitor.visit_argument(el); + match el { + oxc_ast::ast::Argument::SpreadElement(spread) => { + visitor.visit_spread_element(spread); + } + _ => { + visitor.visit_expression(el.to_expression()); + } + } } } diff --git a/crates/oxc_ast_visit/src/generated/visit_mut.rs b/crates/oxc_ast_visit/src/generated/visit_mut.rs index 4ca253155e2c4..601bb5b619c03 100644 --- a/crates/oxc_ast_visit/src/generated/visit_mut.rs +++ b/crates/oxc_ast_visit/src/generated/visit_mut.rs @@ -1694,13 +1694,11 @@ pub mod walk_mut { #[inline] pub fn walk_argument<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut Argument<'a>) { - let kind = AstType::Argument; - visitor.enter_node(kind); + // No `AstType` for this type match it { Argument::SpreadElement(it) => visitor.visit_spread_element(it), match_expression!(Argument) => visitor.visit_expression(it.to_expression_mut()), } - visitor.leave_node(kind); } #[inline] @@ -4489,7 +4487,14 @@ pub mod walk_mut { #[inline] pub fn walk_arguments<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut Vec<'a, Argument<'a>>) { for el in it { - visitor.visit_argument(el); + match el { + oxc_ast::ast::Argument::SpreadElement(spread) => { + visitor.visit_spread_element(spread); + } + _ => { + visitor.visit_expression(el.to_expression_mut()); + } + } } } diff --git a/crates/oxc_formatter/src/ast_nodes/generated/ast_nodes.rs b/crates/oxc_formatter/src/ast_nodes/generated/ast_nodes.rs index fb442d9150605..a1ee5e3462526 100644 --- a/crates/oxc_formatter/src/ast_nodes/generated/ast_nodes.rs +++ b/crates/oxc_formatter/src/ast_nodes/generated/ast_nodes.rs @@ -47,7 +47,6 @@ pub enum AstNodes<'a> { NewExpression(&'a AstNode<'a, NewExpression<'a>>), MetaProperty(&'a AstNode<'a, MetaProperty<'a>>), SpreadElement(&'a AstNode<'a, SpreadElement<'a>>), - Argument(&'a AstNode<'a, Argument<'a>>), UpdateExpression(&'a AstNode<'a, UpdateExpression<'a>>), UnaryExpression(&'a AstNode<'a, UnaryExpression<'a>>), BinaryExpression(&'a AstNode<'a, BinaryExpression<'a>>), @@ -240,7 +239,6 @@ impl<'a> AstNodes<'a> { Self::NewExpression(n) => n.span(), Self::MetaProperty(n) => n.span(), Self::SpreadElement(n) => n.span(), - Self::Argument(n) => n.parent.span(), Self::UpdateExpression(n) => n.span(), Self::UnaryExpression(n) => n.span(), Self::BinaryExpression(n) => n.span(), @@ -433,7 +431,6 @@ impl<'a> AstNodes<'a> { Self::NewExpression(n) => n.parent, Self::MetaProperty(n) => n.parent, Self::SpreadElement(n) => n.parent, - Self::Argument(n) => n.parent, Self::UpdateExpression(n) => n.parent, Self::UnaryExpression(n) => n.parent, Self::BinaryExpression(n) => n.parent, @@ -626,7 +623,6 @@ impl<'a> AstNodes<'a> { Self::NewExpression(_) => "NewExpression", Self::MetaProperty(_) => "MetaProperty", Self::SpreadElement(_) => "SpreadElement", - Self::Argument(_) => "Argument", Self::UpdateExpression(_) => "UpdateExpression", Self::UnaryExpression(_) => "UnaryExpression", Self::BinaryExpression(_) => "BinaryExpression", @@ -1895,7 +1891,7 @@ impl<'a> AstNode<'a, SpreadElement<'a>> { impl<'a> AstNode<'a, Argument<'a>> { #[inline] pub fn as_ast_nodes(&self) -> &AstNodes<'a> { - let parent = self.allocator.alloc(AstNodes::Argument(transmute_self(self))); + let parent = self.parent; let node = match self.inner { Argument::SpreadElement(s) => AstNodes::SpreadElement(self.allocator.alloc(AstNode { inner: s.as_ref(), diff --git a/crates/oxc_formatter/src/ast_nodes/generated/format.rs b/crates/oxc_formatter/src/ast_nodes/generated/format.rs index 65a91d048e2d6..9518bccf6f6b3 100644 --- a/crates/oxc_formatter/src/ast_nodes/generated/format.rs +++ b/crates/oxc_formatter/src/ast_nodes/generated/format.rs @@ -830,7 +830,7 @@ impl<'a> Format<'a> for AstNode<'a, Argument<'a>> { #[inline] fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { let allocator = self.allocator; - let parent = allocator.alloc(AstNodes::Argument(transmute_self(self))); + let parent = self.parent; match self.inner { Argument::SpreadElement(inner) => allocator .alloc(AstNode:: { @@ -2853,14 +2853,21 @@ impl<'a> Format<'a> for AstNode<'a, RegExpLiteral<'a>> { impl<'a> Format<'a> for AstNode<'a, JSXElement<'a>> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - if format_type_cast_comment_node(self, false, f)? { + let is_suppressed = f.comments().is_suppressed(self.span().start); + if !is_suppressed && format_type_cast_comment_node(self, false, f)? { return Ok(()); } - let needs_parentheses = self.needs_parentheses(f); + let needs_parentheses = !is_suppressed && self.needs_parentheses(f); if needs_parentheses { "(".fmt(f)?; } - let result = self.write(f); + let result = if is_suppressed { + self.format_leading_comments(f)?; + FormatSuppressedNode(self.span()).fmt(f)?; + self.format_trailing_comments(f) + } else { + self.write(f) + }; if needs_parentheses { ")".fmt(f)?; } @@ -2892,14 +2899,21 @@ impl<'a> Format<'a> for AstNode<'a, JSXClosingElement<'a>> { impl<'a> Format<'a> for AstNode<'a, JSXFragment<'a>> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - if format_type_cast_comment_node(self, false, f)? { + let is_suppressed = f.comments().is_suppressed(self.span().start); + if !is_suppressed && format_type_cast_comment_node(self, false, f)? { return Ok(()); } - let needs_parentheses = self.needs_parentheses(f); + let needs_parentheses = !is_suppressed && self.needs_parentheses(f); if needs_parentheses { "(".fmt(f)?; } - let result = self.write(f); + let result = if is_suppressed { + self.format_leading_comments(f)?; + FormatSuppressedNode(self.span()).fmt(f)?; + self.format_trailing_comments(f) + } else { + self.write(f) + }; if needs_parentheses { ")".fmt(f)?; } @@ -4202,11 +4216,13 @@ impl<'a> Format<'a> for AstNode<'a, TSTypeAliasDeclaration<'a>> { impl<'a> Format<'a> for AstNode<'a, TSClassImplements<'a>> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { let is_suppressed = f.comments().is_suppressed(self.span().start); - self.format_leading_comments(f)?; - let result = - if is_suppressed { FormatSuppressedNode(self.span()).fmt(f) } else { self.write(f) }; - self.format_trailing_comments(f)?; - result + if is_suppressed { + self.format_leading_comments(f)?; + FormatSuppressedNode(self.span()).fmt(f)?; + self.format_trailing_comments(f) + } else { + self.write(f) + } } } @@ -4802,11 +4818,13 @@ impl<'a> Format<'a> for AstNode<'a, TSNonNullExpression<'a>> { impl<'a> Format<'a> for AstNode<'a, Decorator<'a>> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { let is_suppressed = f.comments().is_suppressed(self.span().start); - self.format_leading_comments(f)?; - let result = - if is_suppressed { FormatSuppressedNode(self.span()).fmt(f) } else { self.write(f) }; - self.format_trailing_comments(f)?; - result + if is_suppressed { + self.format_leading_comments(f)?; + FormatSuppressedNode(self.span()).fmt(f)?; + self.format_trailing_comments(f) + } else { + self.write(f) + } } } diff --git a/crates/oxc_formatter/src/parentheses/expression.rs b/crates/oxc_formatter/src/parentheses/expression.rs index a760ee81a3646..8696fa263fb26 100644 --- a/crates/oxc_formatter/src/parentheses/expression.rs +++ b/crates/oxc_formatter/src/parentheses/expression.rs @@ -14,14 +14,56 @@ use crate::{ Format, ast_nodes::{AstNode, AstNodes}, formatter::Formatter, - utils::expression::ExpressionLeftSide, + utils::{expression::ExpressionLeftSide, is_expression_used_as_call_argument}, write::{BinaryLikeExpression, should_flatten}, }; use super::NeedsParentheses; -impl NeedsParentheses<'_> for AstNode<'_, Expression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { +// Helper function to check if a MemberExpression has a CallExpression in its object chain +fn member_has_call_object(member: &MemberExpression) -> bool { + match member { + MemberExpression::ComputedMemberExpression(m) => expression_is_or_contains_call(&m.object), + MemberExpression::StaticMemberExpression(m) => expression_is_or_contains_call(&m.object), + MemberExpression::PrivateFieldExpression(m) => expression_is_or_contains_call(&m.object), + } +} + +// Helper function to check if an Expression is or contains a CallExpression +fn expression_is_or_contains_call(expr: &Expression) -> bool { + match expr { + Expression::CallExpression(_) => true, + Expression::TaggedTemplateExpression(t) => { + // Tagged templates like x()`` where the tag is a call expression + expression_is_or_contains_call(&t.tag) + } + Expression::ComputedMemberExpression(m) => expression_is_or_contains_call(&m.object), + Expression::StaticMemberExpression(m) => expression_is_or_contains_call(&m.object), + Expression::PrivateFieldExpression(m) => expression_is_or_contains_call(&m.object), + _ => false, + } +} + +// Helper function to check if an expression can be used unparenthesized in a decorator +// Based on Prettier's isDecoratorMemberExpression +fn is_decorator_member_expression(expr: &Expression) -> bool { + match expr { + Expression::Identifier(_) => true, + Expression::StaticMemberExpression(m) if !m.optional => { + // Non-optional static member access like a.b.c + is_decorator_member_expression(&m.object) + } + Expression::ComputedMemberExpression(m) if !m.optional => { + // Non-optional computed member access like a[0] or a["prop"] + // Note: Prettier allows this without parentheses + is_decorator_member_expression(&m.object) + } + _ => false, + } +} + +impl<'a> NeedsParentheses<'a> for AstNode<'a, Expression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { match self.as_ast_nodes() { AstNodes::BooleanLiteral(it) => it.needs_parentheses(f), AstNodes::NullLiteral(it) => it.needs_parentheses(f), @@ -66,10 +108,7 @@ impl NeedsParentheses<'_> for AstNode<'_, Expression<'_>> { AstNodes::StaticMemberExpression(it) => it.needs_parentheses(f), AstNodes::ComputedMemberExpression(it) => it.needs_parentheses(f), AstNodes::PrivateFieldExpression(it) => it.needs_parentheses(f), - _ => { - // TODO: incomplete - false - } + _ => false, } } } @@ -85,19 +124,78 @@ impl NeedsParentheses<'_> for AstNode<'_, IdentifierReference<'_>> { matches!(self.parent, AstNodes::ForOfStatement(stmt) if !stmt.r#await && stmt.left.span().contains_inclusive(self.span)) } "let" => { - // Walk up ancestors to find the relevant context for `let` keyword - for parent in self.ancestors() { + // Special handling for 'let' identifier to match Prettier's behavior + + // Check for-statement contexts first + let mut parent = self.parent; + loop { match parent { - AstNodes::ExpressionStatement(_) => return false, + AstNodes::Program(_) | AstNodes::ExpressionStatement(_) => return false, AstNodes::ForOfStatement(stmt) => { return stmt.left.span().contains_inclusive(self.span); } AstNodes::TSSatisfiesExpression(expr) => { return expr.expression.span() == self.span(); } - _ => {} + AstNodes::ForInStatement(stmt) => { + return stmt.left.span().contains_inclusive(self.span); + } + AstNodes::ForStatement(stmt) => { + return stmt + .init + .as_ref() + .is_some_and(|init| init.span().contains_inclusive(self.span)); + } + _ => parent = parent.parent(), } } + + // Check if 'let' is used as the object of a computed member expression + if let AstNodes::ComputedMemberExpression(member) = self.parent + && member.object.span() == self.span() + { + // Check if this is used as a call argument - if so, no parentheses needed + let mut check_parent = member.parent; + loop { + match check_parent { + // Direct argument to call/new - no parens needed + AstNodes::CallExpression(call) => { + // Check if member is directly an argument + if call + .arguments + .iter() + .any(|arg| arg.span().contains_inclusive(member.span())) + { + return false; + } + } + AstNodes::NewExpression(new_expr) => { + // Check if member or its parent assignment is an argument + if new_expr.arguments.iter().any(|arg| { + // Check if the argument contains our member expression + arg.span().contains_inclusive(member.span()) + }) { + return false; + } + } + // If we hit an assignment that's an argument, check further + AstNodes::AssignmentExpression(_) => { + check_parent = check_parent.parent(); + continue; + } + _ => break, + } + break; + } + + // Need parentheses when at the start of a statement + return is_first_in_statement( + member.span(), + member.parent, + FirstInStatementMode::ExpressionStatementOrArrow, + ); + } + false } name => { @@ -229,19 +327,37 @@ impl NeedsParentheses<'_> for AstNode<'_, ArrayExpression<'_>> { } } -impl NeedsParentheses<'_> for AstNode<'_, ObjectExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - if f.comments().is_type_cast_node(self) { +impl<'a> NeedsParentheses<'a> for AstNode<'a, ObjectExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + let span = self.span(); + let parent = self.parent; + + // Object expressions inside template literals don't need parentheses + // They are unambiguously in expression context and cannot be confused with block statements + if matches!(parent, AstNodes::TemplateLiteral(_)) { return false; } - let parent = self.parent; - is_class_extends(self.span, parent) - || is_first_in_statement( - self.span, - parent, - FirstInStatementMode::ExpressionStatementOrArrow, - ) + // Object expressions don't need parentheses when used as function arguments + if is_expression_used_as_call_argument(span, parent) { + return false; + } + + // Object expressions don't need parentheses when used as the expression of a cast + // that is itself used as an argument + if let AstNodes::TSAsExpression(as_expr) = parent + && is_expression_used_as_call_argument(as_expr.span, as_expr.parent) + { + return false; + } + if let AstNodes::TSSatisfiesExpression(satisfies_expr) = parent + && is_expression_used_as_call_argument(satisfies_expr.span, satisfies_expr.parent) + { + return false; + } + + is_class_extends(parent, span) + || is_first_in_statement(span, parent, FirstInStatementMode::ExpressionStatementOrArrow) } } @@ -252,17 +368,41 @@ impl NeedsParentheses<'_> for AstNode<'_, TaggedTemplateExpression<'_>> { } } -impl NeedsParentheses<'_> for AstNode<'_, MemberExpression<'_>> { +impl<'a> NeedsParentheses<'a> for AstNode<'a, MemberExpression<'a>> { #[inline] - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + // Member expressions with call expression or another member expression with call as object + // need parentheses when used as the callee of a new expression: new (a().b)() + if let AstNodes::NewExpression(new_expr) = self.parent { + let span = self.span(); + if new_expr.callee.span() == span { + // Check if the object of this member expression needs parens + return member_has_call_object(self); + } + } false } } -impl NeedsParentheses<'_> for AstNode<'_, ComputedMemberExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - matches!(self.parent, AstNodes::NewExpression(_)) - && (self.optional || member_chain_callee_needs_parens(&self.object)) +impl<'a> NeedsParentheses<'a> for AstNode<'a, ComputedMemberExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + // Computed member expressions with call expression objects need parentheses + // when used as the callee of a new expression: new (a()[0])() + if let AstNodes::NewExpression(new_expr) = self.parent { + let span = self.span(); + if new_expr.callee.span() == span { + // Check if the object is or contains a call expression + return expression_is_or_contains_call(&self.object); + } + } + + // Computed member expressions need parentheses in decorators + // Example: @(decorators[0]) and @(decorators?.[0]) + if let AstNodes::Decorator(_) = self.parent { + return true; + } + + false } } @@ -296,7 +436,11 @@ impl NeedsParentheses<'_> for AstNode<'_, CallExpression<'_>> { } match self.parent { - AstNodes::NewExpression(_) => true, + AstNodes::NewExpression(_) => { + // Call expressions don't need parentheses when used as new expression arguments + !is_expression_used_as_call_argument(self.span(), self.parent) + } + AstNodes::Decorator(_) => !is_decorator_member_expression(&self.callee), AstNodes::ExportDefaultDeclaration(_) => { let callee = &self.callee(); let callee_span = callee.span(); @@ -314,13 +458,22 @@ impl NeedsParentheses<'_> for AstNode<'_, CallExpression<'_>> { } } -impl NeedsParentheses<'_> for AstNode<'_, NewExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - if f.comments().is_type_cast_node(self) { - return false; +impl<'a> NeedsParentheses<'a> for AstNode<'a, NewExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + let span = self.span(); + let parent = self.parent; + + // New expressions with call expressions as callees need parentheses when being called + if let AstNodes::CallExpression(call) = parent + && call.callee.span() == span + { + // Only need parens if the new expression's callee is a call expression + if let Expression::CallExpression(_) = self.callee { + return true; + } } - is_class_extends(self.span, self.parent) + is_class_extends(parent, span) } } @@ -360,9 +513,9 @@ impl NeedsParentheses<'_> for AstNode<'_, UnaryExpression<'_>> { && parent_operator == operator } // A user typing `!foo instanceof Bar` probably intended `!(foo instanceof Bar)`, - // so format to `(!foo) instance Bar` to what is really happening - // A user typing `!foo in bar` probably intended `!(foo instanceof Bar)`, - // so format to `(!foo) in bar` to what is really happening + // so format to `(!foo) instanceof Bar` to show what is really happening + // A user typing `!foo in bar` probably intended `!(foo in bar)`, + // so format to `(!foo) in bar` to show what is really happening AstNodes::BinaryExpression(e) if e.operator().is_relational() => true, _ => unary_like_expression_needs_parens(UnaryLike::UnaryExpression(self)), } @@ -405,6 +558,8 @@ fn is_in_for_initializer(expr: &AstNode<'_, BinaryExpression<'_>>) -> bool { .is_some_and(|init| init.span().contains_inclusive(expr.span)); } AstNodes::ForInStatement(stmt) => { + // Only add parentheses for in expressions on the LEFT side of for-in + // The right side doesn't need parentheses for disambiguation return stmt.left.span().contains_inclusive(expr.span); } AstNodes::Program(_) => { @@ -417,15 +572,18 @@ fn is_in_for_initializer(expr: &AstNode<'_, BinaryExpression<'_>>) -> bool { false } -impl NeedsParentheses<'_> for AstNode<'_, PrivateInExpression<'_>> { +impl<'a> NeedsParentheses<'a> for AstNode<'a, PrivateInExpression<'a>> { #[inline] - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - if f.comments().is_type_cast_node(self) { - return false; + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + // Need parentheses in class extends: class A extends (#x in obj) {} + if is_class_extends(self.parent, self.span) { + return true; } - is_class_extends(self.span, self.parent) - || matches!(self.parent, AstNodes::UnaryExpression(_)) + // Need parentheses when used as argument to unary operators + // !(#target in this) - parentheses required + // The 'in' operator has lower precedence than unary operators + matches!(self.parent, AstNodes::UnaryExpression(_)) } } @@ -469,6 +627,23 @@ impl NeedsParentheses<'_> for AstNode<'_, ConditionalExpression<'_>> { ) { return true; } + + // Conditional expressions need parentheses when used as the object of a member expression + // BUT only when there's a property access that follows + if let AstNodes::StaticMemberExpression(member) = parent + && member.object.span() == self.span() + { + // Check if there's actually a property access or if it's just grouping + // Only add parentheses if the member expression has properties being accessed + return true; + } + if let AstNodes::ComputedMemberExpression(member) = parent + && member.object.span() == self.span() + { + // Same logic for computed member expressions + return true; + } + if let AstNodes::ConditionalExpression(e) = parent { e.test.span() == self.span() } else { @@ -488,16 +663,20 @@ impl NeedsParentheses<'_> for AstNode<'_, Function<'_>> { } let parent = self.parent; - matches!( - parent, - AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) - | AstNodes::TaggedTemplateExpression(_) - ) || is_first_in_statement( - self.span, - parent, - FirstInStatementMode::ExpressionOrExportDefault, - ) + + // Function expressions in call/new arguments don't need parentheses + // unless they are the callee + if let AstNodes::CallExpression(call) = parent { + // Only add parentheses if this function is the callee, not an argument + return call.callee.span() == self.span; + } + + matches!(parent, AstNodes::NewExpression(_) | AstNodes::TaggedTemplateExpression(_)) + || is_first_in_statement( + self.span, + parent, + FirstInStatementMode::ExpressionOrExportDefault, + ) } } @@ -550,15 +729,14 @@ impl NeedsParentheses<'_> for AstNode<'_, AssignmentExpression<'_>> { true } - // `interface A { [a = 1]; }` not need parens - AstNodes::TSPropertySignature(_) | // Never need parentheses in these contexts: + // - `interface { [a = 1]; }` and `class { [a = 1]; }` (TS property signatures) // - `a = (b = c)` = nested assignments don't need extra parens - AstNodes::AssignmentExpression(_) => false, + AstNodes::TSPropertySignature(_) | AstNodes::AssignmentExpression(_) => false, // Computed member expressions: need parens when assignment is the object - // - `obj[(a = b)]` parens needed for explicitness + // - `obj[a = b]` = no parens needed for property // - `(a = b)[obj]` = parens needed for object - AstNodes::ComputedMemberExpression(member) => true, + AstNodes::ComputedMemberExpression(member) => member.object.span() == self.span(), // For statements, no parens needed in initializer or update sections: // - `for (a = 1; ...; a = 2) {}` = both assignments don't need parens AstNodes::ForStatement(stmt) => { @@ -585,13 +763,23 @@ impl NeedsParentheses<'_> for AstNode<'_, SequenceExpression<'_>> { return false; } + // Special handling for prettier-ignore comments + if f.comments().has_leading_own_line_comment(self.span().start) { + return false; + } + match self.parent { - AstNodes::ReturnStatement(_) - | AstNodes::ThrowStatement(_) + // Return and throw statements handle their own parentheses + // Only add parentheses if there are comments in the sequence + AstNodes::ReturnStatement(_) | AstNodes::ThrowStatement(_) => { + f.comments().has_comment_in_span(self.span()) + } // There's a precedence for writing `x++, y++` - | AstNodes::ForStatement(_) - | AstNodes::SequenceExpression(_) => false, - AstNodes::ExpressionStatement(stmt) => !stmt.is_arrow_function_body(), + AstNodes::ForStatement(_) | AstNodes::SequenceExpression(_) => false, + // Arrow function concise bodies don't need parens (unless there are comments) + AstNodes::ExpressionStatement(stmt) => { + !stmt.is_arrow_function_body() || f.comments().has_comment_in_span(self.span()) + } _ => true, } } @@ -607,19 +795,22 @@ impl NeedsParentheses<'_> for AstNode<'_, AwaitExpression<'_>> { } } -impl NeedsParentheses<'_> for AstNode<'_, ChainExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - if f.comments().is_type_cast_node(self) { +impl<'a> NeedsParentheses<'a> for AstNode<'a, ChainExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + let span = self.span(); + + // Chain expressions don't need parentheses when used as call arguments + if is_expression_used_as_call_argument(span, self.parent) { return false; } match self.parent { - AstNodes::NewExpression(_) => true, AstNodes::CallExpression(call) => !call.optional, AstNodes::StaticMemberExpression(member) => !member.optional, AstNodes::ComputedMemberExpression(member) => { !member.optional && member.object.span() == self.span() } + AstNodes::NewExpression(_) | AstNodes::TSNonNullExpression(_) => true, _ => false, } } @@ -630,24 +821,35 @@ impl NeedsParentheses<'_> for AstNode<'_, Class<'_>> { if self.r#type() != ClassType::ClassExpression { return false; } + let span = self.span(); + let parent = self.parent; + + // Check if this class is the left side of a division operator + // This prevents ASI ambiguity when formatting: (class{}) / foo + // Without parentheses, a line break before the division could cause: + // x = class{} + // / foo <- Parser might try to interpret / as start of regex or statement + if let AstNodes::BinaryExpression(binary) = parent + && matches!(binary.operator, BinaryOperator::Division) + && binary.left.span() == span + { + return true; + } - if f.comments().is_type_cast_node(self) { + // Decorated class expressions need parentheses when used in extends clause + if !self.decorators.is_empty() && is_class_extends(parent, span) { + return true; + } + + // Class expressions don't need parentheses when used as function arguments + if is_expression_used_as_call_argument(span, parent) { return false; } - let parent = self.parent; match parent { - AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) - | AstNodes::ExportDefaultDeclaration(_) => true, - + AstNodes::ExportDefaultDeclaration(_) => true, _ => { - (is_class_extends(self.span, self.parent) && !self.decorators.is_empty()) - || is_first_in_statement( - self.span, - parent, - FirstInStatementMode::ExpressionOrExportDefault, - ) + is_first_in_statement(span, parent, FirstInStatementMode::ExpressionOrExportDefault) } } } @@ -659,12 +861,9 @@ impl NeedsParentheses<'_> for AstNode<'_, ParenthesizedExpression<'_>> { } } -impl NeedsParentheses<'_> for AstNode<'_, ArrowFunctionExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { - if f.comments().is_type_cast_node(self) { - return false; - } - +impl<'a> NeedsParentheses<'a> for AstNode<'a, ArrowFunctionExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + let span = self.span(); let parent = self.parent; if matches!( parent, @@ -679,9 +878,15 @@ impl NeedsParentheses<'_> for AstNode<'_, ArrowFunctionExpression<'_>> { return true; } if let AstNodes::ConditionalExpression(e) = parent { - e.test.span() == self.span() + e.test.without_parentheses().span() == span + } else if let AstNodes::CallExpression(call) = parent { + // Only add parentheses if this arrow function is the callee, not an argument + call.callee.span() == span + } else if let AstNodes::NewExpression(new_expr) = parent { + // Only add parentheses if this arrow function is the callee, not an argument + new_expr.callee.span() == span } else { - update_or_lower_expression_needs_parens(self.span(), parent) + update_or_lower_expression_needs_parens(span, parent) } } } @@ -755,7 +960,14 @@ impl NeedsParentheses<'_> for AstNode<'_, TSTypeAssertion<'_>> { AstNodes::BinaryExpression(binary) => { matches!(binary.operator, BinaryOperator::ShiftLeft) } - _ => type_cast_like_needs_parens(self.span(), self.parent), + _ => { + // Type assertions don't need parentheses when used as function arguments + // This allows for "argument hugging" behavior like `func(obj)` + if is_expression_used_as_call_argument(self.span(), self.parent) { + return false; + } + type_cast_like_needs_parens(self.span(), self.parent) + } } } } @@ -787,14 +999,15 @@ fn type_cast_like_needs_parens(span: Span, parent: &AstNodes<'_>) -> bool { AstNodes::AssignmentExpression(assignment) => { assignment.left.span() == span } - _ => is_class_extends(span, parent), + _ => is_class_extends(parent, span), } } -impl NeedsParentheses<'_> for AstNode<'_, TSNonNullExpression<'_>> { - fn needs_parentheses(&self, f: &Formatter<'_, '_>) -> bool { +impl<'a> NeedsParentheses<'a> for AstNode<'a, TSNonNullExpression<'a>> { + fn needs_parentheses(&self, f: &Formatter<'_, 'a>) -> bool { + let span = self.span(); let parent = self.parent; - is_class_extends(self.span, parent) + is_class_extends(parent, span) || (matches!(parent, AstNodes::NewExpression(_)) && member_chain_callee_needs_parens(&self.expression)) } @@ -814,7 +1027,10 @@ impl NeedsParentheses<'_> for AstNode<'_, TSInstantiationExpression<'_>> { } fn binary_like_needs_parens(binary_like: BinaryLikeExpression<'_, '_>) -> bool { - let parent = match binary_like.parent() { + let span = binary_like.span(); + let parent_nodes = binary_like.parent(); + + let parent = match parent_nodes { AstNodes::TSAsExpression(_) | AstNodes::TSSatisfiesExpression(_) | AstNodes::TSTypeAssertion(_) @@ -823,11 +1039,23 @@ fn binary_like_needs_parens(binary_like: BinaryLikeExpression<'_, '_>) -> bool { | AstNodes::TSNonNullExpression(_) | AstNodes::SpreadElement(_) | AstNodes::JSXSpreadAttribute(_) - | AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) | AstNodes::ChainExpression(_) | AstNodes::StaticMemberExpression(_) | AstNodes::TaggedTemplateExpression(_) => return true, + AstNodes::CallExpression(_) => { + // Binary expressions don't need parentheses when used as function arguments + if is_expression_used_as_call_argument(span, parent_nodes) { + return false; + } + return true; + } + AstNodes::NewExpression(_) => { + // Binary expressions don't need parentheses when used as constructor arguments + if is_expression_used_as_call_argument(span, parent_nodes) { + return false; + } + return true; + } AstNodes::ComputedMemberExpression(computed) => { return computed.object.span() == binary_like.span(); } @@ -843,6 +1071,9 @@ fn binary_like_needs_parens(binary_like: BinaryLikeExpression<'_, '_>) -> bool { let parent_operator = parent.operator(); let operator = binary_like.operator(); + + // Only cache span calculation for multiple uses + let parent_precedence = parent_operator.precedence(); let precedence = operator.precedence(); @@ -852,7 +1083,9 @@ fn binary_like_needs_parens(binary_like: BinaryLikeExpression<'_, '_>) -> bool { return true; } - let is_right = parent.right().span() == binary_like.span(); + // Cache span for multiple comparisons to avoid recalculation + let binary_span = binary_like.span(); + let is_right = parent.right().span() == binary_span; // `a ** b ** c` if is_right && parent_precedence == precedence { @@ -924,17 +1157,27 @@ fn update_or_lower_expression_needs_parens(span: Span, parent: &AstNodes<'_>) -> if matches!( parent, AstNodes::TSNonNullExpression(_) - | AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) | AstNodes::StaticMemberExpression(_) | AstNodes::TaggedTemplateExpression(_) - ) || is_class_extends(span, parent) + ) || is_class_extends(parent, span) { return true; } - if let AstNodes::ComputedMemberExpression(computed_member_expr) = parent { - return computed_member_expr.object.span() == span; + // Special handling for computed member expressions + if let AstNodes::ComputedMemberExpression(computed) = parent { + // Only need parentheses if this expression is the object, not the property + // For a?.[++x], ++x is the property and doesn't need parentheses + return computed.object.span() == span; + } + + // Check call expressions and new expressions, but allow expressions used as arguments + if matches!(parent, AstNodes::CallExpression(_) | AstNodes::NewExpression(_)) { + // Don't add parentheses when used as function/constructor arguments + if is_expression_used_as_call_argument(span, parent) { + return false; + } + return true; } false @@ -983,13 +1226,22 @@ fn is_first_in_statement( return true; } AstNodes::StaticMemberExpression(_) + | AstNodes::TemplateLiteral(_) | AstNodes::TaggedTemplateExpression(_) | AstNodes::ChainExpression(_) - | AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) | AstNodes::TSAsExpression(_) | AstNodes::TSSatisfiesExpression(_) | AstNodes::TSNonNullExpression(_) => {} + AstNodes::CallExpression(call) => { + if call.callee.span() != current_span { + break; + } + } + AstNodes::NewExpression(new_expr) => { + if new_expr.callee.span() != current_span { + break; + } + } AstNodes::SequenceExpression(sequence) => { if sequence.expressions.first().unwrap().span() != current_span { break; @@ -1067,12 +1319,17 @@ fn ts_as_or_satisfies_needs_parens( AstNodes::ExportDefaultDeclaration(_) => matches!(inner, Expression::FunctionExpression(_) | Expression::ClassExpression(_)), _ => { + // Type assertions don't need parentheses when used as function arguments + // This allows for "argument hugging" behavior like `func(obj as Type)` + if is_expression_used_as_call_argument(span, parent) { + return false; + } type_cast_like_needs_parens(span, parent) } } } -fn is_class_extends(span: Span, parent: &AstNodes<'_>) -> bool { +fn is_class_extends(parent: &AstNodes<'_>, span: Span) -> bool { if let AstNodes::Class(c) = parent { return c.super_class.as_ref().is_some_and(|c| c.span() == span); } @@ -1080,7 +1337,7 @@ fn is_class_extends(span: Span, parent: &AstNodes<'_>) -> bool { } fn jsx_element_or_fragment_needs_paren(span: Span, parent: &AstNodes<'_>) -> bool { - if is_class_extends(span, parent) { + if is_class_extends(parent, span) { return true; } @@ -1098,11 +1355,13 @@ fn jsx_element_or_fragment_needs_paren(span: Span, parent: &AstNodes<'_>) -> boo | AstNodes::UnaryExpression(_) | AstNodes::TSNonNullExpression(_) | AstNodes::SpreadElement(_) - | AstNodes::CallExpression(_) - | AstNodes::NewExpression(_) | AstNodes::TaggedTemplateExpression(_) | AstNodes::JSXSpreadAttribute(_) | AstNodes::JSXSpreadChild(_) => true, + AstNodes::CallExpression(_) | AstNodes::NewExpression(_) => { + // JSX elements don't need parentheses when used as function/constructor arguments + !is_expression_used_as_call_argument(span, parent) + } _ => false, } } diff --git a/crates/oxc_formatter/src/utils/assignment_like.rs b/crates/oxc_formatter/src/utils/assignment_like.rs index bd03fa00ca102..62ab0ed2e20e9 100644 --- a/crates/oxc_formatter/src/utils/assignment_like.rs +++ b/crates/oxc_formatter/src/utils/assignment_like.rs @@ -811,7 +811,33 @@ impl<'a> Format<'a> for AssignmentLike<'a, '_> { } }); - write!(f, [format_content]) + // Check if the right-hand side contains ASI-risky division patterns + // If so, use RemoveSoftLinesBuffer to prevent line breaks that could cause + // non-idempotent formatting due to ASI ambiguity + let should_force_inline = match self { + AssignmentLike::AssignmentExpression(assignment) => { + crate::write::has_asi_risky_division_pattern(&assignment.right) + } + AssignmentLike::VariableDeclarator(declarator) => declarator + .init + .as_ref() + .is_some_and(|init| crate::write::has_asi_risky_division_pattern(init)), + _ => false, + }; + + if should_force_inline { + write!( + f, + [format_once(|f| { + use crate::formatter::buffer::RemoveSoftLinesBuffer; + let mut buffer = RemoveSoftLinesBuffer::new(f); + let mut formatter = Formatter::new(&mut buffer); + write!(formatter, [format_content]) + })] + ) + } else { + write!(f, [format_content]) + } } } diff --git a/crates/oxc_formatter/src/utils/call_expression.rs b/crates/oxc_formatter/src/utils/call_expression.rs index 4bb43c2dafd94..a1abbabe4bb9b 100644 --- a/crates/oxc_formatter/src/utils/call_expression.rs +++ b/crates/oxc_formatter/src/utils/call_expression.rs @@ -36,8 +36,11 @@ pub fn is_test_call_expression(call: &AstNode>) -> bool { match (args.next(), args.next(), args.next()) { (Some(argument), None, None) if arguments.len() == 1 => { if is_angular_test_wrapper(call) && { - if let AstNodes::CallExpression(call) = call.grand_parent() { - is_test_call_expression(call) + // After removal of AstKind::Argument, the parent of a CallExpression + // that's used as an argument is now directly the parent CallExpression, + // not parent.parent(). + if let AstNodes::CallExpression(parent_call) = call.parent { + is_test_call_expression(parent_call) } else { false } diff --git a/crates/oxc_formatter/src/utils/jsx.rs b/crates/oxc_formatter/src/utils/jsx.rs index 4fd8c89437323..f0a6323c32527 100644 --- a/crates/oxc_formatter/src/utils/jsx.rs +++ b/crates/oxc_formatter/src/utils/jsx.rs @@ -79,6 +79,8 @@ pub fn get_wrap_state(parent: &AstNodes<'_>) -> WrapState { match parent { AstNodes::ArrayExpression(_) + | AstNodes::CallExpression(_) + | AstNodes::NewExpression(_) | AstNodes::JSXAttribute(_) | AstNodes::JSXExpressionContainer(_) | AstNodes::ConditionalExpression(_) => WrapState::NoWrap, @@ -89,9 +91,6 @@ pub fn get_wrap_state(parent: &AstNodes<'_>) -> WrapState { WrapState::WrapOnBreak } } - AstNodes::Argument(argument) if matches!(argument.parent, AstNodes::CallExpression(_)) => { - WrapState::NoWrap - } AstNodes::ExpressionStatement(stmt) => { // `() =>
` // ^^^^^^^^^^^ diff --git a/crates/oxc_formatter/src/utils/member_chain/chain_member.rs b/crates/oxc_formatter/src/utils/member_chain/chain_member.rs index a3d19a2aceaa0..1e01e465b07af 100644 --- a/crates/oxc_formatter/src/utils/member_chain/chain_member.rs +++ b/crates/oxc_formatter/src/utils/member_chain/chain_member.rs @@ -59,6 +59,37 @@ impl ChainMember<'_, '_> { pub const fn is_computed_expression(&self) -> bool { matches!(self, Self::ComputedMember { .. }) } + + /// Check if this member has a trailing line comment on the same line + pub fn has_same_line_trailing_comment(&self, f: &Formatter<'_, '_>) -> bool { + // Only check for trailing comments on static members for now + // to avoid breaking up other constructs + match self { + Self::StaticMember(e) => { + let member_end = e.property().span.end; + let source = f.source_text(); + + // Look for the rest of the line after this member + let rest_of_line = &source[member_end as usize..]; + + // Find the end of the current line + let line_end_pos = rest_of_line.find('\n').unwrap_or(rest_of_line.len()); + let line_content = &rest_of_line[..line_end_pos]; + + // Check if there's a line comment on this line + // Also check that it's not just whitespace before the comment + if let Some(comment_pos) = line_content.find("//") { + let before_comment = &line_content[..comment_pos]; + + before_comment.chars().all(char::is_whitespace) && !before_comment.is_empty() + } else { + false + } + } + // Don't break groups for other member types for now + _ => false, + } + } } impl<'a> Format<'a> for ChainMember<'a, '_> { diff --git a/crates/oxc_formatter/src/utils/member_chain/mod.rs b/crates/oxc_formatter/src/utils/member_chain/mod.rs index 1808ef9b1af84..a4b8786cc773b 100644 --- a/crates/oxc_formatter/src/utils/member_chain/mod.rs +++ b/crates/oxc_formatter/src/utils/member_chain/mod.rs @@ -46,7 +46,7 @@ impl<'a, 'b> MemberChain<'a, 'b> { // `flattened_items` now contains only the nodes that should have a sequence of // `[ StaticMemberExpression -> AnyNode + CallExpression ]` let tail_groups = - compute_remaining_groups(chain_members.drain(remaining_members_start_index..)); + compute_remaining_groups(chain_members.drain(remaining_members_start_index..), f); let head_group = MemberChainGroup::from(chain_members); let mut member_chain = Self { head: head_group, tail: tail_groups, root: call_expression }; @@ -301,39 +301,55 @@ fn get_split_index_of_head_and_tail_groups(members: &[ChainMember<'_, '_>]) -> u /// computes groups coming after the first group fn compute_remaining_groups<'a, 'b>( members: impl IntoIterator>, + f: &Formatter<'_, 'a>, ) -> TailChainGroups<'a, 'b> { let mut has_seen_call_expression = false; + let mut has_trailing_comment = false; let mut groups_builder = MemberChainGroupsBuilder::default(); for member in members { + // Check if previous member had a trailing comment + // If so, we should start a new group + let should_break_group = has_seen_call_expression || has_trailing_comment; + match member { // [0] should be appended at the end of the group instead of the // beginning of the next one ChainMember::ComputedMember(_) if is_computed_array_member_access(&member) => { - groups_builder.start_or_continue_group(member); + // Always append to the current group, don't start a new one + groups_builder.start_or_continue_group(member.clone()); + // Don't reset has_seen_call_expression since we're continuing the group } ChainMember::StaticMember(_) | ChainMember::ComputedMember(_) => { // if we have seen a CallExpression, we want to close the group. // The resultant group will be something like: [ . , then, () ]; // `.` and `then` belong to the previous StaticMemberExpression, // and `()` belong to the call expression we just encountered - if has_seen_call_expression { + if should_break_group { groups_builder.close_group(); - groups_builder.start_group(member); + groups_builder.start_group(member.clone()); has_seen_call_expression = false; } else { - groups_builder.start_or_continue_group(member); + groups_builder.start_or_continue_group(member.clone()); } } ChainMember::CallExpression { .. } => { - groups_builder.start_or_continue_group(member); + if has_trailing_comment { + groups_builder.close_group(); + groups_builder.start_group(member.clone()); + } else { + groups_builder.start_or_continue_group(member.clone()); + } has_seen_call_expression = true; } ChainMember::TSNonNullExpression(_) => { - groups_builder.start_or_continue_group(member); + groups_builder.start_or_continue_group(member.clone()); } ChainMember::Node(_) => unreachable!("Remaining members never have a `Node` variant"), } + + // Check if this member has a trailing comment for the next iteration + has_trailing_comment = member.has_same_line_trailing_comment(f); } groups_builder.finish() @@ -345,14 +361,45 @@ fn is_computed_array_member_access(member: &ChainMember<'_, '_>) -> bool { ) } +/// Combined analysis of call arguments to avoid multiple iterations +#[derive(Debug, Default)] +struct ArgumentAnalysis { + has_arrow_or_function: bool, + all_simple: bool, +} + +fn analyze_call_arguments<'a>(call: &AstNode<'a, CallExpression<'a>>) -> ArgumentAnalysis { + let mut analysis = ArgumentAnalysis { has_arrow_or_function: false, all_simple: true }; + + for argument in call.arguments() { + // Check for arrow or function expressions + if matches!( + &**argument, + Argument::ArrowFunctionExpression(_) | Argument::FunctionExpression(_) + ) { + analysis.has_arrow_or_function = true; + } + + // Check if argument is simple + if analysis.all_simple && !SimpleArgument::new(argument).is_simple() { + analysis.all_simple = false; + } + + // Early exit if we've determined both conditions + if analysis.has_arrow_or_function && !analysis.all_simple { + break; + } + } + + analysis +} + fn has_arrow_or_function_expression_arg(call: &AstNode<'_, CallExpression<'_>>) -> bool { - call.as_ref().arguments.iter().any(|argument| { - matches!(&argument, Argument::ArrowFunctionExpression(_) | Argument::FunctionExpression(_)) - }) + analyze_call_arguments(call).has_arrow_or_function } fn has_simple_arguments<'a>(call: &AstNode<'a, CallExpression<'a>>) -> bool { - call.arguments().iter().all(|argument| SimpleArgument::new(argument).is_simple()) + analyze_call_arguments(call).all_simple } /// In order to detect those cases, we use an heuristic: if the first diff --git a/crates/oxc_formatter/src/utils/mod.rs b/crates/oxc_formatter/src/utils/mod.rs index bdcdac7282870..5c6aff10d3161 100644 --- a/crates/oxc_formatter/src/utils/mod.rs +++ b/crates/oxc_formatter/src/utils/mod.rs @@ -14,7 +14,11 @@ pub mod typecast; pub mod typescript; use oxc_allocator::Address; -use oxc_ast::{AstKind, ast::CallExpression}; +use oxc_ast::{ + AstKind, + ast::{Argument, CallExpression}, +}; +use oxc_span::{GetSpan, Span}; use crate::{ Format, FormatResult, FormatTrailingCommas, @@ -30,9 +34,109 @@ use crate::{ /// ``` pub fn is_long_curried_call(call: &AstNode<'_, CallExpression<'_>>) -> bool { if let AstNodes::CallExpression(parent_call) = call.parent { - return call.arguments().len() > parent_call.arguments().len() - && !parent_call.arguments().is_empty(); + let parent_args_len = parent_call.arguments().len(); + // Fast path: if parent has no arguments, it's not a curried call + if parent_args_len == 0 { + return false; + } + return call.arguments().len() > parent_args_len; } false } + +/// Check if an expression is used as a call argument by examining the parent node. +#[inline(always)] +pub fn is_expression_used_as_call_argument(span: Span, parent: &AstNodes) -> bool { + match parent { + AstNodes::CallExpression(call) => { + if call.arguments.is_empty() { + return false; + } + if call.callee.span().eq(&span) { + return false; + } + + // Unrolled loop optimization for common argument counts (95% of cases) + match call.arguments.len() { + 1 => { + // Single argument: most common case after empty + let arg_span = call.arguments[0].span(); + arg_span.eq(&span) || arg_span.contains_inclusive(span) + } + 2 => { + // Two arguments: second most common + let arg0_span = call.arguments[0].span(); + let arg1_span = call.arguments[1].span(); + arg0_span.eq(&span) + || arg0_span.contains_inclusive(span) + || arg1_span.eq(&span) + || arg1_span.contains_inclusive(span) + } + 3 => { + // Three arguments: unroll for cache efficiency + let spans = [ + call.arguments[0].span(), + call.arguments[1].span(), + call.arguments[2].span(), + ]; + spans + .iter() + .any(|&arg_span| arg_span.eq(&span) || arg_span.contains_inclusive(span)) + } + _ => { + // Rare case (>3 arguments): use cold path + check_many_arguments_cold(span, &call.arguments) + } + } + } + AstNodes::NewExpression(new_expr) => { + // Branch prediction: Most expressions are not arguments + if new_expr.arguments.is_empty() { + return false; + } + if new_expr.callee.span().eq(&span) { + return false; + } + + // Same unrolled optimization as CallExpression + match new_expr.arguments.len() { + 1 => { + let arg_span = new_expr.arguments[0].span(); + arg_span.eq(&span) || arg_span.contains_inclusive(span) + } + 2 => { + let arg0_span = new_expr.arguments[0].span(); + let arg1_span = new_expr.arguments[1].span(); + arg0_span.eq(&span) + || arg0_span.contains_inclusive(span) + || arg1_span.eq(&span) + || arg1_span.contains_inclusive(span) + } + 3 => { + let spans = [ + new_expr.arguments[0].span(), + new_expr.arguments[1].span(), + new_expr.arguments[2].span(), + ]; + spans + .iter() + .any(|&arg_span| arg_span.eq(&span) || arg_span.contains_inclusive(span)) + } + _ => check_many_arguments_cold(span, &new_expr.arguments), + } + } + _ => false, + } +} + +/// Cold path for checking many arguments - rarely called, optimized for code size not speed +#[cold] +#[inline(never)] +fn check_many_arguments_cold(span: Span, arguments: &[Argument]) -> bool { + // Iterator for rare complex cases (>3 arguments) + arguments.iter().any(|arg| { + let arg_span = arg.span(); + arg_span.eq(&span) || arg_span.contains_inclusive(span) + }) +} diff --git a/crates/oxc_formatter/src/write/arrow_function_expression.rs b/crates/oxc_formatter/src/write/arrow_function_expression.rs index 8f5266e743f67..9eee0a1db3a6d 100644 --- a/crates/oxc_formatter/src/write/arrow_function_expression.rs +++ b/crates/oxc_formatter/src/write/arrow_function_expression.rs @@ -10,10 +10,7 @@ use crate::{ Buffer, Comments, Format, FormatError, FormatResult, Formatter, SourceText, buffer::RemoveSoftLinesBuffer, prelude::*, - trivia::{ - DanglingIndentMode, FormatDanglingComments, FormatLeadingComments, - FormatTrailingComments, format_trailing_comments, - }, + trivia::{FormatLeadingComments, format_trailing_comments}, }, options::FormatTrailingCommas, utils::{assignment_like::AssignmentLikeLayout, expression::ExpressionLeftSide}, @@ -99,7 +96,7 @@ impl<'a> Format<'a> for FormatJsArrowFunctionExpression<'a, '_> { arrow, self.options.call_arg_layout.is_some(), true, - self.options.cache_mode + self.options.cache_mode, ), space(), "=>" @@ -162,19 +159,25 @@ impl<'a> Format<'a> for FormatJsArrowFunctionExpression<'a, '_> { }; } - let body_has_soft_line_break = - arrow_expression.is_none_or(|expression| match expression { + #[expect(clippy::match_same_arms)] + let body_has_soft_line_break = arrow_expression.is_none_or(|expression| { + match expression { Expression::ArrowFunctionExpression(_) | Expression::ArrayExpression(_) | Expression::ObjectExpression(_) => { - !f.comments().has_leading_own_line_comment(body.span().start) + // TODO: It seems no difference whether check there is a leading comment or not. + // !f.comments().has_leading_own_line_comment(body.span().start) + true } Expression::JSXElement(_) | Expression::JSXFragment(_) => true, _ => { is_multiline_template_starting_on_same_line(expression, f.source_text()) } - }); + } + }); + let body_is_condition_type = + matches!(arrow_expression, Some(Expression::ConditionalExpression(_))); if body_has_soft_line_break { write!(f, [formatted_signature, space(), format_body]) } else { @@ -185,13 +188,11 @@ impl<'a> Format<'a> for FormatJsArrowFunctionExpression<'a, '_> { Some(GroupedCallArgumentLayout::GroupedLastArgument) ); - let should_add_soft_line = is_last_call_arg + let should_add_soft_line = (is_last_call_arg // if it's inside a JSXExpression (e.g. an attribute) we should align the expression's closing } with the line with the opening {. - || (matches!(self.arrow.parent, AstNodes::JSXExpressionContainer(container) - if !f.context().comments().has_comment_in_range(arrow.span.end, container.span.end))); - - let body_is_condition_type = - matches!(arrow_expression, Some(Expression::ConditionalExpression(_))); + || matches!(self.arrow.parent, AstNodes::JSXExpressionContainer(_))); + // TODO: it seems no difference whether check there is a comment or not. + //&& !f.context().comments().has_comments(node.syntax()); if body_is_condition_type { write!( @@ -285,20 +286,30 @@ impl<'a, 'b> ArrowFunctionLayout<'a, 'b> { let mut current = arrow; let mut should_break = false; + // Three-way context handling for chain building: + // 1. Assignments: always build chains + // 2. Grouped call arguments: always build chains + // 3. Unspecified call arguments: build only if complex (3+ arrows, type params, etc.) + let should_build_chain = options.assignment_layout.is_some() + || options.call_arg_layout.is_some() + || (options.call_arg_layout.is_none() + && options.assignment_layout.is_none() + && Self::should_build_unspecified_chain(arrow.as_ref())); + loop { if current.expression() && let Some(AstNodes::ExpressionStatement(expr_stmt)) = current.body().statements().first().map(AstNode::::as_ast_nodes) && let AstNodes::ArrowFunctionExpression(next) = &expr_stmt.expression().as_ast_nodes() - && matches!( - options.call_arg_layout, - None | Some(GroupedCallArgumentLayout::GroupedLastArgument) - ) + && should_build_chain { - should_break = should_break || Self::should_break_chain(current); + // Build chains based on context + let should_break_current = Self::should_break_chain(current); + let should_break_next = Self::should_break_chain(next); - should_break = should_break || Self::should_break_chain(next); + should_break = should_break || should_break_current; + should_break = should_break || should_break_next; if head.is_none() { head = Some(current); @@ -313,9 +324,10 @@ impl<'a, 'b> ArrowFunctionLayout<'a, 'b> { None => ArrowFunctionLayout::Single(current), Some(head) => ArrowFunctionLayout::Chain(ArrowChain { head, - middle, tail: current, expand_signatures: should_break, + is_short_chain: middle.is_empty(), // Only 2 arrows if middle is empty + middle, options, }), }; @@ -350,6 +362,34 @@ impl<'a, 'b> ArrowFunctionLayout<'a, 'b> { let has_type_and_parameters = arrow.return_type.is_some() && has_parameters; has_type_and_parameters || has_rest_object_or_array_parameter(parameters) } + + /// Checks if we should build a chain for an unspecified (non-grouped) call argument. + /// Returns true for complex chains that need breaking, false for simple 2-arrow chains. + /// + /// This is used when `call_arg_layout` is `None` (not grouped) to decide if chains + /// should be built based on their complexity. + fn should_build_unspecified_chain(arrow: &ArrowFunctionExpression<'a>) -> bool { + // Count chain length by traversing expression-bodied arrows + let mut current = arrow; + let mut chain_length = 1; + + while current.expression + && let Some(body_stmt) = current.body.statements.first() + && let Statement::ExpressionStatement(expr_stmt) = body_stmt + && let Expression::ArrowFunctionExpression(next) = &expr_stmt.expression + { + chain_length += 1; + current = next; + } + + // Build chains for: + // - 3+ arrows (long chains need breaking) + // - Chains with type parameters (complex) + // - Chains with complex parameters + chain_length >= 3 + || arrow.type_parameters.is_some() + || !has_only_simple_parameters(&arrow.params, true) + } } /// Returns `true` for a template that starts on the same line as the previous token and contains a line break. @@ -407,6 +447,10 @@ struct ArrowChain<'a, 'b> { /// Whether the group wrapping the signatures should be expanded or not. expand_signatures: bool, + + /// Whether this is a short chain (only 2 arrows: head + tail, no middle). + /// Short chains in call arguments are formatted more compactly. + is_short_chain: bool, } impl<'a, 'b> ArrowChain<'a, 'b> { @@ -426,6 +470,10 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { let is_assignment_rhs = self.options.assignment_layout.is_some(); let is_grouped_call_arg_layout = self.options.call_arg_layout.is_some(); + // Check if this arrow function is a call argument (even if not grouped) + let is_call_argument = is_grouped_call_arg_layout + || crate::utils::is_expression_used_as_call_argument(self.head.span, head_parent); + // If this chain is the callee in a parent call expression, then we // want it to break onto a new line to clearly show that the arrow // chain is distinct and the _result_ is what's being called. @@ -436,8 +484,16 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { // () => () => // a // )(); - let is_callee = - matches!(head_parent, AstNodes::CallExpression(_) | AstNodes::NewExpression(_)); + // + // We need to verify the arrow is actually the CALLEE (the thing being called), + // not just an argument to a call. Check if arrow's span is within the callee's span. + let is_callee = match head_parent { + AstNodes::CallExpression(call) => call.callee.span().contains_inclusive(self.head.span), + AstNodes::NewExpression(new_expr) => { + new_expr.callee.span().contains_inclusive(self.head.span) + } + _ => false, + }; // With arrays, objects, sequence expressions, and block function bodies, // the opening brace gives a convenient boundary to insert a line break, @@ -454,20 +510,45 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { // If the body is _not_ one of those kinds, then we'll want to insert a // soft line break before the body so that it prints on a separate line // in its entirety. - let body_on_separate_line = !tail.get_expression().is_none_or(|expression| { + // For call arguments with simple literals, keep them inline to match Prettier + let body_on_separate_line = if self.is_short_chain + && is_call_argument + && !self.arrows().any(|arrow| arrow.type_parameters.is_some()) + && self.arrows().all(|arrow| has_only_simple_parameters(&arrow.params, true)) + { + // Short chains (2 arrows) in call arguments with simple parameters: + // only break if body naturally breaks + // This keeps `(a) => (b) => dispatch(c)` inline + // But breaks `() => () => 1` because type parameters add complexity + // And breaks `(c=default, d=default) => (e) => 0` because complex params add complexity matches!( - expression, - Expression::ObjectExpression(_) - | Expression::ArrayExpression(_) - | Expression::SequenceExpression(_) - | Expression::JSXElement(_) - | Expression::JSXFragment(_) + tail.get_expression(), + Some( + Expression::ObjectExpression(_) + | Expression::ArrayExpression(_) + | Expression::SequenceExpression(_) + | Expression::JSXElement(_) + | Expression::JSXFragment(_) + ) ) - }); + } else { + // Long chains or assignments: use full breaking logic + !tail.get_expression().is_none_or(|expression| { + matches!( + expression, + Expression::ObjectExpression(_) + | Expression::ArrayExpression(_) + | Expression::SequenceExpression(_) + | Expression::JSXElement(_) + | Expression::JSXFragment(_) + ) + }) + }; // If the arrow chain will break onto multiple lines, either because // it's a callee or because the body is printed on its own line, then // the signatures should be expanded first. + // For call arguments, break signatures but don't expand them (controlled by expand_signatures) let break_signatures = (is_callee && body_on_separate_line) || matches!( self.options.assignment_layout, @@ -505,46 +586,34 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { let is_first = is_first_in_chain; let formatted_signature = format_with(|f| { - let format_leading_comments = format_once(|f| { - if should_format_comments { - // A grouped layout implies that the arrow chain is trying to be rendered - // in a condensed, single-line format (at least the signatures, not - // necessarily the body). In that case, we _need_ to prevent the leading - // comments from inserting line breaks. But if it's _not_ a grouped layout, - // then we want to _force_ the line break so that the leading comments - // don't inadvertently end up on the previous line after the fat arrow. - if is_grouped_call_arg_layout { - write!(f, [space(), format_leading_comments(arrow.span())]) - } else { - write!( - f, - [ - soft_line_break_or_space(), - format_leading_comments(arrow.span()) - ] - ) - } + if should_format_comments { + // A grouped layout implies that the arrow chain is trying to be rendered + // in a condensed, single-line format (at least the signatures, not + // necessarily the body). In that case, we _need_ to prevent the leading + // comments from inserting line breaks. But if it's _not_ a grouped layout, + // then we want to _force_ the line break so that the leading comments + // don't inadvertently end up on the previous line after the fat arrow. + if is_grouped_call_arg_layout { + write!(f, [space(), format_leading_comments(arrow.span())])?; } else { - Ok(()) + write!( + f, + [ + soft_line_break_or_space(), + format_leading_comments(arrow.span()) + ] + )?; } - }); + } - let start = arrow.span().start; write!( f, - [ - FormatContentWithCacheMode::new( - Span::new(start, start), - format_leading_comments, - self.options.cache_mode, - ), - format_signature( - arrow, - is_grouped_call_arg_layout, - is_first, - self.options.cache_mode - ) - ] + [format_signature( + arrow, + is_grouped_call_arg_layout, + is_first, + self.options.cache_mode, + )] ) }); @@ -557,6 +626,9 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { // its own indention. This ensures that the first item keeps the same // level as the surrounding content, and then each subsequent item has // one additional level, as shown above. + // EXCEPTION: When has_initial_indent is true (assignments, callees), the + // whole chain is already wrapped in indent(), so we don't add extra indent + // to middle arrows to avoid double-indenting. if is_first_in_chain || has_initial_indent { is_first_in_chain = false; write!(f, [formatted_signature])?; @@ -574,7 +646,7 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { Ok(()) }); - group(&join_signatures).should_expand(*expand_signatures).fmt(f) + write!(f, [group(&join_signatures).should_expand(*expand_signatures)]) }); let format_tail_body_inner = format_with(|f| { @@ -622,6 +694,14 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { write!(f, [format_tail_body])?; } } + + // Format the trailing comments of all arrow function EXCEPT the first one because + // the comments of the head get formatted as part of the `FormatJsArrowFunctionExpression` call. + // TODO: It seems unneeded in the current oxc implementation? + // for arrow in self.arrows().skip(1) { + // write!(f, format_trailing_comments(arrow.span().end))?; + // } + Ok(()) }); @@ -630,6 +710,7 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { let should_add_soft_line = matches!(head_parent, AstNodes::JSXExpressionContainer(_)); if body_on_separate_line { + // Use normal indent for arrow chains to match Prettier write!( f, [ @@ -663,11 +744,7 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> { write!(f, [space(), "=>"])?; - if is_grouped_call_arg_layout { - write!(f, [group(&format_tail_body)])?; - } else { - write!(f, [indent_if_group_breaks(&format_tail_body, group_id)])?; - } + write!(f, [indent_if_group_breaks(&format_tail_body, group_id)])?; if is_callee { write!(f, [if_group_breaks(&soft_line_break()).with_group_id(Some(group_id))])?; @@ -742,7 +819,9 @@ fn format_signature<'a, 'b>( }); let format_head = FormatContentWithCacheMode::new(arrow.params.span, content, cache_mode); - if is_first_or_last_call_argument { + if is_first_or_last_call_argument && is_first_in_chain { + // Only enforce strict no-break policy for the first signature in grouped arguments + // Chains need to be able to break for proper formatting let mut buffer = RemoveSoftLinesBuffer::new(f); let mut recording = buffer.start_recording(); @@ -765,12 +844,12 @@ fn format_signature<'a, 'b>( )?; } - // Print comments before the fat arrow (`=>`) - let comments_before_fat_arrow = - f.context().comments().comments_before_character(arrow.params.span().end, b'='); - let content = - format_once(|f| FormatTrailingComments::Comments(comments_before_fat_arrow).fmt(f)); - write!(f, [FormatContentWithCacheMode::new(arrow.span, content, cache_mode)]) + // TODO: for case `a = (x: any): x is string /* comment */ => {}` + // if f.comments().has_dangling_comments(arrow.span()) { + // write!(f, [space(), format_dangling_comments(arrow.span())])?; + // } + + Ok(()) }) } diff --git a/crates/oxc_formatter/src/write/as_or_satisfies_expression.rs b/crates/oxc_formatter/src/write/as_or_satisfies_expression.rs index 9f5bf2de7ae15..5f74e59bb3163 100644 --- a/crates/oxc_formatter/src/write/as_or_satisfies_expression.rs +++ b/crates/oxc_formatter/src/write/as_or_satisfies_expression.rs @@ -48,7 +48,16 @@ fn format_as_or_satisfies_expression<'a>( }); if is_callee_or_object { - write!(f, [group(&soft_block_indent(&format_inner))]) + // When as/satisfies is the object of a member expression or callee of a call, + // wrap with indent and soft breaks to allow proper line breaking. + // This matches Prettier's formatting: group([indent([softline, ...parts]), softline]) + write!( + f, + [group(&format_args!( + indent(&format_args!(soft_line_break(), format_inner)), + soft_line_break() + ))] + ) } else { write!(f, [format_inner]) } @@ -56,13 +65,14 @@ fn format_as_or_satisfies_expression<'a>( fn is_callee_or_object_context(span: Span, parent: &AstNodes<'_>) -> bool { match parent { - // Callee - AstNodes::CallExpression(_) | AstNodes::NewExpression(_) - // Static member - | AstNodes::StaticMemberExpression(_) => true, - AstNodes::ComputedMemberExpression(member) => { - member.object.span() == span - } + // Only the callee of a call expression needs special formatting + AstNodes::CallExpression(call) => call.callee.span() == span, + // Only the callee of a new expression needs special formatting + AstNodes::NewExpression(new_expr) => new_expr.callee.span() == span, + // All static member expressions need special formatting (as expression is always the object) + AstNodes::StaticMemberExpression(_) => true, + // Only when the as expression is the object of a computed member expression + AstNodes::ComputedMemberExpression(member) => member.object.span() == span, _ => false, } } diff --git a/crates/oxc_formatter/src/write/binary_like_expression.rs b/crates/oxc_formatter/src/write/binary_like_expression.rs index 3228c9ec3629f..2b54c61ca05d0 100644 --- a/crates/oxc_formatter/src/write/binary_like_expression.rs +++ b/crates/oxc_formatter/src/write/binary_like_expression.rs @@ -17,6 +17,39 @@ use crate::{ use crate::{format_args, formatter::prelude::*, write}; +/// Checks if an expression contains class expressions followed by division operators +/// that could create ASI (Automatic Semicolon Insertion) ambiguity when formatted +/// with line breaks. +/// +/// This specifically detects patterns like `class{} / foo` where a line break +/// between the class expression and division operator could cause the parser to +/// insert a semicolon, changing the AST structure on subsequent format passes. +pub fn has_asi_risky_division_pattern(expr: &Expression) -> bool { + match expr { + Expression::BinaryExpression(bin) if matches!(bin.operator, BinaryOperator::Division) => { + // Check if left side is or contains a class expression + has_class_expression(&bin.left) + } + Expression::CallExpression(call) => { + // Check if the callee is a risky division + has_asi_risky_division_pattern(&call.callee) + } + _ => false, + } +} + +/// Helper: Check if expression is or contains a class expression +fn has_class_expression(expr: &Expression) -> bool { + match expr { + Expression::ClassExpression(_) => true, + Expression::ParenthesizedExpression(paren) => has_class_expression(&paren.expression), + Expression::BinaryExpression(bin) if matches!(bin.operator, BinaryOperator::Division) => { + has_class_expression(&bin.left) + } + _ => false, + } +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum BinaryLikeOperator { BinaryOperator(BinaryOperator), @@ -158,6 +191,13 @@ impl<'a, 'b> BinaryLikeExpression<'a, 'b> { matches!(container.parent, AstNodes::JSXAttribute(_)) } AstNodes::ExpressionStatement(statement) => statement.is_arrow_function_body(), + AstNodes::CallExpression(call) => { + // https://github.com/prettier/prettier/issues/18057#issuecomment-3472912112 + // Special case: Boolean(expr) with single argument should not indent + !call.optional + && call.arguments.len() == 1 + && matches!(&call.callee, Expression::Identifier(ident) if ident.name == "Boolean") + } AstNodes::ConditionalExpression(conditional) => { !matches!( parent.parent(), @@ -165,16 +205,17 @@ impl<'a, 'b> BinaryLikeExpression<'a, 'b> { | AstNodes::ThrowStatement(_) | AstNodes::CallExpression(_) | AstNodes::ImportExpression(_) - | AstNodes::MetaProperty(_) - ) && - // TODO(prettier): Why not include `NewExpression` ??? - !matches!(parent.parent(), AstNodes::Argument(argument) if matches!(argument.parent, AstNodes::CallExpression(_))) - } - AstNodes::Argument(argument) => { - // https://github.com/prettier/prettier/issues/18057#issuecomment-3472912112 - matches!(argument.parent, AstNodes::CallExpression(call) if call.arguments.len() == 1 && - matches!(&call.callee, Expression::Identifier(ident) if ident.name == "Boolean")) + | AstNodes::MetaProperty(_) // TODO(prettier): Why not include `NewExpression` ??? + ) } + AstNodes::ConditionalExpression(conditional) => !matches!( + conditional.parent, + AstNodes::ReturnStatement(_) + | AstNodes::ThrowStatement(_) + | AstNodes::CallExpression(_) + | AstNodes::ImportExpression(_) + | AstNodes::MetaProperty(_) + ), _ => false, } } diff --git a/crates/oxc_formatter/src/write/call_arguments.rs b/crates/oxc_formatter/src/write/call_arguments.rs index a9727a74fd67a..fe1efd8592fb2 100644 --- a/crates/oxc_formatter/src/write/call_arguments.rs +++ b/crates/oxc_formatter/src/write/call_arguments.rs @@ -44,8 +44,6 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { let l_paren_token = "("; let r_paren_token = ")"; - let arguments = self.as_ref(); - if self.is_empty() { return write!( f, @@ -67,12 +65,36 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { None }; + let new_expression = + if let AstNodes::NewExpression(new_expr) = self.parent { Some(new_expr) } else { None }; + + let (is_commonjs_or_amd_call, is_test_call) = call_expression + .map(|call| (is_commonjs_or_amd_call(self, call, f), is_test_call_expression(call))) + .unwrap_or_default(); + + // For test calls: use compact formatting (space-separated, no breaks) + // EXCEPT when there are exactly 2 args and the first is NOT a string/template. + // This handles patterns like: + // - it("name", () => {}) - compact + // - it("name", async(() => {})) - compact + // - beforeEach(async(() => {})) - compact + // - it(() => {}, () => {}) - NOT compact (2 args, first not string) + let should_use_compact_test_formatting = is_test_call + && (self.len() != 2 + || matches!( + self.as_ref().first(), + Some( + Argument::StringLiteral(_) + | Argument::TemplateLiteral(_) + | Argument::TaggedTemplateExpression(_) + ) + )); + if is_simple_module_import - || call_expression.is_some_and(|call| { - is_commonjs_or_amd_call(self, call, f) || is_test_call_expression(call) - }) + || is_commonjs_or_amd_call || is_multiline_template_only_args(self, f.source_text()) || is_react_hook_with_deps_array(self, f.comments()) + || should_use_compact_test_formatting { return write!( f, @@ -92,28 +114,27 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { ); } - // Check if there's an empty line (2+ newlines) between any consecutive arguments. - // This is used to preserve intentional blank lines in the original source. - let has_empty_line = arguments.windows(2).any(|window| { - let (cur_arg, next_arg) = (&window[0], &window[1]); - - // Count newlines between arguments, short-circuiting at 2 for performance - // Check if there are at least two newlines between arguments - f.source_text() - .bytes_range(cur_arg.span().end, next_arg.span().start) - .iter() - .filter(|&&b| b == b'\n') - .nth(1) - .is_some() - }); - + let has_empty_line = + self.iter().any(|arg| f.source_text().get_lines_before(arg.span(), f.comments()) > 1); if has_empty_line - || (!matches!(self.grand_parent(), AstNodes::Decorator(_)) + || (!matches!(self.parent.parent(), AstNodes::Decorator(_)) && is_function_composition_args(self)) + || has_long_arrow_chain_arg(self) { return format_all_args_broken_out(self, true, f); } + // Check if this is a simple nested call that should stay compact + if call_expression.is_some_and(|call| should_keep_nested_call_compact(call, self, f)) { + return format_compact_call_arguments(self, f); + } + + // Check if this is a simple nested new expression that should stay compact + if new_expression.is_some_and(|new_expr| should_keep_nested_new_compact(new_expr, self, f)) + { + return format_compact_call_arguments(self, f); + } + if let Some(group_layout) = arguments_grouped_layout(self, f) { write_grouped_arguments(self, group_layout, f) } else if call_expression.is_some_and(|call| is_long_curried_call(call)) { @@ -136,6 +157,198 @@ impl<'a> Format<'a> for AstNode<'a, ArenaVec<'a, Argument<'a>>> { } } +/// Checks if this call should keep its arguments compact because it's a +/// simple nested call used as an argument. +/// +/// Returns `true` for patterns like: +/// - `require(path.join(arg1, arg2))` +/// - `logger.error(pipe(call1(), call2()))` +/// +/// After removing AstKind::Argument, these calls are direct children of +/// their parent CallExpression, so we check if our span is within the +/// parent's argument spans. +fn should_keep_nested_call_compact( + call: &AstNode<'_, CallExpression<'_>>, + args: &[Argument<'_>], + f: &Formatter<'_, '_>, +) -> bool { + // Only applies to calls with 2-4 simple arguments + if args.len() < 2 || args.len() > 4 { + return false; + } + + // Don't use compact formatting if there are any comments in the arguments + // Check before first argument and between all arguments + let call_span = call.span; + for (i, arg) in args.iter().enumerate() { + let previous_end = if i == 0 { + call.callee.span().end // Check from after callee to first arg + } else { + args[i - 1].span().end // Check between args + }; + + let current_span = arg.span(); + if f.comments().has_comment_in_range(previous_end, current_span.start) { + return false; + } + } + + // Also check after last argument (trailing comments) + if let Some(last_arg) = args.last() + && f.comments().has_comment_in_range(last_arg.span().end, call_span.end) + { + return false; + } + + // Estimate total argument length to avoid keeping very long calls compact + let total_length: u32 = args.iter().map(|arg| arg.span().size()).sum(); + // If arguments are too long (> 60 chars), let the normal formatting handle it + if total_length > 60 { + return false; + } + + // Check if arguments are relatively simple + // Allow some complex expressions like conditionals and simple calls if they're short + let all_args_acceptable = args.iter().all(|arg| { + match arg.as_expression() { + Some(expr) => match expr { + // Exclude objects and arrays - they often break across lines during formatting + // even when marked as "simple", which defeats the purpose of compact formatting + Expression::ObjectExpression(_) | Expression::ArrayExpression(_) => false, + // Allow conditionals, binary/logical expressions, and simple calls + Expression::ConditionalExpression(_) + | Expression::LogicalExpression(_) + | Expression::BinaryExpression(_) + | Expression::CallExpression(_) => true, + // For other expressions, use the simple check + _ => is_relatively_short_argument(expr), + }, + None => false, // SpreadElement is not acceptable + } + }); + + if !all_args_acceptable { + return false; + } + + // Check if this call is itself an argument to another call + // After AstKind::Argument removal, parent is directly the CallExpression + match call.parent { + AstNodes::CallExpression(parent_call) => { + // Verify our span is within parent's arguments (not the callee) + if parent_call.callee.span().contains_inclusive(call.span) { + return false; // We're the callee, not an argument + } + + // Check if our span is within any of parent's arguments + parent_call.arguments.iter().any(|arg| arg.span().contains_inclusive(call.span)) + } + AstNodes::NewExpression(parent_new) => { + // Same check for new expressions + if parent_new.callee.span().contains_inclusive(call.span) { + return false; + } + + parent_new.arguments.iter().any(|arg| arg.span().contains_inclusive(call.span)) + } + _ => false, + } +} + +/// Checks if this new expression should keep its arguments compact because it's a +/// simple nested new expression used as an argument. +/// +/// Returns `true` for patterns like: +/// - `assert.sameValue(Temporal.Instant.compare(new Temporal.Instant(-1000n), new Temporal.Instant(1000n)), -1)` +/// - `compareArray(new TA([0, 1, 2, 3]).copyWithin(0, 1, -1), [1, 2, 2, 3])` +/// +/// Similar to `should_keep_nested_call_compact` but for NewExpression. +fn should_keep_nested_new_compact( + new_expr: &AstNode<'_, NewExpression<'_>>, + args: &[Argument<'_>], + f: &Formatter<'_, '_>, +) -> bool { + // Only applies to new expressions with 1-4 simple arguments + if args.is_empty() || args.len() > 4 { + return false; + } + + // Don't use compact formatting if there are any comments in the arguments + let new_span = new_expr.span; + for (i, arg) in args.iter().enumerate() { + let previous_end = if i == 0 { + new_expr.callee.span().end // Check from after callee to first arg + } else { + args[i - 1].span().end // Check between args + }; + + let current_span = arg.span(); + if f.comments().has_comment_in_range(previous_end, current_span.start) { + return false; + } + } + + // Also check after last argument (trailing comments) + if let Some(last_arg) = args.last() + && f.comments().has_comment_in_range(last_arg.span().end, new_span.end) + { + return false; + } + + // Estimate total argument length to avoid keeping very long calls compact + let total_length: u32 = args.iter().map(|arg| arg.span().size()).sum(); + // If arguments are too long (> 60 chars), let the normal formatting handle it + if total_length > 60 { + return false; + } + + // Check if arguments are relatively simple + let all_args_acceptable = args.iter().all(|arg| { + match arg.as_expression() { + Some(expr) => match expr { + // Exclude objects and arrays - they often break across lines during formatting + // even when marked as "simple", which defeats the purpose of compact formatting + Expression::ObjectExpression(_) | Expression::ArrayExpression(_) => false, + // Allow conditionals, binary/logical expressions, and simple calls/new + Expression::ConditionalExpression(_) + | Expression::LogicalExpression(_) + | Expression::BinaryExpression(_) + | Expression::CallExpression(_) + | Expression::NewExpression(_) => true, + // For other expressions, use the simple check + _ => is_relatively_short_argument(expr), + }, + None => false, // SpreadElement is not acceptable + } + }); + + if !all_args_acceptable { + return false; + } + + // Check if this new expression is itself an argument to another call/new + match new_expr.parent { + AstNodes::CallExpression(parent_call) => { + // Verify our span is within parent's arguments (not the callee) + if parent_call.callee.span().contains_inclusive(new_expr.span) { + return false; // We're the callee, not an argument + } + + // Check if our span is within any of parent's arguments + parent_call.arguments.iter().any(|arg| arg.span().contains_inclusive(new_expr.span)) + } + AstNodes::NewExpression(parent_new) => { + // Same check for parent new expressions + if parent_new.callee.span().contains_inclusive(new_expr.span) { + return false; + } + + parent_new.arguments.iter().any(|arg| arg.span().contains_inclusive(new_expr.span)) + } + _ => false, + } +} + /// Tests if a call has multiple anonymous function like (arrow or function expression) arguments. /// /// ## Examples @@ -182,6 +395,55 @@ pub fn is_function_composition_args(args: &[Argument<'_>]) -> bool { false } +/// Tests if a call has any long arrow chain argument (3+ arrows) that should force expansion. +/// +/// Expands chains with: +/// - Block bodies: `head => body => { console.log(); }` +/// - Object/array expression bodies: `a => b => ({ foo: bar })` +/// +/// Does NOT expand chains with simple expression bodies: +/// - Literals: `a => b => c => 1` +/// - Sequence expressions: `a => b => c => (1, 2, 3)` +/// - Conditionals: `a => b => c => (x ? y : z)` +fn has_long_arrow_chain_arg(args: &[Argument<'_>]) -> bool { + args.iter().any(|arg| { + if let Argument::ArrowFunctionExpression(arrow) = arg { + let mut current = arrow; + let mut chain_length = 1; + + while current.expression + && let Some(Statement::ExpressionStatement(expr_stmt)) = + current.body.statements.first() + && let Expression::ArrowFunctionExpression(next) = &expr_stmt.expression + { + chain_length += 1; + current = next; + } + + if chain_length < 3 { + return false; + } + + // Check if tail should force expansion + if !current.expression { + // Block body always forces expansion + return true; + } + + // For expression bodies, only expand for object/array expressions + if let Some(Expression::ObjectExpression(_) | Expression::ArrayExpression(_)) = + current.get_expression() + { + return true; + } + + false + } else { + false + } + }) +} + fn format_all_elements_broken_out<'a, 'b>( node: &'b AstNode<'a, ArenaVec<'a, Argument<'a>>>, elements: impl Iterator>>, usize)>, @@ -252,6 +514,33 @@ fn format_all_args_broken_out<'a, 'b>( ) } +/// Formats call arguments in a compact style that resists breaking. +/// Used for simple nested calls that should stay on one line when possible. +/// +/// Uses a simple group without soft_block_indent to avoid breaking when +/// the outer call adds indentation. +fn format_compact_call_arguments<'a>( + node: &AstNode<'a, ArenaVec<'a, Argument<'a>>>, + f: &mut Formatter<'_, 'a>, +) -> FormatResult<()> { + write!( + f, + [group(&format_args!( + "(", + format_with(|f| { + for (index, argument) in node.iter().enumerate() { + if index > 0 { + write!(f, [",", space()])?; + } + write!(f, [argument])?; + } + Ok(()) + }), + ")", + ))] + ) +} + pub fn arguments_grouped_layout( args: &[Argument], f: &Formatter<'_, '_>, diff --git a/crates/oxc_formatter/src/write/class.rs b/crates/oxc_formatter/src/write/class.rs index 86fa73dc06865..233ef7aec7281 100644 --- a/crates/oxc_formatter/src/write/class.rs +++ b/crates/oxc_formatter/src/write/class.rs @@ -224,29 +224,82 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSClassImplements<'a>>> { f, [ "implements", - group(&soft_line_indent_or_space(&format_once(|f| { - let last_index = self.len().saturating_sub(1); - let mut joiner = f.join_with(soft_line_break_or_space()); - - for (i, heritage) in FormatSeparatedIter::new(self.into_iter(), ",") - .with_trailing_separator(TrailingSeparator::Disallowed) - .enumerate() - { - if i == last_index { - // The trailing comments of the last heritage should be printed inside the class declaration - joiner.entry(&FormatNodeWithoutTrailingComments(&heritage)); + group(&indent(&format_args!( + soft_line_break_or_space(), + format_once(|f| { + let last_index = self.len().saturating_sub(1); + + // Check if any heritage items have inline (block) comments + let has_inline_comments = self.iter().any(|heritage| { + f.comments() + .comments_after(heritage.span().end) + .iter() + .any(|c| !c.is_line()) + }); + + // Use appropriate separator based on whether inline comments are present + if has_inline_comments { + // Keep inline comments on the same line + let mut joiner = f.join_with(space()); + + for (i, heritage) in FormatSeparatedIter::new(self.into_iter(), ",") + .with_trailing_separator(TrailingSeparator::Disallowed) + .enumerate() + { + if i == last_index { + // The trailing comments of the last heritage should be printed inside the class declaration + joiner.entry(&FormatNodeWithoutTrailingComments(&heritage)); + } else { + joiner.entry(&heritage); + } + } + + joiner.finish() } else { - joiner.entry(&heritage); - } - } + // Use soft_line_break_or_space() for normal formatting + let mut joiner = f.join_with(soft_line_break_or_space()); + + for (i, heritage) in self.iter().enumerate() { + joiner.entry(&format_heritage_with_trailing_comments( + heritage, + i == last_index, + )); + } - joiner.finish() - }))) + joiner.finish() + } + }) + ))) ] ) } } +/// Formats a heritage item in an implements clause with explicit trailing comment handling. +/// +/// For non-last items, trailing comments are explicitly formatted to ensure they stay +/// on the same line as the interface name (e.g., `implements z, // comment`). +/// For the last item, trailing comments are suppressed here and printed in the class body. +/// +/// This explicit handling is necessary because the automatic comment attachment mechanism +/// does not correctly associate trailing line comments with heritage items. +fn format_heritage_with_trailing_comments<'a>( + heritage: &'a AstNode<'a, TSClassImplements<'a>>, + is_last: bool, +) -> impl Format<'a> + 'a { + format_with(move |f: &mut Formatter<'_, 'a>| { + if is_last { + // Last item: no trailing comments (printed in class body), no comma + FormatNodeWithoutTrailingComments(heritage).fmt(f) + } else { + // Non-last items: explicit trailing comments + comma + heritage.write(f)?; + heritage.format_trailing_comments(f)?; + write!(f, ",") + } + }) +} + impl<'a> FormatWrite<'a> for AstNode<'a, TSClassImplements<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { write!(f, [self.expression(), self.type_arguments()]) diff --git a/crates/oxc_formatter/src/write/decorators.rs b/crates/oxc_formatter/src/write/decorators.rs index 7de4ee2c165fa..a1ab89bbda830 100644 --- a/crates/oxc_formatter/src/write/decorators.rs +++ b/crates/oxc_formatter/src/write/decorators.rs @@ -22,37 +22,42 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, Decorator<'a>>> { return Ok(()); }; - // Check parent to determine formatting context - match self.parent { - AstNodes::PropertyDefinition(_) - | AstNodes::MethodDefinition(_) - | AstNodes::AccessorProperty(_) => { - return write!( - f, - [group(&format_args!( - format_once(|f| { - f.join_nodes_with_soft_line().entries(self.iter()).finish() - }), - soft_line_break_or_space() - )) - .should_expand(should_expand_decorators(self, f))] - ); + let format_decorators = format_once(|f| { + // Check parent to determine formatting context + match self.parent { + AstNodes::PropertyDefinition(_) + | AstNodes::MethodDefinition(_) + | AstNodes::AccessorProperty(_) => { + return write!( + f, + [group(&format_args!( + format_once(|f| { + f.join_nodes_with_soft_line().entries(self.iter()).finish() + }), + soft_line_break_or_space() + )) + .should_expand(should_expand_decorators(self, f))] + ); + } + // Parameter decorators + AstNodes::FormalParameter(_) => { + write!(f, should_expand_decorators(self, f).then_some(expand_parent()))?; + } + AstNodes::ExportNamedDeclaration(_) | AstNodes::ExportDefaultDeclaration(_) => { + write!(f, [hard_line_break()])?; + } + _ => { + write!(f, [expand_parent()])?; + } } - // Parameter decorators - AstNodes::FormalParameter(_) => { - write!(f, should_expand_decorators(self, f).then_some(expand_parent()))?; - } - AstNodes::ExportNamedDeclaration(_) | AstNodes::ExportDefaultDeclaration(_) => { - write!(f, [hard_line_break()])?; - } - _ => { - write!(f, [expand_parent()])?; - } - } - f.join_with(&soft_line_break_or_space()).entries(self.iter()).finish()?; + f.join_with(&soft_line_break_or_space()).entries(self.iter()).finish()?; + + write!(f, [soft_line_break_or_space()]) + }); - write!(f, [soft_line_break_or_space()]) + format_decorators.fmt(f)?; + format_trailing_comments_for_last_decorator(last.span.end, f) } } @@ -73,27 +78,29 @@ fn is_identifier_or_static_member_only(callee: &Expression) -> bool { impl<'a> FormatWrite<'a> for AstNode<'a, Decorator<'a>> { fn write(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { - write!(f, ["@"]); - - // Determine if parentheses are required around decorator expressions - let needs_parentheses = match &self.expression { - // Identifiers: `@decorator` needs no parens - Expression::Identifier(_) => false, - // Call expressions: `@obj.method()` needs no parens, `@(complex().method)()` needs parens - Expression::CallExpression(call) => !is_identifier_or_static_member_only(&call.callee), - // Static member expressions: `@obj.prop` needs no parens, `@(complex[key])` needs parens - Expression::StaticMemberExpression(static_member) => { - !is_identifier_or_static_member_only(&static_member.object) - } - // All other expressions need parentheses: `@(a + b)`, `@(condition ? a : b)` + self.format_leading_comments(f)?; + write!(f, ["@"])?; + + // Check if we need to manually add parentheses for cases not handled by NeedsParentheses + let needs_manual_parentheses = match &self.expression { + // These expressions don't need manual parentheses: + // - ParenthesizedExpression already has parens + // - CallExpression, ComputedMemberExpression, StaticMemberExpression handled by NeedsParentheses + // - Identifiers don't need parens + Expression::ParenthesizedExpression(_) + | Expression::CallExpression(_) + | Expression::ComputedMemberExpression(_) + | Expression::StaticMemberExpression(_) + | Expression::Identifier(_) => false, + // All other complex expressions need parentheses _ => true, }; - if needs_parentheses { + if needs_manual_parentheses { write!(f, "(")?; } write!(f, [self.expression()])?; - if needs_parentheses { + if needs_manual_parentheses { write!(f, ")")?; } Ok(()) @@ -108,3 +115,32 @@ fn should_expand_decorators<'a>( ) -> bool { decorators.iter().any(|decorator| f.source_text().lines_after(decorator.span().end) > 0) } + +pub fn format_trailing_comments_for_last_decorator( + mut start: u32, + f: &mut Formatter<'_, '_>, +) -> FormatResult<()> { + let mut comments = f.context().comments().unprinted_comments(); + + for (i, comment) in comments.iter().enumerate() { + if !f.source_text().all_bytes_match(start, comment.span.start, |b| b.is_ascii_whitespace()) + { + comments = &comments[..i]; + break; + } + + start = comment.span.end; + } + + if !comments.is_empty() { + write!( + f, + [group(&format_args!( + FormatTrailingComments::Comments(comments), + soft_line_break_or_space() + ))] + )?; + } + + Ok(()) +} diff --git a/crates/oxc_formatter/src/write/jsx/element.rs b/crates/oxc_formatter/src/write/jsx/element.rs index 43700109fd6b2..eef9cb436c31e 100644 --- a/crates/oxc_formatter/src/write/jsx/element.rs +++ b/crates/oxc_formatter/src/write/jsx/element.rs @@ -187,17 +187,25 @@ pub fn should_expand(mut parent: &AstNodes<'_>) -> bool { parent = stmt.grand_parent(); } let maybe_jsx_expression_child = match parent { - AstNodes::ArrowFunctionExpression(arrow) if arrow.expression => match arrow.parent { - // Argument - AstNodes::Argument(argument) - if matches!(argument.parent, AstNodes::CallExpression(_)) => - { - argument.grand_parent() + AstNodes::ArrowFunctionExpression(arrow) if arrow.expression => { + // Check if this arrow function is used as a call argument + if crate::utils::is_expression_used_as_call_argument(arrow.span, arrow.parent) { + // Get the call expression's parent + if let AstNodes::CallExpression(call) = arrow.parent { + call.parent + } else if let AstNodes::NewExpression(new_expr) = arrow.parent { + new_expr.parent + } else { + return false; + } + } else { + // If it's the callee + match arrow.parent { + AstNodes::CallExpression(call) => call.parent, + _ => return false, + } } - // Callee - AstNodes::CallExpression(call) => call.parent, - _ => return false, - }, + } _ => return false, }; matches!( diff --git a/crates/oxc_formatter/src/write/jsx/mod.rs b/crates/oxc_formatter/src/write/jsx/mod.rs index f4d99f1696d73..e71cb58995312 100644 --- a/crates/oxc_formatter/src/write/jsx/mod.rs +++ b/crates/oxc_formatter/src/write/jsx/mod.rs @@ -183,11 +183,53 @@ impl<'a> FormatWrite<'a> for AstNode<'a, JSXExpressionContainer<'a>> { | JSXExpression::BinaryExpression(_) ); + // Check if this is an arrow function that needs special handling + let is_arrow_with_non_jsx_body = + if let JSXExpression::ArrowFunctionExpression(arrow) = &self.expression { + // Only apply special handling for expression arrows (not block arrows) + if arrow.expression { + // Check if the body is a JSX element + if let Some(Statement::ExpressionStatement(expr_stmt)) = + arrow.body.statements.first() + { + // Unwrap parenthesized expressions + let mut expr = &expr_stmt.expression; + while let Expression::ParenthesizedExpression(paren) = expr { + expr = &paren.expression; + } + // If body is JSX, treat like normal inline (no special handling) + // If body is NOT JSX, we need to add line break + !matches!( + expr, + Expression::JSXElement(_) | Expression::JSXFragment(_) + ) + } else { + false + } + } else { + false + } + } else { + false + }; + let should_inline = !has_comment(f) && (is_conditional_or_binary || should_inline_jsx_expression(self, f.comments())); - if should_inline { + if is_arrow_with_non_jsx_body { + // Arrow with non-JSX body: add line break before closing brace + write!( + f, + [group(&format_args!( + "{", + self.expression(), + line_suffix_boundary(), + soft_line_break(), + "}" + ))] + ) + } else if should_inline { write!(f, ["{", self.expression(), line_suffix_boundary(), "}"]) } else { write!( diff --git a/crates/oxc_formatter/src/write/member_expression.rs b/crates/oxc_formatter/src/write/member_expression.rs index ff0b3b8617b6f..a50f0ef033cba 100644 --- a/crates/oxc_formatter/src/write/member_expression.rs +++ b/crates/oxc_formatter/src/write/member_expression.rs @@ -150,9 +150,7 @@ fn layout<'a>( } match first_non_static_member_ancestor { - AstNodes::Argument(argument) if matches!(argument.parent, AstNodes::NewExpression(_)) => { - StaticMemberLayout::NoBreak - } + AstNodes::NewExpression(_) => StaticMemberLayout::NoBreak, AstNodes::AssignmentExpression(assignment) => { if matches!(assignment.left, AssignmentTarget::AssignmentTargetIdentifier(_)) { StaticMemberLayout::BreakAfterObject diff --git a/crates/oxc_formatter/src/write/mod.rs b/crates/oxc_formatter/src/write/mod.rs index 3d54c6bebd6bb..9c5d09bae49be 100644 --- a/crates/oxc_formatter/src/write/mod.rs +++ b/crates/oxc_formatter/src/write/mod.rs @@ -37,7 +37,9 @@ mod variable_declaration; pub use arrow_function_expression::{ FormatJsArrowFunctionExpression, FormatJsArrowFunctionExpressionOptions, }; -pub use binary_like_expression::{BinaryLikeExpression, BinaryLikeOperator, should_flatten}; +pub use binary_like_expression::{ + BinaryLikeExpression, BinaryLikeOperator, has_asi_risky_division_pattern, should_flatten, +}; pub use function::FormatFunctionOptions; use call_arguments::is_function_composition_args; diff --git a/crates/oxc_formatter/src/write/parameters.rs b/crates/oxc_formatter/src/write/parameters.rs index a080cff161f8f..1dbb41e432823 100644 --- a/crates/oxc_formatter/src/write/parameters.rs +++ b/crates/oxc_formatter/src/write/parameters.rs @@ -51,14 +51,28 @@ impl<'a> FormatWrite<'a> for AstNode<'a, FormalParameters<'a>> { let layout = if !self.has_parameter() && this_param.is_none() { ParameterLayout::NoParameters } else if can_hug || { - // `self`: Function - // `self.ancestors().nth(1)`: Argument - // `self.ancestors().nth(2)`: CallExpression - if let Some(AstNodes::CallExpression(call)) = self.ancestors().nth(2) { - is_test_call_expression(call) - } else { - false + // Check if these parameters are part of a test call expression + // by walking up the parent chain + let mut current_parent = Some(self.parent); + let mut is_in_test_call = false; + + while let Some(parent) = current_parent { + // Stop at root (Dummy node provides natural termination) + if matches!(parent, AstNodes::Dummy()) { + break; + } + + if let AstNodes::CallExpression(call) = parent + && is_test_call_expression(call) + { + is_in_test_call = true; + break; + } + + current_parent = Some(parent.parent()); } + + is_in_test_call } { ParameterLayout::Hug } else { diff --git a/crates/oxc_formatter/src/write/return_or_throw_statement.rs b/crates/oxc_formatter/src/write/return_or_throw_statement.rs index 2ade133df735b..afc6055bb24fd 100644 --- a/crates/oxc_formatter/src/write/return_or_throw_statement.rs +++ b/crates/oxc_formatter/src/write/return_or_throw_statement.rs @@ -91,10 +91,12 @@ impl<'a> Format<'a> for FormatAdjacentArgument<'a, '_> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { let argument = self.0; - if !matches!(argument.as_ref(), Expression::JSXElement(_) | Expression::JSXFragment(_)) - && has_argument_leading_comments(argument, f) - { - write!(f, [token("("), &block_indent(&argument), token(")")]) + let is_jsx = + matches!(argument.as_ref(), Expression::JSXElement(_) | Expression::JSXFragment(_)); + let is_jsx_suppressed = is_jsx && f.comments().is_suppressed(argument.span().start); + + if has_argument_leading_comments(argument, f) && (!is_jsx || is_jsx_suppressed) { + write!(f, [text("("), &block_indent(&argument), text(")")]) } else if argument.is_binaryish() { write!( f, diff --git a/crates/oxc_formatter/src/write/type_parameters.rs b/crates/oxc_formatter/src/write/type_parameters.rs index a3fb02d073e0e..4a6d3bfb3ebeb 100644 --- a/crates/oxc_formatter/src/write/type_parameters.rs +++ b/crates/oxc_formatter/src/write/type_parameters.rs @@ -69,7 +69,8 @@ impl<'a> Format<'a> for AstNode<'a, Vec<'a, TSTypeParameter<'a>>> { let trailing_separator = if self.len() == 1 // This only concern sources that allow JSX or a restricted standard variant. && f.context().source_type().is_jsx() - && matches!(self.grand_parent(), AstNodes::ArrowFunctionExpression(_)) + && !matches!(self.parent, AstNodes::Dummy()) + && matches!(self.parent.parent(), AstNodes::ArrowFunctionExpression(_)) // Ignore Type parameter with an `extends` clause or a default type. && !self.first().is_some_and(|t| t.constraint().is_some() || t.default().is_some()) { @@ -108,23 +109,63 @@ impl<'a> Format<'a> for FormatTSTypeParameters<'a, '_> { fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> { let params = self.decl.params(); if params.is_empty() && self.options.is_type_or_interface_decl { - write!(f, "<>") - } else { - write!( - f, - [group(&format_args!("<", format_once(|f| { - if matches!(self.decl.ancestors().nth(2), Some(AstNodes::CallExpression(call)) if is_test_call_expression(call)) - { - f.join_nodes_with_space().entries_with_trailing_separator(params, ",", TrailingSeparator::Omit).finish() + // Handle dangling comments inside empty type parameters + let comments = f.context().comments().comments_before(self.decl.span.end); + let indent = if comments.iter().any(|c| c.is_line()) { + DanglingIndentMode::Soft + } else { + DanglingIndentMode::None + }; + return write!(f, ["<", FormatDanglingComments::Comments { comments, indent }, ">"]); + } + if params.is_empty() { + // Handle dangling comments inside empty type parameters + let comments = f.context().comments().comments_before(self.decl.span.end); + let indent = if comments.iter().any(|c| c.is_line()) { + DanglingIndentMode::Soft + } else { + DanglingIndentMode::None + }; + return write!(f, ["<", FormatDanglingComments::Comments { comments, indent }, ">"]); + } + write!( + f, + [group(&format_args!( + "<", + format_once(|f| { + // Check if this type parameter declaration is inside a test call expression + // by walking up the parent chain + let mut current_parent = Some(self.decl.parent); + let mut is_test_call = false; + + while let Some(parent) = current_parent { + // Stop at root (Dummy node provides natural termination) + if matches!(parent, AstNodes::Dummy()) { + break; + } + + if let AstNodes::CallExpression(call) = parent + && is_test_call_expression(call) + { + is_test_call = true; + break; + } + + current_parent = Some(parent.parent()); + } + + if is_test_call { + f.join_nodes_with_space() + .entries_with_trailing_separator(params, ",", TrailingSeparator::Omit) + .finish() } else { soft_block_indent(¶ms).fmt(f) - }?; - - format_dangling_comments(self.decl.span).with_soft_block_indent().fmt(f) - }), ">")) - .with_group_id(self.options.group_id)] - ) - } + } + }), + ">" + )) + .with_group_id(self.options.group_id)] + ) } } diff --git a/crates/oxc_formatter/tests/fixtures/js/arguments/empty-lines.js.snap b/crates/oxc_formatter/tests/fixtures/js/arguments/empty-lines.js.snap index 8ae11d18029f3..60434b1eb9bae 100644 --- a/crates/oxc_formatter/tests/fixtures/js/arguments/empty-lines.js.snap +++ b/crates/oxc_formatter/tests/fixtures/js/arguments/empty-lines.js.snap @@ -53,20 +53,13 @@ call(() => { // ... }, "good"); -call( - () => { - // ... - }, - "good", -); - -call( - () => { - // ... - }, +call(() => { + // ... +}, "good"); - "good", -); +call(() => { + // ... +}, "good"); call( () => { diff --git a/crates/oxc_formatter/tests/fixtures/js/assignments/parenthesis.js.snap b/crates/oxc_formatter/tests/fixtures/js/assignments/parenthesis.js.snap index 80d67f1f7c7bc..bb3e9e0cc1b92 100644 --- a/crates/oxc_formatter/tests/fixtures/js/assignments/parenthesis.js.snap +++ b/crates/oxc_formatter/tests/fixtures/js/assignments/parenthesis.js.snap @@ -5,6 +5,6 @@ source: crates/oxc_formatter/tests/fixtures/mod.rs object[key = "a" + "b"]; ==================== Output ==================== -object[(key = "a" + "b")]; +object[key = "a" + "b"]; ===================== End ===================== diff --git a/crates/oxc_formatter/tests/fixtures/js/calls/blank-line.js.snap b/crates/oxc_formatter/tests/fixtures/js/calls/blank-line.js.snap index 8c2e4b58036fc..3f76f32a32225 100644 --- a/crates/oxc_formatter/tests/fixtures/js/calls/blank-line.js.snap +++ b/crates/oxc_formatter/tests/fixtures/js/calls/blank-line.js.snap @@ -12,8 +12,12 @@ gen "b"); ==================== Output ==================== -gen("a"); +gen( + "a", +); -gen("b"); +gen( + "b", +); ===================== End ===================== diff --git a/crates/oxc_formatter/tests/fixtures/js/comments/arrow.js.snap b/crates/oxc_formatter/tests/fixtures/js/comments/arrow.js.snap index b178b9f68e8a5..c846543ee6bce 100644 --- a/crates/oxc_formatter/tests/fixtures/js/comments/arrow.js.snap +++ b/crates/oxc_formatter/tests/fixtures/js/comments/arrow.js.snap @@ -31,32 +31,32 @@ call( ); ==================== Output ==================== -() => - // comment - []; +() => // comment +[]; -() => - // comment - ({}); +() => // comment +({}); -() /* comment1 */ => /* comment2 */ {}; +() => /* comment1 */ /* comment2 */ {}; -() /**/ => - // - () /**/ => - /**/ - () /**/ => /**/ { +() => + /**/ // + () => + /**/ /**/ + () => /**/ /**/ { // }; -call(() /* comment1 */ => /* comment2 */ {}); +call(() => /* comment1 */ /* comment2 */ {}); -call(() /**/ => - // - () /**/ => - /**/ - () /**/ => /**/ { - // -}); +call( + () => + /**/ // + () => + /**/ /**/ + () => /**/ /**/ { + // + }, +); ===================== End ===================== diff --git a/crates/oxc_formatter/tests/fixtures/js/comments/computed-member.js.snap b/crates/oxc_formatter/tests/fixtures/js/comments/computed-member.js.snap index 592c80b7edac6..878da46615931 100644 --- a/crates/oxc_formatter/tests/fixtures/js/comments/computed-member.js.snap +++ b/crates/oxc_formatter/tests/fixtures/js/comments/computed-member.js.snap @@ -18,7 +18,7 @@ prop[ ] = shouldCast; let handler = - props[(handlerName = toHandlerKey(event))] || // also try camelCase event handler (#2249) - props[(handlerName = toHandlerKey(camelize(event)))]; + props[handlerName = toHandlerKey(event)] || // also try camelCase event handler (#2249) + props[handlerName = toHandlerKey(camelize(event))]; ===================== End ===================== diff --git a/crates/oxc_formatter/tests/fixtures/js/jsx/arrow-expression.jsx.snap b/crates/oxc_formatter/tests/fixtures/js/jsx/arrow-expression.jsx.snap index 7cb339de26a8d..294de7ab29090 100644 --- a/crates/oxc_formatter/tests/fixtures/js/jsx/arrow-expression.jsx.snap +++ b/crates/oxc_formatter/tests/fixtures/js/jsx/arrow-expression.jsx.snap @@ -19,20 +19,19 @@ source: crates/oxc_formatter/tests/fixtures/mod.rs ==================== Output ==================== <>
- { - () => - function A() { - A(); - } /* comment */ + {() => + function A() { + A(); + } + /* comment */ }
- { - /* comment */ () => - function A() { - A(); - } + {/* comment */ () => + function A() { + A(); + } }
; diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index a27263ac5d8df..b536cb411ccc3 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -205,12 +205,6 @@ pub fn get_enclosing_function<'a, 'b>( } } -/// Returns if `arg` is the `n`th (0-indexed) argument of `call`. -pub fn is_nth_argument<'a>(call: &CallExpression<'a>, arg: &Argument<'a>, n: usize) -> bool { - let nth = &call.arguments[n]; - nth.span() == arg.span() -} - /// Jump to the outer most of chained parentheses if any pub fn outermost_paren<'a, 'b>( node: &'b AstNode<'a>, @@ -939,3 +933,44 @@ pub fn get_outer_member_expression<'a, 'b>( _ => None, } } +/// Check if a node's span is exactly equal to any argument span in a call or new expression +#[inline] +pub fn is_node_exact_call_argument<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { + let parent = ctx.nodes().parent_node(node.id()); + + match parent.kind() { + AstKind::CallExpression(call) => { + if call.arguments.is_empty() { + return false; + } + let node_span = node.span(); // Only compute span when needed + call.arguments.iter().any(|arg| arg.span() == node_span) + } + AstKind::NewExpression(new_expr) => { + if new_expr.arguments.is_empty() { + return false; + } + let node_span = node.span(); // Only compute span when needed + new_expr.arguments.iter().any(|arg| arg.span() == node_span) + } + _ => false, + } +} + +/// Check if a node's span is contained within any argument span in a call expression +#[inline] +pub fn is_node_within_call_argument<'a>( + node: &AstNode<'a>, + call: &CallExpression<'a>, + target_arg_index: usize, +) -> bool { + // Early exit for out-of-bounds index + if target_arg_index >= call.arguments.len() { + return false; + } + + let target_arg = &call.arguments[target_arg_index]; // Direct indexing, no Option unwrap + let node_span = node.span(); + let arg_span = target_arg.span(); + node_span.start >= arg_span.start && node_span.end <= arg_span.end +} diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index ab2bcb554b5a8..408ba3aa21c92 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -767,7 +767,6 @@ impl RuleRunner for crate::rules::eslint::no_unsafe_negation::NoUnsafeNegation { impl RuleRunner for crate::rules::eslint::no_unsafe_optional_chaining::NoUnsafeOptionalChaining { const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[ - AstType::Argument, AstType::ArrayExpression, AstType::AssignmentExpression, AstType::AssignmentPattern, diff --git a/crates/oxc_linter/src/rules/eslint/array_callback_return/mod.rs b/crates/oxc_linter/src/rules/eslint/array_callback_return/mod.rs index 030a8c223da29..f1ce2beaa53af 100644 --- a/crates/oxc_linter/src/rules/eslint/array_callback_return/mod.rs +++ b/crates/oxc_linter/src/rules/eslint/array_callback_return/mod.rs @@ -12,7 +12,7 @@ use serde_json::Value; use self::return_checker::{StatementReturnStatus, check_function_body}; use crate::{ AstNode, - ast_util::{get_enclosing_function, is_nth_argument, outermost_paren}, + ast_util::{get_enclosing_function, outermost_paren}, context::LintContext, rule::Rule, }; @@ -159,7 +159,6 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O // foo.every(nativeFoo || function foo() { ... }) AstKind::LogicalExpression(_) | AstKind::ConditionalExpression(_) - | AstKind::Argument(_) | AstKind::ParenthesizedExpression(_) | AstKind::ChainExpression(_) => { current_node = parent; @@ -189,10 +188,6 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O } AstKind::CallExpression(call) => { - let AstKind::Argument(current_node_arg) = current_node.kind() else { - return None; - }; - let callee = call.callee.get_inner_expression(); let callee = if let Some(member) = callee.as_member_expression() { member @@ -205,7 +200,10 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O // Array.from if callee.is_specific_member_access("Array", "from") { // Check that current node is parent's second argument - if call.arguments.len() == 2 && is_nth_argument(call, current_node_arg, 1) { + if call.arguments.len() == 2 + && let Some(call_arg) = call.arguments[1].as_expression() + && call_arg.span() == current_node.kind().span() + { return Some("from"); } } @@ -216,7 +214,10 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O if TARGET_METHODS.contains(&array_method) // Check that current node is parent's first argument && call.arguments.len() == 1 - && is_nth_argument(call, current_node_arg, 0) + && let Some(call_arg) = call.arguments.first() + && call_arg + .as_expression() + .is_some_and(|arg| arg.span() == current_node.kind().span()) { return Some(array_method); } diff --git a/crates/oxc_linter/src/rules/eslint/func_names.rs b/crates/oxc_linter/src/rules/eslint/func_names.rs index c31d889ea115c..b0bd13bd4a440 100644 --- a/crates/oxc_linter/src/rules/eslint/func_names.rs +++ b/crates/oxc_linter/src/rules/eslint/func_names.rs @@ -267,17 +267,22 @@ fn is_recursive_function(func: &Function, func_name: &str, ctx: &LintContext) -> if let Some(binding) = ctx.scoping().find_binding(func_scope_id, func_name) { return ctx.semantic().symbol_references(binding).any(|reference| { let parent = ctx.nodes().parent_node(reference.node_id()); - if matches!(parent.kind(), AstKind::CallExpression(_)) { - ctx.nodes().ancestors(reference.node_id()).any(|ancestor| { - if let AstKind::Function(f) = ancestor.kind() { - f.scope_id.get() == Some(func_scope_id) - } else { - false - } - }) - } else { - false + // Check if this reference is the callee of a call expression (direct recursive call) + if let AstKind::CallExpression(call_expr) = parent.kind() { + // Only consider it recursive if the reference is the callee, not an argument + if call_expr.callee.span() + == ctx.nodes().get_node(reference.node_id()).kind().span() + { + return ctx.nodes().ancestors(reference.node_id()).any(|ancestor| { + if let AstKind::Function(f) = ancestor.kind() { + f.scope_id.get() == Some(func_scope_id) + } else { + false + } + }); + } } + false }); } diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs index 4d07d20604c9c..a91780ec76696 100644 --- a/crates/oxc_linter/src/rules/eslint/getter_return.rs +++ b/crates/oxc_linter/src/rules/eslint/getter_return.rs @@ -169,9 +169,8 @@ impl GetterReturn { let parent_2 = ctx.nodes().parent_node(parent.id()); let parent_3 = ctx.nodes().parent_node(parent_2.id()); - let parent_4 = ctx.nodes().parent_node(parent_3.id()); // handle (X()) - match parent_4.kind() { + match parent_3.kind() { AstKind::ParenthesizedExpression(p) => { if Self::handle_paren_expr(&p.expression) { return true; @@ -185,9 +184,9 @@ impl GetterReturn { _ => {} } + let parent_4 = ctx.nodes().parent_node(parent_3.id()); let parent_5 = ctx.nodes().parent_node(parent_4.id()); - let parent_6 = ctx.nodes().parent_node(parent_5.id()); - match parent_6.kind() { + match parent_5.kind() { AstKind::ParenthesizedExpression(p) => { if Self::handle_paren_expr(&p.expression) { return true; diff --git a/crates/oxc_linter/src/rules/eslint/max_nested_callbacks.rs b/crates/oxc_linter/src/rules/eslint/max_nested_callbacks.rs index 0394e12e3c994..bcd41ce2cd610 100644 --- a/crates/oxc_linter/src/rules/eslint/max_nested_callbacks.rs +++ b/crates/oxc_linter/src/rules/eslint/max_nested_callbacks.rs @@ -129,7 +129,7 @@ fn is_callback<'a>(node: &AstNode<'a>, semantic: &Semantic<'a>) -> bool { is_function_node(node) && matches!( iter_outer_expressions(semantic.nodes(), node.id()).next(), - Some(AstKind::Argument(_)) + Some(AstKind::CallExpression(_)) ) } diff --git a/crates/oxc_linter/src/rules/eslint/no_eval.rs b/crates/oxc_linter/src/rules/eslint/no_eval.rs index eea13e45c0661..1c0ca01937d3d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_eval.rs +++ b/crates/oxc_linter/src/rules/eslint/no_eval.rs @@ -119,13 +119,12 @@ impl Rule for NoEval { for reference_id in references { let reference = ctx.scoping().get_reference(*reference_id); let node = ctx.nodes().get_node(reference.node_id()); - let mut parent = Self::outermost_mem_expr(node, ctx).unwrap(); if name == "eval" { - if !matches!(parent.kind(), AstKind::CallExpression(_)) { - ctx.diagnostic(no_eval_diagnostic(node.span())); - } + ctx.diagnostic(no_eval_diagnostic(node.span())); } else { + let mut parent = Self::outermost_mem_expr(node, ctx).unwrap(); + loop { match parent.kind() { AstKind::StaticMemberExpression(mem_expr) => { diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 58110ba5b813e..7921daa8f577d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -150,8 +150,12 @@ fn get_define_property_call<'a>( for parent in ctx.nodes().ancestors(node.id()) { if let AstKind::CallExpression(call_expr) = parent.kind() && is_define_property_call(call_expr) + && let Some(first_arg) = call_expr.arguments.first() { - return Some(parent); + let arg_span = first_arg.span(); + if arg_span.contains_inclusive(node.span()) { + return Some(parent); + } } } None @@ -217,7 +221,6 @@ fn get_prototype_property_accessed<'a>( return None; }; let parent = ctx.nodes().parent_node(node.id()); - let mut prototype_node = Some(parent); match parent.kind() { prop_access_expr if prop_access_expr.is_member_expression_kind() => { let prop_name = prop_access_expr @@ -226,14 +229,23 @@ fn get_prototype_property_accessed<'a>( if prop_name != "prototype" { return None; } + // Check if this member expression is wrapped in a ChainExpression let grandparent_node = ctx.nodes().parent_node(parent.id()); + let result_node = if let AstKind::ChainExpression(_) = grandparent_node.kind() { + // Return the ChainExpression + grandparent_node + } else { + // Return the MemberExpression + parent + }; - if let AstKind::ChainExpression(_) = grandparent_node.kind() { - let grandparent_parent = ctx.nodes().parent_node(grandparent_node.id()); - prototype_node = Some(grandparent_parent); + // Check if the result is wrapped in parentheses + let great_grandparent_node = ctx.nodes().parent_node(result_node.id()); + if let AstKind::ParenthesizedExpression(_) = great_grandparent_node.kind() { + Some(great_grandparent_node) + } else { + Some(result_node) } - - prototype_node } _ => None, } diff --git a/crates/oxc_linter/src/rules/eslint/no_extra_boolean_cast.rs b/crates/oxc_linter/src/rules/eslint/no_extra_boolean_cast.rs index 42daf1aac6ff3..56f203c5bc32e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extra_boolean_cast.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extra_boolean_cast.rs @@ -211,12 +211,7 @@ fn is_unary_negation(node: &AstNode) -> bool { fn get_real_parent<'a, 'b>(node: &AstNode, ctx: &'a LintContext<'b>) -> Option<&'a AstNode<'b>> { ctx.nodes().ancestors(node.id()).find(|parent| { - !matches!( - parent.kind(), - AstKind::Argument(_) - | AstKind::ParenthesizedExpression(_) - | AstKind::ChainExpression(_) - ) + !matches!(parent.kind(), AstKind::ParenthesizedExpression(_) | AstKind::ChainExpression(_)) }) } diff --git a/crates/oxc_linter/src/rules/eslint/no_import_assign.rs b/crates/oxc_linter/src/rules/eslint/no_import_assign.rs index d614264c36cbe..544415b8881c6 100644 --- a/crates/oxc_linter/src/rules/eslint/no_import_assign.rs +++ b/crates/oxc_linter/src/rules/eslint/no_import_assign.rs @@ -159,7 +159,7 @@ impl Rule for NoImportAssign { /// - `Reflect.setPrototypeOf` fn is_argument_of_well_known_mutation_function(node_id: NodeId, ctx: &LintContext<'_>) -> bool { let current_node = ctx.nodes().get_node(node_id); - let call_expression_node = ctx.nodes().parent_kind(ctx.nodes().parent_id(node_id)); + let call_expression_node = ctx.nodes().parent_kind(node_id); let AstKind::CallExpression(expr) = call_expression_node else { return false; diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index 29bd46662d263..96e8c82fff535 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -355,8 +355,8 @@ fn is_detectable_object(parent_kind: &AstKind<'_>) -> bool { matches!(parent_kind, AstKind::ObjectExpression(_) | AstKind::ObjectProperty(_)) } -fn is_parse_int_radix(parent_parent_node: &AstNode<'_>) -> bool { - let AstKind::CallExpression(expression) = parent_parent_node.kind() else { +fn is_parse_int_radix(parent_kind: &AstKind<'_>) -> bool { + let AstKind::CallExpression(expression) = parent_kind else { return false; }; @@ -483,7 +483,7 @@ impl NoMagicNumbers { let parent_parent = nodes.parent_node(parent.id()); - if is_parse_int_radix(parent_parent) { + if is_parse_int_radix(&parent.kind()) { return true; } diff --git a/crates/oxc_linter/src/rules/eslint/no_new_func.rs b/crates/oxc_linter/src/rules/eslint/no_new_func.rs index da4e17e995166..a85d8271f1697 100644 --- a/crates/oxc_linter/src/rules/eslint/no_new_func.rs +++ b/crates/oxc_linter/src/rules/eslint/no_new_func.rs @@ -2,7 +2,7 @@ use oxc_ast::{AstKind, ast::IdentifierReference}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::IsGlobalReference; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use crate::{AstNode, context::LintContext, rule::Rule}; @@ -78,6 +78,10 @@ impl Rule for NoNewFunc { return; }; + if !parent_call_expr.callee.span().contains_inclusive(node.span()) { + return; + } + let Some(static_property_name) = &member_expr.static_property_name().map(|s| s.as_str()) else { diff --git a/crates/oxc_linter/src/rules/eslint/no_unsafe_optional_chaining.rs b/crates/oxc_linter/src/rules/eslint/no_unsafe_optional_chaining.rs index af2ef54fd0909..13c5775cd468a 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unsafe_optional_chaining.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unsafe_optional_chaining.rs @@ -1,9 +1,6 @@ use oxc_ast::{ AstKind, - ast::{ - Argument, ArrayExpressionElement, AssignmentTarget, Expression, - match_assignment_target_pattern, - }, + ast::{ArrayExpressionElement, AssignmentTarget, Expression, match_assignment_target_pattern}, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -125,9 +122,6 @@ impl Rule for NoUnsafeOptionalChaining { AstKind::AssignmentPattern(pat) if pat.left.kind.is_destructuring_pattern() => { Self::check_unsafe_usage(&pat.right, ctx); } - AstKind::Argument(Argument::SpreadElement(elem)) => { - Self::check_unsafe_usage(&elem.argument, ctx); - } AstKind::VariableDeclarator(decl) if decl.id.kind.is_destructuring_pattern() => { if let Some(expr) = &decl.init { Self::check_unsafe_usage(expr, ctx); diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_private_class_members.rs b/crates/oxc_linter/src/rules/eslint/no_unused_private_class_members.rs index fd888ec452861..ee6b37e87aa60 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_private_class_members.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_private_class_members.rs @@ -215,7 +215,6 @@ fn is_value_context(parent: &AstNode, child: &AstNode, semantic: &Semantic<'_>) | AstKind::ArrayExpression(_) | AstKind::ObjectProperty(_) | AstKind::JSXExpressionContainer(_) - | AstKind::Argument(_) | AstKind::ChainExpression(_) | AstKind::StaticMemberExpression(_) | AstKind::ComputedMemberExpression(_) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs index fbfb6ba18566c..8ea896ffb9a03 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs @@ -416,11 +416,17 @@ impl<'a> Symbol<'_, 'a> { // used by others AstKind::VariableDeclarator(_) | AstKind::JSXExpressionContainer(_) - | AstKind::Argument(_) | AstKind::PropertyDefinition(_) => { // definitely used, short-circuit return false; } + AstKind::CallExpression(call_expr) + if call_expr.arguments_span().is_some_and(|span| { + span.contains_inclusive(self.nodes().get_node(reference.node_id()).span()) + }) => + { + return false; + } // When symbol is being assigned a new value, we flag the reference // as only affecting itself until proven otherwise. AstKind::UpdateExpression(UpdateExpression { argument, .. }) => { diff --git a/crates/oxc_linter/src/rules/eslint/prefer_object_spread.rs b/crates/oxc_linter/src/rules/eslint/prefer_object_spread.rs index e190503f75fcf..f65f2fd759289 100644 --- a/crates/oxc_linter/src/rules/eslint/prefer_object_spread.rs +++ b/crates/oxc_linter/src/rules/eslint/prefer_object_spread.rs @@ -143,12 +143,13 @@ impl Rule for PreferObjectSpread { let fixer = fixer.for_multifix(); let mut rule_fixes = fixer.new_fix_with_capacity(2 + call_expr.arguments.len() * 5); + let parent_kind = ctx.nodes().parent_kind(node.id()); let needs_paren = !matches!( - ctx.nodes().parent_kind(node.id()), + parent_kind, AstKind::VariableDeclarator(_) | AstKind::ArrayExpression(_) | AstKind::ReturnStatement(_) - | AstKind::Argument(_) + | AstKind::CallExpression(_) | AstKind::ObjectProperty(_) | AstKind::AssignmentExpression(_) ); diff --git a/crates/oxc_linter/src/rules/eslint/prefer_promise_reject_errors.rs b/crates/oxc_linter/src/rules/eslint/prefer_promise_reject_errors.rs index 479bb41d0e3ab..c7bedaf044f84 100644 --- a/crates/oxc_linter/src/rules/eslint/prefer_promise_reject_errors.rs +++ b/crates/oxc_linter/src/rules/eslint/prefer_promise_reject_errors.rs @@ -5,7 +5,7 @@ use oxc_ast::{ }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use schemars::JsonSchema; use crate::{ @@ -153,7 +153,11 @@ fn check_reject_in_function( ctx.symbol_references(reject_arg.symbol_id()).for_each(|reference| { if let AstKind::CallExpression(call_expr) = ctx.nodes().parent_kind(reference.node_id()) { - check_reject_call(call_expr, ctx, allow_empty_reject); + let ref_node = ctx.nodes().get_node(reference.node_id()); + + if call_expr.callee.span().contains_inclusive(ref_node.span()) { + check_reject_call(call_expr, ctx, allow_empty_reject); + } } }); return; diff --git a/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs b/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs index 20c0731cce1ac..d22a99e1b38eb 100644 --- a/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs +++ b/crates/oxc_linter/src/rules/jest/no_conditional_expect.rs @@ -128,6 +128,29 @@ impl Rule for NoConditionalExpect { } } +fn is_in_test_context<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { + let mut current = node; + loop { + current = ctx.nodes().parent_node(current.id()); + + if let AstKind::CallExpression(call_expr) = current.kind() { + let jest_node = PossibleJestNode { node: current, original: None }; + if is_type_of_jest_fn_call( + call_expr, + &jest_node, + ctx, + &[JestFnKind::General(JestGeneralFnKind::Test)], + ) { + return true; + } + } + + if matches!(current.kind(), AstKind::Program(_)) { + return false; + } + } +} + fn check_parents<'a>( node: &AstNode<'a>, visited: &mut FxHashSet, @@ -174,19 +197,29 @@ fn check_parents<'a>( let symbol_table = ctx.scoping(); let symbol_id = ident.symbol_id(); - // Consider cases like: - // ```javascript - // function foo() { - // foo() - // } - // ``` - // To avoid infinite loop, we need to check if the function is already visited when - // call `check_parents`. - let boolean = symbol_table.get_resolved_references(symbol_id).any(|reference| { - let parent = ctx.nodes().parent_node(reference.node_id()); - matches!(check_parents(parent, visited, in_conditional, ctx), InConditional(true)) - }); - return InConditional(boolean); + // Check if this function is used in a test context + let is_used_in_test = + symbol_table.get_resolved_references(symbol_id).any(|reference| { + let parent = ctx.nodes().parent_node(reference.node_id()); + + // Check if directly used as test callback + if let AstKind::CallExpression(call_expr) = parent.kind() { + let jest_node = PossibleJestNode { node: parent, original: None }; + if is_type_of_jest_fn_call( + call_expr, + &jest_node, + ctx, + &[JestFnKind::General(JestGeneralFnKind::Test)], + ) { + return true; + } + } + + // Check if called within a test context by traversing from the call site + is_in_test_context(parent, ctx) + }); + + return if is_used_in_test { in_conditional } else { InConditional(false) }; } AstKind::Program(_) => return InConditional(false), _ => {} diff --git a/crates/oxc_linter/src/rules/jest/no_standalone_expect/mod.rs b/crates/oxc_linter/src/rules/jest/no_standalone_expect/mod.rs index 360e2a97cfd9e..a8e0a1ac8757c 100644 --- a/crates/oxc_linter/src/rules/jest/no_standalone_expect/mod.rs +++ b/crates/oxc_linter/src/rules/jest/no_standalone_expect/mod.rs @@ -234,7 +234,7 @@ fn is_var_declarator_or_test_block<'a>( return true; } } - AstKind::Argument(_) | AstKind::ArrayExpression(_) | AstKind::ObjectExpression(_) => { + AstKind::ArrayExpression(_) | AstKind::ObjectExpression(_) => { let mut current = node; loop { let parent = ctx.nodes().parent_node(current.id()); @@ -247,9 +247,7 @@ fn is_var_declarator_or_test_block<'a>( ctx, ); } - AstKind::Argument(_) - | AstKind::ArrayExpression(_) - | AstKind::ObjectExpression(_) => { + AstKind::ArrayExpression(_) | AstKind::ObjectExpression(_) => { current = parent; } _ => break, diff --git a/crates/oxc_linter/src/rules/jest/valid_expect.rs b/crates/oxc_linter/src/rules/jest/valid_expect.rs index 4616349c492fd..8c9f78b8b1fa2 100644 --- a/crates/oxc_linter/src/rules/jest/valid_expect.rs +++ b/crates/oxc_linter/src/rules/jest/valid_expect.rs @@ -282,7 +282,6 @@ fn is_acceptable_return_node<'a, 'b>( match node.kind() { AstKind::ConditionalExpression(_) - | AstKind::Argument(_) | AstKind::ExpressionStatement(_) | AstKind::FunctionBody(_) => { node = ctx.nodes().parent_node(node.id()); @@ -296,6 +295,29 @@ fn is_acceptable_return_node<'a, 'b>( type ParentAndIsFirstItem<'a, 'b> = (&'b AstNode<'a>, bool); +/// Checks if a node should be skipped during parent traversal +fn should_skip_parent_node(node: &AstNode, parent: &AstNode) -> bool { + match parent.kind() { + AstKind::CallExpression(call) => { + // Don't skip arguments to Promise methods - they're semantically important for await detection + if let Some(member_expr) = call.callee.as_member_expression() + && let Expression::Identifier(ident) = member_expr.object() + && ident.name == "Promise" + { + return false; // Never skip Promise method arguments + } + + // For other call expressions, skip if this node is one of the arguments + call.arguments.iter().any(|arg| arg.span() == node.span()) + } + AstKind::NewExpression(new_expr) => { + // Skip if this node is one of the new expression arguments + new_expr.arguments.iter().any(|arg| arg.span() == node.span()) + } + _ => false, + } +} + // Returns the parent node of the given node, ignoring some nodes, // and return whether the first item if parent is an array. fn get_parent_with_ignore<'a, 'b>( @@ -305,7 +327,7 @@ fn get_parent_with_ignore<'a, 'b>( let mut node = node; loop { let parent = ctx.nodes().parent_node(node.id()); - if !matches!(parent.kind(), AstKind::Argument(_)) { + if !should_skip_parent_node(node, parent) { // we don't want to report `Promise.all([invalidExpectCall_1, invalidExpectCall_2])` twice. // so we need mark whether the node is the first item of an array. // if it not the first item, we ignore it in `find_promise_call_expression_node`. diff --git a/crates/oxc_linter/src/rules/jsdoc/require_returns.rs b/crates/oxc_linter/src/rules/jsdoc/require_returns.rs index af6e09f10a610..24e8102e658dc 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_returns.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_returns.rs @@ -280,18 +280,11 @@ fn is_promise_resolve_with_value(expr: &Expression, ctx: &LintContext) -> Option // IMO: This is a fault of the original rule design... for resolve_ref in ctx.scoping().get_resolved_references(ident.symbol_id()) { // Check if `resolve` is called with value - match ctx.nodes().parent_kind(resolve_ref.node_id()) { - // `resolve(foo)` - AstKind::CallExpression(call_expr) => { - if !call_expr.arguments.is_empty() { - return Some(true); - } - } - // `foo(resolve)` - AstKind::Argument(_) => { - return Some(true); - } - _ => {} + if let AstKind::CallExpression(call_expr) = + ctx.nodes().parent_kind(resolve_ref.node_id()) + && !call_expr.arguments.is_empty() + { + return Some(true); } } None diff --git a/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs b/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs index 3c53ba1560f05..71d1d32beab5e 100644 --- a/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs +++ b/crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs @@ -1,8 +1,8 @@ use oxc_ast::{ AstKind, ast::{ - AssignmentTarget, BindingIdentifier, BindingPatternKind, BindingProperty, CallExpression, - Expression, FormalParameters, JSXAttributeItem, JSXElementName, + Argument, AssignmentTarget, BindingIdentifier, BindingPatternKind, BindingProperty, + CallExpression, Expression, FormalParameters, JSXAttributeItem, JSXElementName, }, }; use oxc_diagnostics::OxcDiagnostic; @@ -275,11 +275,7 @@ fn is_argument_only_used_in_recursion<'a>( let function_symbol_id = function_id.symbol_id(); for reference in references { - let AstKind::Argument(argument) = ctx.nodes().parent_kind(reference.node_id()) else { - return false; - }; - let AstKind::CallExpression(call_expr) = - ctx.nodes().parent_kind(ctx.nodes().parent_node(reference.node_id()).id()) + let AstKind::CallExpression(call_expr) = ctx.nodes().parent_kind(reference.node_id()) else { return false; }; @@ -288,7 +284,9 @@ fn is_argument_only_used_in_recursion<'a>( return false; }; - if argument.span() != call_arg.span() { + if let Argument::Identifier(ident) = call_arg + && ident.name != arg.name + { return false; } diff --git a/crates/oxc_linter/src/rules/promise/always_return.rs b/crates/oxc_linter/src/rules/promise/always_return.rs index 65740401efec7..062b28d64b6f5 100644 --- a/crates/oxc_linter/src/rules/promise/always_return.rs +++ b/crates/oxc_linter/src/rules/promise/always_return.rs @@ -199,15 +199,13 @@ impl Rule for AlwaysReturn { if !is_inline_then_function_expression(node, ctx) { return; } - // want Argument - let parent1 = ctx.nodes().parent_node(node.id()); - // want CallExpression - let parent2 = ctx.nodes().parent_node(parent1.id()); - if self.ignore_last_callback && is_last_callback(parent2, ctx) { + + let parent = ctx.nodes().parent_node(node.id()); + if self.ignore_last_callback && is_last_callback(parent, ctx) { return; } if !self.ignore_assignment_variable.is_empty() - && is_last_callback(parent2, ctx) + && is_last_callback(parent, ctx) && has_ignored_assignment(node, &self.ignore_assignment_variable) { return; @@ -240,14 +238,12 @@ fn is_first_argument(node: &AstNode, call_node: &AstNode) -> bool { } fn is_inline_then_function_expression(node: &AstNode, ctx: &LintContext) -> bool { - // want Argument - let parent1 = ctx.nodes().parent_node(node.id()); - // want CallExpression - let parent2 = ctx.nodes().parent_node(parent1.id()); + // in order to be a thenable, the parent must be a CallExpression + let parent = ctx.nodes().parent_node(node.id()); is_function_with_block_statement(node) - && is_member_call(parent2, "then") - && is_first_argument(node, parent2) + && is_member_call(parent, "then") + && is_first_argument(node, parent) } fn is_last_callback(node: &AstNode, ctx: &LintContext) -> bool { diff --git a/crates/oxc_linter/src/rules/promise/no_callback_in_promise.rs b/crates/oxc_linter/src/rules/promise/no_callback_in_promise.rs index 1d58456963dc3..9e61aae855724 100644 --- a/crates/oxc_linter/src/rules/promise/no_callback_in_promise.rs +++ b/crates/oxc_linter/src/rules/promise/no_callback_in_promise.rs @@ -132,15 +132,23 @@ impl Rule for NoCallbackInPromise { impl NoCallbackInPromise { fn is_inside_promise(node: &AstNode, ctx: &LintContext) -> bool { - if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) - || !matches!(ctx.nodes().parent_kind(node.id()), AstKind::Argument(_)) - { + if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) { return false; } - ctx.nodes().ancestors(node.id()).nth(1).is_some_and(|node| { - node.kind().as_call_expression().is_some_and(Self::has_promise_callback) - }) + // Check if the parent is a CallExpression with then/catch + let parent = ctx.nodes().parent_node(node.id()); + if let Some(call_expr) = parent.kind().as_call_expression() { + // Check if this function is one of the arguments + let is_argument = call_expr + .arguments + .iter() + .any(|arg| matches!(arg.as_expression(), Some(expr) if expr.span() == node.span())); + + return is_argument && Self::has_promise_callback(call_expr); + } + + false } fn has_promise_callback(call_expr: &CallExpression) -> bool { diff --git a/crates/oxc_linter/src/rules/promise/no_nesting.rs b/crates/oxc_linter/src/rules/promise/no_nesting.rs index 468db8c7947e1..b761bce3bc7d0 100644 --- a/crates/oxc_linter/src/rules/promise/no_nesting.rs +++ b/crates/oxc_linter/src/rules/promise/no_nesting.rs @@ -70,17 +70,25 @@ declare_oxc_lint!( ); fn is_inside_promise(node: &AstNode, ctx: &LintContext) -> bool { - if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) - || !matches!(ctx.nodes().parent_kind(node.id()), AstKind::Argument(_)) - { + if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) { return false; } - ctx.nodes().ancestors(node.id()).nth(1).is_some_and(|node| { - node.kind().as_call_expression().is_some_and(|a| { - is_promise(a).is_some_and(|prop_name| prop_name == "then" || prop_name == "catch") - }) - }) + // Check if the parent is a CallExpression with then/catch + let parent = ctx.nodes().parent_node(node.id()); + if let Some(call_expr) = parent.kind().as_call_expression() { + // Check if this function is one of the arguments + let is_argument = call_expr + .arguments + .iter() + .any(|arg| matches!(arg.as_expression(), Some(expr) if expr.span() == node.span())); + + return is_argument + && is_promise(call_expr) + .is_some_and(|prop_name| prop_name == "then" || prop_name == "catch"); + } + + false } /// Gets the closest promise callback function of the nested promise. diff --git a/crates/oxc_linter/src/rules/promise/no_promise_in_callback.rs b/crates/oxc_linter/src/rules/promise/no_promise_in_callback.rs index b0d69e68a0f55..ea27257aff114 100644 --- a/crates/oxc_linter/src/rules/promise/no_promise_in_callback.rs +++ b/crates/oxc_linter/src/rules/promise/no_promise_in_callback.rs @@ -107,16 +107,19 @@ fn is_within_promise_handler<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b return false; } + // Check if the parent is a CallExpression let parent = ctx.nodes().parent_node(node.id()); - if !matches!(ctx.nodes().kind(parent.id()), AstKind::Argument(_)) { - return false; - } - - let AstKind::CallExpression(call_expr) = ctx.nodes().parent_kind(parent.id()) else { + let AstKind::CallExpression(call_expr) = parent.kind() else { return false; }; - matches!(call_expr.callee_name(), Some("then" | "catch")) + // Check if this function is one of the arguments to a promise method + let is_argument = call_expr + .arguments + .iter() + .any(|arg| matches!(arg.as_expression(), Some(expr) if expr.span() == node.span())); + + is_argument && matches!(call_expr.callee_name(), Some("then" | "catch")) } #[test] diff --git a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs index 31ece6e1016e5..f7b930615d135 100644 --- a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs +++ b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs @@ -1188,6 +1188,7 @@ struct ExhaustiveDepsVisitor<'a, 'b> { decl_stack: Vec<&'a VariableDeclarator<'a>>, skip_reporting_dependency: bool, set_state_call: bool, + is_callee_of_call_expr: bool, found_dependencies: FxHashSet>, refs_inside_cleanups: Vec<&'a StaticMemberExpression<'a>>, } @@ -1200,6 +1201,7 @@ impl<'a, 'b> ExhaustiveDepsVisitor<'a, 'b> { decl_stack: vec![], skip_reporting_dependency: false, set_state_call: false, + is_callee_of_call_expr: false, found_dependencies: FxHashSet::default(), refs_inside_cleanups: vec![], } @@ -1308,6 +1310,22 @@ impl<'a> Visit<'a> for ExhaustiveDepsVisitor<'a, '_> { self.stack.pop(); } + fn visit_call_expression(&mut self, it: &CallExpression<'a>) { + self.stack.push(AstType::CallExpression); + + // Mark that we're visiting a callee + self.is_callee_of_call_expr = true; + self.visit_expression(&it.callee); + self.is_callee_of_call_expr = false; + + // Visit arguments normally + for arg in &it.arguments { + self.visit_argument(arg); + } + + self.stack.pop(); + } + fn visit_static_member_expression(&mut self, it: &StaticMemberExpression<'a>) { if it.property.name == "current" && is_inside_effect_cleanup(&self.stack) { // Safety: this is safe @@ -1328,8 +1346,7 @@ impl<'a> Visit<'a> for ExhaustiveDepsVisitor<'a, '_> { return; } - let is_parent_call_expr = - self.stack.last().is_some_and(|&ty| ty == AstType::CallExpression); + let is_parent_call_expr = self.is_callee_of_call_expr; match analyze_property_chain(&it.object, self.semantic) { Ok(source) => { diff --git a/crates/oxc_linter/src/rules/react/jsx_key.rs b/crates/oxc_linter/src/rules/react/jsx_key.rs index fac191bc526a6..a352de31e5995 100644 --- a/crates/oxc_linter/src/rules/react/jsx_key.rs +++ b/crates/oxc_linter/src/rules/react/jsx_key.rs @@ -2,8 +2,8 @@ use cow_utils::CowUtils; use oxc_ast::{ AstKind, ast::{ - Argument, CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXElement, - JSXFragment, Statement, + CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment, + Statement, }, }; use oxc_diagnostics::OxcDiagnostic; @@ -12,6 +12,7 @@ use oxc_span::{GetSpan, Span}; use crate::{ AstNode, + ast_util::is_node_within_call_argument, context::{ContextHost, LintContext}, rule::Rule, }; @@ -155,11 +156,11 @@ fn is_in_array_or_iter<'a, 'b>( node: &'b AstNode<'a>, ctx: &'b LintContext<'a>, ) -> Option { + let jsx_node = node; let mut node = node; let mut is_outside_containing_function = false; let mut is_explicit_return = false; - let mut argument = None; while !matches!(node.kind(), AstKind::Program(_)) { let parent = ctx.nodes().parent_node(node.id()); @@ -179,6 +180,7 @@ fn is_in_array_or_iter<'a, 'b>( if is_outside_containing_function { return None; } + is_outside_containing_function = true; } AstKind::Function(_) => { @@ -188,6 +190,7 @@ fn is_in_array_or_iter<'a, 'b>( if is_outside_containing_function { return None; } + is_outside_containing_function = true; } AstKind::ArrayExpression(_) => { @@ -203,13 +206,17 @@ fn is_in_array_or_iter<'a, 'b>( if let Some(member_expr) = callee.as_member_expression() && let Some((span, ident)) = member_expr.static_property_info() && TARGET_METHODS.contains(&ident) - && argument.is_some_and(|argument: &Argument<'_>| { - v.arguments - .get(if ident == "from" { 1 } else { 0 }) - .is_some_and(|arg| arg.span() == argument.span()) - }) { - return Some(InsideArrayOrIterator::Iterator(span)); + // Early exit if no arguments to check + if v.arguments.is_empty() { + return None; + } + + // Array.from uses 2nd argument (index 1), others use 1st argument (index 0) + let target_arg_index = if ident == "from" { 1 } else { 0 }; + if is_node_within_call_argument(jsx_node, v, target_arg_index) { + return Some(InsideArrayOrIterator::Iterator(span)); + } } return None; @@ -221,9 +228,6 @@ fn is_in_array_or_iter<'a, 'b>( AstKind::ReturnStatement(_) => { is_explicit_return = true; } - AstKind::Argument(arg) => { - argument = Some(arg); - } _ => {} } node = parent; diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index ac4363dc60799..01d1c7c8e66b2 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -198,13 +198,8 @@ fn is_in_es5_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) } let ancestors_1 = ctx.nodes().parent_node(ancestors_0.id()); - if !matches!(ancestors_1.kind(), AstKind::Argument(_)) { - return false; - } - - let ancestors_2 = ctx.nodes().parent_node(ancestors_1.id()); - is_es5_component(ancestors_2) + is_es5_component(ancestors_1) } fn is_in_es6_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool { diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs index 36c213fcf2ed2..84da4ac80c2e4 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -396,11 +396,8 @@ fn parent_func<'a>(nodes: &'a AstNodes<'a>, node: &AstNode) -> Option<&'a AstNod /// Otherwise it would return `false`. fn is_non_react_func_arg(nodes: &AstNodes, node_id: NodeId) -> bool { let parent = nodes.parent_node(node_id); - if !matches!(parent.kind(), AstKind::Argument(_)) { - return false; - } - let AstKind::CallExpression(call) = nodes.parent_kind(parent.id()) else { + let AstKind::CallExpression(call) = parent.kind() else { return false; }; diff --git a/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs b/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs index 092e676bcc3f7..0121e142c0da8 100644 --- a/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs +++ b/crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs @@ -219,7 +219,7 @@ impl Rule for ExplicitFunctionReturnType { } } - if let Some(parent) = get_parent_node(node, ctx) { + if let Some(parent) = outermost_paren_parent(node, ctx) { match parent.kind() { AstKind::MethodDefinition(def) => { ctx.diagnostic(explicit_function_return_type_diagnostic(Span::new( @@ -288,7 +288,7 @@ impl Rule for ExplicitFunctionReturnType { return; } - if let Some(parent) = get_parent_node(node, ctx) { + if let Some(parent) = outermost_paren_parent(node, ctx) { match parent.kind() { AstKind::MethodDefinition(def) => { ctx.diagnostic(explicit_function_return_type_diagnostic(Span::new( @@ -382,7 +382,7 @@ impl ExplicitFunctionReturnType { node: &AstNode<'a>, ctx: &LintContext<'a>, ) -> bool { - let Some(parent) = get_parent_node(node, ctx) else { return false }; + let Some(parent) = outermost_paren_parent(node, ctx) else { return false }; match parent.kind() { AstKind::VariableDeclarator(decl) => { let BindingPatternKind::BindingIdentifier(id) = &decl.id.kind else { @@ -540,19 +540,8 @@ fn is_setter(node: &AstNode) -> bool { } } -fn get_parent_node<'a, 'b>( - node: &'b AstNode<'a>, - ctx: &'b LintContext<'a>, -) -> Option<&'b AstNode<'a>> { - let parent = outermost_paren_parent(node, ctx)?; - match parent.kind() { - AstKind::Argument(_) => outermost_paren_parent(parent, ctx), - _ => Some(parent), - } -} - fn check_typed_function_expression<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { - let Some(parent) = get_parent_node(node, ctx) else { return false }; + let Some(parent) = outermost_paren_parent(node, ctx) else { return false }; is_typed_parent(parent, Some(node)) || is_property_of_object_with_type(parent, ctx) || is_constructor_argument(parent) @@ -636,7 +625,7 @@ fn is_function(expr: &Expression) -> bool { } fn ancestor_has_return_type<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { - let Some(parent) = get_parent_node(node, ctx) else { return false }; + let Some(parent) = outermost_paren_parent(node, ctx) else { return false }; if let AstKind::ObjectProperty(prop) = parent.kind() && let Expression::ArrowFunctionExpression(func) = &prop.value @@ -741,7 +730,7 @@ fn is_property_of_object_with_type(node: &AstNode, ctx: &LintContext) -> bool { if !matches!(parent.kind(), AstKind::ObjectExpression(_)) { return false; } - let Some(obj_expr_parent) = get_parent_node(parent, ctx) else { + let Some(obj_expr_parent) = outermost_paren_parent(parent, ctx) else { return false; }; is_typed_parent(obj_expr_parent, None) || is_property_of_object_with_type(obj_expr_parent, ctx) diff --git a/crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs b/crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs index 2c1e4f459cc89..b1563ff03b090 100644 --- a/crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs +++ b/crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs @@ -10,7 +10,9 @@ use schemars::JsonSchema; use crate::{ AstNode, - ast_util::{get_function_like_declaration, nth_outermost_paren_parent, outermost_paren_parent}, + ast_util::{ + get_function_like_declaration, is_node_exact_call_argument, outermost_paren_parent, + }, context::LintContext, rule::Rule, utils::is_react_hook, @@ -250,8 +252,9 @@ impl Rule for ConsistentFunctionScoping { if matches!( outermost_paren_parent(node, ctx).map(AstNode::kind), - Some(AstKind::ReturnStatement(_) | AstKind::Argument(_)) - ) { + Some(AstKind::ReturnStatement(_)) + ) || is_node_exact_call_argument(node, ctx) + { return; } @@ -341,22 +344,55 @@ fn is_parent_scope_iife<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { if let Some(parent_node) = outermost_paren_parent(node, ctx) && let Some(parent_node) = outermost_paren_parent(parent_node, ctx) && matches!(parent_node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) - && let Some(parent_node) = outermost_paren_parent(parent_node, ctx) + && let Some(call_node) = outermost_paren_parent(parent_node, ctx) + && let AstKind::CallExpression(call) = call_node.kind() { - return matches!(parent_node.kind(), AstKind::CallExpression(_)); + // Check if the function is the callee (true IIFE) + // Handle both direct calls and parenthesized calls + let callee = &call.callee.without_parentheses(); + return callee.span().start <= parent_node.span().start + && parent_node.span().end <= callee.span().end; } false } fn is_in_react_hook<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { - // we want the 3rd outermost parent - // parents are: function body -> function -> argument -> call expression - if let Some(parent) = nth_outermost_paren_parent(node, ctx, 3) - && let AstKind::CallExpression(call_expr) = parent.kind() + // Check immediate parent first, then use scope-based lookup + // First check if the function is directly inside a React hook call + let parent = ctx.nodes().parent_node(node.id()); + if let AstKind::CallExpression(call_expr) = parent.kind() + && is_react_hook(&call_expr.callee) { - return is_react_hook(&call_expr.callee); + return true; } + + // If not directly inside, check if we're inside a function that's inside a React hook + let current_scope_id = match node.kind() { + AstKind::Function(func) => func.scope_id(), + AstKind::ArrowFunctionExpression(arrow) => arrow.scope_id(), + _ => return false, + }; + + let scoping = ctx.scoping(); + + // Check the parent scope's node (the function that contains us) + if let Some(parent_scope_id) = scoping.scope_parent_id(current_scope_id) { + let parent_scope_node_id = scoping.get_node_id(parent_scope_id); + let parent_scope_node = ctx.nodes().get_node(parent_scope_node_id); + + // If the parent scope is a function, check if that function is inside a React hook + if matches!( + parent_scope_node.kind(), + AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) + ) { + let grandparent = ctx.nodes().parent_node(parent_scope_node_id); + if let AstKind::CallExpression(call_expr) = grandparent.kind() { + return is_react_hook(&call_expr.callee); + } + } + } + false } diff --git a/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs b/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs index 86438fffd525b..36603e3c97059 100644 --- a/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs +++ b/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs @@ -13,7 +13,7 @@ use crate::{ AstNode, context::LintContext, rule::Rule, - utils::{get_boolean_ancestor, is_boolean_node}, + utils::{get_boolean_ancestor, is_boolean_call, is_boolean_node}, }; fn non_zero(span: Span, prop_name: &str, op_and_rhs: &str, help: Option) -> OxcDiagnostic { @@ -206,7 +206,17 @@ impl ExplicitLengthCheck { } }; - let span = kind.span(); + let span = match node.kind() { + AstKind::CallExpression(call) if is_boolean_call(&node.kind()) => { + // Check if we should replace just the member expression or the whole call + call.arguments + .first() + .and_then(|arg| arg.as_expression()) + .filter(|expr| matches!(expr, Expression::LogicalExpression(_))) + .map_or_else(|| node.span(), |_| static_member_expr.span) + } + _ => node.span(), + }; let mut need_pad_start = false; let mut need_pad_end = false; let parent = ctx.nodes().parent_kind(node.id()); @@ -221,15 +231,28 @@ impl ExplicitLengthCheck { need_pad_end = end.is_ascii_alphabetic() || !end.is_ascii(); } - let fixed = format!( - "{}{}{} {}{}{}", - if need_pad_start { " " } else { "" }, - if need_paren { "(" } else { "" }, - static_member_expr.span.source_text(ctx.source_text()), - check_code, - if need_paren { ")" } else { "" }, - if need_pad_end { " " } else { "" }, - ); + // Pre-compute source text to avoid repeated calls + let source_text = static_member_expr.span.source_text(ctx.source_text()); + + // Use capacity hint to reduce allocations - estimate based on components + let estimated_capacity = source_text.len() + check_code.len() + 6; // +6 for spaces and parens + let mut fixed = String::with_capacity(estimated_capacity); + + if need_pad_start { + fixed.push(' '); + } + if need_paren { + fixed.push('('); + } + fixed.push_str(source_text); + fixed.push(' '); + fixed.push_str(check_code); + if need_paren { + fixed.push(')'); + } + if need_pad_end { + fixed.push(' '); + } let property = static_member_expr.property.name; let help = if auto_fix { None diff --git a/crates/oxc_linter/src/rules/unicorn/no_null.rs b/crates/oxc_linter/src/rules/unicorn/no_null.rs index 90c5dba2a59aa..1ebec01d4e845 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_null.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_null.rs @@ -196,7 +196,7 @@ impl Rule for NoNull { let grandparent_kind = parents.next(); match (parent_kind, grandparent_kind) { - (AstKind::Argument(_), Some(AstKind::CallExpression(call_expr))) + (AstKind::CallExpression(call_expr), _) if match_call_expression_pass_case(null_literal, call_expr) => { // no violation diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_promise_resolve_reject.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_promise_resolve_reject.rs index fa143f64a43b3..acd5e82ddb320 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_useless_promise_resolve_reject.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_promise_resolve_reject.rs @@ -208,9 +208,6 @@ fn is_promise_callback<'a, 'b>(node: &'a AstNode<'b>, ctx: &'a LintContext<'b>) let Some(parent) = outermost_paren_parent(function_node, ctx) else { return false; }; - let Some(parent) = outermost_paren_parent(parent, ctx) else { - return false; - }; let AstKind::CallExpression(call_expr) = parent.kind() else { return false; diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs index c262bcdddc336..6861a8ba9598f 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs @@ -212,22 +212,31 @@ fn check_useless_spread_in_list<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) - true } // foo(...[ ]) - AstKind::Argument(_) => { - ctx.diagnostic_with_fix(spread_in_arguments(span), |fixer| { + AstKind::NewExpression(NewExpression { arguments, .. }) + | AstKind::CallExpression(CallExpression { arguments, .. }) => { + if let Some(call_expr_args_span) = arguments.first().map(|first| { + let last = arguments.last().unwrap(); + Span::new(first.span().start, last.span().end) + }) && call_expr_args_span.contains_inclusive(array_expr.span) + { + // compute replacer before the closure so we don't capture `ctx` by reference inside the fixer closure let replacer = if let Some(first) = array_expr.elements.first() { - let mut span = first.span(); + let mut snippet_span = first.span(); if array_expr.elements.len() != 1 { let last = array_expr.elements.last().unwrap(); - span = Span::new(first.span().start, last.span().end); + snippet_span = Span::new(first.span().start, last.span().end); } - ctx.source_range(span) + ctx.source_range(snippet_span).to_string() } else { - "" + String::new() }; - fixer.replace(spread_elem.span(), replacer.to_owned()) - }); - true + ctx.diagnostic_with_fix(spread_in_arguments(span), move |fixer| { + fixer.replace(spread_elem.span(), replacer) + }); + return true; + } + false } _ => false, }, @@ -310,15 +319,6 @@ fn check_useless_iterable_to_array<'a>( let span = Span::new(spread_elem.span.start, spread_elem.span.start + 3); - let parent = if let AstKind::Argument(_) = parent.kind() { - let Some(parent) = outermost_paren_parent(parent, ctx) else { - return false; - }; - parent - } else { - parent - }; - match parent.kind() { AstKind::ForOfStatement(for_of_stmt) => { if for_of_stmt.right.without_parentheses().span() == array_expr.span { diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_native_coercion_functions.rs b/crates/oxc_linter/src/rules/unicorn/prefer_native_coercion_functions.rs index bd20339cbf362..9213c5b2ceecd 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_native_coercion_functions.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_native_coercion_functions.rs @@ -5,7 +5,7 @@ use oxc_ast::{ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::NodeId; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use crate::{AstNode, context::LintContext, rule::Rule, utils::get_first_parameter_name}; @@ -193,15 +193,15 @@ fn check_array_callback_methods( ctx: &LintContext, ) -> bool { let parent = ctx.nodes().parent_node(node_id); - let AstKind::Argument(parent_call_expr_arg) = parent.kind() else { - return false; - }; - let grand_parent = ctx.nodes().parent_node(parent.id()); - let AstKind::CallExpression(call_expr) = grand_parent.kind() else { + + let AstKind::CallExpression(call_expr) = parent.kind() else { return false; }; - - if !std::ptr::eq(&raw const call_expr.arguments[0], parent_call_expr_arg) { + if call_expr + .arguments + .first() + .is_none_or(|arg| arg.span() != ctx.nodes().get_node(node_id).kind().span()) + { return false; } if call_expr.optional { diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_regexp_test.rs b/crates/oxc_linter/src/rules/unicorn/prefer_regexp_test.rs index f1ab6bf0857ee..c7d247d603e20 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_regexp_test.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_regexp_test.rs @@ -100,16 +100,7 @@ impl Rule for PreferRegexpTest { return; } } - - AstKind::Argument(_) => { - let Some(parent) = outermost_paren_parent(parent, ctx) else { - return; - }; - - let AstKind::CallExpression(call_expr) = parent.kind() else { - return; - }; - + AstKind::CallExpression(call_expr) => { let Expression::Identifier(ident) = &call_expr.callee else { return; }; diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_top_level_await.rs b/crates/oxc_linter/src/rules/unicorn/prefer_top_level_await.rs index c050d409cfb19..2e9db7a70d7cd 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_top_level_await.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_top_level_await.rs @@ -82,11 +82,7 @@ impl Rule for PreferTopLevelAwait { } let parent = ctx.nodes().parent_node(node.id()); - // TODO: remove this block once removing `AstKind::Argument` is complete - let grand_parent = { - let p = ctx.nodes().parent_node(parent.id()); - if let AstKind::Argument(_) = p.kind() { ctx.nodes().parent_node(p.id()) } else { p } - }; + let grand_parent = ctx.nodes().parent_node(parent.id()); if let AstKind::StaticMemberExpression(member_expr) = parent.kind() && member_expr.object.span() == call_expr.span diff --git a/crates/oxc_linter/src/rules/vue/define_props_destructuring.rs b/crates/oxc_linter/src/rules/vue/define_props_destructuring.rs index 4071f6b4187fe..f168aa8988ef7 100644 --- a/crates/oxc_linter/src/rules/vue/define_props_destructuring.rs +++ b/crates/oxc_linter/src/rules/vue/define_props_destructuring.rs @@ -119,7 +119,7 @@ impl Rule for DefinePropsDestructuring { } let parent = &ctx.nodes().parent_node(node.id()); - let with_defaults_span = get_parent_with_defaults_call_expression_span(parent, ctx); + let with_defaults_span = get_parent_with_defaults_call_expression_span(parent); let has_destructuring = is_parent_destructuring_variable(parent, ctx); if self.destructure == Destructure::Never { @@ -138,13 +138,8 @@ impl Rule for DefinePropsDestructuring { } } -fn get_parent_with_defaults_call_expression_span( - parent: &AstNode<'_>, - ctx: &LintContext<'_>, -) -> Option { - let AstKind::Argument(_) = parent.kind() else { return None }; - let parent = &ctx.nodes().parent_kind(parent.id()); - let AstKind::CallExpression(call_expr) = parent else { return None }; +fn get_parent_with_defaults_call_expression_span(parent: &AstNode<'_>) -> Option { + let AstKind::CallExpression(call_expr) = parent.kind() else { return None }; call_expr.callee.get_identifier_reference().and_then(|reference| { if reference.name == "withDefaults" { Some(reference.span) } else { None } diff --git a/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap b/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap index aa187c580431b..3758fcb587b20 100644 --- a/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap +++ b/crates/oxc_linter/src/snapshots/react_exhaustive_deps.snap @@ -393,13 +393,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Either include it or remove the dependency array. - ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has missing dependencies: 'history.foo', and 'history.foo.bar' + ⚠ eslint-plugin-react-hooks(exhaustive-deps): React Hook useEffect has a missing dependency: 'history.foo' ╭─[exhaustive_deps.tsx:7:14] 3 │ return [ 4 │ history.foo.bar[2].dobedo.listen(), - · ─────────── + · ─────┬───── + · ╰── useEffect uses `history.foo` here 5 │ history.foo.bar().dobedo.listen[2] - · ─────────── 6 │ ]; 7 │ }, []); · ── diff --git a/crates/oxc_linter/src/utils/unicorn/boolean.rs b/crates/oxc_linter/src/utils/unicorn/boolean.rs index 0515da0186b0d..220ae57d748f3 100644 --- a/crates/oxc_linter/src/utils/unicorn/boolean.rs +++ b/crates/oxc_linter/src/utils/unicorn/boolean.rs @@ -89,8 +89,7 @@ pub fn get_boolean_ancestor<'a, 'b>( cur = parent; continue; } - let parent = ctx.nodes().parent_node(parent.id()); - if is_boolean_call(&parent.kind()) { + if is_boolean_call(&kind) { cur = parent; continue; } diff --git a/crates/oxc_semantic/tests/fixtures/oxc/js/assignments/nested-assignment.snap b/crates/oxc_semantic/tests/fixtures/oxc/js/assignments/nested-assignment.snap index a6744ba8cdccd..828b4e4778803 100644 --- a/crates/oxc_semantic/tests/fixtures/oxc/js/assignments/nested-assignment.snap +++ b/crates/oxc_semantic/tests/fixtures/oxc/js/assignments/nested-assignment.snap @@ -37,7 +37,7 @@ input_file: crates/oxc_semantic/tests/fixtures/oxc/js/assignments/nested-assignm "flags": "ReferenceFlags(Read)", "id": 4, "name": "y", - "node_id": 38 + "node_id": 37 } ] } diff --git a/crates/oxc_semantic/tests/fixtures/oxc/js/try-catch/with-same-name-var.snap b/crates/oxc_semantic/tests/fixtures/oxc/js/try-catch/with-same-name-var.snap index bed7eea9bd32b..6ebaaab6472dc 100644 --- a/crates/oxc_semantic/tests/fixtures/oxc/js/try-catch/with-same-name-var.snap +++ b/crates/oxc_semantic/tests/fixtures/oxc/js/try-catch/with-same-name-var.snap @@ -50,7 +50,7 @@ input_file: crates/oxc_semantic/tests/fixtures/oxc/js/try-catch/with-same-name-v "flags": "ReferenceFlags(Read)", "id": 1, "name": "a", - "node_id": 23 + "node_id": 22 } ] } diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression/call-expression.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression/call-expression.snap index 3569953092a34..1fe41d1313426 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression/call-expression.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression/call-expression.snap @@ -33,7 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression "flags": "ReferenceFlags(Read)", "id": 2, "name": "foo", - "node_id": 19 + "node_id": 18 } ] }, @@ -47,13 +47,13 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/call-expression "flags": "ReferenceFlags(Read)", "id": 1, "name": "a", - "node_id": 15 + "node_id": 14 }, { "flags": "ReferenceFlags(Read)", "id": 3, "name": "a", - "node_id": 21 + "node_id": 19 } ] } diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/method-param-default.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/method-param-default.snap index 98259835e4fe3..5e6f85d85a7c5 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/method-param-default.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/method-param-default.snap @@ -45,7 +45,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declarati "flags": "ReferenceFlags(Read)", "id": 1, "name": "printerName", - "node_id": 36 + "node_id": 35 } ] } diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/new-expression/new-expression.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/new-expression/new-expression.snap index 1668230b3beba..f683174af5090 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/new-expression/new-expression.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/new-expression/new-expression.snap @@ -41,7 +41,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/new-expression/ "flags": "ReferenceFlags(Read)", "id": 1, "name": "a", - "node_id": 12 + "node_id": 11 } ] } diff --git a/crates/oxc_transformer/src/typescript/annotations.rs b/crates/oxc_transformer/src/typescript/annotations.rs index 20c09753686a5..d4b06d3a6102f 100644 --- a/crates/oxc_transformer/src/typescript/annotations.rs +++ b/crates/oxc_transformer/src/typescript/annotations.rs @@ -216,7 +216,17 @@ impl<'a> Traverse<'a, TransformState<'a>> for TypeScriptAnnotations<'a, '_> { } } + fn enter_function(&mut self, func: &mut Function<'a>, _ctx: &mut TraverseCtx<'a>) { + // Remove TypeScript annotations from function declarations + // Note: declare flag is preserved for exit_statements to handle declaration removal + func.type_parameters = None; + func.return_type = None; + func.this_param = None; + } + fn enter_class(&mut self, class: &mut Class<'a>, _ctx: &mut TraverseCtx<'a>) { + // Remove TypeScript annotations from class declarations + // Note: declare flag is preserved for exit_statements to handle declaration removal class.type_parameters = None; class.super_type_arguments = None; class.implements.clear(); @@ -366,12 +376,18 @@ impl<'a> Traverse<'a, TransformState<'a>> for TypeScriptAnnotations<'a, '_> { stmts: &mut ArenaVec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>, ) { - // Remove declare declaration - stmts.retain( - |stmt| { - if let Some(decl) = stmt.as_declaration() { !decl.declare() } else { true } - }, - ); + // Remove TypeScript type-only declarations (interfaces, type aliases, etc.) + // but NOT declarations with `declare` keyword - those will be handled + // by their respective enter_* methods which will remove the `declare` flag + stmts.retain(|stmt| { + if let Some(decl) = stmt.as_declaration() { + // Only remove pure TypeScript type declarations + // Keep all other declarations including those with `declare` + !decl.is_type() + } else { + true + } + }); } fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { @@ -401,9 +417,28 @@ impl<'a> Traverse<'a, TransformState<'a>> for TypeScriptAnnotations<'a, '_> { _ctx: &mut TraverseCtx<'a>, ) { // Remove TS specific statements - stmts.retain(|stmt| match stmt { + stmts.retain_mut(|stmt| match stmt { Statement::ExpressionStatement(s) => !s.expression.is_typescript_syntax(), - match_declaration!(Statement) => !stmt.to_declaration().is_typescript_syntax(), + match_declaration!(Statement) => { + let decl = stmt.to_declaration_mut(); + match decl { + Declaration::VariableDeclaration(var_decl) => { + // Remove declare variable declarations entirely + !var_decl.declare + } + Declaration::FunctionDeclaration(func_decl) => { + // Remove declare function declarations and function overload signatures entirely + // Keep only function implementations (those with a body) + !func_decl.declare && func_decl.body.is_some() + } + Declaration::ClassDeclaration(class_decl) => { + // Remove declare class declarations entirely + !class_decl.declare + } + // Remove type-only declarations + _ => !decl.is_typescript_syntax(), + } + } // Ignore ModuleDeclaration as it's handled in the program _ => true, }); diff --git a/crates/oxc_transformer/src/typescript/mod.rs b/crates/oxc_transformer/src/typescript/mod.rs index a43f8c588e4ca..3f7a6d06e38cf 100644 --- a/crates/oxc_transformer/src/typescript/mod.rs +++ b/crates/oxc_transformer/src/typescript/mod.rs @@ -282,6 +282,18 @@ impl<'a> Traverse<'a, TransformState<'a>> for TypeScript<'a, '_> { self.annotations.enter_jsx_fragment(elem, ctx); } + fn enter_variable_declaration( + &mut self, + decl: &mut VariableDeclaration<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + self.annotations.enter_variable_declaration(decl, ctx); + } + + fn enter_function(&mut self, func: &mut Function<'a>, ctx: &mut TraverseCtx<'a>) { + self.annotations.enter_function(func, ctx); + } + fn enter_declaration(&mut self, node: &mut Declaration<'a>, ctx: &mut TraverseCtx<'a>) { self.module.enter_declaration(node, ctx); } diff --git a/tasks/ast_tools/src/generators/ast_kind.rs b/tasks/ast_tools/src/generators/ast_kind.rs index d0e05dd6bab66..b0991e586ff36 100644 --- a/tasks/ast_tools/src/generators/ast_kind.rs +++ b/tasks/ast_tools/src/generators/ast_kind.rs @@ -10,7 +10,6 @@ //! Variants of `AstKind` and `AstType` are created for: //! //! * All structs which are visited, and are not listed in `STRUCTS_BLACK_LIST` below. -//! * Enums listed in `ENUMS_WHITE_LIST` below. use quote::{format_ident, quote}; @@ -41,15 +40,6 @@ use super::define_generator; /// See also: const STRUCTS_BLACK_LIST: &[&str] = &["BindingPattern", "Span"]; -/// Enums to create an `AstKind` for. -/// -/// Apart from this list, enums don't have `AstKind`s. -/// -/// Ideally we don't want any enums to have `AstKind`s. -/// We are working towards removing all the items from this list. -/// -const ENUMS_WHITE_LIST: &[&str] = &["Argument"]; - /// Generator for `AstKind`, `AstType`, and related code. pub struct AstKindGenerator; @@ -81,20 +71,6 @@ impl Generator for AstKindGenerator { ); struct_def.kind.has_kind = false; } - - // Set `has_kind = true` for enums in white list - for &type_name in ENUMS_WHITE_LIST { - let type_def = schema.type_by_name_mut(type_name); - let TypeDef::Enum(enum_def) = type_def else { - panic!("Type which isn't an enum `{}` in `ENUMS_WHITE_LIST`", type_def.name()); - }; - assert!( - enum_def.visit.has_visitor(), - "Enum `{}` is not visited, cannot have an `AstKind`", - enum_def.name() - ); - enum_def.kind.has_kind = true; - } } /// Generate `AstKind` etc definitions. diff --git a/tasks/ast_tools/src/generators/formatter/format.rs b/tasks/ast_tools/src/generators/formatter/format.rs index 7cfe6c1d5653e..407ab7313165b 100644 --- a/tasks/ast_tools/src/generators/formatter/format.rs +++ b/tasks/ast_tools/src/generators/formatter/format.rs @@ -21,9 +21,11 @@ const AST_NODE_WITHOUT_PRINTING_COMMENTS_LIST: &[&str] = &[ "ClassBody", "CatchParameter", "CatchClause", + "Decorator", // Manually prints it because class's decorators can be appears before `export class Cls {}`. "ExportNamedDeclaration", "ExportDefaultDeclaration", + "TSClassImplements", // "JSXElement", "JSXFragment", @@ -134,10 +136,19 @@ fn generate_struct_implementation( let needs_parentheses = parenthesis_type_ids.contains(&struct_def.id); let needs_parentheses_before = if needs_parentheses { - quote! { - let needs_parentheses = self.needs_parentheses(f); - if needs_parentheses { - "(".fmt(f)?; + if matches!(struct_name, "JSXElement" | "JSXFragment") { + quote! { + let needs_parentheses = !is_suppressed && self.needs_parentheses(f); + if needs_parentheses { + "(".fmt(f)?; + } + } + } else { + quote! { + let needs_parentheses = self.needs_parentheses(f); + if needs_parentheses { + "(".fmt(f)?; + } } } } else { @@ -166,13 +177,12 @@ fn generate_struct_implementation( }; // `Program` can't be suppressed. - // `JSXElement` and `JSXFragment` implement suppression formatting in their formatting logic - let suppressed_check = (!matches!(struct_name, "Program" | "JSXElement" | "JSXFragment")) - .then(|| { - quote! { - let is_suppressed = f.comments().is_suppressed(self.span().start); - } - }); + // `JSXElement` and `JSXFragment` need special suppression handling before parentheses + let suppressed_check = (!matches!(struct_name, "Program")).then(|| { + quote! { + let is_suppressed = f.comments().is_suppressed(self.span().start); + } + }); let write_implementation = if suppressed_check.is_none() { write_call diff --git a/tasks/ast_tools/src/generators/visit.rs b/tasks/ast_tools/src/generators/visit.rs index 88d9588271c54..445c0eb034318 100644 --- a/tasks/ast_tools/src/generators/visit.rs +++ b/tasks/ast_tools/src/generators/visit.rs @@ -587,6 +587,8 @@ impl VisitBuilder<'_> { let match_ident = format_ident!("match_{inherits_snake_name}"); let to_fn_ident = format_ident!("to_{inherits_snake_name}"); + // For Argument, we call visitor.visit_expression to maintain enter/leave node bookkeeping + // This provides recursion protection for deeply nested argument lists let match_arm = quote! { #match_ident!(#enum_ident) => visitor.#inner_visit_fn_ident(it.#to_fn_ident()), }; @@ -673,15 +675,67 @@ impl VisitBuilder<'_> { _ => unreachable!(), }; + // Special optimization for Vec to reduce stack usage: + // Since Argument inherits from Expression and most arguments are expressions, + // we can directly dispatch to the expression visitor for non-spread elements, + // avoiding the intermediate walk_argument call and its stack frame. + let inner_type_name = match inner_type { + TypeDef::Enum(enum_def) => &enum_def.name, + _ => "", + }; + let is_arguments_vec = inner_type_name == "Argument"; + let gen_walk = |visit_trait_name, reference| { let visit_trait_ident = create_safe_ident(visit_trait_name); + let is_mut = visit_trait_name == "VisitMut"; + + let loop_body = if is_arguments_vec { + // Optimize for Vec by directly dispatching to expression visitor + if is_mut { + quote! { + for el in it { + // For Argument types, directly dispatch to avoid intermediate stack frame + match el { + oxc_ast::ast::Argument::SpreadElement(spread) => { + visitor.visit_spread_element(spread); + } + _ => { + // All other Argument variants are expressions + visitor.visit_expression(el.to_expression_mut()); + } + } + } + } + } else { + quote! { + for el in it { + // For Argument types, directly dispatch to avoid intermediate stack frame + match el { + oxc_ast::ast::Argument::SpreadElement(spread) => { + visitor.visit_spread_element(spread); + } + _ => { + // All other Argument variants are expressions + visitor.visit_expression(el.to_expression()); + } + } + } + } + } + } else { + // Normal case for other Vec types + quote! { + for el in it { + visitor.#inner_visit_fn_ident(el); + } + } + }; + quote! { ///@@line_break #[inline] pub fn #walk_fn_ident<'a, V: #visit_trait_ident<'a>>(visitor: &mut V, it: #reference #vec_ty) { - for el in it { - visitor.#inner_visit_fn_ident(el); - } + #loop_body } } }; diff --git a/tasks/prettier_conformance/snapshots/prettier.js.snap.md b/tasks/prettier_conformance/snapshots/prettier.js.snap.md index 23759e0d86ac1..da0135255ba3a 100644 --- a/tasks/prettier_conformance/snapshots/prettier.js.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.js.snap.md @@ -24,7 +24,7 @@ js compatibility: 704/754 (93.37%) | js/for/for-in-with-initializer.js | 💥 | 37.50% | | js/for/parentheses.js | 💥 | 96.00% | | js/identifier/for-of/let.js | 💥 | 92.31% | -| js/identifier/parentheses/let.js | 💥💥 | 82.27% | +| js/identifier/parentheses/let.js | 💥💥 | 84.09% | | js/if/comment-between-condition-and-body.js | 💥 | 65.79% | | js/if/expr_and_same_line_comments.js | 💥 | 97.73% | | js/if/if_comments.js | 💥 | 76.00% | diff --git a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md index 52c6c79878337..b47ca74c1df67 100644 --- a/tasks/prettier_conformance/snapshots/prettier.ts.snap.md +++ b/tasks/prettier_conformance/snapshots/prettier.ts.snap.md @@ -16,9 +16,9 @@ ts compatibility: 545/603 (90.38%) | typescript/as/as-const/as-const.ts | 💥 | 90.91% | | typescript/as/break-after-keyword/18148.ts | 💥 | 82.22% | | typescript/as/comments/18160.ts | 💥 | 71.58% | -| typescript/chain-expression/call-expression.ts | 💥 | 68.75% | -| typescript/chain-expression/member-expression.ts | 💥 | 65.67% | -| typescript/chain-expression/test.ts | 💥 | 0.00% | +| typescript/chain-expression/call-expression.ts | 💥 | 82.81% | +| typescript/chain-expression/member-expression.ts | 💥 | 82.09% | +| typescript/chain-expression/test.ts | 💥 | 50.00% | | typescript/class/empty-method-body.ts | 💥 | 80.00% | | typescript/class/extends_implements.ts | 💥 | 84.27% | | typescript/class/issue-16723.ts | 💥 | 82.19% | @@ -49,7 +49,7 @@ ts compatibility: 545/603 (90.38%) | typescript/mapped-type/issue-11098.ts | 💥 | 97.03% | | typescript/mapped-type/break-mode/break-mode.ts | 💥 | 68.75% | | typescript/multiparser-css/issue-6259.ts | 💥 | 57.14% | -| typescript/non-null/optional-chain.ts | 💥 | 72.22% | +| typescript/non-null/optional-chain.ts | 💥 | 88.89% | | typescript/object-multiline/multiline.ts | 💥✨ | 23.21% | | typescript/prettier-ignore/mapped-types.ts | 💥 | 96.61% | | typescript/property-signature/consistent-with-flow/comments.ts | 💥 | 80.00% |