From 3c9e77d66f26556853ef010e27252e47d669fa7b Mon Sep 17 00:00:00 2001 From: Wang Wenzhe Date: Wed, 20 Mar 2024 23:11:45 +0800 Subject: [PATCH] feat(linter/tree-shaking): detect CallExpression in MemberExpression (#2772) --- .../listener_map.rs | 111 +++++++++++++++++- .../no_side_effects_in_initialization/mod.rs | 18 ++- .../no_side_effects_in_initialization.snap | 6 + 3 files changed, 128 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs index 4ee9103a8ff8a..8e293f892d7a8 100644 --- a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs +++ b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs @@ -1,11 +1,12 @@ use std::cell::RefCell; use oxc_ast::ast::{ - ArrayExpressionElement, AssignmentTarget, ComputedMemberExpression, Expression, - IdentifierReference, MemberExpression, PrivateFieldExpression, Program, SimpleAssignmentTarget, - Statement, StaticMemberExpression, + Argument, ArrayExpressionElement, AssignmentTarget, CallExpression, ComputedMemberExpression, + Expression, IdentifierReference, MemberExpression, PrivateFieldExpression, Program, + SimpleAssignmentTarget, Statement, StaticMemberExpression, }; use oxc_semantic::SymbolId; +use oxc_span::GetSpan; use rustc_hash::FxHashSet; use crate::{ast_util::get_symbol_id_of_variable, utils::Value, LintContext}; @@ -66,6 +67,9 @@ impl<'a> ListenerMap for Expression<'a> { Self::Identifier(ident) => { ident.report_effects(options); } + Self::CallExpression(call_expr) => { + call_expr.report_effects(options); + } _ => {} } } @@ -78,6 +82,65 @@ impl<'a> ListenerMap for Expression<'a> { _ => {} } } + fn report_effects_when_called(&self, options: &NodeListenerOptions) { + match self { + Self::CallExpression(expr) => { + expr.report_effects_when_called(options); + } + Self::Identifier(expr) => { + expr.report_effects_when_called(options); + } + _ => {} + } + } +} + +// which kind of Expression defines `report_effects_when_called` method. +fn defined_custom_report_effects_when_called(expr: &Expression) -> bool { + matches!( + expr, + Expression::ArrowFunctionExpression(_) + | Expression::CallExpression(_) + | Expression::ClassExpression(_) + | Expression::ConditionalExpression(_) + | Expression::FunctionExpression(_) + | Expression::Identifier(_) + | Expression::MemberExpression(_) + ) +} + +impl<'a> ListenerMap for CallExpression<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + self.arguments.iter().for_each(|arg| arg.report_effects(options)); + if defined_custom_report_effects_when_called(&self.callee) { + self.callee.report_effects_when_called(options); + } else { + // TODO: Not work now + options.ctx.diagnostic(NoSideEffectsDiagnostic::Call(self.callee.span())); + } + } + fn report_effects_when_called(&self, options: &NodeListenerOptions) { + if let Expression::Identifier(ident) = &self.callee { + if get_symbol_id_of_variable(ident, options.ctx).is_none() { + // TODO: Not work now + options.ctx.diagnostic(NoSideEffectsDiagnostic::CallReturnValue(self.span)); + } + } + } + fn report_effects_when_mutated(&self, options: &NodeListenerOptions) { + options.ctx.diagnostic(NoSideEffectsDiagnostic::MutationOfFunctionReturnValue(self.span)); + } +} + +impl<'a> ListenerMap for Argument<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + match self { + Self::Expression(expr) => expr.report_effects(options), + Self::SpreadElement(spread) => { + spread.argument.report_effects(options); + } + } + } } impl<'a> ListenerMap for AssignmentTarget<'a> { @@ -120,6 +183,16 @@ impl<'a> ListenerMap for IdentifierReference<'a> { } } + fn report_effects_when_called(&self, options: &NodeListenerOptions) { + let ctx = options.ctx; + if get_symbol_id_of_variable(self, ctx).is_none() { + ctx.diagnostic(NoSideEffectsDiagnostic::CallGlobal( + self.name.to_compact_str(), + self.span, + )); + } + } + fn report_effects_when_mutated(&self, options: &NodeListenerOptions) { let ctx = options.ctx; if let Some(symbol_id) = get_symbol_id_of_variable(self, ctx) { @@ -143,6 +216,19 @@ impl<'a> ListenerMap for IdentifierReference<'a> { } impl<'a> ListenerMap for MemberExpression<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + match self { + Self::ComputedMemberExpression(expr) => { + expr.report_effects(options); + } + Self::StaticMemberExpression(expr) => { + expr.report_effects(options); + } + Self::PrivateFieldExpression(expr) => { + expr.report_effects(options); + } + } + } fn report_effects_when_assigned(&self, options: &NodeListenerOptions) { match self { Self::ComputedMemberExpression(expr) => { @@ -161,11 +247,24 @@ impl<'a> ListenerMap for MemberExpression<'a> { } } -impl<'a> ListenerMap for ComputedMemberExpression<'a> {} +impl<'a> ListenerMap for ComputedMemberExpression<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + self.expression.report_effects(options); + self.object.report_effects(options); + } +} -impl<'a> ListenerMap for StaticMemberExpression<'a> {} +impl<'a> ListenerMap for StaticMemberExpression<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + self.object.report_effects(options); + } +} -impl<'a> ListenerMap for PrivateFieldExpression<'a> {} +impl<'a> ListenerMap for PrivateFieldExpression<'a> { + fn report_effects(&self, options: &NodeListenerOptions) { + self.object.report_effects(options); + } +} impl<'a> ListenerMap for ArrayExpressionElement<'a> { fn report_effects(&self, options: &NodeListenerOptions) { diff --git a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs index 936b1e83c48a3..19ae0e67db497 100644 --- a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs +++ b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs @@ -21,6 +21,22 @@ enum NoSideEffectsDiagnostic { #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating `{0}`")] #[diagnostic(severity(warning))] Mutation(CompactStr, #[label] Span), + + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating function return value")] + #[diagnostic(severity(warning))] + MutationOfFunctionReturnValue(#[label] Span), + + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling")] + #[diagnostic(severity(warning))] + Call(#[label] Span), + + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling function return value")] + #[diagnostic(severity(warning))] + CallReturnValue(#[label] Span), + + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `{0}`")] + #[diagnostic(severity(warning))] + CallGlobal(CompactStr, #[label] Span), } /// @@ -353,7 +369,7 @@ fn test() { "ext = 1", "ext += 1", "ext.x = 1", - // "const x = {};x[ext()] = 1", + "const x = {};x[ext()] = 1", // "this.x = 1", // // AssignmentPattern // "const {x = ext()} = {}", diff --git a/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap b/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap index 0a050e37ff380..664174820f882 100644 --- a/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap +++ b/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap @@ -19,3 +19,9 @@ expression: no_side_effects_in_initialization 1 │ ext.x = 1 · ─── ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:16] + 1 │ const x = {};x[ext()] = 1 + · ─── + ╰────