diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 0d436e28c..cc7f562c1 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -1,7 +1,7 @@ use std::hash::{Hash, Hasher}; use oxc_ast::AstKind; -use oxc_semantic::AstNode; +use oxc_semantic::{AstNode, SymbolId}; use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator}; use rustc_hash::FxHasher; @@ -269,11 +269,19 @@ pub fn get_declaration_of_variable<'a, 'b>( ident: &IdentifierReference, ctx: &'b LintContext<'a>, ) -> Option<&'b AstNode<'a>> { + let symbol_id = get_symbol_id_of_variable(ident, ctx)?; + let symbol_table = ctx.semantic().symbols(); + Some(ctx.nodes().get_node(symbol_table.get_declaration(symbol_id))) +} + +pub fn get_symbol_id_of_variable( + ident: &IdentifierReference, + ctx: &LintContext<'_>, +) -> Option { let symbol_table = ctx.semantic().symbols(); let reference_id = ident.reference_id.get()?; let reference = symbol_table.get_reference(reference_id); - let symbol_id = reference.symbol_id()?; - Some(ctx.nodes().get_node(symbol_table.get_declaration(symbol_id))) + reference.symbol_id() } pub fn extract_regex_flags<'a>( 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 746a7beca..4ee9103a8 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,58 +1,79 @@ +use std::cell::RefCell; + use oxc_ast::ast::{ ArrayExpressionElement, AssignmentTarget, ComputedMemberExpression, Expression, IdentifierReference, MemberExpression, PrivateFieldExpression, Program, SimpleAssignmentTarget, Statement, StaticMemberExpression, }; +use oxc_semantic::SymbolId; +use rustc_hash::FxHashSet; -use crate::{ast_util::get_declaration_of_variable, utils::Value, LintContext}; +use crate::{ast_util::get_symbol_id_of_variable, utils::Value, LintContext}; use super::NoSideEffectsDiagnostic; +pub struct NodeListenerOptions<'a, 'b> { + checked_mutated_nodes: RefCell>, + ctx: &'b LintContext<'a>, +} + +impl<'a, 'b> NodeListenerOptions<'a, 'b> { + fn insert_mutated_node(&self, symbol_id: SymbolId) -> bool { + self.checked_mutated_nodes.borrow_mut().insert(symbol_id) + } +} + +impl<'a, 'b> NodeListenerOptions<'a, 'b> { + pub fn new(ctx: &'b LintContext<'a>) -> Self { + Self { checked_mutated_nodes: RefCell::new(FxHashSet::default()), ctx } + } +} + pub trait ListenerMap { - fn report_effects(&self, _ctx: &LintContext) {} - fn report_effects_when_assigned(&self, _ctx: &LintContext) {} - fn report_effects_when_called(&self, _ctx: &LintContext) {} - fn report_effects_when_mutated(&self, _ctx: &LintContext) {} - fn get_value_and_report_effects(&self, _ctx: &LintContext) -> Option { + fn report_effects(&self, _options: &NodeListenerOptions) {} + fn report_effects_when_assigned(&self, _options: &NodeListenerOptions) {} + fn report_effects_when_called(&self, _options: &NodeListenerOptions) {} + fn report_effects_when_mutated(&self, _options: &NodeListenerOptions) {} + fn get_value_and_report_effects(&self, _options: &NodeListenerOptions) -> Option { None } } impl<'a> ListenerMap for Program<'a> { - fn report_effects(&self, ctx: &LintContext) { - self.body.iter().for_each(|stmt| stmt.report_effects(ctx)); + fn report_effects(&self, options: &NodeListenerOptions) { + self.body.iter().for_each(|stmt| stmt.report_effects(options)); } } impl<'a> ListenerMap for Statement<'a> { - fn report_effects(&self, ctx: &LintContext) { + fn report_effects(&self, options: &NodeListenerOptions) { if let Self::ExpressionStatement(expr_stmt) = self { - expr_stmt.expression.report_effects(ctx); + expr_stmt.expression.report_effects(options); } } } impl<'a> ListenerMap for Expression<'a> { - fn report_effects(&self, ctx: &LintContext) { + fn report_effects(&self, options: &NodeListenerOptions) { match self { Self::ArrayExpression(array_expr) => { - array_expr.elements.iter().for_each(|el| el.report_effects(ctx)); + array_expr.elements.iter().for_each(|el| el.report_effects(options)); } Self::AssignmentExpression(assign_expr) => { - assign_expr.left.report_effects_when_assigned(ctx); - assign_expr.right.report_effects(ctx); + assign_expr.left.report_effects_when_assigned(options); + assign_expr.right.report_effects(options); } Self::Identifier(ident) => { - ident.report_effects(ctx); + ident.report_effects(options); } _ => {} } } - fn report_effects_when_mutated(&self, ctx: &LintContext) { + fn report_effects_when_mutated(&self, options: &NodeListenerOptions) { #[allow(clippy::single_match)] match self { Self::Identifier(ident) => { - ident.report_effects_when_mutated(ctx); + ident.report_effects_when_mutated(options); } _ => {} } @@ -60,10 +81,10 @@ impl<'a> ListenerMap for Expression<'a> { } impl<'a> ListenerMap for AssignmentTarget<'a> { - fn report_effects_when_assigned(&self, ctx: &LintContext) { + fn report_effects_when_assigned(&self, options: &NodeListenerOptions) { match self { Self::SimpleAssignmentTarget(target) => { - target.report_effects_when_assigned(ctx); + target.report_effects_when_assigned(options); } Self::AssignmentTargetPattern(_pattern) => {} } @@ -71,18 +92,18 @@ impl<'a> ListenerMap for AssignmentTarget<'a> { } impl<'a> ListenerMap for SimpleAssignmentTarget<'a> { - fn report_effects_when_assigned(&self, ctx: &LintContext) { + fn report_effects_when_assigned(&self, options: &NodeListenerOptions) { match self { Self::AssignmentTargetIdentifier(ident) => { - ident.report_effects_when_assigned(ctx); + ident.report_effects_when_assigned(options); } Self::MemberAssignmentTarget(member) => { - member.report_effects_when_assigned(ctx); + member.report_effects_when_assigned(options); } _ => { // For remain TypeScript AST, just visit its expression if let Some(expr) = self.get_expression() { - expr.report_effects_when_assigned(ctx); + expr.report_effects_when_assigned(options); } } } @@ -90,18 +111,29 @@ impl<'a> ListenerMap for SimpleAssignmentTarget<'a> { } impl<'a> ListenerMap for IdentifierReference<'a> { - fn report_effects_when_assigned(&self, ctx: &LintContext) { - if get_declaration_of_variable(self, ctx).is_none() { - ctx.diagnostic(NoSideEffectsDiagnostic::Assignment( + fn report_effects_when_assigned(&self, options: &NodeListenerOptions) { + if get_symbol_id_of_variable(self, options.ctx).is_none() { + options.ctx.diagnostic(NoSideEffectsDiagnostic::Assignment( self.name.to_compact_str(), self.span, )); } } - fn report_effects_when_mutated(&self, ctx: &LintContext) { - // TODO: check mutation of local variable. - if get_declaration_of_variable(self, ctx).is_none() { + fn report_effects_when_mutated(&self, options: &NodeListenerOptions) { + let ctx = options.ctx; + if let Some(symbol_id) = get_symbol_id_of_variable(self, ctx) { + 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( + self.name.to_compact_str(), + self.span, + )); + } + } + } + } else { ctx.diagnostic(NoSideEffectsDiagnostic::Mutation( self.name.to_compact_str(), self.span, @@ -111,19 +143,19 @@ impl<'a> ListenerMap for IdentifierReference<'a> { } impl<'a> ListenerMap for MemberExpression<'a> { - fn report_effects_when_assigned(&self, ctx: &LintContext) { + fn report_effects_when_assigned(&self, options: &NodeListenerOptions) { match self { Self::ComputedMemberExpression(expr) => { - expr.report_effects(ctx); - expr.object.report_effects_when_mutated(ctx); + expr.report_effects(options); + expr.object.report_effects_when_mutated(options); } Self::StaticMemberExpression(expr) => { - expr.report_effects(ctx); - expr.object.report_effects_when_mutated(ctx); + expr.report_effects(options); + expr.object.report_effects_when_mutated(options); } Self::PrivateFieldExpression(expr) => { - expr.report_effects(ctx); - expr.object.report_effects_when_mutated(ctx); + expr.report_effects(options); + expr.object.report_effects_when_mutated(options); } } } @@ -136,11 +168,11 @@ impl<'a> ListenerMap for StaticMemberExpression<'a> {} impl<'a> ListenerMap for PrivateFieldExpression<'a> {} impl<'a> ListenerMap for ArrayExpressionElement<'a> { - fn report_effects(&self, ctx: &LintContext) { + fn report_effects(&self, options: &NodeListenerOptions) { match self { - Self::Expression(expr) => expr.report_effects(ctx), + Self::Expression(expr) => expr.report_effects(options), Self::SpreadElement(spreed) => { - spreed.argument.report_effects(ctx); + spreed.argument.report_effects(options); } Self::Elision(_) => {} } 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 c5720b05c..936b1e83c 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 @@ -8,7 +8,7 @@ use oxc_span::{CompactStr, Span}; use crate::{context::LintContext, rule::Rule}; -use self::listener_map::ListenerMap; +use self::listener_map::{ListenerMap, NodeListenerOptions}; mod listener_map; @@ -52,8 +52,8 @@ impl Rule for NoSideEffectsInInitialization { fn run_once(&self, ctx: &LintContext) { let Some(root) = ctx.nodes().iter().next() else { return }; let AstKind::Program(program) = root.kind() else { return }; - - program.report_effects(ctx); + let node_listener_options = NodeListenerOptions::new(ctx); + program.report_effects(&node_listener_options); } }