-
Notifications
You must be signed in to change notification settings - Fork 659
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
Add support of docblock method using parent keyword #7414
Conversation
The shepherd error is surprising because the function does not actually exists(removed in phpunit 9) so the suppression should still be used. On another topic, I didn't expect you'd add a new condition here: https://github.com/vimeo/psalm/pull/7414/files#diff-eb58cff4a0b42fdcb5bb7462a60e099ef97f49c766c10c1086b50f63bc1251f3R653 I was looking at two possibilities on my end:
Those two seemed more appropriate for this kind of change. I may be wrong here so if you have a legit reason not to use those, go ahead |
Adding the check of And for the second option, method existence and visibility is checked before, so it's not possible to implements pseudo method here. Maybe my condition is not at the correct place ? Interesting behavior on PHP : https://3v4l.org/tfYli |
I'm fine with the answer :) Can you check why there is an error on shepherd for a non-existing method? After this is fixed, this seems fine to merge |
Maybe a cache issue ? Merge of branch 4.x has resolve the error. |
There's no cache on our CI :p And no, it disappeared after a rebase because I merged this: #7435 But it's concerning that the error was here in the first place. Can you revert my change on your end to check why the error was here? |
Oh, it's a bug of my feature... Because the method do not exists on parent, it will be forwarded to instance method call. So, this code will not raise UndefinedMethod : <?php
class A {}
class B extends A {
public function foo(): string {
return parent::foo();
}
} |
Thanks! |
See: #7411
Currently, when using
parent::
, psalm consider the call as static, so resolve frompseudo_static_method
instead ofpseudo_method
.This fix will forward to instance method analyser when
parent::
call is detected, in instance method context, and if the method is not declared.