From 5e06298ec237f28b631b633c96fe15d0bc9664bf Mon Sep 17 00:00:00 2001 From: mysteryven <33973865+mysteryven@users.noreply.github.com> Date: Sun, 26 May 2024 08:11:48 +0000 Subject: [PATCH] fix(linter): memorize visited block id in `neighbors_filtered_by_edge_weight` (#3407) closes: #3396 we visit the same node too many times, I picked some result from run require-render-return rule on [timeserieseexploere.js](https://github.com/elastic/kibana/blob/main/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js) ```bash NodeIndex(64): 368640, NodeIndex(67): 737280, NodeIndex(70): 2949120, NodeIndex(73): 5971968, NodeIndex(76): 11943936, NodeIndex(43): 184320, NodeIndex(71): 2985984, NodeIndex(65): 368640, NodeIndex(68): 1474560, NodeIndex(74): 5971968, NodeIndex(77): 23887872, NodeIndex(44): 184320, NodeIndex(41): 73728, NodeIndex(35): 36864, NodeIndex(66): 737280, NodeIndex(69): 1474560, NodeIndex(72): 2985984, NodeIndex(75): 11943936, ``` --- .../src/rules/react/require_render_return.rs | 42 +++++++++++++++---- crates/oxc_semantic/src/builder.rs | 6 --- crates/oxc_semantic/src/pg.rs | 6 +++ ..._cfg_files@cond_expr_in_arrow_fn.js-2.snap | 2 - ...g__cfg_files@cond_expr_in_arrow_fn.js.snap | 4 -- ...cfg_files@conditional_expression.js-2.snap | 2 - ...__cfg_files@conditional_expression.js.snap | 4 -- ...egration__cfg__cfg_files@if_else.js-2.snap | 14 +++---- ...ntegration__cfg__cfg_files@if_else.js.snap | 11 ++--- ...cfg__cfg_files@if_stmt_in_for_in.js-2.snap | 10 ++--- ...__cfg__cfg_files@if_stmt_in_for_in.js.snap | 6 +-- 11 files changed, 55 insertions(+), 52 deletions(-) diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index 0cdd101f5da42..f56a2f46de9af 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -110,14 +110,18 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b } for entry in cfg.basic_block(*basic_block_id) { - if let BasicBlockElement::Assignment(to_reg, val) = entry { - if matches!(to_reg, Register::Return) - && matches!(val, AssignmentValue::NotImplicitUndefined) - { - return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH); + match entry { + BasicBlockElement::Assignment(to_reg, val) => { + if matches!(to_reg, Register::Return) + && matches!(val, AssignmentValue::NotImplicitUndefined) + { + return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH); + } } - } else { - // We don't care about other types of instructions. + BasicBlockElement::Unreachable | BasicBlockElement::Throw(_) => { + return (FoundReturn::No, STOP_WALKING_ON_THIS_PATH); + } + BasicBlockElement::Break(_) => {} } } @@ -186,7 +190,31 @@ fn is_in_es6_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) fn test() { use crate::tester::Tester; + // let too_many_if_else = (1..10) + // .map(|i| { + // " + // if (a > i) { + // foo1() + // } else { + // foo2() + // } + // " + // }) + // .collect::(); + + // let too_many_if_else_case = format!( + // " + // class Hello extends React.Component {{ + // render() {{ + // {too_many_if_else} + // return 'div' + // }} + // }} + // ", + // ); + let pass = vec![ + // &too_many_if_else_case, r" class Hello extends React.Component { render() { diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 2be4fb3b76b40..de2de4dcdeaaa 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -801,7 +801,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let after_conditional_graph_ix = self.cfg.new_basic_block(); /* cfg */ - self.cfg.put_unreachable(); self.cfg.add_edge( after_consequent_expr_graph_ix, after_conditional_graph_ix, @@ -1078,12 +1077,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg - bb after if statement joins consequent and alternate */ let after_if_graph_ix = self.cfg.new_basic_block(); - if stmt.alternate.is_some() { - self.cfg.put_unreachable(); - } - // else { self.cfg.add_edge(after_consequent_stmt_graph_ix, after_if_graph_ix, EdgeType::Normal); - // } self.cfg.add_edge( before_if_stmt_graph_ix, diff --git a/crates/oxc_semantic/src/pg.rs b/crates/oxc_semantic/src/pg.rs index 139ab07e13926..8da2e3dc1f534 100644 --- a/crates/oxc_semantic/src/pg.rs +++ b/crates/oxc_semantic/src/pg.rs @@ -1,4 +1,5 @@ use petgraph::{visit::EdgeRef, Direction, Graph}; +use rustc_hash::FxHashSet; use crate::BasicBlockId; @@ -15,6 +16,7 @@ where { let mut q = vec![]; let mut final_states = vec![]; + let mut visited = FxHashSet::default(); // for initial node let (new_state, keep_walking_this_path) = visitor(&node, Default::default()); @@ -27,6 +29,10 @@ where while let Some((graph_ix, state)) = q.pop() { let mut edges = 0; + if visited.contains(&graph_ix) { + continue; + } + visited.insert(graph_ix); for edge in graph.edges_directed(graph_ix, Direction::Outgoing) { if let Some(result_of_edge_filtering) = edge_filter(edge.weight()) { final_states.push(result_of_edge_filtering); diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js-2.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js-2.snap index 5f3545e7e4135..8c28920da0b51 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js-2.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js-2.snap @@ -9,9 +9,7 @@ digraph { 2 [ label = ""] 3 [ label = ""] 4 [ label = ""] - 5 [ label = "Unreachable()"] 0 -> 1 [ ] - 4 -> 5 [ ] 2 -> 4 [ ] 1 -> 2 [ ] 1 -> 3 [ ] diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js.snap index e40268a2a4a40..c02a06dbb6cec 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@cond_expr_in_arrow_fn.js.snap @@ -22,7 +22,3 @@ bb3: { bb4: { } - -bb5: { - Unreachable() -} diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js-2.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js-2.snap index 9895a67b01b22..dc7daddd45d3c 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js-2.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js-2.snap @@ -8,8 +8,6 @@ digraph { 1 [ label = ""] 2 [ label = ""] 3 [ label = ""] - 4 [ label = "Unreachable()"] - 3 -> 4 [ ] 1 -> 3 [ ] 0 -> 1 [ ] 0 -> 2 [ ] diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js.snap index 876e2fb9eb212..6965e2eb88e65 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@conditional_expression.js.snap @@ -18,7 +18,3 @@ bb2: { bb3: { } - -bb4: { - Unreachable() -} diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js-2.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js-2.snap index fde8ce7f5afc7..ee2e7b74d6572 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js-2.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js-2.snap @@ -14,22 +14,20 @@ digraph { 7 [ label = "$return = "] 8 [ label = ""] 9 [ label = "Unreachable()"] - 10 [ label = ""] - 11 [ label = "Unreachable()\n$return = "] - 12 [ label = ""] - 13 [ label = "Unreachable()"] - 14 [ label = ""] + 10 [ label = "$return = "] + 11 [ label = ""] + 12 [ label = "Unreachable()"] + 13 [ label = ""] 0 -> 1 [ ] 1 -> 2 [ ] 1 -> 3 [ ] 2 -> 3 [ ] 5 -> 6 [ ] 8 -> 9 [ ] - 10 -> 11 [ ] 6 -> 10 [ ] 3 -> 4 [ ] 3 -> 7 [ ] 9 -> 10 [ ] - 12 -> 13 [ ] - 0 -> 14 [ ] + 11 -> 12 [ ] + 0 -> 13 [ ] } diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js.snap index 47b5491856222..13edc37e913f0 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_else.js.snap @@ -44,22 +44,17 @@ bb9: { } bb10: { - -} - -bb11: { - Unreachable() $return = } -bb12: { +bb11: { } -bb13: { +bb12: { Unreachable() } -bb14: { +bb13: { } diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js-2.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js-2.snap index 425ba4e9072aa..ab5dfa6d2b891 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js-2.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js-2.snap @@ -17,9 +17,8 @@ digraph { 10 [ label = "Unreachable()"] 11 [ label = ""] 12 [ label = ""] - 13 [ label = "Unreachable()"] + 13 [ label = ""] 14 [ label = ""] - 15 [ label = ""] 0 -> 1 [ ] 5 -> 6 [ ] 5 -> 6 [ ] @@ -29,7 +28,6 @@ digraph { 10 -> 11 [ ] 7 -> 8 [ ] 7 -> 11 [ ] - 12 -> 13 [ ] 6 -> 12 [ ] 4 -> 5 [ ] 4 -> 7 [ ] @@ -37,8 +35,8 @@ digraph { 1 -> 2 [ ] 2 -> 3 [ ] 3 -> 4 [ ] - 13 -> 3 [ ] - 3 -> 14 [ ] + 12 -> 3 [ ] + 3 -> 13 [ ] 9 -> 3 [ ] - 0 -> 15 [ ] + 0 -> 14 [ ] } diff --git a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js.snap b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js.snap index fadca81c036af..72d8cb221fc53 100644 --- a/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js.snap +++ b/crates/oxc_semantic/tests/integration/snapshots/integration__cfg__cfg_files@if_stmt_in_for_in.js.snap @@ -57,13 +57,9 @@ bb12: { } bb13: { - Unreachable() -} - -bb14: { } -bb15: { +bb14: { }