Skip to content

Commit

Permalink
fix(linter): tree_shaking/no_side_effects_in_initialization handle …
Browse files Browse the repository at this point in the history
…JSX correctly (#5450)

#5223 altered `JSXElementName` so `JSXElementName::Identifier` is used only for non-reference JSX names (e.g. `<div>`). `JSXElementName::IdentifierReference` is used where the name is a reference (e.g. `<Foo>`). 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 `<Foo.bar.qux>`) is visited.
  • Loading branch information
overlookmotel committed Sep 5, 2024
1 parent e4ed41d commit 82c0a16
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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),
}
}
}
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <X.Y />
·
· ───────
╰────

eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext`
Expand Down

0 comments on commit 82c0a16

Please sign in to comment.