-
Notifications
You must be signed in to change notification settings - Fork 96
Add BrowserKitAssertionTraitReturnTypeExtension #452
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
base: 2.0.x
Are you sure you want to change the base?
Conversation
24d7802
to
c7e1125
Compare
*/ | ||
private function isSupported(Expr $expr, Scope $scope): bool | ||
{ | ||
if (!($expr instanceof MethodCall) || !($expr->name instanceof Identifier) || $expr->name->name !== self::TRAIT_METHOD_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 don't understand the logic. You're interested in the method name and you're interested where we're calling it from.
But you're not interested on what we're calling the method. This extension might be influencing a total coincidence that's nothing to do with BrowserKitAssertionsTrait::getClient()
.
We're in a class, the class has getClient
method. And the method is declared in BrowserKitAssertionsTrait trait. But the method we're still calling might be unrelated.
My hunch is telling me you're no checking $scope->getMethodReflection($scope->getType($expr->var), $expr->name->name)
but you should be.
Also, method names are case-insensitive, so toLowerString() needs to happen.
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 copied the logic from the extension you gave me as example
phpstan-symfony/src/Type/Symfony/MessengerHandleTraitReturnTypeExtension.php
Lines 67 to 87 in 8820c22
private function isSupported(Expr $expr, Scope $scope): bool | |
{ | |
if (!($expr instanceof MethodCall) || !($expr->name instanceof Identifier) || $expr->name->name !== self::TRAIT_METHOD_NAME) { | |
return false; | |
} | |
if (!$scope->isInClass()) { | |
return false; | |
} | |
$reflectionClass = $scope->getClassReflection()->getNativeReflection(); | |
if (!$reflectionClass->hasMethod(self::TRAIT_METHOD_NAME)) { | |
return false; | |
} | |
$methodReflection = $reflectionClass->getMethod(self::TRAIT_METHOD_NAME); | |
$declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass(); | |
return $declaringClassReflection->getName() === self::TRAIT_NAME; | |
} |
So I assume it should be improve on the other extension too.
d50062e
to
9a6e442
Compare
9a6e442
to
2b57a14
Compare
I tried to fix both BrowserKitAssertionTraitReturnTypeExtension and MessengerHandleTraitReturnTypeExtension then @ondrejmirtes |
No description provided.