From 741aa8df1fcf4ac06cfb4f4e390af5e2f61953dd Mon Sep 17 00:00:00 2001 From: u9g Date: Thu, 24 Aug 2023 23:03:53 -0400 Subject: [PATCH] feat(ast): Add to ChainExpression and ExpressionArrayElement to ASTKind (#785) --- crates/oxc_ast/src/ast_kind.rs | 6 +++ crates/oxc_ast/src/visit.rs | 12 ++++- crates/oxc_ast/src/visit_mut.rs | 6 ++- .../rules/eslint/array_callback_return/mod.rs | 3 +- .../src/rules/eslint/no_extra_boolean_cast.rs | 5 +- .../src/rules/typescript/no_var_requires.rs | 54 +++++++++---------- 6 files changed, 54 insertions(+), 32 deletions(-) diff --git a/crates/oxc_ast/src/ast_kind.rs b/crates/oxc_ast/src/ast_kind.rs index 2e4ef3f16..27c85086c 100644 --- a/crates/oxc_ast/src/ast_kind.rs +++ b/crates/oxc_ast/src/ast_kind.rs @@ -60,6 +60,7 @@ pub enum AstKind<'a> { AwaitExpression(&'a AwaitExpression<'a>), BinaryExpression(&'a BinaryExpression<'a>), CallExpression(&'a CallExpression<'a>), + ChainExpression(&'a ChainExpression<'a>), ConditionalExpression(&'a ConditionalExpression<'a>), LogicalExpression(&'a LogicalExpression<'a>), MemberExpression(&'a MemberExpression<'a>), @@ -81,6 +82,7 @@ pub enum AstKind<'a> { AssignmentTargetWithDefault(&'a AssignmentTargetWithDefault<'a>), ArrayExpressionElement(&'a ArrayExpressionElement<'a>), Elision(Span), + ExpressionArrayElement(&'a Expression<'a>), SpreadElement(&'a SpreadElement<'a>), RestElement(&'a RestElement<'a>), @@ -292,6 +294,7 @@ impl<'a> GetSpan for AstKind<'a> { Self::AwaitExpression(x) => x.span, Self::BinaryExpression(x) => x.span, Self::CallExpression(x) => x.span, + Self::ChainExpression(x) => x.span, Self::ConditionalExpression(x) => x.span, Self::LogicalExpression(x) => x.span, Self::MemberExpression(x) => x.span(), @@ -314,6 +317,7 @@ impl<'a> GetSpan for AstKind<'a> { Self::AssignmentTargetWithDefault(x) => x.span, Self::SpreadElement(x) => x.span, Self::Elision(span) => *span, + Self::ExpressionArrayElement(x) => x.span(), Self::RestElement(x) => x.span, Self::Function(x) => x.span, @@ -444,6 +448,7 @@ impl<'a> AstKind<'a> { Self::AwaitExpression(_) => "AwaitExpression".into(), Self::BinaryExpression(b) => format!("BinaryExpression{}", b.operator.as_str()).into(), Self::CallExpression(_) => "CallExpression".into(), + Self::ChainExpression(_) => "ChainExpression".into(), Self::ConditionalExpression(_) => "ConditionalExpression".into(), Self::LogicalExpression(_) => "LogicalExpression".into(), Self::MemberExpression(_) => "MemberExpression".into(), @@ -466,6 +471,7 @@ impl<'a> AstKind<'a> { Self::AssignmentTargetWithDefault(_) => "AssignmentTargetWithDefault".into(), Self::SpreadElement(_) => "SpreadElement".into(), Self::Elision(_) => "Elision".into(), + Self::ExpressionArrayElement(_) => "ExpressionArrayElement".into(), Self::RestElement(_) => "RestElement".into(), Self::Function(x) => format!( diff --git a/crates/oxc_ast/src/visit.rs b/crates/oxc_ast/src/visit.rs index d903ea0e7..6e0648073 100644 --- a/crates/oxc_ast/src/visit.rs +++ b/crates/oxc_ast/src/visit.rs @@ -530,7 +530,7 @@ pub trait Visit<'a>: Sized { self.enter_node(kind); match arg { ArrayExpressionElement::SpreadElement(spread) => self.visit_spread_element(spread), - ArrayExpressionElement::Expression(expr) => self.visit_expression(expr), + ArrayExpressionElement::Expression(expr) => self.visit_expression_array_element(expr), ArrayExpressionElement::Elision(span) => self.visit_elision(*span), } self.leave_node(kind); @@ -553,6 +553,13 @@ pub trait Visit<'a>: Sized { self.leave_node(kind); } + fn visit_expression_array_element(&mut self, expr: &'a Expression<'a>) { + let kind = AstKind::ExpressionArrayElement(expr); + self.enter_node(kind); + self.visit_expression(expr); + self.leave_node(kind); + } + fn visit_elision(&mut self, span: Span) { let kind = AstKind::Elision(span); self.enter_node(kind); @@ -607,7 +614,10 @@ pub trait Visit<'a>: Sized { } fn visit_chain_expression(&mut self, expr: &'a ChainExpression<'a>) { + let kind = AstKind::ChainExpression(expr); + self.enter_node(kind); self.visit_chain_element(&expr.expression); + self.leave_node(kind); } fn visit_chain_element(&mut self, elem: &'a ChainElement<'a>) { diff --git a/crates/oxc_ast/src/visit_mut.rs b/crates/oxc_ast/src/visit_mut.rs index ea0346bda..e55d78c37 100644 --- a/crates/oxc_ast/src/visit_mut.rs +++ b/crates/oxc_ast/src/visit_mut.rs @@ -401,7 +401,7 @@ pub trait VisitMut<'a, 'b>: Sized { fn visit_array_expression_element(&mut self, arg: &'b mut ArrayExpressionElement<'a>) { match arg { ArrayExpressionElement::SpreadElement(spread) => self.visit_spread_element(spread), - ArrayExpressionElement::Expression(expr) => self.visit_expression(expr), + ArrayExpressionElement::Expression(expr) => self.visit_expression_array_element(expr), ArrayExpressionElement::Elision(span) => self.visit_elision(*span), } } @@ -417,6 +417,10 @@ pub trait VisitMut<'a, 'b>: Sized { self.visit_expression(&mut elem.argument); } + fn visit_expression_array_element(&mut self, expr: &'b mut Expression<'a>) { + self.visit_expression(expr); + } + fn visit_elision(&mut self, _span: Span) {} fn visit_assignment_expression(&mut self, expr: &'b mut AssignmentExpression<'a>) { 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 fb0632b22..1c63433e4 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 @@ -151,7 +151,8 @@ pub fn get_array_method_name<'a>( AstKind::LogicalExpression(_) | AstKind::ConditionalExpression(_) | AstKind::Argument(_) - | AstKind::ParenthesizedExpression(_) => { + | AstKind::ParenthesizedExpression(_) + | AstKind::ChainExpression(_) => { current_node = parent; } 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 e59039d00..d9c558e66 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 @@ -192,7 +192,10 @@ fn get_real_parent<'a, 'b>(node: &AstNode, ctx: &'a LintContext<'b>) -> Option<& for (_, parent) in ctx.nodes().iter_parents(node.id()).tuple_windows::<(&AstNode<'b>, &AstNode<'b>)>() { - if let AstKind::Argument(_) | AstKind::ParenthesizedExpression(_) = parent.kind() { + if let AstKind::Argument(_) + | AstKind::ParenthesizedExpression(_) + | AstKind::ChainExpression(_) = parent.kind() + { continue; } diff --git a/crates/oxc_linter/src/rules/typescript/no_var_requires.rs b/crates/oxc_linter/src/rules/typescript/no_var_requires.rs index 76e7da404..61b7c90d1 100644 --- a/crates/oxc_linter/src/rules/typescript/no_var_requires.rs +++ b/crates/oxc_linter/src/rules/typescript/no_var_requires.rs @@ -4,7 +4,7 @@ use oxc_diagnostics::{ thiserror::Error, }; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -42,40 +42,38 @@ impl Rule for NoVarRequires { return; } let AstKind::CallExpression(expr) = node.kind() else { return }; - if expr.is_require_call() { - if ctx.scopes().get_bindings(node.scope_id()).contains_key("require") { - return; - } - if let Some(parent_node) = ctx.nodes().parent_node(node.id()) { - if let AstKind::Argument(_) = parent_node.kind() { - if let Some(parent_node) = ctx.nodes().parent_node(parent_node.id()) { - if is_target_node(&parent_node.kind()) { - ctx.diagnostic(NoVarRequiresDiagnostic(expr.span)); - } - } - } + if expr.is_require_call() + && !ctx.scopes().get_bindings(node.scope_id()).contains_key("require") + { + // If the parent is an expression statement => this is a top level require() + // Or, if the parent is a chain expression (require?.()) and + // the grandparent is an expression statement => this is a top level require() + let is_expression_statement = { + let parent_node = ctx.nodes().parent_node(node.id()); + let grandparent_node = parent_node.and_then(|x| ctx.nodes().parent_node(x.id())); + matches!( + ( + parent_node.map(oxc_semantic::AstNode::kind), + grandparent_node.map(oxc_semantic::AstNode::kind) + ), + (Some(AstKind::ExpressionStatement(_)), _) + | ( + Some(AstKind::ChainExpression(_)), + Some(AstKind::ExpressionStatement(_)) + ) + ) + }; - if is_target_node(&parent_node.kind()) { - ctx.diagnostic(NoVarRequiresDiagnostic(expr.span)); - } + // If this is an expression statement, it means the `require()`'s return value is unused. + // If the return value is unused, this isn't a problem. + if !is_expression_statement { + ctx.diagnostic(NoVarRequiresDiagnostic(node.kind().span())); } } } } -fn is_target_node(node_kind: &AstKind<'_>) -> bool { - matches!( - node_kind, - AstKind::CallExpression(_) - | AstKind::MemberExpression(_) - | AstKind::NewExpression(_) - | AstKind::TSAsExpression(_) - | AstKind::TSTypeAssertion(_) - | AstKind::VariableDeclarator(_) - ) -} - #[test] fn test() { use crate::tester::Tester;