From 61bae74f76648e325561a896680ec8e04e23491d Mon Sep 17 00:00:00 2001
From: rzvxa <3788964+rzvxa@users.noreply.github.com>
Date: Thu, 6 Jun 2024 07:55:37 +0000
Subject: [PATCH] improvement(semantic/cfg): better CFG API (#3472)

---
 .../src/rules/react/rules_of_hooks.rs         |  7 +--
 crates/oxc_semantic/examples/cfg.rs           |  3 +-
 crates/oxc_semantic/src/control_flow/mod.rs   | 48 +++++++++++++------
 3 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs
index b9fc7c03e00d3..fce19e4f33a5c 100644
--- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs
+++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs
@@ -228,12 +228,7 @@ impl Rule for RulesOfHooks {
             return;
         }
 
-        if !petgraph::algo::has_path_connecting(
-            &semantic.cfg().graph,
-            func_cfg_id,
-            node_cfg_id,
-            None,
-        ) {
+        if !ctx.semantic().cfg().is_reachabale(func_cfg_id, node_cfg_id) {
             // There should always be a control flow path between a parent and child node.
             // If there is none it means we always do an early exit before reaching our hook call.
             // In some cases it might mean that we are operating on an invalid `cfg` but in either
diff --git a/crates/oxc_semantic/examples/cfg.rs b/crates/oxc_semantic/examples/cfg.rs
index 0ab461b0ad188..1817d925b84a7 100644
--- a/crates/oxc_semantic/examples/cfg.rs
+++ b/crates/oxc_semantic/examples/cfg.rs
@@ -111,7 +111,8 @@ fn main() -> std::io::Result<()> {
                     }
                 });
                 format!(
-                    "xlabel = \"nodes [{}]\\l\", label = \"bb{}\n{}\"",
+                    "xlabel = \"nodes{} [{}]\\l\", label = \"bb{}\n{}\"",
+                    node.1,
                     nodes,
                     node.1,
                     semantic.semantic.cfg().basic_blocks[*node.1]
diff --git a/crates/oxc_semantic/src/control_flow/mod.rs b/crates/oxc_semantic/src/control_flow/mod.rs
index c45a48cba4883..e609ed7c3b7b0 100644
--- a/crates/oxc_semantic/src/control_flow/mod.rs
+++ b/crates/oxc_semantic/src/control_flow/mod.rs
@@ -1,14 +1,13 @@
 mod builder;
 mod dot;
 
-use itertools::Itertools;
 use oxc_span::CompactStr;
 use oxc_syntax::operator::{
     AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator, UpdateOperator,
 };
 use petgraph::{
     stable_graph::NodeIndex,
-    visit::{depth_first_search, Dfs, DfsEvent, Walker},
+    visit::{depth_first_search, Control, DfsEvent},
     Graph,
 };
 
@@ -192,19 +191,28 @@ impl ControlFlowGraph {
     }
 
     pub fn is_reachabale(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
+        if from == to {
+            return true;
+        }
         let graph = &self.graph;
-        let mut dfs = Dfs::empty(graph);
-        dfs.reset(graph);
-        dfs.move_to(from);
-        dfs.iter(graph)
-            .take_while_inclusive(|it| {
-                !self
-                    .basic_block(*it)
-                    .instructions()
-                    .iter()
-                    .any(|it| matches!(it, Instruction { kind: InstructionKind::Unreachable, .. }))
-            })
-            .any(|x| x == to)
+        depth_first_search(&self.graph, Some(from), |event| match event {
+            DfsEvent::TreeEdge(a, b) => {
+                let unreachable = graph.edges_connecting(a, b).all(|edge| {
+                    matches!(edge.weight(), EdgeType::NewFunction | EdgeType::Unreachable)
+                });
+
+                if unreachable {
+                    Control::Prune
+                } else if b == to {
+                    return Control::Break(true);
+                } else {
+                    Control::Continue
+                }
+            }
+            _ => Control::Continue,
+        })
+        .break_value()
+        .unwrap_or(false)
     }
 
     pub fn is_cyclic(&self, node: BasicBlockId) -> bool {
@@ -214,6 +222,18 @@ impl ControlFlowGraph {
         })
         .is_err()
     }
+
+    pub fn has_conditional_path(&self, from: BasicBlockId, to: BasicBlockId) -> bool {
+        let graph = &self.graph;
+        // All nodes should be able to reach the `to` node, Otherwise we have a conditional/branching flow.
+        petgraph::algo::dijkstra(graph, from, Some(to), |e| match e.weight() {
+            EdgeType::NewFunction => 1,
+            EdgeType::Jump | EdgeType::Unreachable | EdgeType::Backedge | EdgeType::Normal => 0,
+        })
+        .into_iter()
+        .filter(|(_, val)| *val == 0)
+        .any(|(f, _)| !self.is_reachabale(f, to))
+    }
 }
 
 pub struct PreservedExpressionState {