From b634e1a1b77c05e88a9f592e80d019112483a271 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Tue, 20 Mar 2018 22:59:22 -0400 Subject: [PATCH] Add more refined treatment of InvalidIterator --- config.xsd | 2 + docs/issues.md | 18 +++++++ .../Statements/Block/ForeachChecker.php | 47 +++++++++++++++---- src/Psalm/Issue/PossiblyFalseIterator.php | 6 +++ src/Psalm/Issue/PossiblyInvalidIterator.php | 6 +++ 5 files changed, 69 insertions(+), 10 deletions(-) create mode 100644 src/Psalm/Issue/PossiblyFalseIterator.php create mode 100644 src/Psalm/Issue/PossiblyInvalidIterator.php diff --git a/config.xsd b/config.xsd index aa87cbc0f..eae54576e 100644 --- a/config.xsd +++ b/config.xsd @@ -190,6 +190,7 @@ + @@ -198,6 +199,7 @@ + diff --git a/docs/issues.md b/docs/issues.md index c6b691c6b..f545adf5a 100644 --- a/docs/issues.md +++ b/docs/issues.md @@ -1150,6 +1150,15 @@ function foo(string $s) : void { } ``` +### PossiblyFalseIterator + +Emitted when trying to iterate over a value that may be `false` + +```php +$arr = rand(0, 1) ? [1, 2, 3] : false; +foreach ($arr as $a) {} +``` + ### PossiblyFalseOperand Emitted when using a possibly `false` value as part of an operation (e.g. `+`, `.`, `^` etc.`) @@ -1242,6 +1251,15 @@ $a = rand(0, 1) ? 5 : function() : int { return 5; }; $b = $a(); ``` +### PossiblyInvalidIterator + +Emitted when trying to iterate over a value that may be invalid + +```php +$arr = rand(0, 1) ? [1, 2, 3] : "hello"; +foreach ($arr as $a) {} +``` + ### PossiblyInvalidMethodCall Emitted when trying to call a method on a value that may not be an object diff --git a/src/Psalm/Checker/Statements/Block/ForeachChecker.php b/src/Psalm/Checker/Statements/Block/ForeachChecker.php index d3db966f0..a14a9dcfe 100644 --- a/src/Psalm/Checker/Statements/Block/ForeachChecker.php +++ b/src/Psalm/Checker/Statements/Block/ForeachChecker.php @@ -13,6 +13,8 @@ use Psalm\Exception\DocblockParseException; use Psalm\Issue\InvalidDocblock; use Psalm\Issue\InvalidIterator; use Psalm\Issue\NullIterator; +use Psalm\Issue\PossiblyFalseIterator; +use Psalm\Issue\PossiblyInvalidIterator; use Psalm\Issue\PossiblyNullIterator; use Psalm\Issue\RawObjectIteration; use Psalm\IssueBuffer; @@ -87,7 +89,7 @@ class ForeachChecker } } elseif ($iterator_type->isFalsable() && !$iterator_type->ignore_falsable_issues) { if (IssueBuffer::accepts( - new InvalidIterator( + new PossiblyFalseIterator( 'Cannot iterate over falsable var ' . $iterator_type, new CodeLocation($statements_checker->getSource(), $stmt->expr) ), @@ -97,11 +99,15 @@ class ForeachChecker } } + $has_valid_iterator = false; + $invalid_iterator_types = []; + foreach ($iterator_type->getTypes() as $iterator_type) { // if it's an empty array, we cannot iterate over it if ($iterator_type instanceof Type\Atomic\TArray && $iterator_type->type_params[1]->isEmpty() ) { + $has_valid_iterator = true; continue; } @@ -129,27 +135,22 @@ class ForeachChecker } else { $key_type = Type::combineUnionTypes($key_type, $key_type_part); } + + $has_valid_iterator = true; continue; } if ($iterator_type instanceof Type\Atomic\Scalar || $iterator_type instanceof Type\Atomic\TVoid ) { - if (IssueBuffer::accepts( - new InvalidIterator( - 'Cannot iterate over ' . $iterator_type->getKey(), - new CodeLocation($statements_checker->getSource(), $stmt->expr) - ), - $statements_checker->getSuppressedIssues() - )) { - return false; - } + $invalid_iterator_types[] = $iterator_type->getKey(); $value_type = Type::getMixed(); } elseif ($iterator_type instanceof Type\Atomic\TObject || $iterator_type instanceof Type\Atomic\TMixed || $iterator_type instanceof Type\Atomic\TEmpty ) { + $has_valid_iterator = true; $value_type = Type::getMixed(); } elseif ($iterator_type instanceof Type\Atomic\TNamedObject) { if ($iterator_type->value !== 'Traversable' && @@ -165,6 +166,8 @@ class ForeachChecker } } + $has_valid_iterator = true; + if ($iterator_type instanceof Type\Atomic\TGenericObject && (strtolower($iterator_type->value) === 'iterable' || strtolower($iterator_type->value) === 'traversable' || @@ -263,6 +266,30 @@ class ForeachChecker } } } + + if ($invalid_iterator_types) { + if ($has_valid_iterator) { + if (IssueBuffer::accepts( + new PossiblyInvalidIterator( + 'Cannot iterate over ' . $invalid_iterator_types[0], + new CodeLocation($statements_checker->getSource(), $stmt->expr) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + } else { + if (IssueBuffer::accepts( + new InvalidIterator( + 'Cannot iterate over ' . $invalid_iterator_types[0], + new CodeLocation($statements_checker->getSource(), $stmt->expr) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + } + } } if ($stmt->keyVar && $stmt->keyVar instanceof PhpParser\Node\Expr\Variable && is_string($stmt->keyVar->name)) { diff --git a/src/Psalm/Issue/PossiblyFalseIterator.php b/src/Psalm/Issue/PossiblyFalseIterator.php new file mode 100644 index 000000000..38b6a99d2 --- /dev/null +++ b/src/Psalm/Issue/PossiblyFalseIterator.php @@ -0,0 +1,6 @@ +