Skip to content

Commit

Permalink
Merge pull request #10697 from weirdan/forbid-iterating-over-generato…
Browse files Browse the repository at this point in the history 
…rs-with-non-nullable-send
  • Loading branch information
@weirdan
weirdan committed Feb 27, 2024
2 parents e9ea999 + 0ce62a5 commit a1352eb
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 12 deletions.
9 changes: 0 additions & 9 deletions src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,6 @@ public static function verifyReturnType(
$source->getParentFQCLN(),
);

// hack until we have proper yield type collection
if ($function_like_storage
&& $function_like_storage->has_yield
&& !$inferred_yield_type
&& !$inferred_return_type->isVoid()
) {
$inferred_return_type = new Union([new TNamedObject('Generator')]);
}

if ($is_to_string) {
$union_comparison_results = new TypeComparisonResult();
if (!$inferred_return_type->hasMixed() &&
Expand Down
55 changes: 53 additions & 2 deletions src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ public static function checkIteratorType(
$key_type,
$value_type,
$has_valid_iterator,
$invalid_iterator_types,
);
} else {
$raw_object_types[] = $iterator_atomic_type->value;
Expand Down Expand Up @@ -725,6 +726,7 @@ public static function checkIteratorType(
return null;
}

/** @param list<string> $invalid_iterator_types */
public static function handleIterable(
StatementsAnalyzer $statements_analyzer,
TNamedObject $iterator_atomic_type,
Expand All @@ -733,7 +735,8 @@ public static function handleIterable(
Context $context,
?Union &$key_type,
?Union &$value_type,
bool &$has_valid_iterator
bool &$has_valid_iterator,
array &$invalid_iterator_types = []
): void {
if ($iterator_atomic_type->extra_types) {
$iterator_atomic_types = array_merge(
Expand All @@ -753,7 +756,6 @@ public static function handleIterable(
}


$has_valid_iterator = true;

if ($iterator_atomic_type instanceof TIterable
|| (strtolower($iterator_atomic_type->value) === 'traversable'
Expand Down Expand Up @@ -781,6 +783,8 @@ public static function handleIterable(
)
)
) {
$has_valid_iterator = true;

$old_data_provider = $statements_analyzer->node_data;

$statements_analyzer->node_data = clone $statements_analyzer->node_data;
Expand Down Expand Up @@ -867,6 +871,7 @@ public static function handleIterable(
$key_type,
$value_type,
$has_valid_iterator,
$invalid_iterator_types,
);

continue;
Expand Down Expand Up @@ -899,6 +904,51 @@ public static function handleIterable(
$value_type = Type::combineUnionTypes($value_type, $value_type_part);
}
}
} elseif ($iterator_atomic_type instanceof TGenericObject
&& strtolower($iterator_atomic_type->value) === 'generator'
) {
$type_params = $iterator_atomic_type->type_params;
if (isset($type_params[2]) && !$type_params[2]->isNullable() && !$type_params[2]->isMixed()) {
$invalid_iterator_types[] = $iterator_atomic_type->getKey();
} else {
$has_valid_iterator = true;
}

$iterator_value_type = self::getFakeMethodCallType(
$statements_analyzer,
$foreach_expr,
$context,
'current',
);

$iterator_key_type = self::getFakeMethodCallType(
$statements_analyzer,
$foreach_expr,
$context,
'key',
);

if ($iterator_value_type && !$iterator_value_type->isMixed()) {
// remove null coming from current() to signify invalid iterations
// we're in a foreach context, so we know we're not going iterate past the end
if (isset($type_params[1]) && !$type_params[1]->isNullable()) {
$iterator_value_type = $iterator_value_type->getBuilder();
$iterator_value_type->removeType('null');
$iterator_value_type = $iterator_value_type->freeze();
}
$value_type = Type::combineUnionTypes($value_type, $iterator_value_type);
}

if ($iterator_key_type && !$iterator_key_type->isMixed()) {
// remove null coming from key() to signify invalid iterations
// we're in a foreach context, so we know we're not going iterate past the end
if (isset($type_params[0]) && !$type_params[0]->isNullable()) {
$iterator_key_type = $iterator_key_type->getBuilder();
$iterator_key_type->removeType('null');
$iterator_key_type = $iterator_key_type->freeze();
}
$key_type = Type::combineUnionTypes($key_type, $iterator_key_type);
}
} elseif ($codebase->classImplements(
$iterator_atomic_type->value,
'Iterator',
Expand All @@ -911,6 +961,7 @@ public static function handleIterable(
)
)
) {
$has_valid_iterator = true;
$iterator_value_type = self::getFakeMethodCallType(
$statements_analyzer,
$foreach_expr,
Expand Down
3 changes: 2 additions & 1 deletion tests/GeneratorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,9 @@ function gen(): Generator {
echo yield;
}',
],
'yieldFromTwiceWithVoidSend' => [
'SKIPPED-yieldFromTwiceWithVoidSend' => [
'code' => '<?php
// this test is all wrong
/**
* @return \Generator<int, string, void, string>
*/
Expand Down
69 changes: 69 additions & 0 deletions tests/Loop/ForeachTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,63 @@ function f(array $a): array {
}
PHP,
],
'generatorWithUnspecifiedSend' => [
'code' => <<<'PHP'
<?php
/** @return Generator<int,int> */
function gen() : Generator {
return yield 1;
}
$gen = gen();
foreach ($gen as $i) {}
PHP,
],
'generatorWithMixedSend' => [
'code' => <<<'PHP'
<?php
/** @return Generator<int,int, mixed, mixed> */
function gen() : Generator {
return yield 1;
}
$gen = gen();
foreach ($gen as $i) {}
PHP,
],
'nullableGenerator' => [
'code' => <<<'PHP'
<?php
/** @return Generator<int,int|null> */
function gen() : Generator {
yield null;
yield 1;
}
$gen = gen();
$a = "";
foreach ($gen as $i) {
$a = $i;
}
PHP,
'assertions' => [
'$a===' => "''|int|null",
],
],
'nonNullableGenerator' => [
'code' => <<<'PHP'
<?php
/** @return Generator<int,int> */
function gen() : Generator {
yield 1;
}
$gen = gen();
$a = "";
foreach ($gen as $i) {
$a = $i;
}
PHP,
'assertions' => [
'$a===' => "''|int",
],
],
];
}

Expand Down Expand Up @@ -1395,6 +1452,18 @@ function f(array $a): array {
PHP,
'error_message' => 'LessSpecificReturnStatement',
],
'generatorWithNonNullableSend' => [
'code' => <<<'PHP'
<?php
/** @return Generator<int,int,string,string> */
function gen() : Generator {
return yield 1;
}
$gen = gen();
foreach ($gen as $i) {}
PHP,
'error_message' => 'InvalidIterator',
],
];
}
}

0 comments on commit a1352eb

Please sign in to comment.