Skip to content

Commit

Permalink
fix type leaking when using ternaries
Browse files Browse the repository at this point in the history
  • Loading branch information
@orklah
orklah committed Oct 23, 2021
1 parent 307635f commit 0339815
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 40 deletions.
46 changes: 30 additions & 16 deletions src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
);
}
}
Expand All @@ -237,13 +230,34 @@ 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],
$t_else_context->vars_in_scope[$redef_var_id]
);
}

//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,
Expand Down
21 changes: 9 additions & 12 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ public static function reconcile(
$code_location,
$suppressed_issues,
$failed_reconciliation,
false,
$inside_loop
false
);
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}
Expand Down
21 changes: 9 additions & 12 deletions src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ public static function reconcile(
$failed_reconciliation,
$is_equality,
$is_strict_equality,
false,
$inside_loop
false
);
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -725,29 +723,28 @@ 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,
$suppressed_issues,
$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);
}
}
Expand Down
15 changes: 15 additions & 0 deletions tests/Template/ConditionalReturnTypeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,21 @@ function scope(bool $list_output = true): array
}
'
],
'dontChokeOnFalsyAssertionsWithTemplatesOutsideLoop' => [
'<?php
class A {}
class B {}
/**
* @psalm-return ($a is true ? list<A> : list<B>)
*/
function get_liste_designation_client(bool $a = false) {
(!$a ? "a" : "b");
(!$a ? "a" : "b");
return [];
}
'
],
];
}
}

0 comments on commit 0339815

Please sign in to comment.