From e99c1951ae99f71eab44615748f9b9d2971a70b6 Mon Sep 17 00:00:00 2001 From: orklah Date: Thu, 2 Sep 2021 21:47:10 +0200 Subject: [PATCH 1/6] Make Psalm understand infinite while loop in control actions --- src/Psalm/Internal/Analyzer/ScopeAnalyzer.php | 16 ++++++++ tests/Loop/WhileTest.php | 40 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php index ccc82414485..61616b79f83 100644 --- a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php @@ -345,6 +345,22 @@ function ($action) { return $action !== self::ACTION_NONE; } ); + + if ($stmt instanceof PhpParser\Node\Stmt\While_ + && $nodes + && ($stmt_expr_type = $nodes->getType($stmt->cond)) + && $stmt_expr_type->isTrue() + ) { + //infinite while loop that only return don't have an exit path + $have_exit_path = (bool)array_diff( + $control_actions, + [self::ACTION_END, self::ACTION_RETURN] + ); + + if (!$have_exit_path) { + return array_values(array_unique($control_actions)); + } + } } if ($stmt instanceof PhpParser\Node\Stmt\TryCatch) { diff --git a/tests/Loop/WhileTest.php b/tests/Loop/WhileTest.php index a60999486ea..98aef78ade5 100644 --- a/tests/Loop/WhileTest.php +++ b/tests/Loop/WhileTest.php @@ -708,6 +708,46 @@ function bar(A $a): void { if ($a->foo !== null) {} }' ], + 'whileTrueDontHaveExitPathForReturn' => [ + ' [ + 'getResult(); + } catch (Throwable $exception) { + if ($attempt >= $this->retryAttempts) { + throw $exception; + } + + $attempt++; + + continue; + } + } + } + }' + ], ]; } From c8cf5033677fcf8fa627370456c57b6be6d3e29f Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 4 Sep 2021 13:56:13 +0200 Subject: [PATCH 2/6] introduce isAlwaysFalsy and isAlwaysTruthy --- src/Psalm/Internal/Analyzer/ScopeAnalyzer.php | 2 +- .../Block/IfConditionalAnalyzer.php | 2 +- src/Psalm/Type/Union.php | 125 ++++++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php index 61616b79f83..6070187f19a 100644 --- a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php @@ -349,7 +349,7 @@ function ($action) { if ($stmt instanceof PhpParser\Node\Stmt\While_ && $nodes && ($stmt_expr_type = $nodes->getType($stmt->cond)) - && $stmt_expr_type->isTrue() + && $stmt_expr_type->isTruthy() ) { //infinite while loop that only return don't have an exit path $have_exit_path = (bool)array_diff( diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php index edb08413c00..571179c6f33 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php @@ -218,7 +218,7 @@ function (Type\Union $_): bool { $cond_type = $statements_analyzer->node_data->getType($cond); if ($cond_type !== null) { - if ($cond_type->isFalse()) { + if ($cond_type->isAlwaysFalsy()) { if ($cond_type->from_docblock) { if (IssueBuffer::accepts( new DocblockTypeContradiction( diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 118416c7718..a29e627a504 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -958,11 +958,136 @@ public function isFalse(): bool return count($this->types) === 1 && isset($this->types['false']); } + public function isAlwaysFalsy(): bool + { + foreach ($this->getAtomicTypes() as $atomic_type) { + if ($atomic_type instanceof Type\Atomic\TFalse) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralInt && $atomic_type->value === 0) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralFloat && $atomic_type->value === 0.0) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralString && + ($atomic_type->value === '' || $atomic_type->value === '0') + ) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TNull) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TKeyedArray && + $atomic_type->sealed === true && + count($atomic_type->properties) === 0) { + continue; + } + + if ($atomic_type instanceof TTemplateParam && $atomic_type->as->isAlwaysFalsy()) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TIntRange && + $atomic_type->min_bound === 0 && + $atomic_type->max_bound === 0 + ) { + continue; + } + + //we can't be sure the type is always falsy + return false; + } + + return true; + } + public function isTrue(): bool { return count($this->types) === 1 && isset($this->types['true']); } + public function isTruthy(): bool + { + foreach ($this->getAtomicTypes() as $atomic_type) { + if ($atomic_type instanceof Type\Atomic\TTrue) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralInt && $atomic_type->value !== 0) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralFloat && $atomic_type->value !== 0.0) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralString && + ($atomic_type->value !== '' && $atomic_type->value !== '0') + ) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TNonFalsyString) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TNonEmptyArray) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TNonEmptyList) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TObject) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TNamedObject) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TIntRange && !$atomic_type->contains(0)) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TPositiveInt) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TLiteralClassString) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TClassString) { + continue; + } + + if ($atomic_type instanceof Type\Atomic\TKeyedArray) { + foreach ($atomic_type->properties as $property) { + if ($property->possibly_undefined === false) { + continue; + } + } + } + + if ($atomic_type instanceof TTemplateParam && $atomic_type->as->isAlwaysTruthy()) { + continue; + } + + //we can't be sure the type is always truthy + return false; + } + + return true; + } + public function isVoid(): bool { return isset($this->types['void']); From f9b37cea5b481624182a9d94bb39d024da6bccfb Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 4 Sep 2021 14:08:04 +0200 Subject: [PATCH 3/6] name error --- src/Psalm/Internal/Analyzer/ScopeAnalyzer.php | 2 +- src/Psalm/Type/Union.php | 12 +++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php index 6070187f19a..dfc9b630fb0 100644 --- a/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/ScopeAnalyzer.php @@ -349,7 +349,7 @@ function ($action) { if ($stmt instanceof PhpParser\Node\Stmt\While_ && $nodes && ($stmt_expr_type = $nodes->getType($stmt->cond)) - && $stmt_expr_type->isTruthy() + && $stmt_expr_type->isAlwaysTruthy() ) { //infinite while loop that only return don't have an exit path $have_exit_path = (bool)array_diff( diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index a29e627a504..653a5d06a98 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -983,12 +983,6 @@ public function isAlwaysFalsy(): bool continue; } - if ($atomic_type instanceof Type\Atomic\TKeyedArray && - $atomic_type->sealed === true && - count($atomic_type->properties) === 0) { - continue; - } - if ($atomic_type instanceof TTemplateParam && $atomic_type->as->isAlwaysFalsy()) { continue; } @@ -1012,7 +1006,7 @@ public function isTrue(): bool return count($this->types) === 1 && isset($this->types['true']); } - public function isTruthy(): bool + public function isAlwaysTruthy(): bool { foreach ($this->getAtomicTypes() as $atomic_type) { if ($atomic_type instanceof Type\Atomic\TTrue) { @@ -1053,9 +1047,9 @@ public function isTruthy(): bool continue; } - if ($atomic_type instanceof Type\Atomic\TIntRange && !$atomic_type->contains(0)) { + /*if ($atomic_type instanceof Type\Atomic\TIntRange && !$atomic_type->contains(0)) { continue; - } + }*/ if ($atomic_type instanceof Type\Atomic\TPositiveInt) { continue; From de2769301510f90a97eb6fc8267ca18afeefbed3 Mon Sep 17 00:00:00 2001 From: orklah Date: Sat, 4 Sep 2021 14:10:12 +0200 Subject: [PATCH 4/6] change test to show while loop with truthy value is still inferred --- tests/Loop/WhileTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Loop/WhileTest.php b/tests/Loop/WhileTest.php index 98aef78ade5..8550ed5a219 100644 --- a/tests/Loop/WhileTest.php +++ b/tests/Loop/WhileTest.php @@ -712,7 +712,7 @@ function bar(A $a): void { ' Date: Sat, 4 Sep 2021 14:17:55 +0200 Subject: [PATCH 5/6] fix tests --- tests/TypeReconciliation/AssignmentInConditionalTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TypeReconciliation/AssignmentInConditionalTest.php b/tests/TypeReconciliation/AssignmentInConditionalTest.php index dfe0aecf882..140bcd18360 100644 --- a/tests/TypeReconciliation/AssignmentInConditionalTest.php +++ b/tests/TypeReconciliation/AssignmentInConditionalTest.php @@ -383,7 +383,7 @@ function foo(string $str): ?int { 'repeatedSet' => [ ' [ ' Date: Sat, 4 Sep 2021 14:19:49 +0200 Subject: [PATCH 6/6] add test --- tests/TypeReconciliation/ConditionalTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 5872f54252c..5f584bc5593 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -3046,6 +3046,13 @@ function foo(Exception $e) : void { }', 'error_message' => 'TypeDoesNotContainType', ], + 'falsyValuesInIf' => [ + ' 'TypeDoesNotContainType', + ], ]; } }