Skip to content

Commit a2acf64

Browse files
ondrejmirtesondrejmirtes
committed
Fix iterable key and value type - prefer generic type variables instead of key()/current() typehints
1 parent 6dc7e3a commit a2acf64

File tree

3 files changed

+171
-16
lines changed

3 files changed

+171
-16
lines changed

‎src/Type/ObjectType.php

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -680,11 +680,27 @@ public function isIterableAtLeastOnce(): TrinaryLogic
680680

681681
public function getIterableKeyType(): Type
682682
{
683-
$classReflection = $this->getClassReflection();
684-
if ($classReflection === null) {
685-
return new ErrorType();
683+
$isTraversable = false;
684+
if ($this->isInstanceOf(Traversable::class)->yes()) {
685+
$isTraversable = true;
686+
$tKey = GenericTypeVariableResolver::getType($this, Traversable::class, 'TKey');
687+
if ($tKey !== null) {
688+
if (!$tKey instanceof MixedType || $tKey->isExplicitMixed()) {
689+
$classReflection = $this->getClassReflection();
690+
if ($classReflection === null) {
691+
return $tKey;
692+
}
693+
694+
return TypeTraverser::map($tKey, static function (Type $type, callable $traverse) use ($classReflection): Type {
695+
if ($type instanceof StaticType) {
696+
return $type->changeBaseClass($classReflection)->getStaticObjectType();
697+
}
698+
699+
return $traverse($type);
700+
});
701+
}
702+
}
686703
}
687-
688704
if ($this->isInstanceOf(Iterator::class)->yes()) {
689705
return RecursionGuard::run($this, fn (): Type => ParametersAcceptorSelector::selectSingle(
690706
$this->getMethod('key', new OutOfClassScope())->getVariants(),
@@ -695,17 +711,13 @@ public function getIterableKeyType(): Type
695711
$keyType = RecursionGuard::run($this, fn (): Type => ParametersAcceptorSelector::selectSingle(
696712
$this->getMethod('getIterator', new OutOfClassScope())->getVariants(),
697713
)->getReturnType()->getIterableKeyType());
714+
$isTraversable = true;
698715
if (!$keyType instanceof MixedType || $keyType->isExplicitMixed()) {
699716
return $keyType;
700717
}
701718
}
702719

703-
if ($this->isInstanceOf(Traversable::class)->yes()) {
704-
$tKey = GenericTypeVariableResolver::getType($this, Traversable::class, 'TKey');
705-
if ($tKey !== null) {
706-
return $tKey;
707-
}
708-
720+
if ($isTraversable) {
709721
return new MixedType();
710722
}
711723

@@ -714,6 +726,28 @@ public function getIterableKeyType(): Type
714726

715727
public function getIterableValueType(): Type
716728
{
729+
$isTraversable = false;
730+
if ($this->isInstanceOf(Traversable::class)->yes()) {
731+
$isTraversable = true;
732+
$tValue = GenericTypeVariableResolver::getType($this, Traversable::class, 'TValue');
733+
if ($tValue !== null) {
734+
if (!$tValue instanceof MixedType || $tValue->isExplicitMixed()) {
735+
$classReflection = $this->getClassReflection();
736+
if ($classReflection === null) {
737+
return $tValue;
738+
}
739+
740+
return TypeTraverser::map($tValue, static function (Type $type, callable $traverse) use ($classReflection): Type {
741+
if ($type instanceof StaticType) {
742+
return $type->changeBaseClass($classReflection)->getStaticObjectType();
743+
}
744+
745+
return $traverse($type);
746+
});
747+
}
748+
}
749+
}
750+
717751
if ($this->isInstanceOf(Iterator::class)->yes()) {
718752
return RecursionGuard::run($this, fn (): Type => ParametersAcceptorSelector::selectSingle(
719753
$this->getMethod('current', new OutOfClassScope())->getVariants(),
@@ -724,17 +758,13 @@ public function getIterableValueType(): Type
724758
$valueType = RecursionGuard::run($this, fn (): Type => ParametersAcceptorSelector::selectSingle(
725759
$this->getMethod('getIterator', new OutOfClassScope())->getVariants(),
726760
)->getReturnType()->getIterableValueType());
761+
$isTraversable = true;
727762
if (!$valueType instanceof MixedType || $valueType->isExplicitMixed()) {
728763
return $valueType;
729764
}
730765
}
731766

732-
if ($this->isInstanceOf(Traversable::class)->yes()) {
733-
$tValue = GenericTypeVariableResolver::getType($this, Traversable::class, 'TValue');
734-
if ($tValue !== null) {
735-
return $tValue;
736-
}
737-
767+
if ($isTraversable) {
738768
return new MixedType();
739769
}
740770

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,7 @@ public function dataFileAsserts(): iterable
626626
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6404.php');
627627
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6399.php');
628628
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4357.php');
629+
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5817.php');
629630
}
630631

631632
/**

‎tests/PHPStan/Analyser/data/bug-5817.php

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug5817;
6+
7+
use ArrayAccess;
8+
use Countable;
9+
use DateTimeInterface;
10+
use Iterator;
11+
use JsonSerializable;
12+
13+
use function count;
14+
use function current;
15+
use function key;
16+
use function next;
17+
use function PHPStan\Testing\assertType;
18+
19+
/**
20+
* @implements ArrayAccess<int, DateTimeInterface>
21+
* @implements Iterator<int, DateTimeInterface>
22+
*/
23+
class MyContainer implements
24+
ArrayAccess,
25+
Countable,
26+
Iterator,
27+
JsonSerializable
28+
{
29+
/** @var array<int, DateTimeInterface> */
30+
protected array $items = [];
31+
32+
public function add(DateTimeInterface $item, int $offset = null): self
33+
{
34+
$this->offsetSet($offset, $item);
35+
return $this;
36+
}
37+
38+
public function count(): int
39+
{
40+
return count($this->items);
41+
}
42+
43+
/** @return DateTimeInterface|false */
44+
public function current()
45+
{
46+
return current($this->items);
47+
}
48+
49+
/** @return DateTimeInterface|false */
50+
public function next()
51+
{
52+
return next($this->items);
53+
}
54+
55+
/** @return int|null */
56+
public function key(): ?int
57+
{
58+
return key($this->items);
59+
}
60+
61+
public function valid(): bool
62+
{
63+
return $this->key() !== null;
64+
}
65+
66+
/** @return DateTimeInterface|false */
67+
public function rewind()
68+
{
69+
return reset($this->items);
70+
}
71+
72+
/** @param mixed $offset */
73+
public function offsetExists($offset): bool
74+
{
75+
return isset($this->items[$offset]);
76+
}
77+
78+
/** @param mixed $offset */
79+
public function offsetGet($offset): ?DateTimeInterface
80+
{
81+
return $this->items[$offset] ?? null;
82+
}
83+
84+
/**
85+
* @param mixed $offset
86+
* @param mixed $value
87+
*/
88+
public function offsetSet($offset, $value): void
89+
{
90+
assert($value instanceof DateTimeInterface);
91+
if ($offset === null) { // append
92+
$this->items[] = $value;
93+
} else {
94+
$this->items[$offset] = $value;
95+
}
96+
}
97+
98+
/** @param mixed $offset */
99+
public function offsetUnset($offset): void
100+
{
101+
unset($this->items[$offset]);
102+
}
103+
104+
/** @return DateTimeInterface[] */
105+
public function jsonSerialize(): array
106+
{
107+
return $this->items;
108+
}
109+
}
110+
111+
class Foo
112+
{
113+
114+
public function doFoo()
115+
{
116+
$container = (new MyContainer())->add(new \DateTimeImmutable());
117+
118+
foreach ($container as $k => $item) {
119+
assertType('int', $k);
120+
assertType(DateTimeInterface::class, $item);
121+
}
122+
}
123+
124+
}

0 commit comments

Comments
 (0)