Skip RemoveUnusedRequestParamRector on override abstract method - #394
Conversation
|
Fixes #366 |
|
I dont know how to fix the rector failing, that has probably nothing to do with my changes. The PHPstan fails because undefined method but it does work can i add it to the ignores? |
|
@samsonasik It seems the BetterTokenIterator::getTokens() was removed. Do you have any idea how this happened? |
|
Latest php-doc-parser already have it, so I removed it as it extends the
php-doc-parser may need to be updated to at least version |
|
I create PR to ensure rector-src got latest phpdoc-parser when loaded rectorphp/rector-src#3691 |
|
Ah, I see now 😁 |
|
Let's restart build. |
|
I will look into why it not load latest phpdoc-parser, @JohJohan I may cherry-pick your commit in case I can't push to your branch :) |
Sounds good, good luck :) |
|
It seems the autoload is overlapped with phpstan.phar's inside build release 1.10.4 which still uses php-doc-parser pinned to rector-src provide preload.php and preload-split-package.php, that's why it works there, I will check if it can be tweaked here :) |
|
Latest phpstan's dev-main already uses pinned php-doc-parser 1.20.3 https://github.com/phpstan/phpstan-src/blob/14f56abbd40a172259a6010820dc7e613ce7a516/composer.json#L28 kind of aliasing can work, but I will check if there is better solution. |
|
Let's restart build ... |
|
@JohJohan I fixed the rector error 🎉 , I will let you fix the phpstan notice part :) |
| - '#Parameter \#1 \$className of method Rector\\Core\\Reflection\\ReflectionResolver\:\:resolveMethodReflection\(\) expects class\-string, string given#' | ||
|
|
||
| - | ||
| message: '#Call to an undefined method PHPStan\\Reflection\\ExtendedMethodReflection\:\:isAbstract\(\)#' |
There was a problem hiding this comment.
This should be fixed with set actual object with isAbstract() method, you can debug actual object with dump($variable)
There was a problem hiding this comment.
The actual class is PHPStan\Reflection\Php\PhpMethodReflection which has isAbstract but when i do something like assert($extendedMethodReflection instanceof PHPStan\Reflection\Php\PhpMethodReflection); i get error Class Rector\Symfony\Rector\ClassMethod\PHPStan\Reflection\Php\PhpMethodReflection not found.
There was a problem hiding this comment.
You need to use FQCN, or that will refer current namespace:
-assert($extendedMethodReflection instanceof PHPStan\Reflection\Php\PhpMethodReflection);
+assert($extendedMethodReflection instanceof \PHPStan\Reflection\Php\PhpMethodReflection);
samsonasik
left a comment
samsonasik
left a comment
There was a problem hiding this comment.
Please update PR title to something like:
Skip RemoveUnusedRequestParamRector on override abstract method
| try { | ||
| $extendedMethodReflection = $reflectionParentClass->getNativeMethod($methodName); | ||
| assert($extendedMethodReflection instanceof PhpMethodReflection); | ||
| if (! $extendedMethodReflection->isAbstract()) { |
There was a problem hiding this comment.
There is already service to verify it, see
There was a problem hiding this comment.
Nice find, i thought there must have been something but i couldn't find it i keep my private function to do the reflection and checking see hasAbstractParentClassMethod is that okay?
There was a problem hiding this comment.
That's fine as you need to get ClassReflection instance 👍
| abstract public function fooAction(Request $request): Response; | ||
| } | ||
|
|
||
| class DemoController extends AbstractController |
There was a problem hiding this comment.
It needs to be more specific name so it ease to look on the future, eg:
| class DemoController extends AbstractController | |
| class DontOverrideParentAbstractClassMethodController extends AbstractController |
| @@ -0,0 +1,55 @@ | |||
| <?php | |||
There was a problem hiding this comment.
please rename fixture file to something like: do_not_override_parent_abstract_class_method.php.inc
# Failing Test for RemoveUnusedRequestParamRector
|
Thank you @JohJohan |
…ectorphp#394) # Failing Test for RemoveUnusedRequestParamRector Co-authored-by: johan Vlaar <johan@adivare.nl>
3 participants
Fixes #366