diff --git a/CHANGELOG.md b/CHANGELOG.md index e20620f6..434f0ba4 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index ddc6cfc8..7bad58a0 100644 --- a/UPGRADE-4.0.md +++ b/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. diff --git a/docs/pointcuts.md b/docs/pointcuts.md index 8db5edac..dd079a70 100644 --- a/docs/pointcuts.md +++ b/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 diff --git a/src/Aop/Pointcut/NamePointcut.php b/src/Aop/Pointcut/NamePointcut.php index a9a781e5..cfce2d8e 100644 --- a/src/Aop/Pointcut/NamePointcut.php +++ b/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( diff --git a/src/Aop/Pointcut/ReturnTypePointcut.php b/src/Aop/Pointcut/ReturnTypePointcut.php index 74cd2d3d..08c95fe0 100644 --- a/src/Aop/Pointcut/ReturnTypePointcut.php +++ b/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 { diff --git a/tests/Aop/Pointcut/NamePointcutTest.php b/tests/Aop/Pointcut/NamePointcutTest.php index bc7a5720..b3144b81 100644 --- a/tests/Aop/Pointcut/NamePointcutTest.php +++ b/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( diff --git a/tests/Aop/Pointcut/PointcutSemanticsTest.php b/tests/Aop/Pointcut/PointcutSemanticsTest.php index 2e30f642..9cd3093c 100644 --- a/tests/Aop/Pointcut/PointcutSemanticsTest.php +++ b/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 + */ + 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); diff --git a/tests/Aop/Pointcut/ReturnTypePointcutTest.php b/tests/Aop/Pointcut/ReturnTypePointcutTest.php index be85952c..08008b1a 100644 --- a/tests/Aop/Pointcut/ReturnTypePointcutTest.php +++ b/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],