diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index fde7463af5d6..d598fb48e115 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -1690,6 +1690,10 @@ impl<'a> VariableDeclaration<'a> { pub fn is_typescript_syntax(&self) -> bool { self.modifiers.contains(ModifierKind::Declare) } + + pub fn has_init(&self) -> bool { + self.declarations.iter().any(|decl| decl.init.is_some()) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index b4bb890ec810..597a841edc1e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -95,6 +95,7 @@ mod eslint { pub mod no_ternary; pub mod no_this_before_super; pub mod no_undef; + pub mod no_unreachable; pub mod no_unsafe_finally; pub mod no_unsafe_negation; pub mod no_unsafe_optional_chaining; @@ -483,6 +484,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_shadow_restricted_names, eslint::no_sparse_arrays, eslint::no_undef, + eslint::no_unreachable, eslint::no_unsafe_finally, eslint::no_unsafe_negation, eslint::no_unsafe_optional_chaining, diff --git a/crates/oxc_linter/src/rules/eslint/no_unreachable.rs b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs new file mode 100644 index 000000000000..bb4cce11a94a --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_unreachable.rs @@ -0,0 +1,307 @@ +use oxc_ast::{ast::VariableDeclarationKind, AstKind}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::{ + petgraph::{ + visit::{depth_first_search, Control, DfsEvent, EdgeRef}, + Direction, + }, + EdgeType, InstructionKind, +}; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule}; + +fn no_unreachable_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::error("eslint(no-unreachable): Unreachable code.").with_labels([span.into()]) +} + +/// +#[derive(Debug, Default, Clone)] +pub struct NoUnreachable; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow unreachable code after `return`, `throw`, `continue`, and `break` statements + /// + NoUnreachable, + correctness +); + +impl Rule for NoUnreachable { + fn run_once(&self, ctx: &LintContext) { + let nodes = ctx.nodes(); + let Some(root) = nodes.root_node() else { return }; + let cfg = ctx.semantic().cfg(); + 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 + // node in our paths then it should be unreachable by definition. + let mut unreachables = vec![true; cfg.basic_blocks.len()]; + + // All of the end points of infinite loops we encountered. + let mut infinite_loops = Vec::new(); + + // Set the root as reachable. + 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| match event { + DfsEvent::Finish(node, _) => { + let bb = cfg.basic_block(node); + let unreachable = if bb.unreachable { + true + } else { + graph + .edges_directed(node, Direction::Incoming) + .any(|edge| matches!(edge.weight(), EdgeType::Join)) + }; + unreachables[node.index()] = unreachable; + + if let Some(it) = cfg.is_infinite_loop_start(node, nodes) { + infinite_loops.push(it); + } + + Control::Continue + } + _ => Control::Continue, + }); + + // In the second path we go for each infinite loop end block and follow it marking all + // edges as unreachable unless they have a reachable jump (eg. break). + for loop_ in infinite_loops { + // A loop end block usually is also its condition and start point but what is common + // in all cases is that it may have `Jump` or `Backedge` edges so we only want to + // follow the `Normal` edges as these are the exiting edges. + let starts: Vec<_> = graph + .edges_directed(loop_.1, Direction::Outgoing) + .filter(|it| matches!(it.weight(), EdgeType::Normal)) + .map(|it| it.target()) + .collect(); + + // Search with all `Normal` edges as starting point(s). + let _: Control<()> = depth_first_search(graph, starts, |event| match event { + DfsEvent::Discover(node, _) => { + let mut incoming = graph.edges_directed(node, Direction::Incoming); + if incoming.any(|e| match e.weight() { + // `NewFunction` is always reachable + | EdgeType::NewFunction + // `Finalize` can be reachable if we encounter an error in the loop. + | EdgeType::Finalize => true, + + // If we have an incoming `Jump` and it is from a `Break` instruction, + // We know with high confidence that we are visiting a reachable block. + // NOTE: May cause false negatives but I couldn't think of one. + EdgeType::Jump + if cfg + .basic_block(e.source()) + .instructions() + .iter() + .any(|it| matches!(it.kind, InstructionKind::Break(_))) => + { + true + } + _ => false, + }) { + // We prune this branch if it is reachable from this point forward. + Control::Prune + } else { + // Otherwise we set it to unreachable and continue. + unreachables[node.index()] = true; + Control::Continue + } + } + _ => Control::Continue, + }); + } + for node in ctx.nodes().iter() { + // exit early if we are not visiting a statement. + if !node.kind().is_statement() { + continue; + } + + // exit early if it is an empty statement. + if matches!(node.kind(), AstKind::EmptyStatement(_)) { + continue; + } + + if matches! { + node.kind(), + AstKind::VariableDeclaration(decl) + if matches!(decl.kind, VariableDeclarationKind::Var) && !decl.has_init() + } { + // Skip `var` declarations without any initialization, + // These work because of the JavaScript hoisting rules. + continue; + } + + if unreachables[node.cfg_id().index()] { + ctx.diagnostic(no_unreachable_diagnostic(node.kind().span())); + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "function foo() { function bar() { return 1; } return bar(); }", + "function foo() { return bar(); function bar() { return 1; } }", + "function foo() { return x; var x; }", + "function foo() { var x = 1; var y = 2; }", + "function foo() { var x = 1; var y = 2; return; }", + "while (true) { switch (foo) { case 1: x = 1; x = 2;} }", + "while (true) { break; var x; }", + "while (true) { continue; var x, y; }", + "while (true) { throw 'message'; var x; }", + "while (true) { if (true) break; var x = 1; }", + "while (true) continue;", + "switch (foo) { case 1: break; var x; }", + "switch (foo) { case 1: break; var x; default: throw true; };", + "const arrow_direction = arrow => { switch (arrow) { default: throw new Error(); };}", + "var x = 1; y = 2; throw 'uh oh'; var y;", + "function foo() { var x = 1; if (x) { return; } x = 2; }", + "function foo() { var x = 1; if (x) { } else { return; } x = 2; }", + "function foo() { var x = 1; switch (x) { case 0: break; default: return; } x = 2; }", + "function foo() { var x = 1; while (x) { return; } x = 2; }", + "function foo() { var x = 1; for (x in {}) { return; } x = 2; }", + "function foo() { var x = 1; try { return; } finally { x = 2; } }", + "function foo() { var x = 1; for (;;) { if (x) break; } x = 2; }", + "A: { break A; } foo()", + "function* foo() { try { yield 1; return; } catch (err) { return err; } }", + "function foo() { try { bar(); return; } catch (err) { return err; } }", + "function foo() { try { a.b.c = 1; return; } catch (err) { return err; } }", + "class C { foo = reachable; }", + "class C { foo = reachable; constructor() {} }", + "class C extends B { foo = reachable; }", + "class C extends B { foo = reachable; constructor() { super(); } }", + "class C extends B { static foo = reachable; constructor() {} }", + "function foo() { var x = 1; for (;x == 1;) { if (x) continue; } x = 2; }", + " + if (a) { + a(); + } else { + for (let i = 1; i <= 10; i++) { + b(); + } + + for (let i = 1; i <= 10; i++) { + c(); + } + } + ", + " + try { + throw 'error'; + } catch (err) { + b(); + } + c(); + ", + " + export const getPagePreviewText = (page) => { + if (!a) { + return ''; + } + while (a && b > c && d-- > 0) { + } + }; + ", + " + try { + for (const a of b) { + c(); + } + + while (true) { + d(); + } + } finally { + } + ", + " + switch (authType) { + case 1: + return a(); + case 2: + return b(); + case 3: + return c(); + } + d(); + ", + " + try { + a(); + } catch (e) { + b(); + } finally { + c(); + } + d(); + ", + " + try { + while (true) { + a(); + } + } finally { + b(); + } + ", + ]; + + let fail = vec![ + //[{ messageId: "unreachableCode", type: "VariableDeclaration" }] + "function foo() { return x; var x = 1; }", + //[{ messageId: "unreachableCode", type: "VariableDeclaration" }] + "function foo() { return x; var x, y = 1; }", + "while (true) { break; var x = 1; }", + //[{ messageId: "unreachableCode", type: "VariableDeclaration" }] + "while (true) { continue; var x = 1; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { return; x = 1; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { throw error; x = 1; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "while (true) { break; x = 1; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "while (true) { continue; x = 1; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { switch (foo) { case 1: return; x = 1; } }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { switch (foo) { case 1: throw e; x = 1; } }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "while (true) { switch (foo) { case 1: break; x = 1; } }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "while (true) { switch (foo) { case 1: continue; x = 1; } }", + //[{ messageId: "unreachableCode", type: "VariableDeclaration" }] + "var x = 1; throw 'uh oh'; var y = 2;", + // [{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; if (x) { return; } else { throw e; } x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; if (x) return; else throw -1; x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; try { return; } finally {} x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; try { } finally { return; } x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; do { return; } while (x); x = 2; }", + // [{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; while (x) { if (x) break; else continue; x = 2; } }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; for (;;) { if (x) continue; } x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; while (true) { } x = 2; }", + //[{ messageId: "unreachableCode", type: "ExpressionStatement" }] + "function foo() { var x = 1; do { } while (true); x = 2; }", + ]; + + Tester::new(NoUnreachable::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_unreachable.snap b/crates/oxc_linter/src/snapshots/no_unreachable.snap new file mode 100644 index 000000000000..d060e0edc817 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_unreachable.snap @@ -0,0 +1,135 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_unreachable +--- + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:28] + 1 │ function foo() { return x; var x = 1; } + · ────────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:28] + 1 │ function foo() { return x; var x, y = 1; } + · ───────────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:23] + 1 │ while (true) { break; var x = 1; } + · ────────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:26] + 1 │ while (true) { continue; var x = 1; } + · ────────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:26] + 1 │ function foo() { return; x = 1; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:31] + 1 │ function foo() { throw error; x = 1; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:23] + 1 │ while (true) { break; x = 1; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:26] + 1 │ while (true) { continue; x = 1; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:49] + 1 │ function foo() { switch (foo) { case 1: return; x = 1; } } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:50] + 1 │ function foo() { switch (foo) { case 1: throw e; x = 1; } } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:46] + 1 │ while (true) { switch (foo) { case 1: break; x = 1; } } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:49] + 1 │ while (true) { switch (foo) { case 1: continue; x = 1; } } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:27] + 1 │ var x = 1; throw 'uh oh'; var y = 2; + · ────────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:66] + 1 │ function foo() { var x = 1; if (x) { return; } else { throw e; } x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:59] + 1 │ function foo() { var x = 1; if (x) return; else throw -1; x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:56] + 1 │ function foo() { var x = 1; try { return; } finally {} x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:57] + 1 │ function foo() { var x = 1; try { } finally { return; } x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:55] + 1 │ function foo() { var x = 1; do { return; } while (x); x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:70] + 1 │ function foo() { var x = 1; while (x) { if (x) break; else continue; x = 2; } } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:59] + 1 │ function foo() { var x = 1; for (;;) { if (x) continue; } x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:46] + 1 │ function foo() { var x = 1; while (true) { } x = 2; } + · ────── + ╰──── + + ⚠ eslint(no-unreachable): Unreachable code. + ╭─[no_unreachable.tsx:1:50] + 1 │ function foo() { var x = 1; do { } while (true); x = 2; } + · ────── + ╰──── diff --git a/crates/oxc_semantic/src/control_flow/mod.rs b/crates/oxc_semantic/src/control_flow/mod.rs index 8132bd0f8764..80233ac26c3c 100644 --- a/crates/oxc_semantic/src/control_flow/mod.rs +++ b/crates/oxc_semantic/src/control_flow/mod.rs @@ -1,17 +1,19 @@ mod builder; mod dot; +use itertools::Itertools; +use oxc_ast::AstKind; use oxc_span::CompactStr; use oxc_syntax::operator::{ AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator, UpdateOperator, }; use petgraph::{ stable_graph::NodeIndex, - visit::{depth_first_search, Control, DfsEvent}, - Graph, + visit::{depth_first_search, Control, DfsEvent, EdgeRef}, + Direction, Graph, }; -use crate::AstNodeId; +use crate::{AstNodeId, AstNodes}; pub use builder::{ControlFlowGraphBuilder, CtxCursor, CtxFlags}; pub use dot::{DebugDot, DebugDotContext, DisplayDot}; @@ -240,8 +242,8 @@ impl ControlFlowGraph { 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) + let unreachable = !graph.edges_connecting(a, b).any(|edge| { + !matches!(edge.weight(), EdgeType::NewFunction | EdgeType::Unreachable) }); if unreachable { @@ -258,6 +260,91 @@ impl ControlFlowGraph { .unwrap_or(false) } + /// Returns `None` the given node isn't the cyclic point of an infinite loop. + /// Otherwise returns `Some(loop_start, loop_end)`. + pub fn is_infinite_loop_start( + &self, + node: BasicBlockId, + nodes: &AstNodes, + ) -> Option<(BasicBlockId, BasicBlockId)> { + enum EvalConstConditionResult { + NotFound, + Fail, + Eval(bool), + } + fn try_eval_const_condition( + instruction: &Instruction, + nodes: &AstNodes, + ) -> EvalConstConditionResult { + use EvalConstConditionResult::{Eval, Fail, NotFound}; + match instruction { + Instruction { kind: InstructionKind::Condition, node_id: Some(id) } => { + match nodes.kind(*id) { + AstKind::BooleanLiteral(lit) => Eval(lit.value), + _ => Fail, + } + } + _ => NotFound, + } + } + + fn get_jump_target( + graph: &Graph, + node: BasicBlockId, + ) -> Option { + graph + .edges_directed(node, Direction::Outgoing) + .find_or_first(|e| matches!(e.weight(), EdgeType::Jump)) + .map(|it| it.target()) + } + + let basic_block = self.basic_block(node); + let mut backedges = self + .graph + .edges_directed(node, Direction::Incoming) + .filter(|e| matches!(e.weight(), EdgeType::Backedge)); + + // if this node doesn't have an backedge it isn't a loop starting point. + let backedge = backedges.next()?; + + debug_assert!( + backedges.next().is_none(), + "there should only be one backedge to each basic block." + ); + + // if instructions are empty we might be in a `for(;;)`. + if basic_block.instructions().is_empty() + && !self + .graph + .edges_directed(node, Direction::Outgoing) + .any(|e| matches!(e.weight(), EdgeType::Backedge)) + { + return get_jump_target(&self.graph, node).map(|it| (it, node)); + } + + // if there are more than one instruction in this block it can't be a valid loop start. + let Ok(only_instruction) = basic_block.instructions().iter().exactly_one() else { + return None; + }; + + // if there is exactly one and it is a condition instruction we are in a loop so we + // check the condition to infer if it is always true. + if let EvalConstConditionResult::Eval(true) = + try_eval_const_condition(only_instruction, nodes) + { + get_jump_target(&self.graph, node).map(|it| (it, node)) + } else if let EvalConstConditionResult::Eval(true) = + self.basic_block(backedge.source()).instructions().iter().exactly_one().map_or_else( + |_| EvalConstConditionResult::NotFound, + |it| try_eval_const_condition(it, nodes), + ) + { + get_jump_target(&self.graph, node).map(|it| (node, it)) + } else { + None + } + } + pub fn is_cyclic(&self, node: BasicBlockId) -> bool { depth_first_search(&self.graph, Some(node), |event| match event { DfsEvent::BackEdge(_, id) if id == node => Err(()), diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs index ae73a53c4676..2e1a8d20518e 100644 --- a/crates/oxc_semantic/src/node.rs +++ b/crates/oxc_semantic/src/node.rs @@ -69,6 +69,14 @@ impl<'a> AstNodes<'a> { self.nodes.iter() } + pub fn len(&self) -> usize { + self.nodes.len() + } + + pub fn is_empty(&self) -> bool { + self.nodes.len() == 0 + } + /// Walk up the AST, iterating over each parent node. /// /// The first node produced by this iterator is the first parent of the node