fix(weaving): handle #[\Override] on intercepted trait methods

CHANGELOG.md

@@ -38,6 +38,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide.
 * [BC BREAK] **Removed unused API** — the `#[Aspect]` attribute, `Pointcut::KIND_TRAIT` (`KIND_ALL` is `119`), `InterceptedConstructorGenerator`, `FunctionParameterList` and a few unused methods; see UPGRADE-4.0.
 * [BC BREAK] **Pointcut semantics** — space-separated modifiers must all match and `|` alternatives bind tighter, so `final public|protected` no longer matches every non-final public method. `Foo+` also matches `Foo` itself and classes using the trait `Foo`. `@execution`/`@within`/`@access` match subclasses of the attribute. Return-type patterns accept the `?Foo` nullable marker, and regular-expression failures raise a `PointcutSyntaxException` instead of silently not matching.
 * [BC BREAK] **Methods returning by reference** can not be intercepted: weaving one fails with a `WeavingException` instead of silently returning a copy. The new `matchReturningByReference()` pointcut excludes them (`&& !matchReturningByReference()`).
+* **Trait methods with `#[\Override]`** — a woven trait strips `#[\Override]` from its intercepted methods like a woven class does, instead of failing to load. A class importing such a method can not intercept it: weaving fails with a `WeavingException` instead of a fatal error at load time; exclude it with `&& !matchInherited()` or advise the trait itself (#761).
 * [BC BREAK] **Renames** — `TransformerResultEnum::RESULT_*` becomes `TransformerResult::{Aborted, Abstain, Transformed}`; enum cases use PascalCase everywhere (`FieldAccessType::Read`/`Write`, and the `Proxy\Generator` enums `Visibility`, `PropertyModifier`, `ClassModifier`); `AspectContainer::AOP_PROXIED_SUFFIX` becomes `ORIGINAL_TRAIT_SUFFIX` plus `ORIGINAL_TRAIT_FILE_SUFFIX`.
 
 **Performance**

docs/advices.md

@@ -59,6 +59,14 @@ The joinpoints declare `@template` types, so PHPStan and IDEs know the types of
 - **Returning by reference.** A method or function declared `function &name()` can not be intercepted: joinpoints
   and advices return values, so the caller would get a copy instead of the reference. Weaving one fails with a
   `WeavingException`; exclude such methods from a broad pointcut with `&& !matchReturningByReference()`.
+- **Trait methods with `#[\Override]`.** A method that a class imports from a trait and that carries `#[\Override]`
+  can not be intercepted in that class: PHP copies the attribute to the private alias the proxy calls, which
+  overrides nothing. Weaving one fails with a `WeavingException`; exclude trait methods from the pointcut with
+  `&& !matchInherited()`, exclude only that method (`&& !execution(public Foo->hello(*))`), or remove
+  `#[\Override]` from the trait. To intercept the method anyway, advise the trait itself
+  (`execution(public HelloTrait->hello(*))`): the trait is woven, which works with `#[\Override]`, and the advice
+  applies in every class using the trait. Trait methods that a class inherits from its parent class are not
+  affected.
 - **Generators.** For a method that contains `yield`, the advice runs when the method is called and
   `proceed()` returns the `Generator` object. The advice does not see the yielded values, which are produced later,
   while the caller iterates.

docs/pointcuts.md

@@ -17,7 +17,7 @@ creation. It is the argument of an advice attribute, e.g. `#[Before('execution(p
 | `@execution(Attribute)`                  | Methods that carry the attribute |
 | `@access(Attribute)`                     | Properties that carry the attribute |
 | `@within(Attribute)`                     | Every member of classes that carry the attribute |
-| `matchInherited()`                       | Only members inherited from a parent class; `!matchInherited()` excludes them |
+| `matchInherited()`                       | Only members inherited from a parent class or imported from a trait; `!matchInherited()` excludes them |
 | `matchReturningByReference()`            | Methods and functions declared `function &name()`; `!matchReturningByReference()` excludes them |
 | `$this->pointcutName`                    | A named pointcut declared with `#[Pointcut]` on a method of the same aspect |
 | `Aspect\Class->pointcutName`             | A named pointcut of another aspect |

src/Instrument/AGENTS.md

@@ -78,7 +78,10 @@ Woven trait MUST preserve original source line numbers for XDebug breakpoints.
 
 ## PHP compat: #[\Override] on intercepted methods (8.3+)
 When intercepted method has #[\Override], PHP copies attribute to trait alias — fatal error (alias doesn't override anything).
-WeavingTransformer::convertClassToTrait() strips #[\Override] from trait for every intercepted method. Attribute preserved on proxy's override method (proxy extends same parent).
+WeavingTransformer::stripOverrideAttributeFromInterceptedMethods() strips #[\Override] from the woven source for every intercepted own method: classes (convertClassToTrait), enums (convertEnumToTrait) and woven traits (renamed original trait, its methods aliased by TraitProxyGenerator). Attribute preserved on the proxy method (MethodGenerator copies it from the AST), so PHP still checks the override.
+- Limitation (#761): a method a class/enum/trait imports from ANOTHER trait with #[\Override] can not be intercepted — the attribute lives in the shared trait file, can't be stripped per class. assertNoTraitImportedOverride() (processSingleClass, before the generators; native reflection hides the trait) throws WeavingException naming class, method, trait. Covers aliases, static methods, enums, woven traits using a trait.
+- Workaround: `&& !matchInherited()` (excludes trait-imported methods too), narrower `&& !execution(public Foo->hello(*))`, drop #[\Override] from the trait method, or advise the trait itself (`execution(public HelloTrait->hello(*))`, intercepts it in every class using the trait).
+- Trait methods inherited via a parent class are fine: proxy calls `parent::m(...)`, no alias.
 
 ## Aspects themselves
 Classes implementing \Go\Aop\Aspect: unconditionally skipped by WeavingTransformer. Aspects cannot weave themselves.

src/Instrument/Transformer/WeavingTransformer.php

@@ -175,13 +175,16 @@ private function processSingleClass(
         $originalImports = $namespace->getNamespaceAliases();
         // Intercepted methods are looked up by name several times per method, here and in the proxy generators
         $methods = ClassProxyGenerator::indexMethods($class);
+        $this->assertNoTraitImportedOverride($class, $advices, $methods);
 
         // For traits: rename the trait (legacy approach, TraitProxyGenerator generates a child trait).
         // For enums: convert the enum body to a trait (cases extracted to proxy enum by EnumProxyGenerator).
         // For classes: convert the class body to a trait (new trait-based engine).
         if ($class->isTrait()) {
             $this->removeInterceptedPropertiesFromTraitBody($class, $advices, $metadata);
             $this->adjustOriginalTrait($class, $metadata, $newClassName);
+            // The proxy trait aliases every intercepted method of the renamed trait, PHP would copy #[\Override] there
+            $this->stripOverrideAttributeFromInterceptedMethods($class, $advices, $methods, $metadata);
             $childProxyGenerator = new TraitProxyGenerator($class, $newFqcn, $advices, $originalImports, $methods);
         } elseif ($class->isEnum()) {
             $this->convertEnumToTrait($class, $advices, $methods, $metadata, $newClassName);
@@ -223,6 +226,61 @@ private function isAspectSafe(ReflectionClass $class): bool
         }
     }
 
+    /**
+     * Rejects the interception of a method imported from a trait that carries #[\Override] (issue #761)
+     *
+     * Every intercepted own method is aliased in the trait-use block of the proxy (`m as private mOriginalAlias`)
+     * and PHP copies the attributes of the method to that alias, which overrides nothing: #[\Override] on it is a
+     * fatal error at load time. For a method declared in the class body the attribute is stripped from the woven
+     * source (see stripOverrideAttributeFromInterceptedMethods()), but the attribute of a trait method lives in the
+     * trait file, which is shared by every class using it and can not be edited per class. The proxy generators can
+     * not detect the case: native reflection reports the using class as the declaring class of a trait method.
+     *
+     * Covered: trait aliases (`use T { foo as bar; }`), static methods, enums (EnumProxyGenerator aliases every
+     * intercepted method) and woven traits using another trait. Trait methods arriving through a parent class keep
+     * the parent in their `class` property and are called through `parent::m(...)` without an alias, so they pass.
+     * Methods of a trait used by an imported trait (nested traits) are not collected by parser-reflection 5.0.1, so
+     * they can not get here.
+     *
+     * @param array<string, array<string, list<string|\Go\Aop\Framework\GeneratedInterceptor>>> $advices
+     * @param array<string, \ReflectionMethod> $methods Methods of the class by name
+     */
+    private function assertNoTraitImportedOverride(ReflectionClass $class, array $advices, array $methods): void
+    {
+        $interceptedNames = array_merge(
+            array_keys($advices[AspectContainer::METHOD_PREFIX] ?? []),
+            array_keys($advices[AspectContainer::STATIC_METHOD_PREFIX] ?? []),
+        );
+
+        foreach ($interceptedNames as $methodName) {
+            $method = $methods[$methodName] ?? null;
+            // Methods inherited from a parent class (including the ones it imports from traits) are not aliased
+            if (!$method instanceof ReflectionMethod || $method->class !== $class->name) {
+                continue;
+            }
+            $declaringClass = $method->getDeclaringClass();
+            if ($declaringClass->name === $class->name || !$declaringClass->isTrait()) {
+                continue;
+            }
+            foreach ($method->getNode()->attrGroups as $attrGroup) {
+                foreach ($attrGroup->attrs as $attribute) {
+                    if (!self::isOverrideAttribute($attribute)) {
+                        continue;
+                    }
+                    throw new WeavingException(sprintf(
+                        'Method %s::%s() is imported from trait %s and has #[\Override], so it can not be '
+                        . 'intercepted: PHP copies the attribute to the private "%sOriginalAlias" alias of the '
+                        . 'proxy, which overrides nothing. Exclude it from the pointcut with "&& !matchInherited()".',
+                        $class->name,
+                        $methodName,
+                        $declaringClass->name,
+                        $methodName,
+                    ));
+                }
+            }
+        }
+    }
+
     /**
      * Adjust definition of original trait source to enable extending
      */
@@ -611,7 +669,8 @@ private function convertEnumToTrait(
      * `SomeTrait::method as private methodOriginalAlias`), PHP copies the method's attributes to
      * the alias. If the original method had `#[\Override]`, the alias name has no matching
      * parent method → fatal error. We strip the attribute only from methods that will be aliased
-     * (those with dynamic or static method advices).
+     * (those with dynamic or static method advices). Applies to woven classes, enums and traits: the proxy trait
+     * of a woven trait aliases its intercepted methods the same way, and the proxy methods keep #[\Override].
      *
      * @param array<string, array<string, list<string|\Go\Aop\Framework\GeneratedInterceptor>>> $advices
      * @param array<string, \ReflectionMethod> $methods Methods of the class by name
@@ -629,21 +688,31 @@ private function stripOverrideAttributeFromInterceptedMethods(
 
         foreach ($interceptedNames as $methodName) {
             // The declaring class, not the `class` property: a method imported from a trait reports the using
-            // class there, while its attributes live in the trait source, not in this token stream
+            // class there, while its attributes live in the trait source, not in this token stream. Interception
+            // of such a method with #[\Override] is rejected earlier, by assertNoTraitImportedOverride()
             $method = $methods[$methodName] ?? null;
             if (!$method instanceof ReflectionMethod || $method->getDeclaringClass()->name !== $class->name) {
                 continue;
             }
-            // Attribute names are compared as resolved by the parser, so aliases (`use Override as O; #[O]`),
-            // qualified names and attributes that merely end with "Override" are told apart correctly
             $this->stripAttributes(
                 $method->getNode()->attrGroups,
-                static fn(Attribute $attribute): bool => self::resolveAttributeName($attribute) === 'Override',
+                self::isOverrideAttribute(...),
                 $streamMetaData,
             );
         }
     }
 
+    /**
+     * Checks whether the attribute is #[\Override], compared by the name resolved by the parser
+     *
+     * Aliases (`use Override as O; #[O]`), qualified names and attributes that merely end with "Override" are told
+     * apart correctly.
+     */
+    private static function isOverrideAttribute(Attribute $attribute): bool
+    {
+        return self::resolveAttributeName($attribute) === 'Override';
+    }
+
     /**
      * Removes intercepted property declarations from the woven trait body.
      *

tests/Aop/Pointcut/PointcutSemanticsTest.php

@@ -15,6 +15,8 @@
 use Attribute;
 use Go\Aop\Exception\PointcutSyntaxException;
 use Go\Aop\Pointcut;
+use Go\Instrument\Transformer\Stubs\ClassUsingOverrideTrait;
+use Go\ParserReflection\ReflectionClass as ParserReflectionClass;
 use Go\Stubs\ByReferenceStub;
 use PhpParser\PrettyPrinter\Standard;
 use PHPUnit\Framework\Attributes\DataProvider;
@@ -125,6 +127,20 @@ public function testMethodsReturningByReferenceCanBeFilteredOut(): void
         );
     }
 
+    /**
+     * `!matchInherited()` also excludes the methods imported from a trait: the workaround for trait methods with
+     * #[\Override], which can not be intercepted (issue #761). Parser reflection is used, as the weaver does:
+     * native reflection reports the using class as the declaring class of a trait method.
+     */
+    public function testNotMatchInheritedExcludesTraitImportedMethods(): void
+    {
+        $class    = new ParserReflectionClass(ClassUsingOverrideTrait::class);
+        $pointcut = self::parse('execution(public ' . ClassUsingOverrideTrait::class . '->*(*)) && !matchInherited()');
+
+        $this->assertTrue($pointcut->matches($class, $class->getMethod('ownMethod')));
+        $this->assertFalse($pointcut->matches($class, $class->getMethod('count')));
+    }
+
     public function testNullableReturnTypeMarker(): void
     {
         $class    = new ReflectionClass(PointcutSemanticsTarget::class);

tests/Fixtures/project/src/Application/Issue761Base.php

@@ -0,0 +1,15 @@
+<?php
+declare(strict_types=1);
+
+namespace Go\Tests\TestProject\Application;
+
+/**
+ * Test fixture (issue #761): parent class whose method is overridden by a trait method with #[\Override]
+ */
+class Issue761Base
+{
+    public function hello(): string
+    {
+        return 'base';
+    }
+}

tests/Fixtures/project/src/Application/Issue761Child.php

@@ -0,0 +1,18 @@
+<?php
+declare(strict_types=1);
+
+namespace Go\Tests\TestProject\Application;
+
+/**
+ * Test fixture (issue #761): a class importing a trait method with #[\Override]. Its pointcut excludes the trait
+ * method with `!matchInherited()`, so the class is woven for its own method and stays loadable.
+ */
+class Issue761Child extends Issue761Base
+{
+    use Issue761OverrideTrait;
+
+    public function own(): string
+    {
+        return 'own';
+    }
+}

tests/Fixtures/project/src/Application/Issue761OverrideTrait.php

@@ -0,0 +1,16 @@
+<?php
+declare(strict_types=1);
+
+namespace Go\Tests\TestProject\Application;
+
+/**
+ * Test fixture (issue #761): trait method with #[\Override], it can not be intercepted in a class using the trait
+ */
+trait Issue761OverrideTrait
+{
+    #[\Override]
+    public function hello(): string
+    {
+        return 'trait';
+    }
+}

tests/Fixtures/project/src/Application/Issue761WovenOverrideTrait.php

@@ -0,0 +1,17 @@
+<?php
+declare(strict_types=1);
+
+namespace Go\Tests\TestProject\Application;
+
+/**
+ * Test fixture (issue #761): a woven trait whose own intercepted method has #[\Override]. The attribute is stripped
+ * from the renamed original trait and kept on the proxy trait method.
+ */
+trait Issue761WovenOverrideTrait
+{
+    #[\Override]
+    public function hello(): string
+    {
+        return 'woven-trait';
+    }
+}

tests/Fixtures/project/src/Application/Issue761WovenOverrideTraitUser.php

@@ -0,0 +1,12 @@
+<?php
+declare(strict_types=1);
+
+namespace Go\Tests\TestProject\Application;
+
+/**
+ * Test fixture (issue #761): uses a woven trait whose intercepted method has #[\Override] and overrides the parent
+ */
+class Issue761WovenOverrideTraitUser extends Issue761Base
+{
+    use Issue761WovenOverrideTrait;
+}

tests/Fixtures/project/src/Aspect/TraitCompositionAspect.php

@@ -4,6 +4,7 @@
 namespace Go\Tests\TestProject\Aspect;
 
 use Go\Aop\Aspect;
+use Go\Aop\Intercept\MethodInvocation;
 use Go\Lang\Attribute as Pointcut;
 
 /**
@@ -27,4 +28,21 @@ public function afterClassUsingTraitMethod(): void {}
      */
     #[Pointcut\After("execution(private|protected Go\Tests\TestProject\Application\ClassWithPrivateMethods->*(*))")]
     public function afterNonPublicMethod(): void {}
+
+    /**
+     * Intercepts the own methods of Issue761Child only: its trait method with #[\Override] can not be
+     * intercepted (issue #761), `!matchInherited()` excludes the methods imported from traits.
+     */
+    #[Pointcut\After("execution(public Go\Tests\TestProject\Application\Issue761Child->*(*)) && !matchInherited()")]
+    public function afterIssue761OwnMethod(): void {}
+
+    /**
+     * Intercepts the own #[\Override] method of a woven trait (issue #761): the result is prefixed, so a functional
+     * test sees that the advice runs in the class using the trait.
+     */
+    #[Pointcut\Around("execution(public Go\Tests\TestProject\Application\Issue761WovenOverrideTrait->hello(*))")]
+    public function aroundIssue761WovenTraitMethod(MethodInvocation $invocation): mixed
+    {
+        return 'advised:' . $invocation->proceed();
+    }
 }