-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
[BUG] Method matching positive on abstract classes which does not implements pointcuted method from interface #335
Comments
Hi! I can see for the abstract class, that it doesn't contain an abstract method after proxy generation. because // temporary disable override of final methods
if (!$method->isFinal() && !$method->isAbstract()) {
$this->override($method->name, $this->getJoinpointInvocationBody($method));
} This check prevents interception of final or abstract methods (including methods from interfaces). But I can agree that pointcut currently matches abstract and final methods too. Not sure if it's good or bad, just possible to match abstract method. It can be used for example for introduction pointcuts to introduce new properties, methods or body for interfaces. |
I think it is bad, it should not match what can not be weaved.
Isn't that different story here? Introductions would introduce new Interfaces to a class, and via traits interfaces are implemented, right? |
Yes, for example, TYPO3 just adds properties and methods into specific class. But I've decided to switch to traits and interfaces to have an ability to put breakpoints in traits. And it also gives me control over line numbering in the classes which was important.
Ok, let's fix this 👍 |
I haven't checked, but I think it will be possible to do this on grammar level by giving to the $modifierMatcherFilter = new ModifierMatcherFilter();
$modifierMatcherFilter->notMatch(\ReflectionMethod::IS_ABSTRACT); |
…g-class-non-declared-methods-and-properties Exclude abstract methods from pointcut matching #335
Cleaned up tests, base for functional tests setup, basic test for #335 written.
* origin/2.x: AdviceMatcher should reject abstract method as fix for #335
Fixed in #337 |
Consider usecase (precondition):
Consider that ALL stated Interface/Classes are within same namespace.
If matcher is execution(public *->foo()), abstract class gets weaved -> it is interpreted as if there is a method foo, but there is not.
Possible fix: check PHP Tokenizer and class metadata for better introspection of implemented methods.
The text was updated successfully, but these errors were encountered: