From 60feb5fd719aad4512197fe152248d161045af1d Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 12 Feb 2022 20:31:39 +0100 Subject: [PATCH 1/3] allow collapsing enum cases under named object in combiner --- src/Psalm/Internal/Type/TypeCombiner.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php index 935e45994e2..c28f119ef45 100644 --- a/src/Psalm/Internal/Type/TypeCombiner.php +++ b/src/Psalm/Internal/Type/TypeCombiner.php @@ -18,6 +18,7 @@ use Psalm\Type\Atomic\TClassString; use Psalm\Type\Atomic\TClassStringMap; use Psalm\Type\Atomic\TEmptyMixed; +use Psalm\Type\Atomic\TEnumCase; use Psalm\Type\Atomic\TFalse; use Psalm\Type\Atomic\TFloat; use Psalm\Type\Atomic\TGenericObject; @@ -334,6 +335,11 @@ public static function combine( } if ($combination->named_object_types !== null) { + foreach ($combination->value_types as $key => $atomic_type) { + if ($atomic_type instanceof TEnumCase && isset($combination->named_object_types[$atomic_type->value])) { + unset($combination->value_types[$key]); + } + } $combination->value_types += $combination->named_object_types; } From 954884eb4850d435adffd83ee469359f44cc374c Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 12 Feb 2022 20:50:06 +0100 Subject: [PATCH 2/3] consistency between TernaryAnalyzer and IfElseAnalyzer --- .../Statements/Expression/TernaryAnalyzer.php | 80 +++++++++++++------ 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php index 2bd6307fda8..f81f80fbc23 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php @@ -5,6 +5,7 @@ use PhpParser; use Psalm\CodeLocation; use Psalm\Context; +use Psalm\Exception\ComplicatedExpressionException; use Psalm\Exception\ScopeAnalysisException; use Psalm\Internal\Algebra; use Psalm\Internal\Algebra\FormulaGenerator; @@ -15,6 +16,7 @@ use Psalm\Internal\Clause; use Psalm\Internal\Scope\IfScope; use Psalm\Internal\Type\AssertionReconciler; +use Psalm\Node\Expr\VirtualBooleanNot; use Psalm\Storage\Assertion\Truthy; use Psalm\Type; use Psalm\Type\Reconciler; @@ -27,6 +29,7 @@ use function array_map; use function array_merge; use function array_values; +use function count; use function in_array; use function preg_match; use function preg_quote; @@ -56,6 +59,7 @@ public static function analyze( $context->branch_point ?: (int) $stmt->getAttribute('startFilePos') ); + // this is the context for stuff that happens within the first operand of the ternary $if_context = $if_conditional_scope->if_context; $cond_referenced_var_ids = $if_conditional_scope->cond_referenced_var_ids; @@ -64,19 +68,21 @@ public static function analyze( return false; } - $codebase = $statements_analyzer->getCodebase(); - - $cond_id = spl_object_id($stmt->cond); + $cond_object_id = spl_object_id($stmt->cond); $if_clauses = FormulaGenerator::getFormula( - $cond_id, - $cond_id, + $cond_object_id, + $cond_object_id, $stmt->cond, $context->self, $statements_analyzer, $codebase ); + if (count($if_clauses) > 200) { + $if_clauses = []; + } + $mixed_var_ids = []; foreach ($context->vars_in_scope as $var_id => $type) { @@ -95,7 +101,7 @@ public static function analyze( /** * @return Clause */ - function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { + function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause { $keys = array_keys($c->possibilities); $mixed_var_ids = array_diff($mixed_var_ids, $keys); @@ -103,7 +109,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { foreach ($keys as $key) { foreach ($mixed_var_ids as $mixed_var_id) { if (preg_match('/^' . preg_quote($mixed_var_id, '/') . '(\[|-)/', $key)) { - return new Clause([], $cond_id, $cond_id, true); + return new Clause([], $cond_object_id, $cond_object_id, true); } } } @@ -113,6 +119,8 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { $if_clauses ); + $entry_clauses = $context->clauses; + // this will see whether any of the clauses in set A conflict with the clauses in set B AlgebraAnalyzer::checkForParadox( $context->clauses, @@ -122,34 +130,58 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { $assigned_in_conditional_var_ids ); - $ternary_clauses = array_merge($context->clauses, $if_clauses); + $if_clauses = Algebra::simplifyCNF($if_clauses); + + $ternary_context_clauses = array_merge($entry_clauses, $if_clauses); if ($if_context->reconciled_expression_clauses) { $reconciled_expression_clauses = $if_context->reconciled_expression_clauses; - $ternary_clauses = array_values( + $ternary_context_clauses = array_values( array_filter( - $ternary_clauses, + $ternary_context_clauses, fn($c): bool => !in_array($c->hash, $reconciled_expression_clauses) ) ); - } - $ternary_clauses = Algebra::simplifyCNF($ternary_clauses); + if (count($if_context->clauses) === 1 + && $if_context->clauses[0]->wedge + && !$if_context->clauses[0]->possibilities + ) { + $if_context->clauses = []; + $if_context->reconciled_expression_clauses = []; + } + } - $negated_clauses = Algebra::negateFormula($if_clauses); + try { + $if_scope->negated_clauses = Algebra::negateFormula($if_clauses); + } catch (ComplicatedExpressionException $e) { + try { + $if_scope->negated_clauses = FormulaGenerator::getFormula( + $cond_object_id, + $cond_object_id, + new VirtualBooleanNot($stmt->cond), + $context->self, + $statements_analyzer, + $codebase, + false + ); + } catch (ComplicatedExpressionException $e) { + $if_scope->negated_clauses = []; + } + } - $negated_if_types = Algebra::getTruthsFromFormula( + $if_scope->negated_types = Algebra::getTruthsFromFormula( Algebra::simplifyCNF( - array_merge($context->clauses, $negated_clauses) + array_merge($context->clauses, $if_scope->negated_clauses) ) ); $active_if_types = []; $reconcilable_if_types = Algebra::getTruthsFromFormula( - $ternary_clauses, - $cond_id, + $ternary_context_clauses, + $cond_object_id, $cond_referenced_var_ids, $active_if_types ); @@ -189,16 +221,16 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { $t_else_context->clauses = Algebra::simplifyCNF( array_merge( $t_else_context->clauses, - $negated_clauses + $if_scope->negated_clauses ) ); - if ($negated_if_types) { - $changed_var_ids = []; + $changed_var_ids = []; - $t_else_vars_in_scope_reconciled = Reconciler::reconcileKeyedTypes( - $negated_if_types, - $negated_if_types, + if ($if_scope->negated_types) { + $else_vars_reconciled = Reconciler::reconcileKeyedTypes( + $if_scope->negated_types, + $if_scope->negated_types, $t_else_context->vars_in_scope, $t_else_context->references_in_scope, $changed_var_ids, @@ -209,7 +241,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause { new CodeLocation($statements_analyzer->getSource(), $stmt->else) ); - $t_else_context->vars_in_scope = $t_else_vars_in_scope_reconciled; + $t_else_context->vars_in_scope = $else_vars_reconciled; $t_else_context->clauses = Context::removeReconciledClauses($t_else_context->clauses, $changed_var_ids)[0]; } From 9cc82d55f779ab80ab1cd1ba91786472a9ffb7b4 Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 12 Feb 2022 21:40:51 +0100 Subject: [PATCH 3/3] add test --- tests/EnumTest.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/EnumTest.php b/tests/EnumTest.php index 199126d28ea..3297151ee2d 100644 --- a/tests/EnumTest.php +++ b/tests/EnumTest.php @@ -382,6 +382,29 @@ enum Status: int { 'ignored_issues' => [], 'php_version' => '8.1', ], + 'EnumCollapsing' => [ + 'code' => ' [ + '$code' => 'Code|int', + ], + 'ignored_issues' => [], + 'php_version' => '8.1', + ], ]; }