From 0339815070605fce98ae5bad04d863aad73d353c Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 23 Oct 2021 17:30:34 +0200 Subject: [PATCH] fix type leaking when using ternaries --- .../Statements/Expression/TernaryAnalyzer.php | 46 ++++++++++++------- .../Type/SimpleAssertionReconciler.php | 21 ++++----- .../Type/SimpleNegatedAssertionReconciler.php | 21 ++++----- tests/Template/ConditionalReturnTypeTest.php | 15 ++++++ 4 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php index ec95304e074..0145c48d85c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php @@ -16,6 +16,7 @@ use function array_filter; use function array_intersect; +use function array_intersect_key; use function array_keys; use function array_map; use function array_merge; @@ -174,12 +175,6 @@ function ($c) use ($reconciled_expression_clauses): bool { return false; } - foreach ($if_context->vars_in_scope as $var_id => $type) { - if (isset($context->vars_in_scope[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes($context->vars_in_scope[$var_id], $type); - } - } - $context->referenced_var_ids = array_merge( $context->referenced_var_ids, $if_context->referenced_var_ids @@ -217,18 +212,16 @@ function ($c) use ($reconciled_expression_clauses): bool { return false; } - foreach ($t_else_context->vars_in_scope as $var_id => $type) { - if (isset($context->vars_in_scope[$var_id])) { - $context->vars_in_scope[$var_id] = Type::combineUnionTypes( - $context->vars_in_scope[$var_id], - $type - ); - } elseif (isset($if_context->vars_in_scope[$var_id]) - && isset($if_context->assigned_var_ids[$var_id]) - ) { + $assign_var_ifs = $if_context->assigned_var_ids; + $assign_var_else = $t_else_context->assigned_var_ids; + $assign_all = array_intersect_key($assign_var_ifs, $assign_var_else); + + //if the same var was assigned in both branches + foreach ($assign_all as $var_id => $_) { + if (isset($if_context->vars_in_scope[$var_id]) && isset($t_else_context->vars_in_scope[$var_id])) { $context->vars_in_scope[$var_id] = Type::combineUnionTypes( $if_context->vars_in_scope[$var_id], - $type + $t_else_context->vars_in_scope[$var_id] ); } } @@ -237,6 +230,7 @@ function ($c) use ($reconciled_expression_clauses): bool { $redef_var_else = array_keys($t_else_context->getRedefinedVars($context->vars_in_scope)); $redef_all = array_intersect($redef_var_ifs, $redef_var_else); + //these vars were changed in both branches foreach ($redef_all as $redef_var_id) { $context->vars_in_scope[$redef_var_id] = Type::combineUnionTypes( $if_context->vars_in_scope[$redef_var_id], @@ -244,6 +238,26 @@ function ($c) use ($reconciled_expression_clauses): bool { ); } + //these vars were changed in the if and existed before + foreach ($redef_var_ifs as $redef_var_ifs_id) { + if (isset($context->vars_in_scope[$redef_var_ifs_id])) { + $context->vars_in_scope[$redef_var_ifs_id] = Type::combineUnionTypes( + $context->vars_in_scope[$redef_var_ifs_id], + $if_context->vars_in_scope[$redef_var_ifs_id] + ); + } + } + + //these vars were changed in the else and existed before + foreach ($redef_var_else as $redef_var_else_id) { + if (isset($context->vars_in_scope[$redef_var_else_id])) { + $context->vars_in_scope[$redef_var_else_id] = Type::combineUnionTypes( + $context->vars_in_scope[$redef_var_else_id], + $t_else_context->vars_in_scope[$redef_var_else_id] + ); + } + } + $context->vars_possibly_in_scope = array_merge( $context->vars_possibly_in_scope, $if_context->vars_possibly_in_scope, diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php index 932787779ad..9d8760e6a6d 100644 --- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php @@ -143,8 +143,7 @@ public static function reconcile( $code_location, $suppressed_issues, $failed_reconciliation, - false, - $inside_loop + false ); } @@ -2219,8 +2218,7 @@ private static function reconcileFalsyOrEmpty( ?CodeLocation $code_location, array $suppressed_issues, int &$failed_reconciliation, - bool $recursive_check, - bool $inside_loop + bool $recursive_check ) : Union { $old_var_type_string = $existing_var_type->getId(); $existing_var_atomic_types = $existing_var_type->getAtomicTypes(); @@ -2360,27 +2358,26 @@ private static function reconcileFalsyOrEmpty( $existing_var_type->addType(new TEmptyNumeric()); } - foreach ($existing_var_atomic_types as $existing_var_atomic_type) { + foreach ($existing_var_atomic_types as $type_key => $existing_var_atomic_type) { if ($existing_var_atomic_type instanceof TTemplateParam) { if (!$existing_var_atomic_type->as->isMixed()) { $template_did_fail = 0; - $tmp_existing_var_atomic_type = clone $existing_var_atomic_type; + $existing_var_atomic_type = clone $existing_var_atomic_type; - $reconciled_type = self::reconcileFalsyOrEmpty( + $existing_var_atomic_type->as = self::reconcileFalsyOrEmpty( $assertion, - $tmp_existing_var_atomic_type->as, + $existing_var_atomic_type->as, $key, $negated, $code_location, $suppressed_issues, $template_did_fail, - $recursive_check, - $inside_loop + $recursive_check ); - if (!$template_did_fail && !$inside_loop) { - $existing_var_atomic_type->as = $reconciled_type; + if (!$template_did_fail) { + $existing_var_type->removeType($type_key); $existing_var_type->addType($existing_var_atomic_type); } } diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php index 4ff27e08974..e8c344716c8 100644 --- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php +++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php @@ -180,8 +180,7 @@ public static function reconcile( $failed_reconciliation, $is_equality, $is_strict_equality, - false, - $inside_loop + false ); } @@ -571,8 +570,7 @@ private static function reconcileFalsyOrEmpty( int &$failed_reconciliation, bool $is_equality, bool $is_strict_equality, - bool $recursive_check, - bool $inside_loop + bool $recursive_check ) : Type\Union { $old_var_type_string = $existing_var_type->getId(); $existing_var_atomic_types = $existing_var_type->getAtomicTypes(); @@ -725,16 +723,16 @@ private static function reconcileFalsyOrEmpty( } } - foreach ($existing_var_atomic_types as $existing_var_atomic_type) { + foreach ($existing_var_atomic_types as $type_key => $existing_var_atomic_type) { if ($existing_var_atomic_type instanceof TTemplateParam) { if (!$is_equality && !$existing_var_atomic_type->as->isMixed()) { $template_did_fail = 0; - $tmp_existing_var_atomic_type = clone $existing_var_atomic_type; + $existing_var_atomic_type = clone $existing_var_atomic_type; - $reconciled_type = self::reconcileFalsyOrEmpty( + $existing_var_atomic_type->as = self::reconcileFalsyOrEmpty( $assertion, - $tmp_existing_var_atomic_type->as, + $existing_var_atomic_type->as, $key, $negated, $code_location, @@ -742,12 +740,11 @@ private static function reconcileFalsyOrEmpty( $template_did_fail, $is_equality, $is_strict_equality, - true, - $inside_loop + true ); - if (!$template_did_fail && !$inside_loop) { - $existing_var_atomic_type->as = $reconciled_type; + if (!$template_did_fail) { + $existing_var_type->removeType($type_key); $existing_var_type->addType($existing_var_atomic_type); } } diff --git a/tests/Template/ConditionalReturnTypeTest.php b/tests/Template/ConditionalReturnTypeTest.php index 6c97ebaa3cb..0cbab4dc0fa 100644 --- a/tests/Template/ConditionalReturnTypeTest.php +++ b/tests/Template/ConditionalReturnTypeTest.php @@ -826,6 +826,21 @@ function scope(bool $list_output = true): array } ' ], + 'dontChokeOnFalsyAssertionsWithTemplatesOutsideLoop' => [ + ' : list) + */ + function get_liste_designation_client(bool $a = false) { + (!$a ? "a" : "b"); + (!$a ? "a" : "b"); + return []; + } + ' + ], ]; } }