-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
TASK: Migrate to PHPStan for Flow 8 #3218
Conversation
8e188c4
to
0e6568a
Compare
We would need to adjust https://github.com/neos/flow-development-distribution to provide phpstan |
Neos.Flow/Classes/Reflection/Exception/ClassLoadingForReflectionFailedException.php
Outdated
Show resolved
Hide resolved
e6c795c
to
3e1ba73
Compare
Neos.Flow/Classes/Security/Authorization/Interceptor/RequireAuthentication.php
Show resolved
Hide resolved
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.
.
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.
Mostly comments, but one real change request for the ResourceCommandController
Neos.Flow/Classes/Aop/Builder/AdvicedConstructorInterceptorBuilder.php
Outdated
Show resolved
Hide resolved
Neos.Flow/Classes/Security/Authorization/Interceptor/RequireAuthentication.php
Show resolved
Hide resolved
536cccc
to
49ab156
Compare
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 think I am fine with this overall for now, thanks for putting the work in!
Neos.Flow/Classes/Aop/Builder/AdvicedConstructorInterceptorBuilder.php
Outdated
Show resolved
Hide resolved
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.
Yeah, why not.
… but fix the PHPStan complaints first. 🤣 |
…uctor ... as flows proxy building doesnt like it when on multiple level of inheritance: Cannot override final method Neos\Flow\Security\Authentication\Provider\AbstractProvider::__construct() in /home/runner/work/flow-development-collection/flow-development-collection/flow-development-distribution/Data/Temporary/Testing/Cache/Code/Flow_Object_Classes/Neos_Flow_Security_Authentication_Provider_TestingProvider.php on line 130 see also https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static
The method `buildMethodParametersCode` does not exist and most likely `$proxyMethod->buildMethodParametersCode` is meant. > This dates back to Andi and I believe the solution is as simple as this being > $proxyMethod->setMethodParametersCode($proxyMethod->buildMethodParametersCode($declaringClassName, $methodName, true)); > Which becomes even clearer looking at the sibling code in AdvicedMethodInterceptorBuilder where it is like I suggested #3218 (comment)
49ab156
to
31f5dff
Compare
Thank you for your quick reviews ❤️ |
- fix types where neos depends on it (by correcting types and adding `never`) - adjust unit test as `never` cannot be doubled eventually this will be fixed via: sebastianbergmann/phpunit#5048 - fix ci and style as neos 9 followup for #3218
With #3218 PHPStan level 1 was added to the whole Flow code base and CI for Flow 8. This upmerged change needs some adjustments to pass the CI in Flow 9 - fix types and remove deprecated code - fix types where neos depends on it (by correcting types and adding `never`) - adjust unit test as `never` cannot be doubled (eventually this will be fixed via: sebastianbergmann/phpunit#5048) - fix ci and style as neos 9 followup for #3218
The method `buildMethodParametersCode` does not exist and most likely `$proxyMethod->buildMethodParametersCode` is meant. > This dates back to Andi and I believe the solution is as simple as this being > $proxyMethod->setMethodParametersCode($proxyMethod->buildMethodParametersCode($declaringClassName, $methodName, true)); > Which becomes even clearer looking at the sibling code in AdvicedMethodInterceptorBuilder where it is like I suggested neos/flow-development-collection#3218 (comment)
With neos/flow-development-collection#3218 PHPStan level 1 was added to the whole Flow code base and CI for Flow 8. This upmerged change needs some adjustments to pass the CI in Flow 9 - fix types and remove deprecated code - fix types where neos depends on it (by correcting types and adding `never`) - adjust unit test as `never` cannot be doubled (eventually this will be fixed via: sebastianbergmann/phpunit#5048) - fix ci and style as neos 9 followup for neos/flow-development-collection#3218
With neos/flow-development-collection#3218 PHPStan level 1 was added to the whole Flow code base and CI for Flow 8. This upmerged change needs some adjustments to pass the CI in Flow 9 - fix types and remove deprecated code - fix types where neos depends on it (by correcting types and adding `never`) - adjust unit test as `never` cannot be doubled (eventually this will be fixed via: sebastianbergmann/phpunit#5048) - fix ci and style as neos 9 followup for neos/flow-development-collection#3218
With neos/flow-development-collection#3218 PHPStan level 1 was added to the whole Flow code base and CI for Flow 8. This upmerged change needs some adjustments to pass the CI in Flow 9 - fix types and remove deprecated code - fix types where neos depends on it (by correcting types and adding `never`) - adjust unit test as `never` cannot be doubled (eventually this will be fixed via: sebastianbergmann/phpunit#5048) - fix ci and style as neos 9 followup for neos/flow-development-collection#3218
This is a backport of #3216
Adds PHPStan level 1 to the whole Flow code base and CI.
Psalm was removed.
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions