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 3bff0b27b0d24..4ae1d27ebcc83 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -1,13 +1,10 @@ -use itertools::{FoldWhile, Itertools}; use oxc_ast::{ ast::{ArrowFunctionExpression, Function}, AstKind, }; use oxc_macros::declare_oxc_lint; use oxc_semantic::{ - petgraph::{self}, - pg::neighbors_filtered_by_edge_weight, - AstNodeId, AstNodes, BasicBlockId, EdgeType, Instruction, InstructionKind, + algo, petgraph::visit::Control, AstNodeId, AstNodes, BasicBlockId, EdgeType, InstructionKind, }; use oxc_span::{Atom, CompactStr}; use oxc_syntax::operator::AssignmentOperator; @@ -241,87 +238,43 @@ impl Rule for RulesOfHooks { return ctx.diagnostic(diagnostics::loop_hook(span, hook_name)); } - if Self::is_conditional(ctx, func_cfg_id, node_cfg_id) - || Self::breaks_early(ctx, func_cfg_id, node_cfg_id) - { + if has_conditional_path_accept_throw(ctx, func_cfg_id, node_cfg_id) { #[allow(clippy::needless_return)] return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name)); } } } -// TODO: all `dijkstra` algorithms can be merged together for better performance. -impl RulesOfHooks { - #[inline] - fn is_conditional( - ctx: &LintContext, - func_cfg_id: BasicBlockId, - node_cfg_id: BasicBlockId, - ) -> bool { - let cfg = ctx.semantic().cfg(); - let graph = &cfg.graph; - // All nodes should be reachable from our hook, Otherwise we have a conditional/branching flow. - petgraph::algo::dijkstra(graph, func_cfg_id, Some(node_cfg_id), |e| match e.weight() { - EdgeType::NewFunction => 1, - EdgeType::Jump | EdgeType::Unreachable | EdgeType::Backedge | EdgeType::Normal => 0, +fn has_conditional_path_accept_throw( + ctx: &LintContext<'_>, + from: BasicBlockId, + to: BasicBlockId, +) -> bool { + let cfg = ctx.semantic().cfg(); + let graph = &cfg.graph; + // All nodes should be able to reach the hook node, Otherwise we have a conditional/branching flow. + 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, _)| { + !cfg.is_reachabale_filtered(f, to, |it| { + if cfg + .basic_block(it) + .instructions() + .iter() + .any(|i| matches!(i.kind, InstructionKind::Throw)) + { + Control::Break(true) + } else { + Control::Continue + } }) - .into_iter() - .filter(|(_, val)| *val == 0) - .any(|(f, _)| !cfg.is_reachabale(f, node_cfg_id)) - } - - #[inline] - fn breaks_early( - ctx: &LintContext, - func_cfg_id: BasicBlockId, - node_cfg_id: BasicBlockId, - ) -> bool { - let cfg = ctx.semantic().cfg(); - neighbors_filtered_by_edge_weight( - &cfg.graph, - func_cfg_id, - &|e| match e { - EdgeType::Jump | EdgeType::Normal => None, - EdgeType::Unreachable | EdgeType::Backedge | EdgeType::NewFunction => { - Some(State::default()) - } - }, - &mut |id: &BasicBlockId, mut state: State| { - if node_cfg_id == *id { - return (state, false); - } - - let (push, keep_walking) = cfg - .basic_block(*id) - .instructions - .iter() - .fold_while((false, true), |acc, Instruction { kind, .. }| match kind { - InstructionKind::Break(_) => FoldWhile::Done((true, false)), - InstructionKind::Unreachable - | InstructionKind::Throw - | InstructionKind::Return(_) => FoldWhile::Continue((acc.0, false)), - InstructionKind::Statement | InstructionKind::Continue(_) => { - FoldWhile::Continue(acc) - } - }) - .into_inner(); - - if push { - state.0.push(*id); - } - (state, keep_walking) - }, - ) - .iter() - .flat_map(|it| it.0.iter()) - .next() - .is_some() - } + }) } -#[derive(Debug, Default, Clone)] -struct State(Vec); - fn parent_func<'a>(nodes: &'a AstNodes<'a>, node: &AstNode) -> Option<&'a AstNode<'a>> { nodes.ancestors(node.id()).map(|id| nodes.get_node(id)).find(|it| it.kind().is_function_like()) } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 211056a1b8b23..d69bad0e48062 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -1118,7 +1118,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { // todo - append unreachable after throw statement /* cfg */ - self.cfg.push_throw(node_id); + self.cfg.append_throw(node_id); /* cfg */ self.leave_node(kind); diff --git a/crates/oxc_semantic/src/control_flow/builder/mod.rs b/crates/oxc_semantic/src/control_flow/builder/mod.rs index 1d5422a9abc86..12e0694db98ce 100644 --- a/crates/oxc_semantic/src/control_flow/builder/mod.rs +++ b/crates/oxc_semantic/src/control_flow/builder/mod.rs @@ -96,8 +96,9 @@ impl<'a> ControlFlowGraphBuilder<'a> { self.push_instruction(InstructionKind::Return(kind), Some(node)); } - pub fn push_throw(&mut self, node: AstNodeId) { + pub fn append_throw(&mut self, node: AstNodeId) { self.push_instruction(InstructionKind::Throw, Some(node)); + self.append_unreachable(); } pub fn append_break(&mut self, node: AstNodeId, label: Option<&'a str>) { diff --git a/crates/oxc_semantic/src/control_flow/mod.rs b/crates/oxc_semantic/src/control_flow/mod.rs index e609ed7c3b7b0..988f6823fcec2 100644 --- a/crates/oxc_semantic/src/control_flow/mod.rs +++ b/crates/oxc_semantic/src/control_flow/mod.rs @@ -191,12 +191,25 @@ impl ControlFlowGraph { } pub fn is_reachabale(&self, from: BasicBlockId, to: BasicBlockId) -> bool { + self.is_reachabale_filtered(from, to, |_| Control::Continue) + } + + pub fn is_reachabale_filtered Control>( + &self, + from: BasicBlockId, + to: BasicBlockId, + filter: F, + ) -> bool { if from == to { return true; } let graph = &self.graph; depth_first_search(&self.graph, Some(from), |event| match event { DfsEvent::TreeEdge(a, b) => { + let filter_result = filter(a); + if !matches!(filter_result, Control::Continue) { + return filter_result; + } let unreachable = graph.edges_connecting(a, b).all(|edge| { matches!(edge.weight(), EdgeType::NewFunction | EdgeType::Unreachable) }); diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 869634096ae12..f2a4b08b77071 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -16,6 +16,7 @@ mod symbol; use std::{rc::Rc, sync::Arc}; pub use petgraph; +pub use petgraph::algo; pub use builder::{SemanticBuilder, SemanticBuilderReturn}; use class::ClassTable;