diff --git a/src/analyzer/expr/exit_analyzer.rs b/src/analyzer/expr/exit_analyzer.rs index 3df02373..40fc5515 100644 --- a/src/analyzer/expr/exit_analyzer.rs +++ b/src/analyzer/expr/exit_analyzer.rs @@ -2,6 +2,7 @@ use std::rc::Rc; use crate::expression_analyzer; use crate::function_analysis_data::FunctionAnalysisData; +use crate::scope::control_action::ControlAction; use crate::scope::BlockContext; use crate::scope_analyzer::ScopeAnalyzer; use crate::statements_analyzer::StatementsAnalyzer; @@ -9,9 +10,9 @@ use crate::stmt_analyzer::AnalysisError; use hakana_code_info::code_location::HPos; use hakana_code_info::function_context::FunctionLikeIdentifier; use hakana_code_info::functionlike_parameter::FunctionLikeParameter; +use hakana_code_info::ttype::{get_arraykey, get_mixed_any, get_nothing}; use hakana_code_info::VarId; use hakana_str::StrId; -use hakana_code_info::ttype::{get_arraykey, get_mixed_any, get_nothing}; use oxidized::ast_defs::Pos; use oxidized::{aast, ast_defs}; @@ -62,6 +63,9 @@ pub(crate) fn analyze( context.inside_general_use = false; } + context.has_returned = true; + context.control_actions.insert(ControlAction::End); + analysis_data.set_expr_type(call_pos, get_nothing()); Ok(()) diff --git a/src/analyzer/stmt/switch_analyzer.rs b/src/analyzer/stmt/switch_analyzer.rs index 958df46a..fd0684d0 100644 --- a/src/analyzer/stmt/switch_analyzer.rs +++ b/src/analyzer/stmt/switch_analyzer.rs @@ -70,53 +70,6 @@ pub(crate) fn analyze( switch_var_id }; - let original_context = context.clone(); - - let mut last_case_exit_type = ControlAction::Break; - - let mut case_exit_types = FxHashMap::default(); - - let has_default = stmt.2.is_some(); - - let mut case_action_map = FxHashMap::default(); - - let mut cases = stmt.1.iter().enumerate().collect::>(); - cases.reverse(); - - if let Some(default_case) = stmt.2 { - update_case_exit_map( - codebase, - statements_analyzer.get_interner(), - &default_case.1 .0, - analysis_data, - statements_analyzer.get_file_analyzer().resolved_names, - &mut case_action_map, - cases.len(), - &mut last_case_exit_type, - &mut case_exit_types, - ); - } - - for (i, case) in &cases { - update_case_exit_map( - codebase, - statements_analyzer.get_interner(), - &case.1 .0, - analysis_data, - statements_analyzer.get_file_analyzer().resolved_names, - &mut case_action_map, - *i, - &mut last_case_exit_type, - &mut case_exit_types, - ); - } - - let mut switch_scope = SwitchScope::new(); - - let mut all_options_returned = true; - - cases.reverse(); - let mut condition_is_fake = false; let fake_switch_condition = if switch_var_id.starts_with("$-tmp_switch-") { @@ -134,17 +87,24 @@ pub(crate) fn analyze( None }; - let mut previous_empty_cases = vec![]; + let original_context = context.clone(); - for (i, case) in &cases { - let case_exit_type = case_exit_types.get(i).unwrap(); + let mut last_case_exit_type = ControlAction::Break; - if case_exit_type != &ControlAction::Return { - all_options_returned = false; - } + let has_default = stmt.2.is_some(); + + let mut cases = stmt.1.iter().enumerate().collect::>(); + cases.reverse(); + + let mut switch_scope = SwitchScope::new(); + + let mut all_options_returned = true; + + cases.reverse(); - let case_actions = &case_action_map[i]; + let mut previous_empty_cases = vec![]; + for (i, case) in &cases { if !case .1 .iter() @@ -167,8 +127,6 @@ pub(crate) fn analyze( analysis_data, context, &original_context, - case_exit_type, - case_actions, *i == (cases.len() - 1) && stmt.2.is_none(), &mut switch_scope, loop_scope, @@ -178,17 +136,7 @@ pub(crate) fn analyze( } if let Some(default_case) = stmt.2 { - let i = cases.len(); - - let case_exit_type = case_exit_types.get(&i).unwrap(); - - if case_exit_type != &ControlAction::Return { - all_options_returned = false; - } - - let case_actions = case_action_map.get(&i).unwrap(); - - analyze_case( + let case_exit_type = analyze_case( statements_analyzer, stmt, fake_switch_condition.as_ref().unwrap_or(stmt.0), @@ -201,12 +149,14 @@ pub(crate) fn analyze( analysis_data, context, &original_context, - case_exit_type, - case_actions, true, &mut switch_scope, loop_scope, )?; + + if case_exit_type != ControlAction::Return { + all_options_returned = false; + } } let mut possibly_redefined_vars = switch_scope.possibly_redefined_vars.unwrap_or_default(); @@ -246,32 +196,6 @@ pub(crate) fn analyze( Ok(()) } -fn update_case_exit_map( - codebase: &CodebaseInfo, - interner: &Interner, - case_stmts: &Vec>, - analysis_data: &mut FunctionAnalysisData, - resolved_names: &FxHashMap, - case_action_map: &mut FxHashMap>, - i: usize, - last_case_exit_type: &mut ControlAction, - case_exit_types: &mut FxHashMap, -) { - let case_actions = control_analyzer::get_control_actions( - codebase, - interner, - resolved_names, - case_stmts, - analysis_data, - vec![BreakContext::Switch], - true, - ); - - case_action_map.insert(i, case_actions.clone()); - *last_case_exit_type = get_last_action(case_actions).unwrap_or(last_case_exit_type.clone()); - case_exit_types.insert(i, last_case_exit_type.clone()); -} - fn get_last_action(case_actions: FxHashSet) -> Option { if !case_actions.contains(&ControlAction::None) { if case_actions.len() == 1 && case_actions.contains(&ControlAction::End) { diff --git a/src/analyzer/stmt/switch_case_analyzer.rs b/src/analyzer/stmt/switch_case_analyzer.rs index 0d46fd3d..4105cd40 100644 --- a/src/analyzer/stmt/switch_case_analyzer.rs +++ b/src/analyzer/stmt/switch_case_analyzer.rs @@ -72,17 +72,10 @@ pub(crate) fn analyze_case( analysis_data: &mut FunctionAnalysisData, context: &mut BlockContext, original_context: &BlockContext, - case_exit_type: &ControlAction, - case_actions: &FxHashSet, is_last: bool, switch_scope: &mut SwitchScope, loop_scope: &mut Option, -) -> Result<(), AnalysisError> { - let has_ending_statements = - case_actions.len() == 1 && case_actions.contains(&ControlAction::End); - let has_leaving_statements = has_ending_statements - || (!case_actions.is_empty() && !case_actions.contains(&ControlAction::None)); - +) -> Result { let mut case_context = original_context.clone(); let mut old_node_data = analysis_data.expr_types.clone(); @@ -192,7 +185,9 @@ pub(crate) fn analyze_case( let case_stmts = leftover_statements; - if !has_leaving_statements && !is_last { + if (case_stmts.is_empty() || &case_stmts.last().unwrap().1 == &aast::Stmt_::Fallthrough) + && !is_last + { // this is safe for non-defaults, and defaults are always last let case_equality_expression = case_equality_expr.unwrap(); let case_cond = case_cond.unwrap(); @@ -235,7 +230,7 @@ pub(crate) fn analyze_case( analysis_data.case_scopes.pop(); - return Ok(()); + return Ok(ControlAction::None); } if let Some(leftover_case_equality_expr) = &switch_scope.leftover_case_equality_expr { @@ -416,7 +411,7 @@ pub(crate) fn analyze_case( statements_analyzer.analyze(&case_stmts, analysis_data, &mut case_context, loop_scope)?; if analysis_data.case_scopes.is_empty() { - return Ok(()); + return Ok(ControlAction::None); } let case_scope = analysis_data.case_scopes.pop().unwrap(); @@ -425,7 +420,12 @@ pub(crate) fn analyze_case( old_node_data.extend(new_node_data); analysis_data.expr_types = old_node_data; - if !matches!(case_exit_type, ControlAction::Return) { + if case_context.control_actions.is_empty() + || !case_context + .control_actions + .iter() + .all(|a| a == &ControlAction::Return || a == &ControlAction::End) + { handle_non_returning_case( statements_analyzer, switch_var_id, @@ -435,7 +435,6 @@ pub(crate) fn analyze_case( context, &case_context, original_context, - case_exit_type, switch_scope, )?; } @@ -505,7 +504,7 @@ pub(crate) fn analyze_case( } } - Ok(()) + Ok(ControlAction::None) } pub(crate) fn handle_non_returning_case( @@ -517,7 +516,6 @@ pub(crate) fn handle_non_returning_case( context: &mut BlockContext, case_context: &BlockContext, original_context: &BlockContext, - case_exit_type: &ControlAction, switch_scope: &mut SwitchScope, ) -> Result<(), AnalysisError> { if is_default_case { @@ -542,86 +540,81 @@ pub(crate) fn handle_non_returning_case( let codebase = statements_analyzer.get_codebase(); - if !matches!(case_exit_type, ControlAction::Continue) { - let mut removed_var_ids = FxHashSet::default(); - let case_redefined_vars = case_context.get_redefined_locals( - &original_context.locals, - false, - &mut removed_var_ids, + let mut removed_var_ids = FxHashSet::default(); + let case_redefined_vars = + case_context.get_redefined_locals(&original_context.locals, false, &mut removed_var_ids); + + if let Some(ref mut possibly_redefined_var_ids) = switch_scope.possibly_redefined_vars { + for (var_id, var_type) in &case_redefined_vars { + possibly_redefined_var_ids.insert( + var_id.clone(), + combine_optional_union_types( + Some(var_type), + possibly_redefined_var_ids.get(var_id), + codebase, + ), + ); + } + } else { + switch_scope.possibly_redefined_vars = Some( + case_redefined_vars + .clone() + .into_iter() + .filter(|(var_id, _)| context.locals.contains_key(var_id)) + .collect(), ); + } - if let Some(ref mut possibly_redefined_var_ids) = switch_scope.possibly_redefined_vars { - for (var_id, var_type) in &case_redefined_vars { - possibly_redefined_var_ids.insert( + if let Some(ref mut redefined_vars) = switch_scope.redefined_vars { + for (var_id, var_type) in redefined_vars.clone() { + if let Some(break_var_type) = case_redefined_vars.get(&var_id) { + redefined_vars.insert( var_id.clone(), - combine_optional_union_types( - Some(var_type), - possibly_redefined_var_ids.get(var_id), + Rc::new(combine_union_types( + break_var_type, + &var_type, codebase, - ), + false, + )), ); + } else { + redefined_vars.remove(&var_id); } - } else { - switch_scope.possibly_redefined_vars = Some( - case_redefined_vars - .clone() - .into_iter() - .filter(|(var_id, _)| context.locals.contains_key(var_id)) - .collect(), - ); - } - - if let Some(ref mut redefined_vars) = switch_scope.redefined_vars { - for (var_id, var_type) in redefined_vars.clone() { - if let Some(break_var_type) = case_redefined_vars.get(&var_id) { - redefined_vars.insert( - var_id.clone(), - Rc::new(combine_union_types( - break_var_type, - &var_type, - codebase, - false, - )), - ); - } else { - redefined_vars.remove(&var_id); - } - } - } else { - switch_scope.redefined_vars = Some( - case_redefined_vars - .into_iter() - .map(|(k, v)| (k, Rc::new(v))) - .collect(), - ); } + } else { + switch_scope.redefined_vars = Some( + case_redefined_vars + .into_iter() + .map(|(k, v)| (k, Rc::new(v))) + .collect(), + ); + } - if let Some(ref mut new_locals) = switch_scope.new_locals { - for (var_id, var_type) in new_locals.clone() { - if case_context.locals.contains_key(&var_id) { - new_locals.insert( - var_id.clone(), - Rc::new(combine_union_types( - case_context.locals.get(&var_id).unwrap(), - &var_type, - codebase, - false, - )), - ); - } else { - new_locals.remove(&var_id); - } + if let Some(ref mut new_locals) = switch_scope.new_locals { + for (var_id, var_type) in new_locals.clone() { + if case_context.locals.contains_key(&var_id) { + new_locals.insert( + var_id.clone(), + Rc::new(combine_union_types( + case_context.locals.get(&var_id).unwrap(), + &var_type, + codebase, + false, + )), + ); + } else { + new_locals.remove(&var_id); } - } else { - switch_scope.new_locals = Some( - case_context - .locals - .clone() - .into_iter() - .filter(|(k, _)| !context.locals.contains_key(k)) - .collect(), - ); } + } else { + switch_scope.new_locals = Some( + case_context + .locals + .clone() + .into_iter() + .filter(|(k, _)| !context.locals.contains_key(k)) + .collect(), + ); } Ok(()) diff --git a/tests/inference/SwitchType/defaultExits/input.hack b/tests/inference/SwitchType/defaultExits/input.hack new file mode 100644 index 00000000..be027211 --- /dev/null +++ b/tests/inference/SwitchType/defaultExits/input.hack @@ -0,0 +1,14 @@ +function foo(): void { + $a = rand(0, 10); + + switch ($a) { + case 1: + case 2: + $b = 5; + + default: + exit(1); + } + + echo $b; +} diff --git a/tests/inference/SwitchType/varDefinedInDefault/input.hack b/tests/inference/SwitchType/varDefinedInDefault/input.hack new file mode 100644 index 00000000..baa6542c --- /dev/null +++ b/tests/inference/SwitchType/varDefinedInDefault/input.hack @@ -0,0 +1,14 @@ +function foo(): void { + $a = rand(0, 10); + + switch ($a) { + case 1: + case 2: + return; + + default: + $b = 5; + } + + echo $b; +}