Skip to content

Commit a1352eb

Browse files
weirdanweirdan
authored
Merge pull request #10697 from weirdan/forbid-iterating-over-generators-with-non-nullable-send
2 parents e9ea999 + 0ce62a5 commit a1352eb

File tree

4 files changed

+124
-12
lines changed

4 files changed

+124
-12
lines changed

‎src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -307,15 +307,6 @@ public static function verifyReturnType(
307307
$source->getParentFQCLN(),
308308
);
309309

310-
// hack until we have proper yield type collection
311-
if ($function_like_storage
312-
&& $function_like_storage->has_yield
313-
&& !$inferred_yield_type
314-
&& !$inferred_return_type->isVoid()
315-
) {
316-
$inferred_return_type = new Union([new TNamedObject('Generator')]);
317-
}
318-
319310
if ($is_to_string) {
320311
$union_comparison_results = new TypeComparisonResult();
321312
if (!$inferred_return_type->hasMixed() &&

‎src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,7 @@ public static function checkIteratorType(
657657
$key_type,
658658
$value_type,
659659
$has_valid_iterator,
660+
$invalid_iterator_types,
660661
);
661662
} else {
662663
$raw_object_types[] = $iterator_atomic_type->value;
@@ -725,6 +726,7 @@ public static function checkIteratorType(
725726
return null;
726727
}
727728

729+
/** @param list<string> $invalid_iterator_types */
728730
public static function handleIterable(
729731
StatementsAnalyzer $statements_analyzer,
730732
TNamedObject $iterator_atomic_type,
@@ -733,7 +735,8 @@ public static function handleIterable(
733735
Context $context,
734736
?Union &$key_type,
735737
?Union &$value_type,
736-
bool &$has_valid_iterator
738+
bool &$has_valid_iterator,
739+
array &$invalid_iterator_types = []
737740
): void {
738741
if ($iterator_atomic_type->extra_types) {
739742
$iterator_atomic_types = array_merge(
@@ -753,7 +756,6 @@ public static function handleIterable(
753756
}
754757

755758

756-
$has_valid_iterator = true;
757759

758760
if ($iterator_atomic_type instanceof TIterable
759761
|| (strtolower($iterator_atomic_type->value) === 'traversable'
@@ -781,6 +783,8 @@ public static function handleIterable(
781783
)
782784
)
783785
) {
786+
$has_valid_iterator = true;
787+
784788
$old_data_provider = $statements_analyzer->node_data;
785789

786790
$statements_analyzer->node_data = clone $statements_analyzer->node_data;
@@ -867,6 +871,7 @@ public static function handleIterable(
867871
$key_type,
868872
$value_type,
869873
$has_valid_iterator,
874+
$invalid_iterator_types,
870875
);
871876

872877
continue;
@@ -899,6 +904,51 @@ public static function handleIterable(
899904
$value_type = Type::combineUnionTypes($value_type, $value_type_part);
900905
}
901906
}
907+
} elseif ($iterator_atomic_type instanceof TGenericObject
908+
&& strtolower($iterator_atomic_type->value) === 'generator'
909+
) {
910+
$type_params = $iterator_atomic_type->type_params;
911+
if (isset($type_params[2]) && !$type_params[2]->isNullable() && !$type_params[2]->isMixed()) {
912+
$invalid_iterator_types[] = $iterator_atomic_type->getKey();
913+
} else {
914+
$has_valid_iterator = true;
915+
}
916+
917+
$iterator_value_type = self::getFakeMethodCallType(
918+
$statements_analyzer,
919+
$foreach_expr,
920+
$context,
921+
'current',
922+
);
923+
924+
$iterator_key_type = self::getFakeMethodCallType(
925+
$statements_analyzer,
926+
$foreach_expr,
927+
$context,
928+
'key',
929+
);
930+
931+
if ($iterator_value_type && !$iterator_value_type->isMixed()) {
932+
// remove null coming from current() to signify invalid iterations
933+
// we're in a foreach context, so we know we're not going iterate past the end
934+
if (isset($type_params[1]) && !$type_params[1]->isNullable()) {
935+
$iterator_value_type = $iterator_value_type->getBuilder();
936+
$iterator_value_type->removeType('null');
937+
$iterator_value_type = $iterator_value_type->freeze();
938+
}
939+
$value_type = Type::combineUnionTypes($value_type, $iterator_value_type);
940+
}
941+
942+
if ($iterator_key_type && !$iterator_key_type->isMixed()) {
943+
// remove null coming from key() to signify invalid iterations
944+
// we're in a foreach context, so we know we're not going iterate past the end
945+
if (isset($type_params[0]) && !$type_params[0]->isNullable()) {
946+
$iterator_key_type = $iterator_key_type->getBuilder();
947+
$iterator_key_type->removeType('null');
948+
$iterator_key_type = $iterator_key_type->freeze();
949+
}
950+
$key_type = Type::combineUnionTypes($key_type, $iterator_key_type);
951+
}
902952
} elseif ($codebase->classImplements(
903953
$iterator_atomic_type->value,
904954
'Iterator',
@@ -911,6 +961,7 @@ public static function handleIterable(
911961
)
912962
)
913963
) {
964+
$has_valid_iterator = true;
914965
$iterator_value_type = self::getFakeMethodCallType(
915966
$statements_analyzer,
916967
$foreach_expr,

‎tests/GeneratorTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,9 @@ function gen(): Generator {
217217
echo yield;
218218
}',
219219
],
220-
'yieldFromTwiceWithVoidSend' => [
220+
'SKIPPED-yieldFromTwiceWithVoidSend' => [
221221
'code' => '<?php
222+
// this test is all wrong
222223
/**
223224
* @return \Generator<int, string, void, string>
224225
*/

‎tests/Loop/ForeachTest.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,63 @@ function f(array $a): array {
11691169
}
11701170
PHP,
11711171
],
1172+
'generatorWithUnspecifiedSend' => [
1173+
'code' => <<<'PHP'
1174+
<?php
1175+
/** @return Generator<int,int> */
1176+
function gen() : Generator {
1177+
return yield 1;
1178+
}
1179+
$gen = gen();
1180+
foreach ($gen as $i) {}
1181+
PHP,
1182+
],
1183+
'generatorWithMixedSend' => [
1184+
'code' => <<<'PHP'
1185+
<?php
1186+
/** @return Generator<int,int, mixed, mixed> */
1187+
function gen() : Generator {
1188+
return yield 1;
1189+
}
1190+
$gen = gen();
1191+
foreach ($gen as $i) {}
1192+
PHP,
1193+
],
1194+
'nullableGenerator' => [
1195+
'code' => <<<'PHP'
1196+
<?php
1197+
/** @return Generator<int,int|null> */
1198+
function gen() : Generator {
1199+
yield null;
1200+
yield 1;
1201+
}
1202+
$gen = gen();
1203+
$a = "";
1204+
foreach ($gen as $i) {
1205+
$a = $i;
1206+
}
1207+
PHP,
1208+
'assertions' => [
1209+
'$a===' => "''|int|null",
1210+
],
1211+
],
1212+
'nonNullableGenerator' => [
1213+
'code' => <<<'PHP'
1214+
<?php
1215+
/** @return Generator<int,int> */
1216+
function gen() : Generator {
1217+
yield 1;
1218+
}
1219+
$gen = gen();
1220+
$a = "";
1221+
foreach ($gen as $i) {
1222+
$a = $i;
1223+
}
1224+
PHP,
1225+
'assertions' => [
1226+
'$a===' => "''|int",
1227+
],
1228+
],
11721229
];
11731230
}
11741231

@@ -1395,6 +1452,18 @@ function f(array $a): array {
13951452
PHP,
13961453
'error_message' => 'LessSpecificReturnStatement',
13971454
],
1455+
'generatorWithNonNullableSend' => [
1456+
'code' => <<<'PHP'
1457+
<?php
1458+
/** @return Generator<int,int,string,string> */
1459+
function gen() : Generator {
1460+
return yield 1;
1461+
}
1462+
$gen = gen();
1463+
foreach ($gen as $i) {}
1464+
PHP,
1465+
'error_message' => 'InvalidIterator',
1466+
],
13981467
];
13991468
}
14001469
}

0 commit comments

Comments
 (0)