From fde1dc7b5cc73812a13a68c6204b68f1e5b24ea4 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:09:00 +0000 Subject: [PATCH] refactor(semantic): simplify checking if function is part of `if` statement (#18290) `is_function_part_of_if_statement` had some convoluted code to check if a `Function` is the `consequent` or `alternate` of an `IfStatement`. Simplify it by checking if the `Function` is a function declaration, then just a check for whether the parent node is an `IfStatement`. Rationale is: * If a `Function` is a declaration, it can only be the `consequent` or `alternate` of an `IfStatement`. It can't be the `test`, because that's an expression not a declaration. So if a function declaration's parent is an `IfStatement`, it's definitely either the `consequent` or `alternate`. * If a `Function` is an expression, it's parent can't be `consequent` or `alternate` of an `IfStatement` because the function would need to be wrapped in some other node e.g. `ExpressionStatement` (and then `IfStatement` wouldn't be the function's parent). Also add a comment explaining why we do the check for strict/sloppy mode too. --- crates/oxc_semantic/src/checker/javascript.rs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/crates/oxc_semantic/src/checker/javascript.rs b/crates/oxc_semantic/src/checker/javascript.rs index 8e313d373f94f..ac9cc09d4b2a1 100644 --- a/crates/oxc_semantic/src/checker/javascript.rs +++ b/crates/oxc_semantic/src/checker/javascript.rs @@ -1,5 +1,3 @@ -use std::ptr; - use memchr::memchr_iter; use rustc_hash::FxHashMap; @@ -605,23 +603,21 @@ pub fn check_variable_declarator_redeclaration( /// Check for Annex B `if (foo) function a() {} else function b() {}` pub fn is_function_part_of_if_statement(function: &Function, builder: &SemanticBuilder) -> bool { - if builder.current_scope_flags().is_strict_mode() { + if !function.is_declaration() { return false; } - let AstKind::IfStatement(stmt) = builder.nodes.parent_kind(builder.current_node_id) else { + + // Function declarations cannot be `consequent` or `alternate` of an `IfStatement` in strict mode. + // This check is redundant - parent kind lookup below will always return `false` in strict mode. + // But this check is cheaper, and strict mode code is more common than sloppy mode, so we do this cheap check first. + if builder.current_scope_flags().is_strict_mode() { return false; - }; - if let Statement::FunctionDeclaration(func) = &stmt.consequent - && ptr::eq(func.as_ref(), function) - { - return true; - } - if let Some(Statement::FunctionDeclaration(func)) = &stmt.alternate - && ptr::eq(func.as_ref(), function) - { - return true; } - false + + // A function declaration whose parent is an `IfStatement` can only be + // either that `IfStatement`'s `consequent` or `alternate` + // (can't be `test` because that's an expression) + matches!(builder.nodes.parent_kind(builder.current_node_id), AstKind::IfStatement(_)) } // It is a Syntax Error if the LexicallyDeclaredNames of StatementList contains any duplicate entries,