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 8e293f892..47f33e3fb 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,18 +1,24 @@ use std::cell::RefCell; -use oxc_ast::ast::{ - Argument, ArrayExpressionElement, AssignmentTarget, CallExpression, ComputedMemberExpression, - Expression, IdentifierReference, MemberExpression, PrivateFieldExpression, Program, - SimpleAssignmentTarget, Statement, StaticMemberExpression, +use oxc_ast::{ + ast::{ + Argument, ArrayExpressionElement, AssignmentTarget, CallExpression, + ComputedMemberExpression, Expression, IdentifierReference, MemberExpression, + PrivateFieldExpression, Program, SimpleAssignmentTarget, Statement, StaticMemberExpression, + }, + AstKind, }; -use oxc_semantic::SymbolId; +use oxc_semantic::{AstNode, SymbolId}; use oxc_span::GetSpan; use rustc_hash::FxHashSet; -use crate::{ast_util::get_symbol_id_of_variable, utils::Value, LintContext}; +use crate::{ + ast_util::{get_declaration_of_variable, get_symbol_id_of_variable}, + utils::{get_write_expr, Value}, + LintContext, +}; use super::NoSideEffectsDiagnostic; - pub struct NodeListenerOptions<'a, 'b> { checked_mutated_nodes: RefCell>, ctx: &'b LintContext<'a>, @@ -54,6 +60,23 @@ impl<'a> ListenerMap for Statement<'a> { } } +// we don't need implement all AstNode +// it's same as `reportSideEffectsInDefinitionWhenCalled` in eslint-plugin-tree-shaking +// +impl<'a> ListenerMap for AstNode<'a> { + fn report_effects_when_called(&self, options: &NodeListenerOptions) { + #[allow(clippy::single_match)] + match self.kind() { + AstKind::VariableDeclarator(decl) => { + if let Some(init) = &decl.init { + init.report_effects_when_called(options); + } + } + _ => {} + } + } +} + impl<'a> ListenerMap for Expression<'a> { fn report_effects(&self, options: &NodeListenerOptions) { match self { @@ -79,7 +102,10 @@ impl<'a> ListenerMap for Expression<'a> { Self::Identifier(ident) => { ident.report_effects_when_mutated(options); } - _ => {} + _ => { + // Default behavior + options.ctx.diagnostic(NoSideEffectsDiagnostic::Mutation(self.span())); + } } } fn report_effects_when_called(&self, options: &NodeListenerOptions) { @@ -90,7 +116,10 @@ impl<'a> ListenerMap for Expression<'a> { Self::Identifier(expr) => { expr.report_effects_when_called(options); } - _ => {} + _ => { + // Default behavior + options.ctx.diagnostic(NoSideEffectsDiagnostic::Call(self.span())); + } } } } @@ -120,9 +149,18 @@ impl<'a> ListenerMap for CallExpression<'a> { } } fn report_effects_when_called(&self, options: &NodeListenerOptions) { + let ctx = options.ctx; if let Expression::Identifier(ident) = &self.callee { - if get_symbol_id_of_variable(ident, options.ctx).is_none() { - // TODO: Not work now + if let Some(node) = get_declaration_of_variable(ident, ctx) { + let Some(parent) = ctx.nodes().parent_kind(node.id()) else { + return; + }; + // TODO: `isLocalVariableAWhitelistedModule` + if matches!(parent, AstKind::ImportDeclaration(_)) { + return; + } + options.ctx.diagnostic(NoSideEffectsDiagnostic::CallReturnValue(self.span)); + } else { options.ctx.diagnostic(NoSideEffectsDiagnostic::CallReturnValue(self.span)); } } @@ -185,7 +223,20 @@ 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() { + if let Some(symbol_id) = get_symbol_id_of_variable(self, ctx) { + let symbol_table = ctx.semantic().symbols(); + for reference in symbol_table.get_resolved_references(symbol_id) { + if reference.is_write() { + let node_id = reference.node_id(); + if let Some(expr) = get_write_expr(node_id, ctx) { + expr.report_effects_when_called(options); + } + } + } + let symbol_table = ctx.semantic().symbols(); + let node = ctx.nodes().get_node(symbol_table.get_declaration(symbol_id)); + node.report_effects_when_called(options); + } else { ctx.diagnostic(NoSideEffectsDiagnostic::CallGlobal( self.name.to_compact_str(), self.span, @@ -199,7 +250,12 @@ impl<'a> ListenerMap for IdentifierReference<'a> { if options.insert_mutated_node(symbol_id) { for reference in ctx.symbols().get_resolved_references(symbol_id) { if reference.is_write() { - ctx.diagnostic(NoSideEffectsDiagnostic::Mutation( + let node_id = reference.node_id(); + if let Some(expr) = get_write_expr(node_id, ctx) { + expr.report_effects_when_mutated(options); + } + + ctx.diagnostic(NoSideEffectsDiagnostic::MutationWithName( self.name.to_compact_str(), self.span, )); @@ -207,7 +263,7 @@ impl<'a> ListenerMap for IdentifierReference<'a> { } } } else { - ctx.diagnostic(NoSideEffectsDiagnostic::Mutation( + ctx.diagnostic(NoSideEffectsDiagnostic::MutationWithName( self.name.to_compact_str(), self.span, )); 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 19ae0e67d..c716cc324 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 @@ -18,9 +18,13 @@ enum NoSideEffectsDiagnostic { #[diagnostic(severity(warning))] Assignment(CompactStr, #[label] Span), + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating")] + #[diagnostic(severity(warning))] + Mutation(#[label] Span), + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating `{0}`")] #[diagnostic(severity(warning))] - Mutation(CompactStr, #[label] Span), + MutationWithName(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))] @@ -387,7 +391,7 @@ fn test() { // "(()=>{})(ext(), 1)", // "(()=>{})(1, ext())", // // CallExpression when called - // "const x = ()=>ext; const y = x(); y()", + "const x = ()=>ext; const y = x(); y()", // // CallExpression when mutated // "const x = ()=>ext; const y = x(); y.z = 1", // // CatchClause 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 664174820..216c9820b 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 @@ -25,3 +25,9 @@ expression: no_side_effects_in_initialization 1 │ const x = {};x[ext()] = 1 · ─── ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling function return value + ╭─[no_side_effects_in_initialization.tsx:1:30] + 1 │ const x = ()=>ext; const y = x(); y() + · ─── + ╰──── diff --git a/crates/oxc_linter/src/utils/tree_shaking.rs b/crates/oxc_linter/src/utils/tree_shaking.rs index 3cba28a14..31e1e1cea 100644 --- a/crates/oxc_linter/src/utils/tree_shaking.rs +++ b/crates/oxc_linter/src/utils/tree_shaking.rs @@ -1,5 +1,21 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_semantic::AstNodeId; + +use crate::LintContext; + #[allow(dead_code)] pub enum Value { Boolean(bool), Number(f64), } + +pub fn get_write_expr<'a, 'b>( + node_id: AstNodeId, + ctx: &'b LintContext<'a>, +) -> Option<&'b Expression<'a>> { + let parent = ctx.nodes().parent_kind(node_id)?; + match parent { + AstKind::AssignmentExpression(assign_expr) => Some(&assign_expr.right), + _ => None, + } +}