Add more refined treatment of InvalidIterator

config.xsd

@@ -190,6 +190,7 @@
             <xs:element name="ParadoxicalCondition" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ParentNotFound" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyFalseArgument" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="PossiblyFalseIterator" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyFalseOperand" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyFalsePropertyAssignmentValue" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyFalseReference" type="IssueHandlerType" minOccurs="0" />
@@ -198,6 +199,7 @@
             <xs:element name="PossiblyInvalidArrayAssignment" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidArrayOffset" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidFunctionCall" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="PossiblyInvalidIterator" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidMethodCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidOperand" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="PossiblyInvalidPropertyAssignment" type="IssueHandlerType" minOccurs="0" />

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

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(
+                    $invalid_iterator_types[] = $iterator_type->getKey();
-                        new InvalidIterator(
-                            'Cannot iterate over ' . $iterator_type->getKey(),
-                            new CodeLocation($statements_checker->getSource(), $stmt->expr)
-                        ),
-                        $statements_checker->getSuppressedIssues()
-                    )) {
-                        return false;
-                    }
 
                     $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)) {

src/Psalm/Issue/PossiblyFalseIterator.php

@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Issue;
+
+class PossiblyFalseIterator extends CodeIssue
+{
+}

src/Psalm/Issue/PossiblyInvalidIterator.php

@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Issue;
+
+class PossiblyInvalidIterator extends CodeIssue
+{
+}