-
-
Notifications
You must be signed in to change notification settings - Fork 431
Fix adding string[] in explode() for ClassMethodArrayDocblockParamFromLocalCallsRector #7331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
TomasVotruba
commented
Sep 25, 2025
- add fixture
- fix adding string[] in explode() for ClassMethodArrayDocblockParamFromLocalCallsRector
| } | ||
| } | ||
|
|
||
| // correction for explode() that always returns array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumption is only true for php8+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is getType(), not getNativeType(), I guess it ok for most usage that allow based on docblock, but I think we can add check PhpVersionProvider->isAtLeastPhpVersion() for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why rector re-implements the type inference for such functions instead of using return-type extensions from PHPStan?
(Which would already do php version specific types)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, looking for fixture, use for docblock array iterable value type reason.
Other type resolver and corrector are for common resolve type that are needed, eg Intersection string to string, Intersection array to array, also node verify by its attribute, eg, Name with its attribute, eg
rector-src/src/NodeTypeResolver/NodeTypeResolver/NameTypeResolver.php
Lines 41 to 43 in fe64244
| if (! $node instanceof FullyQualified && $node->hasAttribute(AttributeKey::NAMESPACED_NAME)) { | |
| return $this->resolve(new FullyQualified($node->getAttribute(AttributeKey::NAMESPACED_NAME))); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked locally, without this, it seems got mixed[] on explode() function:
- * @param string[] $items
+ * @param mixed[] $itemsWhen using scope->getType(), it got Intersection:
PHPStan\Type\IntersectionType #11668
types: array (3)
| 0 => PHPStan\Type\ArrayType #13424
| | itemType: PHPStan\Type\IntersectionType #11020
| | | types: array (3)
| | | | 0 => PHPStan\Type\Accessory\AccessoryLowercaseStringType #11687
| | | | 1 => PHPStan\Type\Accessory\AccessoryUppercaseStringType #11035
| | | | 2 => PHPStan\Type\StringType #10771
| | | sortedTypes: false
| | keyType: PHPStan\Type\IntegerRangeType #11763
| | | min: 0
| | | max: null
| 1 => PHPStan\Type\Accessory\AccessoryArrayListType #12617
| 2 => PHPStan\Type\Accessory\NonEmptyArrayType #10765
sortedTypes: false
It possibly can be handled in new corrector, eg: AccessoryNonEmptyArrayTypeCorrector, I will create new PR if this can be handled there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.