mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
refactor(semantic): make control flow generation optional. (#3737)
For maximum backward compatibility, we generate CFG by default. Note: It can't be done with a simple method since lifetimes make it impossible(at least without unsafe trickery) I've tried to do it without a macro but it was just unintuitive.
This commit is contained in:
parent
0537d298db
commit
d8ad321687
11 changed files with 551 additions and 412 deletions
|
|
@ -124,6 +124,10 @@ pub struct ControlFlowGraph {
|
|||
}
|
||||
|
||||
impl ControlFlowGraph {
|
||||
pub fn graph(&self) -> &Graph<usize, EdgeType> {
|
||||
&self.graph
|
||||
}
|
||||
|
||||
/// # Panics
|
||||
pub fn basic_block(&self, id: BasicBlockId) -> &BasicBlock {
|
||||
let ix = *self.graph.node_weight(id).expect("expected a valid node id in self.graph");
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use oxc_ast::{
|
|||
use oxc_diagnostics::OxcDiagnostic;
|
||||
|
||||
use oxc_cfg::{
|
||||
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, InstructionKind,
|
||||
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, InstructionKind,
|
||||
ReturnInstructionKind,
|
||||
};
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
|
|
@ -55,16 +55,19 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for GetterReturn {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
// https://eslint.org/docs/latest/rules/getter-return#handled_by_typescript
|
||||
if ctx.source_type().is_typescript() {
|
||||
return;
|
||||
}
|
||||
match node.kind() {
|
||||
AstKind::Function(func) if !func.is_typescript_syntax() => {
|
||||
self.run_diagnostic(node, ctx, func.span);
|
||||
self.run_diagnostic(node, ctx, cfg, func.span);
|
||||
}
|
||||
AstKind::ArrowFunctionExpression(expr) => {
|
||||
self.run_diagnostic(node, ctx, expr.span);
|
||||
self.run_diagnostic(node, ctx, cfg, expr.span);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
|
@ -178,15 +181,19 @@ impl GetterReturn {
|
|||
false
|
||||
}
|
||||
|
||||
fn run_diagnostic<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>, span: Span) {
|
||||
fn run_diagnostic<'a>(
|
||||
&self,
|
||||
node: &AstNode<'a>,
|
||||
ctx: &LintContext<'a>,
|
||||
cfg: &ControlFlowGraph,
|
||||
span: Span,
|
||||
) {
|
||||
if !Self::is_wanted_node(node, ctx) {
|
||||
return;
|
||||
}
|
||||
|
||||
let cfg = ctx.semantic().cfg();
|
||||
|
||||
let output = neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
cfg.graph(),
|
||||
node.cfg_id(),
|
||||
&|edge| match edge {
|
||||
EdgeType::Jump | EdgeType::Normal => None,
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use oxc_cfg::{
|
|||
visit::{neighbors_filtered_by_edge_weight, EdgeRef},
|
||||
Direction,
|
||||
},
|
||||
BasicBlockId, EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
BasicBlockId, ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
|
|
@ -89,13 +89,15 @@ impl Rule for NoFallthrough {
|
|||
}
|
||||
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let AstKind::SwitchStatement(switch) = node.kind() else { return };
|
||||
|
||||
let switch_id = node.cfg_id();
|
||||
let cfg = ctx.semantic().cfg();
|
||||
let graph = &cfg.graph;
|
||||
let graph = cfg.graph();
|
||||
|
||||
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);
|
||||
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, cfg, node, switch);
|
||||
|
||||
let Some(default_or_exit) = default.or(exit) else {
|
||||
// TODO: our `get_switch_semantic_cases` can't evaluate cfg_ids for switch statements
|
||||
|
|
@ -260,6 +262,7 @@ impl NoFallthrough {
|
|||
// Issue: <https://github.com/oxc-project/oxc/issues/3662>
|
||||
fn get_switch_semantic_cases(
|
||||
ctx: &LintContext,
|
||||
cfg: &ControlFlowGraph,
|
||||
node: &AstNode,
|
||||
switch: &SwitchStatement,
|
||||
) -> (
|
||||
|
|
@ -268,8 +271,7 @@ fn get_switch_semantic_cases(
|
|||
/* default */ Option<BasicBlockId>,
|
||||
/* exit */ Option<BasicBlockId>,
|
||||
) {
|
||||
let cfg = &ctx.semantic().cfg();
|
||||
let graph = &cfg.graph;
|
||||
let graph = cfg.graph();
|
||||
let has_default = switch.cases.iter().any(SwitchCase::is_default_case);
|
||||
let (tests, exit) = graph
|
||||
.edges_directed(node.cfg_id(), Direction::Outgoing)
|
||||
|
|
|
|||
|
|
@ -58,7 +58,8 @@ enum DefinitelyCallsThisBeforeSuper {
|
|||
impl Rule for NoThisBeforeSuper {
|
||||
fn run_once(&self, ctx: &LintContext) {
|
||||
let semantic = ctx.semantic();
|
||||
let cfg = semantic.cfg();
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
// first pass -> find super calls and local violations
|
||||
let mut wanted_nodes = Vec::new();
|
||||
|
|
|
|||
|
|
@ -31,10 +31,12 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for NoUnreachable {
|
||||
fn run_once(&self, ctx: &LintContext) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let nodes = ctx.nodes();
|
||||
let Some(root) = nodes.root_node() else { return };
|
||||
let cfg = ctx.semantic().cfg();
|
||||
let graph = &cfg.graph;
|
||||
let graph = cfg.graph();
|
||||
|
||||
// A pre-allocated vector containing the reachability status of all the basic blocks.
|
||||
// We initialize this vector with all nodes set to `unreachable` since if we don't visit a
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ use oxc_ast::{ast::Expression, AstKind};
|
|||
use oxc_diagnostics::OxcDiagnostic;
|
||||
|
||||
use oxc_cfg::{
|
||||
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, Instruction, InstructionKind,
|
||||
ReturnInstructionKind,
|
||||
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, Instruction,
|
||||
InstructionKind, ReturnInstructionKind,
|
||||
};
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_span::{GetSpan, Span};
|
||||
|
|
@ -51,6 +51,9 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for RequireRenderReturn {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
if !matches!(node.kind(), AstKind::ArrowFunctionExpression(_) | AstKind::Function(_)) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -64,7 +67,7 @@ impl Rule for RequireRenderReturn {
|
|||
return;
|
||||
}
|
||||
|
||||
if !contains_return_statement(node, ctx) {
|
||||
if !contains_return_statement(node, cfg) {
|
||||
match parent.kind() {
|
||||
AstKind::MethodDefinition(method) => {
|
||||
ctx.diagnostic(require_render_return_diagnostic(method.key.span()));
|
||||
|
|
@ -91,10 +94,9 @@ enum FoundReturn {
|
|||
const KEEP_WALKING_ON_THIS_PATH: bool = true;
|
||||
const STOP_WALKING_ON_THIS_PATH: bool = false;
|
||||
|
||||
fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool {
|
||||
let cfg = ctx.semantic().cfg();
|
||||
fn contains_return_statement(node: &AstNode, cfg: &ControlFlowGraph) -> bool {
|
||||
let state = neighbors_filtered_by_edge_weight(
|
||||
&cfg.graph,
|
||||
cfg.graph(),
|
||||
node.cfg_id(),
|
||||
&|edge| match edge {
|
||||
// We only care about normal edges having a return statement.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ use oxc_ast::{
|
|||
};
|
||||
use oxc_cfg::{
|
||||
graph::{algo, visit::Control},
|
||||
EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
};
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_semantic::{AstNodeId, AstNodes};
|
||||
|
|
@ -107,6 +107,9 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for RulesOfHooks {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let AstKind::CallExpression(call) = node.kind() else { return };
|
||||
|
||||
if !is_react_hook(&call.callee) {
|
||||
|
|
@ -227,7 +230,7 @@ impl Rule for RulesOfHooks {
|
|||
return;
|
||||
}
|
||||
|
||||
if !ctx.semantic().cfg().is_reachable(func_cfg_id, node_cfg_id) {
|
||||
if !cfg.is_reachable(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
|
||||
|
|
@ -236,11 +239,11 @@ impl Rule for RulesOfHooks {
|
|||
}
|
||||
|
||||
// Is this node cyclic?
|
||||
if semantic.cfg().is_cyclic(node_cfg_id) {
|
||||
if cfg.is_cyclic(node_cfg_id) {
|
||||
return ctx.diagnostic(diagnostics::loop_hook(span, hook_name));
|
||||
}
|
||||
|
||||
if has_conditional_path_accept_throw(ctx, parent_func, node) {
|
||||
if has_conditional_path_accept_throw(cfg, parent_func, node) {
|
||||
#[allow(clippy::needless_return)]
|
||||
return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name));
|
||||
}
|
||||
|
|
@ -248,14 +251,13 @@ impl Rule for RulesOfHooks {
|
|||
}
|
||||
|
||||
fn has_conditional_path_accept_throw(
|
||||
ctx: &LintContext<'_>,
|
||||
cfg: &ControlFlowGraph,
|
||||
from: &AstNode<'_>,
|
||||
to: &AstNode<'_>,
|
||||
) -> bool {
|
||||
let from_graph_id = from.cfg_id();
|
||||
let to_graph_id = to.cfg_id();
|
||||
let cfg = ctx.semantic().cfg();
|
||||
let graph = &cfg.graph;
|
||||
let graph = cfg.graph();
|
||||
if graph
|
||||
.edges(to_graph_id)
|
||||
.any(|it| matches!(it.weight(), EdgeType::Error(ErrorEdgeKind::Explicit)))
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ fn main() -> std::io::Result<()> {
|
|||
let semantic = SemanticBuilder::new(&source_text, source_type)
|
||||
.with_check_syntax_error(true)
|
||||
.with_trivias(ret.trivias)
|
||||
.with_cfg(true)
|
||||
.build(program);
|
||||
|
||||
if !semantic.errors.is_empty() {
|
||||
|
|
@ -59,16 +60,19 @@ fn main() -> std::io::Result<()> {
|
|||
return Ok(());
|
||||
}
|
||||
|
||||
let cfg = semantic
|
||||
.semantic
|
||||
.cfg()
|
||||
.expect("we set semantic to build the control flow (`with_cfg`) for us so it should always be `Some`");
|
||||
|
||||
let mut ast_nodes_by_block = HashMap::<_, Vec<_>>::new();
|
||||
for node in semantic.semantic.nodes().iter() {
|
||||
let block = node.cfg_id();
|
||||
let block_ix = semantic.semantic.cfg().graph.node_weight(block).unwrap();
|
||||
let block_ix = cfg.graph.node_weight(block).unwrap();
|
||||
ast_nodes_by_block.entry(*block_ix).or_default().push(node);
|
||||
}
|
||||
|
||||
let basic_blocks_printed = semantic
|
||||
.semantic
|
||||
.cfg()
|
||||
let basic_blocks_printed = cfg
|
||||
.basic_blocks
|
||||
.iter()
|
||||
.map(DisplayDot::display_dot)
|
||||
|
|
@ -93,13 +97,13 @@ fn main() -> std::io::Result<()> {
|
|||
let cfg_dot_diagram = format!(
|
||||
"{:?}",
|
||||
Dot::with_attr_getters(
|
||||
&semantic.semantic.cfg().graph,
|
||||
cfg.graph(),
|
||||
&[Config::EdgeNoLabel, Config::NodeNoLabel],
|
||||
&|_graph, edge| {
|
||||
let weight = edge.weight();
|
||||
let label = format!("label = \"{weight:?}\"");
|
||||
if matches!(weight, EdgeType::Unreachable)
|
||||
|| semantic.semantic.cfg().basic_block(edge.source()).unreachable
|
||||
|| cfg.basic_block(edge.source()).unreachable
|
||||
{
|
||||
format!("{label}, style = \"dotted\" ")
|
||||
} else {
|
||||
|
|
@ -124,9 +128,7 @@ fn main() -> std::io::Result<()> {
|
|||
node.1,
|
||||
nodes,
|
||||
node.1,
|
||||
semantic.semantic.cfg().basic_blocks[*node.1]
|
||||
.debug_dot(semantic.semantic.nodes().into())
|
||||
.trim()
|
||||
cfg.basic_blocks[*node.1].debug_dot(semantic.semantic.nodes().into()).trim()
|
||||
)
|
||||
}
|
||||
)
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -56,7 +56,7 @@ pub struct Semantic<'a> {
|
|||
|
||||
unused_labels: FxHashSet<AstNodeId>,
|
||||
|
||||
cfg: ControlFlowGraph,
|
||||
cfg: Option<ControlFlowGraph>,
|
||||
}
|
||||
|
||||
impl<'a> Semantic<'a> {
|
||||
|
|
@ -108,8 +108,8 @@ impl<'a> Semantic<'a> {
|
|||
&self.unused_labels
|
||||
}
|
||||
|
||||
pub fn cfg(&self) -> &ControlFlowGraph {
|
||||
&self.cfg
|
||||
pub fn cfg(&self) -> Option<&ControlFlowGraph> {
|
||||
self.cfg.as_ref()
|
||||
}
|
||||
|
||||
pub fn is_unresolved_reference(&self, node_id: AstNodeId) -> bool {
|
||||
|
|
|
|||
|
|
@ -102,29 +102,29 @@ impl<'a> SemanticTester<'a> {
|
|||
|
||||
pub fn basic_blocks_count(&self) -> usize {
|
||||
let built = self.build();
|
||||
built.cfg().basic_blocks.len()
|
||||
built.cfg().map_or(0, |cfg| cfg.basic_blocks.len())
|
||||
}
|
||||
|
||||
pub fn basic_blocks_printed(&self) -> String {
|
||||
let built = self.build();
|
||||
built
|
||||
.cfg()
|
||||
.basic_blocks
|
||||
.iter()
|
||||
.map(DisplayDot::display_dot)
|
||||
.enumerate()
|
||||
.map(|(i, it)| {
|
||||
format!(
|
||||
"bb{i}: {{\n{}\n}}",
|
||||
it.lines().map(|x| format!("\t{}", x.trim())).join("\n")
|
||||
)
|
||||
})
|
||||
.join("\n\n")
|
||||
built.cfg().map_or_else(String::default, |cfg| {
|
||||
cfg.basic_blocks
|
||||
.iter()
|
||||
.map(DisplayDot::display_dot)
|
||||
.enumerate()
|
||||
.map(|(i, it)| {
|
||||
format!(
|
||||
"bb{i}: {{\n{}\n}}",
|
||||
it.lines().map(|x| format!("\t{}", x.trim())).join("\n")
|
||||
)
|
||||
})
|
||||
.join("\n\n")
|
||||
})
|
||||
}
|
||||
|
||||
pub fn cfg_dot_diagram(&self) -> String {
|
||||
let semantic = self.build();
|
||||
semantic.cfg().debug_dot(semantic.nodes().into())
|
||||
semantic.cfg().map_or_else(String::default, |cfg| cfg.debug_dot(semantic.nodes().into()))
|
||||
}
|
||||
|
||||
/// Tests that a symbol with the given name exists at the top-level scope and provides a
|
||||
|
|
|
|||
Loading…
Reference in a new issue