From 6ead4dda8c8b7f277466b374252970bdda8ab3e7 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 10:55:33 +0100 Subject: [PATCH 1/6] Describe Traversable --- src/Type/ObjectType.php | 32 ++++++----------- src/Type/UnionType.php | 29 +++++++++++----- .../Rules/Methods/ReturnTypeRuleTest.php | 5 +++ .../PHPStan/Rules/Methods/data/bug-12102.php | 34 +++++++++++++++++++ 4 files changed, 70 insertions(+), 30 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-12102.php diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index a51539df05..741db8005f 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -5,11 +5,6 @@ use ArrayAccess; use Closure; use Countable; -use DateTime; -use DateTimeImmutable; -use DateTimeInterface; -use Error; -use Exception; use Iterator; use IteratorAggregate; use PHPStan\Analyser\OutOfClassScope; @@ -1560,23 +1555,16 @@ private function getInterfaces(): array public function tryRemove(Type $typeToRemove): ?Type { - if ($this->getClassName() === DateTimeInterface::class) { - if ($typeToRemove instanceof ObjectType && $typeToRemove->getClassName() === DateTimeImmutable::class) { - return new ObjectType(DateTime::class); - } - - if ($typeToRemove instanceof ObjectType && $typeToRemove->getClassName() === DateTime::class) { - return new ObjectType(DateTimeImmutable::class); - } - } - - if ($this->getClassName() === Throwable::class) { - if ($typeToRemove instanceof ObjectType && $typeToRemove->getClassName() === Error::class) { - return new ObjectType(Exception::class); // phpcs:ignore SlevomatCodingStandard.Exceptions.ReferenceThrowableOnly.ReferencedGeneralException - } - - if ($typeToRemove instanceof ObjectType && $typeToRemove->getClassName() === Exception::class) { // phpcs:ignore SlevomatCodingStandard.Exceptions.ReferenceThrowableOnly.ReferencedGeneralException - return new ObjectType(Error::class); + foreach (UnionType::EQUAL_UNION_CLASSES as $baseClass => $classes) { + if ($this->getClassName() === $baseClass && $typeToRemove instanceof ObjectType) { + foreach ($classes as $index => $class) { + if ($typeToRemove->getClassName() === $class) { + unset($classes[$index]); + return TypeCombinator::union( + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes) + ); + } + } } } diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index 1505fa1bb2..6157c16810 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -5,6 +5,10 @@ use DateTime; use DateTimeImmutable; use DateTimeInterface; +use Error; +use Exception; +use Iterator; +use IteratorAggregate; use PHPStan\Php\PhpVersion; use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode; @@ -26,6 +30,8 @@ use PHPStan\Type\Generic\TemplateTypeVariance; use PHPStan\Type\Generic\TemplateUnionType; use PHPStan\Type\Traits\NonGeneralizableTypeTrait; +use Throwable; +use Traversable; use function array_diff_assoc; use function array_fill_keys; use function array_map; @@ -45,6 +51,12 @@ class UnionType implements CompoundType use NonGeneralizableTypeTrait; + public const EQUAL_UNION_CLASSES = [ + DateTimeInterface::class => [DateTimeImmutable::class, DateTime::class], + Throwable::class => [Error::class, Exception::class], + Traversable::class => [IteratorAggregate::class, Iterator::class], + ]; + private bool $sortedTypes = false; /** @var array */ @@ -183,14 +195,15 @@ public function getConstantStrings(): array public function accepts(Type $type, bool $strictTypes): AcceptsResult { - if ( - $type->equals(new ObjectType(DateTimeInterface::class)) - && $this->accepts( - new UnionType([new ObjectType(DateTime::class), new ObjectType(DateTimeImmutable::class)]), - $strictTypes, - )->yes() - ) { - return AcceptsResult::createYes(); + foreach (self::EQUAL_UNION_CLASSES as $baseClass => $classes) { + if ($type->equals(new ObjectType($baseClass))) { + $union = TypeCombinator::union( + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes) + ); + if ($this->accepts($union, $strictTypes)->yes()) { + return AcceptsResult::createYes(); + } + } } $result = AcceptsResult::createNo(); diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index f60107aa79..c812661e67 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1096,4 +1096,9 @@ public function testBug11857(): void $this->analyse([__DIR__ . '/data/bug-11857-builder.php'], []); } + public function testBug12102(): void + { + $this->analyse([__DIR__ . '/data/bug-12102.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-12102.php b/tests/PHPStan/Rules/Methods/data/bug-12102.php new file mode 100644 index 0000000000..ca594cd653 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12102.php @@ -0,0 +1,34 @@ + $traversable */ + public function sayHello(Traversable $traversable): ?Iterator + { + if (!$traversable instanceof IteratorAggregate) { + return $traversable; + } + + return null; + } + + /** @param iterable $iterable */ + public function sayHello2(iterable $iterable): ?Iterator + { + if (\is_array($iterable)) { + return null; + } + + if (!$iterable instanceof IteratorAggregate) { + return $iterable; + } + + return null; + } +} From c4ede85aa561571f48af4f9d8e0f759ee228d316 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 11:01:26 +0100 Subject: [PATCH 2/6] Fix --- phpstan-baseline.neon | 2 +- src/Type/ObjectType.php | 19 ++++++++++--------- src/Type/UnionType.php | 19 ++++++++++--------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3b47d2e5fc..d3e9a54a9d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1422,7 +1422,7 @@ parameters: - message: '#^Doing instanceof PHPStan\\Type\\ObjectType is error\-prone and deprecated\. Use Type\:\:isObject\(\) or Type\:\:getObjectClassNames\(\) instead\.$#' identifier: phpstanApi.instanceofType - count: 6 + count: 3 path: src/Type/ObjectType.php - diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index 741db8005f..2d3912f684 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -41,7 +41,6 @@ use PHPStan\Type\Traits\NonGeneralizableTypeTrait; use PHPStan\Type\Traits\NonGenericTypeTrait; use PHPStan\Type\Traits\UndecidedComparisonTypeTrait; -use Throwable; use Traversable; use function array_key_exists; use function array_map; @@ -1556,14 +1555,16 @@ private function getInterfaces(): array public function tryRemove(Type $typeToRemove): ?Type { foreach (UnionType::EQUAL_UNION_CLASSES as $baseClass => $classes) { - if ($this->getClassName() === $baseClass && $typeToRemove instanceof ObjectType) { - foreach ($classes as $index => $class) { - if ($typeToRemove->getClassName() === $class) { - unset($classes[$index]); - return TypeCombinator::union( - ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes) - ); - } + if ($this->getClassName() !== $baseClass || !($typeToRemove instanceof ObjectType)) { + continue; + } + + foreach ($classes as $index => $class) { + if ($typeToRemove->getClassName() === $class) { + unset($classes[$index]); + return TypeCombinator::union( + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes), + ); } } } diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index 6157c16810..5c781e175a 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -6,7 +6,6 @@ use DateTimeImmutable; use DateTimeInterface; use Error; -use Exception; use Iterator; use IteratorAggregate; use PHPStan\Php\PhpVersion; @@ -53,7 +52,7 @@ class UnionType implements CompoundType public const EQUAL_UNION_CLASSES = [ DateTimeInterface::class => [DateTimeImmutable::class, DateTime::class], - Throwable::class => [Error::class, Exception::class], + Throwable::class => [Error::class, Throwable::class], Traversable::class => [IteratorAggregate::class, Iterator::class], ]; @@ -196,13 +195,15 @@ public function getConstantStrings(): array public function accepts(Type $type, bool $strictTypes): AcceptsResult { foreach (self::EQUAL_UNION_CLASSES as $baseClass => $classes) { - if ($type->equals(new ObjectType($baseClass))) { - $union = TypeCombinator::union( - ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes) - ); - if ($this->accepts($union, $strictTypes)->yes()) { - return AcceptsResult::createYes(); - } + if (!$type->equals(new ObjectType($baseClass))) { + continue; + } + + $union = TypeCombinator::union( + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes), + ); + if ($this->accepts($union, $strictTypes)->yes()) { + return AcceptsResult::createYes(); } } From 4df65659746663b002cff4d1fc1b5ae9e817a89f Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 11:19:48 +0100 Subject: [PATCH 3/6] Improve --- src/Type/ObjectType.php | 23 +++++++++++++---------- src/Type/UnionType.php | 1 + 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index 2d3912f684..36e06201ad 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -1554,17 +1554,20 @@ private function getInterfaces(): array public function tryRemove(Type $typeToRemove): ?Type { - foreach (UnionType::EQUAL_UNION_CLASSES as $baseClass => $classes) { - if ($this->getClassName() !== $baseClass || !($typeToRemove instanceof ObjectType)) { - continue; - } + if ($typeToRemove instanceof ObjectType) { + foreach (UnionType::EQUAL_UNION_CLASSES as $baseClass => $classes) { + if ($this->getClassName() !== $baseClass) { + continue; + } - foreach ($classes as $index => $class) { - if ($typeToRemove->getClassName() === $class) { - unset($classes[$index]); - return TypeCombinator::union( - ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes), - ); + foreach ($classes as $index => $class) { + if ($typeToRemove->getClassName() === $class) { + unset($classes[$index]); + + return TypeCombinator::union( + ...array_map(static fn(string $objectClass): Type => new ObjectType($objectClass), $classes), + ); + } } } } diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index 5c781e175a..e099e862d2 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -205,6 +205,7 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult if ($this->accepts($union, $strictTypes)->yes()) { return AcceptsResult::createYes(); } + break; } $result = AcceptsResult::createNo(); From 8d1dead8e9b5cb872f8f48becb45ad76ceab3603 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 12:06:26 +0100 Subject: [PATCH 4/6] Fix --- src/Type/UnionType.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index e099e862d2..134942e660 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -6,6 +6,7 @@ use DateTimeImmutable; use DateTimeInterface; use Error; +use Exception; use Iterator; use IteratorAggregate; use PHPStan\Php\PhpVersion; @@ -52,7 +53,7 @@ class UnionType implements CompoundType public const EQUAL_UNION_CLASSES = [ DateTimeInterface::class => [DateTimeImmutable::class, DateTime::class], - Throwable::class => [Error::class, Throwable::class], + Throwable::class => [Error::class, Exception::class], // phpcs:ignore SlevomatCodingStandard.Exceptions.ReferenceThrowableOnly.ReferencedGeneralException Traversable::class => [IteratorAggregate::class, Iterator::class], ]; From 84030b29dbe66c87204866fb0c8ae3d6a6ac9f7f Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 12:21:16 +0100 Subject: [PATCH 5/6] Fix --- src/Type/ObjectType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index 36e06201ad..3290fb97c7 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -1565,7 +1565,7 @@ public function tryRemove(Type $typeToRemove): ?Type unset($classes[$index]); return TypeCombinator::union( - ...array_map(static fn(string $objectClass): Type => new ObjectType($objectClass), $classes), + ...array_map(static fn (string $objectClass): Type => new ObjectType($objectClass), $classes), ); } } From b897b8d98d79cf634ad7cdb20887f8d16c008ba2 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 22 Nov 2024 15:03:44 +0100 Subject: [PATCH 6/6] Remove iterator part --- src/Type/UnionType.php | 4 --- .../Rules/Methods/ReturnTypeRuleTest.php | 5 --- .../PHPStan/Rules/Methods/data/bug-12102.php | 34 ------------------- 3 files changed, 43 deletions(-) delete mode 100644 tests/PHPStan/Rules/Methods/data/bug-12102.php diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index 134942e660..5d7627b306 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -7,8 +7,6 @@ use DateTimeInterface; use Error; use Exception; -use Iterator; -use IteratorAggregate; use PHPStan\Php\PhpVersion; use PHPStan\PhpDocParser\Ast\Type\TypeNode; use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode; @@ -31,7 +29,6 @@ use PHPStan\Type\Generic\TemplateUnionType; use PHPStan\Type\Traits\NonGeneralizableTypeTrait; use Throwable; -use Traversable; use function array_diff_assoc; use function array_fill_keys; use function array_map; @@ -54,7 +51,6 @@ class UnionType implements CompoundType public const EQUAL_UNION_CLASSES = [ DateTimeInterface::class => [DateTimeImmutable::class, DateTime::class], Throwable::class => [Error::class, Exception::class], // phpcs:ignore SlevomatCodingStandard.Exceptions.ReferenceThrowableOnly.ReferencedGeneralException - Traversable::class => [IteratorAggregate::class, Iterator::class], ]; private bool $sortedTypes = false; diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index c812661e67..f60107aa79 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1096,9 +1096,4 @@ public function testBug11857(): void $this->analyse([__DIR__ . '/data/bug-11857-builder.php'], []); } - public function testBug12102(): void - { - $this->analyse([__DIR__ . '/data/bug-12102.php'], []); - } - } diff --git a/tests/PHPStan/Rules/Methods/data/bug-12102.php b/tests/PHPStan/Rules/Methods/data/bug-12102.php deleted file mode 100644 index ca594cd653..0000000000 --- a/tests/PHPStan/Rules/Methods/data/bug-12102.php +++ /dev/null @@ -1,34 +0,0 @@ - $traversable */ - public function sayHello(Traversable $traversable): ?Iterator - { - if (!$traversable instanceof IteratorAggregate) { - return $traversable; - } - - return null; - } - - /** @param iterable $iterable */ - public function sayHello2(iterable $iterable): ?Iterator - { - if (\is_array($iterable)) { - return null; - } - - if (!$iterable instanceof IteratorAggregate) { - return $iterable; - } - - return null; - } -}