fix(pointcut): follow the documented name wildcard and alternation rules

CHANGELOG.md

@@ -79,6 +79,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide.
 * [Fixed] `cache:warmup:aop` and `debug:advisor` no longer fail with "Cannot rewind a generator that was already run" when more than one include path is configured (#751).
 * [Fixed] The proxy of a class declared in the second (or a later) block of a namespace that a file declares several times copies the imports of its own block instead of the first block's.
 * [Fixed] A file declaring several woven classes gets one proxy file per class, below the file name like a file inside a phar (`Holder.php/App/First.php`): the proxies used to share one cache file, so all classes but the last one were not found (#760).
+* [Fixed] [BC BREAK] Name patterns follow `docs/pointcuts.md` (#774): `*` matches any characters of one name part, including none (`Sensor*` now matches `Sensor`, `get*` a method named `get`, the return type pattern `Exc*` the type `Exc`), and `|` alternatives apply to the name part they are written in (`Ns\A|B` is `Ns\A` or `Ns\B`, no longer `Ns\A` or the global class `B`). `NamePointcut` translates a pattern part by part.
 
 **Internal**
 * [Internal] Tooling: PHPUnit 13 with strict flags, PHPStan 2 at level 10 over `src/` and `tests/`, php-cs-fixer with the PER-CS rule set, and `composer test`/`analyze`/`cs`/`check` scripts.

UPGRADE-4.0.md

@@ -178,6 +178,13 @@ Pointcuts
 - **`Foo+` includes `Foo`.** It now also matches the class or interface `Foo` itself and classes that use the trait
   `Foo`. Add `&& !within(Foo)` to exclude the type itself.
 - **Attribute pointcuts match subclasses.** `@execution(Attr)` and friends also match attributes extending `Attr`.
+- **`*` also matches nothing (#774).** In name and return-type patterns `*` stands for any characters of one name
+  part, including none, as `docs/pointcuts.md` always said. In 3.x it needed at least one character: `Sensor*`
+  skipped the class `Sensor` itself and `set*` skipped a method named `set()`. Such pointcuts now match them as well;
+  exclude them with `&& !execution(public Foo->set(*))` or `&& !within(Foo)` if needed.
+- **`|` alternatives stay inside their name part (#774).** `Ns\A|B` selects `Ns\A` and `Ns\B`. In 3.x it meant `Ns\A`
+  or the global class `B`. A pattern that listed classes of different namespaces (`Foo\A|Bar\B`) has to be written
+  as two pointcuts: `execution(public Foo\A->*(*)) || execution(public Bar\B->*(*))`.
 - **`ModifierPointcut` is final and immutable.** The masks are constructor arguments, and
   `andMatch()`, `orMatch()` and `notMatch()` return a new instance.
 

docs/pointcuts.md

@@ -28,9 +28,9 @@ The argument list is always `(*)`: arguments are not matched.
 
 | Pattern       | Meaning |
 |---------------|---------|
-| `*`           | Any part of a name, e.g. `get*`, `*Service`, `App\*\Controller` |
+| `*`           | Any characters of one name part, including none, never crossing a `\`: `get*` (also `get`), `*Service` (also `Service`), `App\*\Controller` (exactly one namespace level between) |
 | `**`          | Any namespace depth: `App\**` is every class below `App`, `**->*(*)` is every method |
-| `a\|b`        | Alternatives within one name part, e.g. `get*\|is*` |
+| `a\|b`        | Alternatives within one name part, e.g. `get*\|is*`. They apply to the part they are written in: `App\Cart\|Order` is `App\Cart` or `App\Order`, never a global `Order`. For classes of different namespaces join two pointcuts with `\|\|` |
 | `Foo+`        | `Foo` itself, its subclasses and implementers, and classes that use the trait `Foo` (directly or through a parent class) |
 
 Names are matched case-sensitively. Write namespaces with a single backslash; in a PHP string with single quotes

src/Aop/Pointcut/NamePointcut.php

@@ -43,20 +43,39 @@
      */
     public function __construct(
         public int $kind,
-        /** Element name to match, can contain wildcards **,*,| */
+        /** Element name to match, can contain wildcards `**`, `*` and `|`, see compilePattern() */
         private string $name,
         /** Switch to matching context name instead of reflector */
         private bool $useContextForMatching = false,
     ) {
-        // Special parenthesis is needed for stricter matching, see https://github.com/goaop/framework/issues/115
-        $this->regexp = '/^(' . strtr(
-            preg_quote($this->name, '/'),
-            [
-                '\\*'    => '[^\\\\]+?',
-                '\\*\\*' => '.+?',
-                '\\|'    => '|',
-            ],
-        ) . ')$/';
+        $this->regexp = self::compilePattern($this->name);
+    }
+
+    /**
+     * Translates a name pattern into a regular expression, one name part (a segment between namespace separators)
+     * at a time:
+     *  - `*` is any characters, including none, inside the name part, it never crosses a namespace separator;
+     *  - `**` is any characters, including namespace separators (at least one);
+     *  - `|` separates alternatives of the name part it is written in, so `Ns\A|B` is `Ns\A` or `Ns\B`, never a
+     *    global class `B`. The group around the alternatives keeps the anchors strict for each of them, see
+     *    https://github.com/goaop/framework/issues/115
+     */
+    private static function compilePattern(string $pattern): string
+    {
+        $nameParts = [];
+        foreach (explode('\\', $pattern) as $namePart) {
+            $regexp = strtr(
+                preg_quote($namePart, '/'),
+                [
+                    '\\*'    => '[^\\\\]*?',
+                    '\\*\\*' => '.+?',
+                    '\\|'    => '|',
+                ],
+            );
+            $nameParts[] = str_contains($namePart, '|') ? "(?:{$regexp})" : $regexp;
+        }
+
+        return '/^' . implode('\\\\', $nameParts) . '$/';
     }
 
     public function matches(

src/Aop/Pointcut/ReturnTypePointcut.php

@@ -94,7 +94,7 @@ public function __construct(string $returnTypeName)
         $atomRegexps = [];
         foreach ($this->patternGroups as $patternGroup) {
             foreach ($patternGroup as $patternAtom) {
-                $atomRegexps[$patternAtom] = '/^(' . strtr(preg_quote($patternAtom, '/'), ['\\*' => '[^\\\\]+']) . ')$/';
+                $atomRegexps[$patternAtom] = '/^(' . strtr(preg_quote($patternAtom, '/'), ['\\*' => '[^\\\\]*']) . ')$/';
             }
         }
         $this->atomRegexps = $atomRegexps;
@@ -221,7 +221,8 @@ private static function splitAtDepthZero(string $type): array
     }
 
     /**
-     * Checks whether one atomic type pattern (with a possible '*' wildcard) matches an atomic type.
+     * Checks whether one atomic type pattern (with a possible '*' wildcard, any characters inside one name part,
+     * including none) matches an atomic type.
      */
     private function atomMatches(string $pattern, string $actual): bool
     {

tests/Aop/Pointcut/NamePointcutTest.php

@@ -14,7 +14,9 @@
 
 use Go\Aop\Exception\PointcutSyntaxException;
 use Go\Aop\Pointcut;
+use ArrayObject;
 use Go\Stubs\First;
+use Go\Stubs\FirstStatic;
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
 use ReflectionMethod;
@@ -103,6 +105,62 @@ public function testIssue115(): void
         $this->assertFalse($matched, "Pointcut should match strict");
     }
 
+    /**
+     * Star is any characters of one name part, including none: `get*` is also `get`, `Foo*` is also `Foo`
+     */
+    public function testStarMatchesEmptyString(): void
+    {
+        $method  = new ReflectionMethod(First::class, 'publicMethod');
+        $context = new ReflectionClass(First::class);
+
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_METHOD, 'publicMethod*'))->matches($context, $method));
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_METHOD, '*publicMethod'))->matches($context, $method));
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_ALL, First::class . '*', true))->matches($context));
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_ALL, First::class . '*', true))->matches(new ReflectionClass(FirstStatic::class)));
+    }
+
+    /**
+     * Star never crosses a namespace separator, double star does
+     */
+    public function testStarStaysInsideOneNamePart(): void
+    {
+        $context = new ReflectionClass(First::class);
+
+        $this->assertFalse((new NamePointcut(Pointcut::KIND_ALL, 'Go*', true))->matches($context));
+        $this->assertFalse((new NamePointcut(Pointcut::KIND_ALL, 'Go\\*', true))->matches($context));
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_ALL, 'Go\\*\\First', true))->matches($context));
+        $this->assertTrue((new NamePointcut(Pointcut::KIND_ALL, 'Go\\**', true))->matches($context));
+    }
+
+    /**
+     * Alternatives belong to the name part they are written in: `Go\Stubs\First|FirstStatic` is
+     * `Go\Stubs\First` or `Go\Stubs\FirstStatic`, not `Go\Stubs\First` or a global class `FirstStatic`
+     */
+    public function testAlternativesAreScopedToTheirNamePart(): void
+    {
+        $pointcut = new NamePointcut(Pointcut::KIND_ALL, 'Go\\Stubs\\First|FirstStatic', true);
+
+        $this->assertTrue($pointcut->matches(new ReflectionClass(First::class)));
+        $this->assertTrue($pointcut->matches(new ReflectionClass(FirstStatic::class)));
+
+        // ArrayObject is a global class: it must not match the alternative of a nested name part
+        $globalPointcut = new NamePointcut(Pointcut::KIND_ALL, 'Go\\Stubs\\First|ArrayObject', true);
+        $this->assertFalse($globalPointcut->matches(new ReflectionClass(ArrayObject::class)));
+        $this->assertTrue($globalPointcut->matches(new ReflectionClass(First::class)));
+    }
+
+    /**
+     * Alternatives of an inner name part keep the rest of the namespace on both sides
+     */
+    public function testAlternativesInTheMiddleOfAName(): void
+    {
+        $pointcut = new NamePointcut(Pointcut::KIND_ALL, 'Go\\Stubs|Aop\\First', true);
+
+        $this->assertTrue($pointcut->matches(new ReflectionClass(First::class)));
+        $this->assertFalse($pointcut->matches(new ReflectionClass(FirstStatic::class)));
+        $this->assertFalse($pointcut->matches(new ReflectionClass(ArrayObject::class)));
+    }
+
     public function testMatchesAnyContextWithoutReflector(): void
     {
         $pointcut = new NamePointcut(

tests/Aop/Pointcut/PointcutSemanticsTest.php

@@ -17,7 +17,10 @@
 use Go\Aop\Pointcut;
 use Go\Instrument\Transformer\Stubs\ClassUsingOverrideTrait;
 use Go\ParserReflection\ReflectionClass as ParserReflectionClass;
+use ClassWithoutNamespace;
 use Go\Stubs\ByReferenceStub;
+use Go\Stubs\First;
+use Go\Stubs\FirstStatic;
 use PhpParser\PrettyPrinter\Standard;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
@@ -141,6 +144,39 @@ public function testNotMatchInheritedExcludesTraitImportedMethods(): void
         $this->assertFalse($pointcut->matches($class, $class->getMethod('count')));
     }
 
+    /**
+     * Wildcards and alternatives of class and member names, see the "Names and wildcards" table of docs/pointcuts.md
+     *
+     * @return array<string, array{string, class-string, string, bool}>
+     */
+    public static function nameCases(): array
+    {
+        return [
+            'star matches no characters of a class name'        => ['execution(public Go\Stubs\First*->publicMethod(*))', First::class, 'publicMethod', true],
+            'star matches characters of a class name'           => ['execution(public Go\Stubs\First*->publicMethod(*))', FirstStatic::class, 'publicMethod', true],
+            'star matches no characters of a member name'       => ['execution(public Go\Stubs\First->publicMethod*(*))', First::class, 'publicMethod', true],
+            'star in a class name skips other namespaces'       => ['execution(public Go\Stubs\First*->publicMethod(*))', ClassWithoutNamespace::class, 'publicMethod', false],
+            'star does not cross a namespace separator'         => ['execution(public Go*->publicMethod(*))', First::class, 'publicMethod', false],
+            'alternatives of a class name: first'               => ['execution(public Go\Stubs\First|FirstStatic->publicMethod(*))', First::class, 'publicMethod', true],
+            'alternatives of a class name: second'              => ['execution(public Go\Stubs\First|FirstStatic->publicMethod(*))', FirstStatic::class, 'publicMethod', true],
+            'alternatives stay inside the namespace'            => ['execution(public Go\Stubs\First|ClassWithoutNamespace->publicMethod(*))', ClassWithoutNamespace::class, 'publicMethod', false],
+            'alternatives of a member name: first'              => ['execution(public Go\Stubs\First->publicMethod|publicFinalMethod(*))', First::class, 'publicMethod', true],
+            'alternatives of a member name: second'             => ['execution(public Go\Stubs\First->publicMethod|publicFinalMethod(*))', First::class, 'publicFinalMethod', true],
+            'alternatives of a member name match strictly'      => ['execution(public Go\Stubs\First->publicMethod|publicFinalMethod(*))', First::class, 'publicMethodWithAttribute', false],
+        ];
+    }
+
+    /**
+     * @param class-string $className
+     */
+    #[DataProvider('nameCases')]
+    public function testNamePatternsFollowTheDocumentedWildcards(string $expression, string $className, string $methodName, bool $expected): void
+    {
+        $class = new ReflectionClass($className);
+
+        $this->assertSame($expected, self::parse($expression)->matches($class, $class->getMethod($methodName)));
+    }
+
     public function testNullableReturnTypeMarker(): void
     {
         $class    = new ReflectionClass(PointcutSemanticsTarget::class);

tests/Aop/Pointcut/ReturnTypePointcutTest.php

@@ -39,10 +39,14 @@ public static function returnTypeMatchesDataProvider(): array
         $intersectionMethod = new ReflectionMethod(ClassWithComplexTypes::class, 'publicMethodWithIntersectionTypeReturn');
         $dnfMethod          = new ReflectionMethod(ClassWithComplexTypes::class, 'publicMethodWithDNFTypeReturn');
         $nullableMethod     = new ReflectionMethod(CachePathManager::class, 'queryCacheState');
+        $namespacedMethod   = new ReflectionMethod(NamePointcut::class, 'compileToPhp');
 
         return [
             'Exact match (int)' => ['int', new ReflectionFunction('strlen'), true],
             'Star match (bool)' => ['b*l', new ReflectionMethod(ReturnTypePointcut::class, 'matches'), true],
+            'Star matches no characters (int)' => ['int*', new ReflectionFunction('strlen'), true],
+            'Star matches no characters (namespaced type)' => ['PhpParser\\Node\\Expr*', $namespacedMethod, true],
+            'Star does not cross a namespace separator' => ['*Expr', $namespacedMethod, false],
             'Question mark is not a wildcard' => ['?nt', new ReflectionFunction('strlen'), false],
             'No match (int)' => ['array', new ReflectionFunction('strlen'), false],