diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs index a0a733a527ce5..68409c15200a6 100644 --- a/crates/oxc_linter/src/rules/eslint/getter_return.rs +++ b/crates/oxc_linter/src/rules/eslint/getter_return.rs @@ -234,72 +234,73 @@ impl GetterReturn { break 'returns true; } } - let output = set_depth_first_search(graph, Some(node.cfg_id()), |event| { - match event { - // We only need to check paths that are normal or jump. - DfsEvent::TreeEdge(a, b) => { - let edges = graph.edges_connecting(a, b).collect::>(); - if edges.iter().any(|e| { - matches!( - e.weight(), - EdgeType::Normal - | EdgeType::Jump - | EdgeType::Error(ErrorEdgeKind::Explicit) - ) - }) { - Control::Continue - } else { - Control::Prune + let output = + set_depth_first_search(graph, Some(ctx.nodes().cfg_id(node.id())), |event| { + match event { + // We only need to check paths that are normal or jump. + DfsEvent::TreeEdge(a, b) => { + let edges = graph.edges_connecting(a, b).collect::>(); + if edges.iter().any(|e| { + matches!( + e.weight(), + EdgeType::Normal + | EdgeType::Jump + | EdgeType::Error(ErrorEdgeKind::Explicit) + ) + }) { + Control::Continue + } else { + Control::Prune + } } - } - DfsEvent::Discover(basic_block_id, _) => { - let return_instruction = - cfg.basic_block(basic_block_id).instructions().iter().find(|it| { - match it.kind { - // Throws are classified as returning. - InstructionKind::Return(_) | InstructionKind::Throw => true, - - // Ignore irrelevant elements. - InstructionKind::ImplicitReturn - | InstructionKind::Break(_) - | InstructionKind::Continue(_) - | InstructionKind::Iteration(_) - | InstructionKind::Unreachable - | InstructionKind::Condition - | InstructionKind::Statement => false, - } + DfsEvent::Discover(basic_block_id, _) => { + let return_instruction = + cfg.basic_block(basic_block_id).instructions().iter().find(|it| { + match it.kind { + // Throws are classified as returning. + InstructionKind::Return(_) | InstructionKind::Throw => true, + + // Ignore irrelevant elements. + InstructionKind::ImplicitReturn + | InstructionKind::Break(_) + | InstructionKind::Continue(_) + | InstructionKind::Iteration(_) + | InstructionKind::Unreachable + | InstructionKind::Condition + | InstructionKind::Statement => false, + } + }); + + let does_return = return_instruction.is_some_and(|ret| { + !matches!( ret.kind, + InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined) + if !self.allow_implicit + ) }); - let does_return = return_instruction.is_some_and(|ret| { - !matches!( ret.kind, - InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined) - if !self.allow_implicit - ) - }); - - // Return true as the second argument to signify we should - // continue walking this branch, as we haven't seen anything - // that will signify to us that this path of the program will - // definitely return or throw. - if graph.edges_directed(basic_block_id, Direction::Outgoing).any(|e| { - matches!( - e.weight(), - EdgeType::Jump - | EdgeType::Normal - | EdgeType::Backedge - | EdgeType::Error(ErrorEdgeKind::Explicit) - ) - }) { - Control::Continue - } else if does_return { - Control::Prune - } else { - Control::Break(()) + // Return true as the second argument to signify we should + // continue walking this branch, as we haven't seen anything + // that will signify to us that this path of the program will + // definitely return or throw. + if graph.edges_directed(basic_block_id, Direction::Outgoing).any(|e| { + matches!( + e.weight(), + EdgeType::Jump + | EdgeType::Normal + | EdgeType::Backedge + | EdgeType::Error(ErrorEdgeKind::Explicit) + ) + }) { + Control::Continue + } else if does_return { + Control::Prune + } else { + Control::Break(()) + } } + _ => Control::Continue, } - _ => Control::Continue, - } - }); + }); output.break_value().is_none() }; diff --git a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs index e9fe723731ca0..c4b36cd854d43 100644 --- a/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs +++ b/crates/oxc_linter/src/rules/eslint/no_fallthrough.rs @@ -260,7 +260,7 @@ impl Rule for NoFallthrough { let AstKind::SwitchStatement(switch) = node.kind() else { return }; let cfg = ctx.cfg(); - let switch_id = node.cfg_id(); + let switch_id = ctx.nodes().cfg_id(node.id()); let graph = cfg.graph(); let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch); @@ -438,7 +438,7 @@ fn get_switch_semantic_cases( let graph = cfg.graph(); let has_default = switch.cases.iter().any(SwitchCase::is_default_case); let (mut cfg_ids, tests, exit) = graph - .edges_directed(node.cfg_id(), Direction::Outgoing) + .edges_directed(ctx.nodes().cfg_id(node.id()), Direction::Outgoing) .fold((Vec::new(), Vec::new(), None), |(mut cfg_ids, mut conds, exit), it| { let target = it.target(); if !matches!(it.weight(), EdgeType::Normal) { 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 index e34840a832cdc..71254b9c89e4c 100644 --- a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs +++ b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs @@ -75,7 +75,7 @@ impl Rule for NoThisBeforeSuper { } } AstKind::Super(_) => { - let basic_block_id = node.cfg_id(); + let basic_block_id = ctx.nodes().cfg_id(node.id()); if let AstKind::CallExpression(call_expr) = ctx.nodes().parent_kind(node.id()) { let has_this_or_super_in_args = Self::contains_this_or_super_in_args(&call_expr.arguments); @@ -92,7 +92,7 @@ impl Rule for NoThisBeforeSuper { } } AstKind::ThisExpression(_) => { - let basic_block_id = node.cfg_id(); + let basic_block_id = ctx.nodes().cfg_id(node.id()); if !basic_blocks_with_super_called.contains(&basic_block_id) { basic_blocks_with_local_violations .entry(basic_block_id) @@ -109,7 +109,7 @@ impl Rule for NoThisBeforeSuper { for node in wanted_nodes { let output = Self::analyze( cfg, - node.cfg_id(), + ctx.nodes().cfg_id(node.id()), &basic_blocks_with_super_called, &basic_blocks_with_local_violations, false, diff --git a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs index 56b871cd45759..75bb430ac2b83 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs @@ -68,12 +68,13 @@ impl Rule for NoUnreachable { let mut infinite_loops = Vec::new(); // Set the root as reachable. - unreachables[root.cfg_id().index()] = false; + let root_cfg_id = ctx.nodes().cfg_id(root.id()); + unreachables[root_cfg_id.index()] = false; // In our first path we first check if each block is definitely unreachable, If it is then // we set it as such, If we encounter an infinite loop we keep its end block since it can // prevent other reachable blocks from ever getting executed. - let _: Control<()> = depth_first_search(graph, Some(root.cfg_id()), |event| { + let _: Control<()> = depth_first_search(graph, Some(root_cfg_id), |event| { if let DfsEvent::Finish(node, _) = event { let unreachable = cfg.basic_block(node).is_unreachable(); unreachables[node.index()] = unreachable; @@ -168,7 +169,7 @@ impl Rule for NoUnreachable { continue; } - if unreachables[node.cfg_id().index()] { + if unreachables[ctx.nodes().cfg_id(node.id()).index()] { ctx.diagnostic(no_unreachable_diagnostic(node.kind().span())); } } diff --git a/crates/oxc_linter/src/rules/promise/always_return.rs b/crates/oxc_linter/src/rules/promise/always_return.rs index aba2cae9b8a40..a6ebb51c2addd 100644 --- a/crates/oxc_linter/src/rules/promise/always_return.rs +++ b/crates/oxc_linter/src/rules/promise/always_return.rs @@ -332,7 +332,7 @@ fn is_nodejs_terminal_statement(node: &AstNode) -> bool { fn has_no_return_code_path(node: &AstNode, ctx: &LintContext) -> bool { let cfg = ctx.cfg(); let graph = cfg.graph(); - let output = set_depth_first_search(graph, Some(node.cfg_id()), |event| { + let output = set_depth_first_search(graph, Some(ctx.nodes().cfg_id(node.id())), |event| { match event { // We only need to check paths that are normal or jump. DfsEvent::TreeEdge(a, b) => { diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index b4d60a00f62f6..ac64a679b269c 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -116,7 +116,7 @@ fn contains_return_statement(node: &AstNode, ctx: &LintContext) -> bool { let cfg = ctx.cfg(); let state = neighbors_filtered_by_edge_weight( cfg.graph(), - node.cfg_id(), + ctx.nodes().cfg_id(node.id()), &|edge| match edge { // We only care about normal edges having a return statement. EdgeType::Jump | EdgeType::Normal => None, 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 ff34d3a6176e0..6afda78180771 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -275,8 +275,8 @@ impl Rule for RulesOfHooks { return; } - let node_cfg_id = node.cfg_id(); - let func_cfg_id = parent_func.cfg_id(); + let node_cfg_id = ctx.nodes().cfg_id(node.id()); + let func_cfg_id = ctx.nodes().cfg_id(parent_func.id()); // there is no branch between us and our parent function if node_cfg_id == func_cfg_id { @@ -296,7 +296,7 @@ impl Rule for RulesOfHooks { return ctx.diagnostic(diagnostics::loop_hook(span, hook_name)); } - if has_conditional_path_accept_throw(cfg, parent_func, node) { + if has_conditional_path_accept_throw(ctx.nodes(), cfg, parent_func, node) { #[expect(clippy::needless_return)] return ctx.diagnostic(diagnostics::conditional_hook(span, hook_name)); } @@ -304,12 +304,13 @@ impl Rule for RulesOfHooks { } fn has_conditional_path_accept_throw( + nodes: &AstNodes<'_>, cfg: &ControlFlowGraph, from: &AstNode<'_>, to: &AstNode<'_>, ) -> bool { - let from_graph_id = from.cfg_id(); - let to_graph_id = to.cfg_id(); + let from_graph_id = nodes.cfg_id(from.id()); + let to_graph_id = nodes.cfg_id(to.id()); let graph = cfg.graph(); if graph .edges(to_graph_id) diff --git a/crates/oxc_semantic/examples/cfg.rs b/crates/oxc_semantic/examples/cfg.rs index 6d0842789c399..4f0fad597fbeb 100644 --- a/crates/oxc_semantic/examples/cfg.rs +++ b/crates/oxc_semantic/examples/cfg.rs @@ -96,7 +96,7 @@ fn main() -> std::io::Result<()> { let mut ast_nodes_by_block = FxHashMap::<_, Vec<_>>::default(); for node in semantic.semantic.nodes() { - let block = node.cfg_id(); + let block = semantic.semantic.nodes().cfg_id(node.id()); let block_ix = cfg.graph.node_weight(block).unwrap(); ast_nodes_by_block.entry(*block_ix).or_default().push(node); } diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs index cd6af68cdd531..816e43e35b895 100644 --- a/crates/oxc_semantic/src/node.rs +++ b/crates/oxc_semantic/src/node.rs @@ -22,10 +22,6 @@ pub struct AstNode<'a> { /// Associated Scope (initialized by binding) scope_id: ScopeId, - - /// Associated `BasicBlockId` in CFG (initialized by control_flow) - #[cfg(feature = "cfg")] - cfg_id: BlockNodeId, } impl<'a> AstNode<'a> { @@ -34,10 +30,10 @@ impl<'a> AstNode<'a> { pub(crate) fn new( kind: AstKind<'a>, scope_id: ScopeId, - cfg_id: BlockNodeId, + _cfg_id: BlockNodeId, id: NodeId, ) -> Self { - Self { id, kind, scope_id, cfg_id } + Self { id, kind, scope_id } } #[cfg(not(feature = "cfg"))] @@ -51,15 +47,6 @@ impl<'a> AstNode<'a> { self.id } - /// ID of the control flow graph node this node is in. - /// - /// See [oxc_cfg::ControlFlowGraph] for more information. - #[inline] - #[cfg(feature = "cfg")] - pub fn cfg_id(&self) -> BlockNodeId { - self.cfg_id - } - /// Access the underlying struct from [`oxc_ast`]. #[inline] pub fn kind(&self) -> AstKind<'a> { @@ -99,6 +86,9 @@ pub struct AstNodes<'a> { parent_ids: IndexVec, /// `node` -> `flags` flags: IndexVec, + /// `node` -> `cfg_id` (control flow graph node) + #[cfg(feature = "cfg")] + cfg_ids: IndexVec, /// Stores a set of bits of a fixed size, where each bit represents a single [`AstKind`]. If the bit is set (1), /// then the AST contains at least one node of that kind. If the bit is not set (0), then the AST does not contain /// any nodes of that kind. @@ -197,6 +187,15 @@ impl<'a> AstNodes<'a> { &mut self.flags[node_id] } + /// ID of the control flow graph node this node is in. + /// + /// See [oxc_cfg::ControlFlowGraph] for more information. + #[inline] + #[cfg(feature = "cfg")] + pub fn cfg_id(&self, node_id: NodeId) -> BlockNodeId { + self.cfg_ids[node_id] + } + /// Get the [`Program`] that's also the root of the AST. #[inline] pub fn program(&self) -> &'a Program<'a> { @@ -228,6 +227,7 @@ impl<'a> AstNodes<'a> { let node = AstNode::new(kind, scope_id, cfg_id, node_id); self.nodes.push(node); self.flags.push(flags); + self.cfg_ids.push(cfg_id); self.node_kinds_set.set(kind.ty()); node_id } @@ -271,6 +271,7 @@ impl<'a> AstNodes<'a> { self.parent_ids.push(NodeId::ROOT); self.nodes.push(AstNode::new(kind, scope_id, cfg_id, NodeId::ROOT)); self.flags.push(flags); + self.cfg_ids.push(cfg_id); self.node_kinds_set.set(AstType::Program); NodeId::ROOT } @@ -300,6 +301,8 @@ impl<'a> AstNodes<'a> { self.nodes.reserve(additional); self.parent_ids.reserve(additional); self.flags.reserve(additional); + #[cfg(feature = "cfg")] + self.cfg_ids.reserve(additional); } /// Checks if the AST contains any nodes of the given types.