[Known-issue] No access to annotations from a parent method in children classes

[TheCelavi]

Consider example:

---------- file A.php ------------

use Some\Annotation as Annotation;

abstract class A {
    /**
     * @Annotation\Cacheable()
     */
    public foo () {}
}

---------- file B.php ------------

abstract class B extends A {}

We define our pointcut to target every public method annotated with annotation \Some\Annotation\Cacheable

Class A will get weaved, class B will get weaved as well. Exception will occur on class B since:

1. method foo() will be defined and weaved in class B as well because of A and pointcut
2. method foo() in weaved B will get docblock comment from foo() from A (@Annotation\Cacheable())

Exception will be thrown because @Annotation\Cacheable() is not imported into weaved B.

This is easy to reproduce.

Fix is easy - instead of using class aliases for docblock annotations, in process of compiling classes and creating proxies, for all annotations FQCN should be used, eg, instead of @Annotation\Cacheable() -> write @Some\Annotation\Cacheable()

[lisachenko]

Hi, @TheCelavi!

This should be related to the https://github.com/doctrine/annotations/pull/89 and https://github.com/goaop/framework/issues/273.

But I don't like the idea of writing FQN into annotations, however this should solve this issue. Another one option is to fix goaop/parser-reflection, because we should ask annotations for the parent class, that contains method definition and imports as well.

So, if we have a method and it was declared in parent class, then we should ask annotations from that class, not from the child.

[TheCelavi]

But I don't like the idea of writing FQN into annotations, however this should solve this issue.

I was thinking about simplest, bug free idea. Class A could have "use X as Y" and B could have "use Z as Y" -> so, when you do copy annotations to subclass within weaving process, you can get these naming collisions.

If you copy annotations, but you use FQCN -> there are no possibilities for collisions.

On top of that, you did nothing wrong, still, debugging and breakpoints are not affected.

Another one option is to fix goaop/parser-reflection, because we should ask annotations for the parent class, that contains method definition and imports as well. So, if we have a method and it was declared in parent class, then we should ask annotations from that class, not from the child.

Don't think that that is viable option -> I do not know why, see our example, method foo() is copied from A to B, isn't weaving for A sufficient?

If foo() must be copied from A to B, then annotations should be copied as well, and B then declares foo(), so B->foo() should have same annotation.

[lisachenko]

Don't think that that is viable option -> I do not know why, see our example, method foo() is copied from A to B, isn't weaving for A sufficient?

If foo() must be copied from A to B, then annotations should be copied as well, and B then declares foo(), so B->foo() should have same annotation.

Probably, you haven't understand the idea :)

So, let me clarify a little bit more. When we process class A we will get only methods from A class and we have all info about annotations and can run pointcut on them and generate proxy. For class B ReflectionClass->getMethods() will return collection of methods (public and protected methods from class A and all methods from class B).

Then, during pointcut matching we should ask annotations for methods where they were declared, eg. during generation of proxy for class B we will ask annotations for method foo from the class A, not B and everything will work.

[TheCelavi]

Oh, there has been misunderstanding here as well! We can go back and forward like this, I will have to provide you with full example so you have full insight on the issue - that way you can come to best possible solution. Give me some time, sorry.

[TheCelavi]

We have annotation class:

/**
 * @Annotation
 */
class Marker\Cache {
    public $ttl = 3600;
}

Then we have abstract provider:

namespace Network;

use Marker as Aop;

abstract class Provider {

    /**
     * @Aop\Cache()
     */
    public function foo() {}
}

and then we have several providers:

class ProviderA extends Provider {}
class ProviderB extends Provider {}

and so on... only difference is with provider C, which uses different caching settings

use Marker as Aop;

class ProviderC extends Provider {
    /**
     * @Aop\Cache(ttl=86400)
     */
     public foo() {
         return parent::foo()
     }
}

Our aspect will intercept execution of every public method annotated with Marker\Cache and read $ttl to determine cache lifetime.

GoAop will generate proxies for classes Provider (abstract class), ProviderA, ProviderB and ProviderC. Because of abstract class Provider, ProviderA and ProviderB in their proxies have annotation as well -> even if they do not declare method foo().

So -> this is a runtime issue, not weaving time. When we ask in runtime for TTL instance of class ProviderC -> everything is fine because of "use Marker as Aop" in class ProviderC.

However when we do that for instance of class, per exampe, ProviderA -> we get exception.

Now, have in mind that GoAOP generates:

class ProviderA extends ProviderA__AopProxied {
    /**
     * @Aop\Cache()
     */
    public function foo()
    {
        return self::$__joinPoints['foo']->__invoke($this);
    }
}

which is fine, but where is that @Aop\Cache() comes from?

That is why I am proposing to weave this class ProviderA with annotations containing FCQN:

class ProviderA extends ProviderA__AopProxied {
    /**
     * @Marker\Cache()
     */
    public function foo()
    {
        return self::$__joinPoints['foo']->__invoke($this);
    }
}

That will solve our problem when asking instance of class ProviderA for details about attached annotations.

Makes sense?

[lisachenko]

That will solve our problem when asking instance of class ProviderA for details about attached annotations.

Makes sense?

Yes, have understood this issue. It definitely requires FQN for annotation classes.

[lisachenko]

Actually, I don't like copy-pasting docBlocks for methods, but this is only possible way to keep some attached metadata. I like the idea to have inheritdoc on proxy docBlock to inherit annotations and metadata from the parent method. And there were https://github.com/doctrine/annotations/pull/55 or https://github.com/doctrine/annotations/pull/54 but both PR have been rejected.

So, yes, only possible way to fix this issue is to change annotation aliases to FQN in children classes.

[lisachenko]

Mark this as known-issue for now. It's unlikely that it used by developers very often.

[lisachenko]

No more relevant after switching to the version 4 and native PHP8 attributes, there should be proper imports. However, current code-generators do not support attributes at all, this should be fixed separately.