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.
oxc-project · Jun 18, 2024 · 8bfea7b · 8bfea7b
1 parent 97bcb64
commit 8bfea7b
Show file tree

Hide file tree

Showing 11 changed files with 557 additions and 418 deletions.
diff --git a/crates/oxc_cfg/src/lib.rs b/crates/oxc_cfg/src/lib.rs
@@ -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");

diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs
@@ -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,

diff --git a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs
@@ -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)

diff --git a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs
@@ -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();

diff --git a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs
@@ -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

diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs
@@ -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.

diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs
@@ -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,26 +239,25 @@ 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));
         }
     }
 }
 
 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)))

diff --git a/crates/oxc_semantic/examples/cfg.rs b/crates/oxc_semantic/examples/cfg.rs
@@ -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()
                 )
             }
         )