From 82c0a167c426c7bccc4b199e872cbf3f45eee5e8 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Thu, 5 Sep 2024 02:06:19 +0000 Subject: [PATCH] fix(linter): `tree_shaking/no_side_effects_in_initialization` handle JSX correctly (#5450) #5223 altered `JSXElementName` so `JSXElementName::Identifier` is used only for non-reference JSX names (e.g. `
`). `JSXElementName::IdentifierReference` is used where the name is a reference (e.g. ``). Similarly `JSXMemberExpressionObject`'s `object` is always an `IdentifierReference` now. So, the net result is that `JSXIdentifier` is now never a reference, it's just the JSX equivalent of `IdentifierName`. So I don't think `JSXIdentifier` can ever have side-effects. This PR: 1. Removes `impl ListenerMap for JSXIdentifier` 2. Adds `impl ListenerMap for JSXMemberExpression` and makes sure the root `IdentifierReference` (`Foo` in ``) is visited. --- .../listener_map.rs | 50 ++++++------------- .../no_side_effects_in_initialization.snap | 6 +-- 2 files changed, 19 insertions(+), 37 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 65c94af6d89c4..ce20e455bce0e 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 @@ -7,11 +7,11 @@ use oxc_ast::{ ConditionalExpression, Declaration, ExportSpecifier, Expression, ForStatementInit, FormalParameter, Function, IdentifierReference, JSXAttribute, JSXAttributeItem, JSXAttributeValue, JSXChild, JSXElement, JSXElementName, JSXExpression, - JSXExpressionContainer, JSXFragment, JSXIdentifier, JSXOpeningElement, LogicalExpression, - MemberExpression, NewExpression, ObjectExpression, ObjectPropertyKind, - ParenthesizedExpression, PrivateFieldExpression, Program, PropertyKey, SequenceExpression, - SimpleAssignmentTarget, Statement, StaticMemberExpression, SwitchCase, ThisExpression, - UnaryExpression, VariableDeclarator, + JSXExpressionContainer, JSXFragment, JSXMemberExpression, JSXMemberExpressionObject, + JSXOpeningElement, LogicalExpression, MemberExpression, NewExpression, ObjectExpression, + ObjectPropertyKind, ParenthesizedExpression, PrivateFieldExpression, Program, PropertyKey, + SequenceExpression, SimpleAssignmentTarget, Statement, StaticMemberExpression, SwitchCase, + ThisExpression, UnaryExpression, VariableDeclarator, }, AstKind, }; @@ -788,41 +788,24 @@ impl<'a> ListenerMap for JSXOpeningElement<'a> { impl<'a> ListenerMap for JSXElementName<'a> { fn report_effects_when_called(&self, options: &NodeListenerOptions) { match self { - Self::Identifier(ident) => ident.report_effects_when_called(options), + Self::Identifier(_) | Self::NamespacedName(_) => {} Self::IdentifierReference(ident) => ident.report_effects_when_called(options), - Self::NamespacedName(name) => name.property.report_effects_when_called(options), - Self::MemberExpression(member) => { - member.property.report_effects_when_called(options); - } + Self::MemberExpression(member) => member.report_effects_when_called(options), } } } -impl<'a> ListenerMap for JSXIdentifier<'a> { +impl<'a> ListenerMap for JSXMemberExpression<'a> { fn report_effects_when_called(&self, options: &NodeListenerOptions) { - if self.name.chars().next().is_some_and(char::is_uppercase) { - let Some(symbol_id) = options.ctx.symbols().get_symbol_id_from_name(&self.name) else { - options.ctx.diagnostic(super::call_global(self.name.as_str(), self.span)); - return; - }; + self.object.report_effects_when_called(options); + } +} - for reference in options.ctx.symbols().get_resolved_references(symbol_id) { - if reference.is_write() { - let node_id = reference.node_id(); - if let Some(expr) = get_write_expr(node_id, options.ctx) { - let old_val = options.called_with_new.get(); - options.called_with_new.set(true); - expr.report_effects_when_called(options); - options.called_with_new.set(old_val); - } - } - } - let symbol_table = options.ctx.semantic().symbols(); - let node = options.ctx.nodes().get_node(symbol_table.get_declaration(symbol_id)); - let old_val = options.called_with_new.get(); - options.called_with_new.set(true); - node.report_effects_when_called(options); - options.called_with_new.set(old_val); +impl<'a> ListenerMap for JSXMemberExpressionObject<'a> { + fn report_effects_when_called(&self, options: &NodeListenerOptions) { + match self { + Self::IdentifierReference(ident) => ident.report_effects_when_called(options), + Self::MemberExpression(member) => member.report_effects_when_called(options), } } } @@ -1140,7 +1123,6 @@ impl<'a> ListenerMap for IdentifierReference<'a> { ); if is_used_in_jsx { - // TODO: remove the repeat the same logic in `JSXIdentifier::report_effects_when_called` method for reference in options.ctx.symbols().get_resolved_references(symbol_id) { if reference.is_write() { let node_id = reference.node_id(); 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 8c26f7b3a32be..ce78f67d6e604 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 @@ -817,10 +817,10 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `Y` - ╭─[no_side_effects_in_initialization.tsx:1:34] + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling + ╭─[no_side_effects_in_initialization.tsx:1:11] 1 │ const X = {Y: ext}; const x = - · ─ + · ──────── ╰──── ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext`