compiler/crates/react_compiler_validation/src/validate_locals_not_reassigned_after_render.rs RUST 282 lines View on github.com → Search inside
1/*2 * Copyright (c) Meta Platforms, Inc. and affiliates.3 *4 * This source code is licensed under the MIT license found in the5 * LICENSE file in the root directory of this source tree.6 */78use rustc_hash::{FxHashMap, FxHashSet};910use react_compiler_diagnostics::{CompilerDiagnostic, CompilerDiagnosticDetail, ErrorCategory};11use react_compiler_hir::environment::Environment;12use react_compiler_hir::visitors::{13    each_instruction_lvalue_ids, each_instruction_value_operand, each_terminal_operand,14};15use react_compiler_hir::{16    Effect, HirFunction, Identifier, IdentifierId, IdentifierName, InstructionValue, Place, Type,17};1819/// Validates that local variables cannot be reassigned after render.20/// This prevents a category of bugs in which a closure captures a21/// binding from one render but does not update.22pub fn validate_locals_not_reassigned_after_render(func: &HirFunction, env: &mut Environment) {23    let mut context_variables: FxHashSet<IdentifierId> = FxHashSet::default();24    let mut diagnostics: Vec<CompilerDiagnostic> = Vec::new();2526    let reassignment = get_context_reassignment(27        func,28        &env.identifiers,29        &env.types,30        &env.functions,31        env,32        &mut context_variables,33        false,34        false,35        &mut diagnostics,36    );3738    // Record accumulated errors (from async function checks in inner functions) first39    for diagnostic in diagnostics {40        env.record_diagnostic(diagnostic);41    }4243    // Then record the top-level reassignment error if any44    if let Some(reassignment_place) = reassignment {45        let variable_name = format_variable_name(&reassignment_place, &env.identifiers);46        env.record_diagnostic(47            CompilerDiagnostic::new(48                ErrorCategory::Immutability,49                "Cannot reassign variable after render completes",50                Some(format!(51                    "Reassigning {} after render has completed can cause inconsistent \52                     behavior on subsequent renders. Consider using state instead",53                    variable_name54                )),55            )56            .with_detail(CompilerDiagnosticDetail::Error {57                loc: reassignment_place.loc,58                message: Some(format!(59                    "Cannot reassign {} after render completes",60                    variable_name61                )),62                identifier_name: None,63            }),64        );65    }66}6768/// Format a variable name for error messages. Uses the named identifier if69/// available, otherwise falls back to "variable".70fn format_variable_name(place: &Place, identifiers: &[Identifier]) -> String {71    let identifier = &identifiers[place.identifier.0 as usize];72    match &identifier.name {73        Some(IdentifierName::Named(name)) => format!("`{}`", name),74        _ => "variable".to_string(),75    }76}7778/// Recursively checks whether a function (or its dependencies) reassigns a79/// context variable. Returns the reassigned place if found, or None.80///81/// Side effects: accumulates async-function reassignment diagnostics into `diagnostics`.82fn get_context_reassignment(83    func: &HirFunction,84    identifiers: &[Identifier],85    types: &[Type],86    functions: &[HirFunction],87    env: &Environment,88    context_variables: &mut FxHashSet<IdentifierId>,89    is_function_expression: bool,90    is_async: bool,91    diagnostics: &mut Vec<CompilerDiagnostic>,92) -> Option<Place> {93    // Maps identifiers to the place that they reassign94    let mut reassigning_functions: FxHashMap<IdentifierId, Place> = FxHashMap::default();9596    for (_block_id, block) in &func.body.blocks {97        for &instruction_id in &block.instructions {98            let instr = &func.instructions[instruction_id.0 as usize];99100            match &instr.value {101                InstructionValue::FunctionExpression { lowered_func, .. }102                | InstructionValue::ObjectMethod { lowered_func, .. } => {103                    let inner_function = &functions[lowered_func.func.0 as usize];104                    let inner_is_async = is_async || inner_function.is_async;105106                    // Recursively check the inner function107                    let mut reassignment = get_context_reassignment(108                        inner_function,109                        identifiers,110                        types,111                        functions,112                        env,113                        context_variables,114                        true,115                        inner_is_async,116                        diagnostics,117                    );118119                    // If the function itself doesn't reassign, check if one of its120                    // dependencies (operands) is a reassigning function121                    if reassignment.is_none() {122                        for context_place in &inner_function.context {123                            if let Some(reassignment_place) =124                                reassigning_functions.get(&context_place.identifier)125                            {126                                reassignment = Some(reassignment_place.clone());127                                break;128                            }129                        }130                    }131132                    // If the function or its dependencies reassign, handle it133                    if let Some(ref reassignment_place) = reassignment {134                        if inner_is_async {135                            // Async functions that reassign get an immediate error136                            let variable_name =137                                format_variable_name(reassignment_place, identifiers);138                            diagnostics.push(139                                CompilerDiagnostic::new(140                                    ErrorCategory::Immutability,141                                    "Cannot reassign variable in async function",142                                    Some(143                                        "Reassigning a variable in an async function can cause \144                                         inconsistent behavior on subsequent renders. \145                                         Consider using state instead"146                                            .to_string(),147                                    ),148                                )149                                .with_detail(150                                    CompilerDiagnosticDetail::Error {151                                        loc: reassignment_place.loc,152                                        message: Some(format!("Cannot reassign {}", variable_name)),153                                        identifier_name: None,154                                    },155                                ),156                            );157                            // Return null (don't propagate further) — matches TS behavior158                            return None;159                        } else {160                            // Propagate reassignment info on the lvalue161                            reassigning_functions162                                .insert(instr.lvalue.identifier, reassignment_place.clone());163                        }164                    }165                }166167                InstructionValue::StoreLocal { lvalue, value, .. } => {168                    if let Some(reassignment_place) = reassigning_functions.get(&value.identifier) {169                        let reassignment_place = reassignment_place.clone();170                        reassigning_functions171                            .insert(lvalue.place.identifier, reassignment_place.clone());172                        reassigning_functions.insert(instr.lvalue.identifier, reassignment_place);173                    }174                }175176                InstructionValue::LoadLocal { place, .. } => {177                    if let Some(reassignment_place) = reassigning_functions.get(&place.identifier) {178                        reassigning_functions179                            .insert(instr.lvalue.identifier, reassignment_place.clone());180                    }181                }182183                InstructionValue::DeclareContext { lvalue, .. } => {184                    if !is_function_expression {185                        context_variables.insert(lvalue.place.identifier);186                    }187                }188189                InstructionValue::StoreContext { lvalue, value, .. } => {190                    // If we're inside a function expression and the target is a191                    // context variable from the outer scope, this is a reassignment192                    if is_function_expression193                        && context_variables.contains(&lvalue.place.identifier)194                    {195                        return Some(lvalue.place.clone());196                    }197198                    // In the outer function, track context variables199                    if !is_function_expression {200                        context_variables.insert(lvalue.place.identifier);201                    }202203                    // Propagate reassigning function info through StoreContext204                    if let Some(reassignment_place) = reassigning_functions.get(&value.identifier) {205                        let reassignment_place = reassignment_place.clone();206                        reassigning_functions207                            .insert(lvalue.place.identifier, reassignment_place.clone());208                        reassigning_functions.insert(instr.lvalue.identifier, reassignment_place);209                    }210                }211212                _ => {213                    // For calls with noAlias signatures, only check the callee/receiver214                    // (not args) to avoid false positives from callbacks that reassign215                    // context variables.216                    let operands: Vec<Place> = match &instr.value {217                        InstructionValue::CallExpression { callee, .. } => {218                            if env.has_no_alias_signature(callee.identifier) {219                                vec![callee.clone()]220                            } else {221                                each_instruction_value_operand(&instr.value, env)222                            }223                        }224                        InstructionValue::MethodCall {225                            receiver, property, ..226                        } => {227                            if env.has_no_alias_signature(property.identifier) {228                                vec![receiver.clone(), property.clone()]229                            } else {230                                each_instruction_value_operand(&instr.value, env)231                            }232                        }233                        InstructionValue::TaggedTemplateExpression { tag, .. } => {234                            if env.has_no_alias_signature(tag.identifier) {235                                vec![tag.clone()]236                            } else {237                                each_instruction_value_operand(&instr.value, env)238                            }239                        }240                        _ => each_instruction_value_operand(&instr.value, env),241                    };242243                    for operand in &operands {244                        // Invariant: effects must be inferred before this pass runs245                        assert!(246                            operand.effect != Effect::Unknown,247                            "Expected effects to be inferred prior to \248                             ValidateLocalsNotReassignedAfterRender"249                        );250251                        if let Some(reassignment_place) =252                            reassigning_functions.get(&operand.identifier).cloned()253                        {254                            if operand.effect == Effect::Freeze {255                                // Functions that reassign local variables are inherently256                                // mutable and unsafe to pass where a frozen value is expected.257                                return Some(reassignment_place);258                            } else {259                                // If the operand is not frozen but does reassign, then the260                                // lvalues of the instruction could also be reassigning261                                for lvalue_id in each_instruction_lvalue_ids(instr) {262                                    reassigning_functions263                                        .insert(lvalue_id, reassignment_place.clone());264                                }265                            }266                        }267                    }268                }269            }270        }271272        // Check terminal operands for reassigning functions273        for operand in each_terminal_operand(&block.terminal) {274            if let Some(reassignment_place) = reassigning_functions.get(&operand.identifier) {275                return Some(reassignment_place.clone());276            }277        }278    }279280    None281}

Code quality findings 9

Critical: Use of 'unsafe' keyword bypasses Rust's safety guarantees. Requires careful auditing, clear justification (FFI, specific optimizations), and minimal scope.
error safety unsafe-block
// mutable and unsafe to pass where a frozen value is expected.
Warning: Direct indexing (e.g., `vec[i]`, `slice[i]`) panics on out-of-bounds access. Prefer using `.get(index)` or `.get_mut(index)` which return Option<&T>/Option<&mut T>.
warning correctness unchecked-indexing
let identifier = &identifiers[place.identifier.0 as usize];
Warning: Direct indexing (e.g., `vec[i]`, `slice[i]`) panics on out-of-bounds access. Prefer using `.get(index)` or `.get_mut(index)` which return Option<&T>/Option<&mut T>.
warning correctness unchecked-indexing
let instr = &func.instructions[instruction_id.0 as usize];
Warning: Direct indexing (e.g., `vec[i]`, `slice[i]`) panics on out-of-bounds access. Prefer using `.get(index)` or `.get_mut(index)` which return Option<&T>/Option<&mut T>.
warning correctness unchecked-indexing
let inner_function = &functions[lowered_func.func.0 as usize];
Info: Ensure 'match' statements are exhaustive. If matching on enums, consider adding a wildcard arm `_ => {}` only if necessary and intentional, as it suppresses warnings about unhandled variants.
info correctness match-wildcard
match &identifier.name {
Performance Info: Frequent cloning, especially of Strings, Vecs, or other heap-allocated types inside loops, can be expensive. Consider using references/borrowing where possible.
info performance clone-in-loop
reassignment = Some(reassignment_place.clone());
Performance Info: Frequent cloning, especially of Strings, Vecs, or other heap-allocated types inside loops, can be expensive. Consider using references/borrowing where possible.
info performance clone-in-loop
vec![tag.clone()]
Performance Info: Frequent cloning, especially of Strings, Vecs, or other heap-allocated types inside loops, can be expensive. Consider using references/borrowing where possible.
info performance clone-in-loop
.insert(lvalue_id, reassignment_place.clone());
Performance Info: Frequent cloning, especially of Strings, Vecs, or other heap-allocated types inside loops, can be expensive. Consider using references/borrowing where possible.
info performance clone-in-loop
return Some(reassignment_place.clone());

Get this view in your editor

Same data, no extra tab — call code_get_file + code_get_findings over MCP from Claude/Cursor/Copilot.