diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_fallthrough_switch_clause.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_fallthrough_switch_clause.rs index 6ee17701ec6..7ebe8807ef5 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_fallthrough_switch_clause.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_fallthrough_switch_clause.rs @@ -1,10 +1,12 @@ -use rome_rowan::{AstNode, AstNodeList}; +use rome_control_flow::{ExceptionHandlerKind, InstructionKind}; +use rome_rowan::{AstNode, AstNodeList, NodeOrToken}; -use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic}; +use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic}; use rome_console::markup; -use rome_js_syntax::{ - AnyJsStatement, AnyJsSwitchClause, JsBlockStatement, JsStatementList, JsSwitchStatement, -}; +use rome_js_syntax::{AnyJsSwitchClause, JsLanguage, JsSwitchStatement}; +use rustc_hash::FxHashSet; + +use crate::ControlFlowGraph; declare_rule! { /// Disallow fallthrough of `switch` clauses. @@ -47,29 +49,93 @@ declare_rule! { } impl Rule for NoFallthroughSwitchClause { - type Query = Ast; + type Query = ControlFlowGraph; type State = AnyJsSwitchClause; type Signals = Vec; type Options = (); fn run(ctx: &RuleContext) -> Self::Signals { - let query = ctx.query(); - let mut signals: Self::Signals = Vec::new(); - let mut cases = query.cases().into_iter().peekable(); - - while let Some(any_case) = cases.next() { - let is_last = cases.peek().is_none(); - - if is_last { + let cfg = ctx.query(); + let mut fallthrough: Vec = Vec::new(); + // block to process. + let mut block_stack = Vec::new(); + let mut visited_blocks = FxHashSet::default(); + block_stack.push(0u32); + visited_blocks.insert(0u32); + // Traverse the control flow graph and search for switch statements. + while let Some(block_index) = block_stack.pop() { + // SAFETY: this is a safe conversion because it is already an index for `cfg.blocks`. + let block_index = block_index as usize; + let Some(block) = cfg.blocks.get(block_index) else { continue; + }; + // Register exception handlers as blocks to process + // Ignore finally handler: they are already in the Control Flow Graph. + for exception_handler in block + .exception_handlers + .iter() + .filter(|x| matches!(x.kind, ExceptionHandlerKind::Catch)) + { + if visited_blocks.insert(exception_handler.target) { + block_stack.push(exception_handler.target); + } } - - if case_fell(&any_case) { - signals.push(any_case); + // Traverse the instructions of the block searching for a switch statement. + let mut is_switch = false; + let mut has_default_clause = false; + let mut switch_clause_blocks = FxHashSet::default(); + for instruction in block.instructions.iter() { + match instruction.kind { + InstructionKind::Statement => { + if let Some(node) = &instruction.node { + if let Some(switch_stmt) = + node.parent().and_then(JsSwitchStatement::cast) + { + if is_switch { + unreachable!("A block cannot contain two switch statements.") + } + is_switch = true; + has_default_clause = + switch_stmt.cases().iter().any(|switch_clause| { + switch_clause.as_js_default_clause().is_some() + }); + } + } + } + InstructionKind::Jump { + conditional, block, .. + } => { + let jump_block_index = block.index(); + // Avoid cycles and redundant checks. + if visited_blocks.insert(jump_block_index) { + block_stack.push(jump_block_index); + } + // If the last statement is the discriminant of a switch statements, + // then all succeeding jumps are jumps to switch clauses. + // The unconditional jump jumps to the default switch clause if it exists. + if is_switch && (conditional || has_default_clause) { + // Take the unconditional jump into account only if a default clause is present. + switch_clause_blocks.insert(jump_block_index); + } + if !conditional { + // The next instructions are unreachable. + break; + } + } + InstructionKind::Return => { + // The next instructions are unreachable. + break; + } + } + } + if !switch_clause_blocks.is_empty() { + // Analyze the found switch statement to detect any fallthrough. + let mut new_fallthrough = + get_switch_clause_fallthrough(&switch_clause_blocks, &visited_blocks, cfg); + fallthrough.append(&mut new_fallthrough); } } - - signals + fallthrough } fn diagnostic(_: &RuleContext, reference: &Self::State) -> Option { @@ -88,31 +154,72 @@ impl Rule for NoFallthroughSwitchClause { } } -fn case_fell(case: &AnyJsSwitchClause) -> bool { - let statements = case.consequent(); - !has_fall_blocker_statement(&statements) && statements.iter().count() != 0 -} - -fn has_fall_blocker_statement(statements: &JsStatementList) -> bool { - for statement in statements.iter() { - if is_fall_blocker_statement(&statement) { - return true; +fn get_switch_clause_fallthrough( + switch_clause_blocks: &FxHashSet, + visited_blocks: &FxHashSet, + cfg: &rome_control_flow::ControlFlowGraph, +) -> Vec { + // Register all switch clauses as block to process. + let mut block_stack: Vec = switch_clause_blocks.iter().copied().collect(); + let mut visited_blocks = visited_blocks.clone(); + let mut fallthrough = Vec::new(); + // Traverse the control flow graph + while let Some(block_index) = block_stack.pop() { + // SAFETY: this is a safe conversion because it is already an index for `cfg.blocks`. + let block_index = block_index as usize; + let Some(block) = cfg.blocks.get(block_index) else { + continue; + }; + // Register exception handlers as blocks to process + // Ignore finally handler: they are already in the Control Flow Graph. + for exception_handler in block + .exception_handlers + .iter() + .filter(|x| matches!(x.kind, ExceptionHandlerKind::Catch)) + { + if visited_blocks.insert(exception_handler.target) { + block_stack.push(exception_handler.target); + } } - if let Some(block_statement) = JsBlockStatement::cast_ref(statement.syntax()) { - if has_fall_blocker_statement(&block_statement.statements()) { - return true; + let mut first_statement = None; + for instruction in block.instructions.iter() { + match instruction.kind { + InstructionKind::Statement => { + if first_statement.is_none() { + if let Some(NodeOrToken::Node(node)) = &instruction.node { + first_statement = Some(node); + } + } + } + InstructionKind::Jump { + conditional, block, .. + } => { + let jump_block_index = block.index(); + // Avoid cycles and redundant checks. + if visited_blocks.insert(jump_block_index) { + block_stack.push(jump_block_index); + } + if !conditional { + if switch_clause_blocks.contains(&jump_block_index) { + if let Some(last_statement) = first_statement { + // This is a fallthrough + if let Some(switch_clause) = + last_statement.ancestors().find_map(AnyJsSwitchClause::cast) + { + fallthrough.push(switch_clause); + } + } + } + // The next instructions are unreachable. + break; + } + } + InstructionKind::Return => { + // The next instructions are unreachable. + break; + } } } } - false -} - -fn is_fall_blocker_statement(statement: &AnyJsStatement) -> bool { - matches!( - statement, - AnyJsStatement::JsBreakStatement(_) - | AnyJsStatement::JsReturnStatement(_) - | AnyJsStatement::JsThrowStatement(_) - | AnyJsStatement::JsContinueStatement(_) - ) + fallthrough }