Skip to content

Commit

Permalink
fix: better throw control flow.
Browse files Browse the repository at this point in the history
  • Loading branch information
rzvxa committed Jun 1, 2024
1 parent 7e143f2 commit 9aa8f48
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 4 deletions.
36 changes: 34 additions & 2 deletions crates/oxc_linter/src/rules/react/rules_of_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use oxc_ast::{
AstKind,
};
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{AstNodeId, AstNodes};
use oxc_semantic::{
algo, petgraph::visit::Control, AstNodeId, AstNodes, BasicBlockId, EdgeType, InstructionKind,
};
use oxc_span::{Atom, CompactStr};
use oxc_syntax::operator::AssignmentOperator;

Expand Down Expand Up @@ -236,13 +238,43 @@ impl Rule for RulesOfHooks {
return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
}

if semantic.cfg().has_conditional_path(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));
}
}
}

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
}
})
})
}

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())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,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);
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_semantic/src/control_flow/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
13 changes: 13 additions & 0 deletions crates/oxc_semantic/src/control_flow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<F: Fn(BasicBlockId) -> Control<bool>>(
&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)
});
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9aa8f48

Please sign in to comment.