From cc7e893634c42475477d16dc11d18df9401c67b5 Mon Sep 17 00:00:00 2001 From: mysteryven <33973865+mysteryven@users.noreply.github.com> Date: Thu, 11 Jul 2024 05:28:35 +0000 Subject: [PATCH] fix(linter/tree-shaking): avoid recursive function stackoverflow (#4191) fixes: #4164 --- .../listener_map.rs | 22 ++++---- .../no_side_effects_in_initialization/mod.rs | 29 ++++++++++ .../no_side_effects_in_initialization.snap | 56 +++---------------- crates/oxc_linter/src/utils/tree_shaking.rs | 6 ++ 4 files changed, 54 insertions(+), 59 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 da3080c4d92e..aa86a72ac8b9 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 @@ -488,7 +488,6 @@ impl<'a> ListenerMap for BindingIdentifier<'a> { } } } - let symbol_table = ctx.semantic().symbols(); let node = ctx.nodes().get_node(symbol_table.get_declaration(symbol_id)); node.report_effects_when_called(options); } @@ -578,7 +577,6 @@ impl<'a> ListenerMap for Expression<'a> { } fn report_effects_when_mutated(&self, options: &NodeListenerOptions) { - #[allow(clippy::single_match)] match self { Self::Identifier(ident) => { ident.report_effects_when_mutated(options); @@ -1135,18 +1133,20 @@ impl<'a> ListenerMap for IdentifierReference<'a> { let ctx = options.ctx; 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); + if options.insert_called_node(symbol_id) { + 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); } - 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(super::call_global(self.name.as_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 f276ecd0b481..1735d330c922 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 @@ -503,6 +503,25 @@ fn test() { "function* x(){yield ext}; x()", // Supports TypeScript nodes "interface Blub {}", + " + function a() { + a + } + function b() { + b + } + export { + a, + b + } + ", + " + const Comp = () => { +
+ +
+ } + ", ]; let fail = vec![ @@ -788,6 +807,16 @@ fn test() { "function* x(){yield ext()}; x()", // YieldExpression when called "function* x(){yield ext()}; x()", + " + function f() { + try { + f(); + } catch(e) { + a.map(v => v + 1); + } + } + f(); + ", ]; // test options 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 d8484099aea9..8c26f7b3a32b 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 @@ -541,18 +541,6 @@ source: crates/oxc_linter/src/tester.rs · ─ ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling function parameter - ╭─[no_side_effects_in_initialization.tsx:1:12] - 1 │ function x(a){a(); a(); a()}; x(ext) - · ─ - ╰──── - - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling function parameter - ╭─[no_side_effects_in_initialization.tsx:1:12] - 1 │ function x(a){a(); a(); a()}; x(ext) - · ─ - ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating function parameter ╭─[no_side_effects_in_initialization.tsx:1:12] 1 │ function x(a){a.y = 1}; x(ext) @@ -583,30 +571,6 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:14] - 1 │ function x(){ext = 1}; x(); x(); x() - · ─── - ╰──── - - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:14] - 1 │ function x(){ext = 1}; x(); x(); x() - · ─── - ╰──── - - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:14] - 1 │ function x(){ext = 1}; const y = new x(); y = new x(); y = new x() - · ─── - ╰──── - - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:14] - 1 │ function x(){ext = 1}; const y = new x(); y = new x(); y = new x() - · ─── - ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` ╭─[no_side_effects_in_initialization.tsx:1:14] 1 │ function x(){ext = 1}; const y = new x(); y = new x(); y = new x() @@ -709,18 +673,6 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:16] - 1 │ const x = ()=>{ext = 1}; x(); x(); x() - · ─── - ╰──── - - ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of assignment to `ext` - ╭─[no_side_effects_in_initialization.tsx:1:16] - 1 │ const x = ()=>{ext = 1}; x(); x(); x() - · ─── - ╰──── - ⚠ 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:31] 1 │ const x = ()=>{}; const {y} = x(); y() @@ -1303,6 +1255,14 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling member function + ╭─[no_side_effects_in_initialization.tsx:6:13] + 5 │ } catch(e) { + 6 │ a.map(v => v + 1); + · ───── + 7 │ } + ╰──── + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling member function ╭─[no_side_effects_in_initialization.tsx:1:1] 1 │ Object.freeze({}) diff --git a/crates/oxc_linter/src/utils/tree_shaking.rs b/crates/oxc_linter/src/utils/tree_shaking.rs index 5fe45096eb7f..1068dda2371c 100644 --- a/crates/oxc_linter/src/utils/tree_shaking.rs +++ b/crates/oxc_linter/src/utils/tree_shaking.rs @@ -16,6 +16,7 @@ mod pure_functions; pub struct NodeListenerOptions<'a, 'b> { pub checked_mutated_nodes: RefCell>, + pub checked_called_nodes: RefCell>, pub ctx: &'b LintContext<'a>, pub has_valid_this: Cell, pub called_with_new: Cell, @@ -27,6 +28,7 @@ impl<'a, 'b> NodeListenerOptions<'a, 'b> { pub fn new(ctx: &'b LintContext<'a>) -> Self { Self { checked_mutated_nodes: RefCell::new(FxHashSet::default()), + checked_called_nodes: RefCell::new(FxHashSet::default()), ctx, has_valid_this: Cell::new(false), called_with_new: Cell::new(false), @@ -46,6 +48,10 @@ impl<'a, 'b> NodeListenerOptions<'a, 'b> { pub fn insert_mutated_node(&self, symbol_id: SymbolId) -> bool { self.checked_mutated_nodes.borrow_mut().insert(symbol_id) } + + pub fn insert_called_node(&self, symbol_id: SymbolId) -> bool { + self.checked_called_nodes.borrow_mut().insert(symbol_id) + } } #[derive(Debug, Default, Clone)]