From 4117b7d54da22df1de15ee2ad9826f2b5f7aa7d5 Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko Date: Mon, 1 Apr 2024 19:09:51 +0300 Subject: [PATCH] [BC Break] Refactor framework core for PHP8.2 features and phpstan level 9 checks --- src/Aop/Framework/AbstractInterceptor.php | 90 +++++++++---------- src/Aop/Framework/AbstractInvocation.php | 16 +--- src/Aop/Framework/AbstractJoinpoint.php | 50 +++-------- .../Framework/AbstractMethodInvocation.php | 59 ++++++------ src/Aop/Framework/AfterInterceptor.php | 5 +- .../Framework/AfterThrowingInterceptor.php | 4 +- src/Aop/Framework/AroundInterceptor.php | 5 +- src/Aop/Framework/BeforeInterceptor.php | 5 +- src/Aop/Framework/ClassFieldAccess.php | 71 +++++++-------- src/Aop/Framework/DeclareErrorInterceptor.php | 68 +++++--------- .../DynamicClosureMethodInvocation.php | 58 ++++++------ .../DynamicInvocationMatcherInterceptor.php | 26 ++---- .../ReflectionConstructorInvocation.php | 62 ++++++------- .../ReflectionFunctionInvocation.php | 40 ++++----- .../StaticClosureMethodInvocation.php | 55 +++++++----- .../StaticInitializationJoinpoint.php | 41 ++++----- src/Aop/Framework/TraitIntroductionInfo.php | 29 ++---- src/Aop/Intercept/ClassJoinpoint.php | 8 +- src/Aop/Intercept/ConstructorInvocation.php | 2 + src/Aop/Intercept/FieldAccess.php | 4 +- src/Aop/Intercept/FunctionInvocation.php | 8 +- src/Aop/Intercept/Interceptor.php | 27 +----- src/Aop/Intercept/Invocation.php | 7 +- src/Aop/Intercept/Joinpoint.php | 20 ++--- src/Aop/Intercept/MethodInvocation.php | 19 ++-- src/Aop/{Framework => }/OrderedAdvice.php | 4 +- src/Proxy/ClassProxyGenerator.php | 2 +- .../Aop/Framework/AbstractJoinpointTest.php | 1 + .../AbstractMethodInvocationTest.php | 3 +- .../Go/Aop/Framework/BaseInterceptorTest.php | 2 +- .../ReflectionConstructorInvocationTest.php | 8 -- tests/Go/Stubs/AbstractInterceptorMock.php | 18 ---- 32 files changed, 320 insertions(+), 497 deletions(-) rename src/Aop/{Framework => }/OrderedAdvice.php (90%) diff --git a/src/Aop/Framework/AbstractInterceptor.php b/src/Aop/Framework/AbstractInterceptor.php index d122a842..5600874d 100644 --- a/src/Aop/Framework/AbstractInterceptor.php +++ b/src/Aop/Framework/AbstractInterceptor.php @@ -13,7 +13,9 @@ namespace Go\Aop\Framework; use Closure; +use Go\Aop\AspectException; use Go\Aop\Intercept\Interceptor; +use Go\Aop\OrderedAdvice; use Go\Core\AspectKernel; use ReflectionFunction; use ReflectionMethod; @@ -31,7 +33,8 @@ * * After and before interceptors are simple closures that will be invoked after and before main invocation. * - * Framework models an interceptor as an PHP-closure, maintaining a chain of interceptors "around" the joinpoint: + * Framework models an interceptor as an PHP {@see Closure}, maintaining a chain of interceptors "around" the joinpoint: + *
  *   public function (Joinpoint $joinPoint)
  *   {
  *      echo 'Before action';
@@ -41,78 +44,69 @@
  *
  *      return $result;
  *   }
+ * 
*/ abstract class AbstractInterceptor implements Interceptor, OrderedAdvice { /** - * Local cache of advices for faster unserialization on big projects - * - * @var array - */ - protected static array $localAdvicesCache = []; - - /** - * Pointcut expression string which was used for this interceptor - */ - protected string $pointcutExpression; - - /** - * Closure to call - */ - protected Closure $adviceMethod; - - /** - * Advice order + * @var (array&array) Local hashmap of advices for faster unserialization */ - private int $adviceOrder; + private static array $localAdvicesCache = []; /** * Default constructor for interceptor */ - public function __construct(Closure $adviceMethod, int $adviceOrder = 0, string $pointcutExpression = '') - { - $this->adviceMethod = $adviceMethod; - $this->adviceOrder = $adviceOrder; - $this->pointcutExpression = $pointcutExpression; - } + public function __construct( + protected readonly Closure $adviceMethod, + private readonly int $adviceOrder = 0, + protected readonly string $pointcutExpression = '' + ) {} /** - * Serialize advice method into array + * Serializes advice closure into array + * + * @return array{name: string, class?: string} */ public static function serializeAdvice(Closure $adviceMethod): array { - $refAdvice = new ReflectionFunction($adviceMethod); + $reflectionAdvice = new ReflectionFunction($adviceMethod); + $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); + + $packedAdvice = ['name' => $reflectionAdvice->name]; + if (!isset($scopeReflectionClass)) { + throw new AspectException('Could not pack an interceptor without aspect name'); + } + $packedAdvice['class'] = $scopeReflectionClass->name; - return [ - 'method' => $refAdvice->name, - 'class' => $refAdvice->getClosureScopeClass()->name - ]; + return $packedAdvice; } /** * Unserialize an advice * - * @param array $adviceData Information about advice + * @param array{name: string, class?: string} $adviceData Information about advice */ public static function unserializeAdvice(array $adviceData): Closure { + // General unpacking supports only aspect's advices + if (!isset($adviceData['class'])) { + throw new AspectException('Could not unpack an interceptor without aspect name'); + } $aspectName = $adviceData['class']; - $methodName = $adviceData['method']; + $methodName = $adviceData['name']; - if (!isset(static::$localAdvicesCache["$aspectName->$methodName"])) { - $aspect = AspectKernel::getInstance()->getContainer()->getAspect($aspectName); - $refMethod = new ReflectionMethod($aspectName, $methodName); - $advice = $refMethod->getClosure($aspect); + // With aspect name and method name, we can restore back a closure for it + if (!isset(self::$localAdvicesCache["$aspectName->$methodName"])) { + $aspect = AspectKernel::getInstance()->getContainer()->getAspect($aspectName); + $advice = (new ReflectionMethod($aspectName, $methodName))->getClosure($aspect); - static::$localAdvicesCache["$aspectName->$methodName"] = $advice; + assert(isset($advice), 'getClosure() can not be null on modern PHP versions'); + self::$localAdvicesCache["$aspectName->$methodName"] = $advice; } - return static::$localAdvicesCache["$aspectName->$methodName"]; + return self::$localAdvicesCache["$aspectName->$methodName"]; } - /** - * Returns the advice order - */ public function getAdviceOrder(): int { return $this->adviceOrder; @@ -120,6 +114,8 @@ public function getAdviceOrder(): int /** * Getter for extracting the advice closure from Interceptor + * + * @internal */ public function getRawAdvice(): Closure { @@ -127,12 +123,16 @@ public function getRawAdvice(): Closure } /** - * Serializes an interceptor into it's representation + * Serializes an interceptor into it's array shape representation + * + * @return non-empty-array */ final public function __serialize(): array { + // Compressing state representation to avoid default values, eg pointcutExpression = '' or adviceOrder = 0 $state = array_filter(get_object_vars($this)); + // Override closure with array representation to enable serialization $state['adviceMethod'] = static::serializeAdvice($this->adviceMethod); return $state; @@ -141,7 +141,7 @@ final public function __serialize(): array /** * Un-serializes an interceptor from it's stored state * - * @param array $state The stored representation of the interceptor. + * @param array{adviceMethod: array{name: string, class?: string}} $state The stored representation of the interceptor. */ final public function __unserialize(array $state): void { diff --git a/src/Aop/Framework/AbstractInvocation.php b/src/Aop/Framework/AbstractInvocation.php index 750da35c..c363b788 100644 --- a/src/Aop/Framework/AbstractInvocation.php +++ b/src/Aop/Framework/AbstractInvocation.php @@ -20,26 +20,16 @@ abstract class AbstractInvocation extends AbstractJoinpoint implements Invocation { /** - * Arguments for invocation + * @var array Arguments for invocation, can be mutated by the {@see setArguments()} method */ protected array $arguments = []; - /** - * Gets arguments for current invocation - * - * @api - */ - public function getArguments(): array + final public function getArguments(): array { return $this->arguments; } - /** - * Sets arguments for current invocation - * - * @api - */ - public function setArguments(array $arguments): void + final public function setArguments(array $arguments): void { $this->arguments = $arguments; } diff --git a/src/Aop/Framework/AbstractJoinpoint.php b/src/Aop/Framework/AbstractJoinpoint.php index 0685d02d..d4cc539a 100644 --- a/src/Aop/Framework/AbstractJoinpoint.php +++ b/src/Aop/Framework/AbstractJoinpoint.php @@ -18,8 +18,7 @@ use Go\Aop\AdviceBefore; use Go\Aop\Intercept\Interceptor; use Go\Aop\Intercept\Joinpoint; - -use function is_array; +use Go\Aop\OrderedAdvice; /** * Abstract joinpoint for framework @@ -33,26 +32,11 @@ */ abstract class AbstractJoinpoint implements Joinpoint { - /** - * List of advices (interceptors) - * - * NB: All current children assume that each advice is Interceptor now. - * Whereas, it isn't correct logically, this can be used to satisfy PHPStan check now. - * - * @var array - */ - protected array $advices = []; - /** * Current advice index */ protected int $current = 0; - /** - * Stack frames to work with recursive calls or with cross-calls inside object - */ - protected array $stackFrames = []; - /** * Recursion level for invocation */ @@ -63,10 +47,7 @@ abstract class AbstractJoinpoint implements Joinpoint * * @param array $advices List of advices (interceptors) */ - public function __construct(array $advices) - { - $this->advices = $advices; - } + public function __construct(protected readonly array $advices = []) {} /** * Sorts advices by priority @@ -80,23 +61,12 @@ public static function sortAdvices(array $advices): array $sortedAdvices = $advices; uasort( $sortedAdvices, - function (Advice $first, Advice $second) { - switch (true) { - case $first instanceof AdviceBefore && !($second instanceof AdviceBefore): - return -1; - - case $first instanceof AdviceAround && !($second instanceof AdviceAround): - return 1; - - case $first instanceof AdviceAfter && !($second instanceof AdviceAfter): - return $second instanceof AdviceBefore ? 1 : -1; - - case ($first instanceof OrderedAdvice && $second instanceof OrderedAdvice): - return $first->getAdviceOrder() - $second->getAdviceOrder(); - - default: - return 0; - } + fn(Advice $first, Advice $second) => match (true) { + $first instanceof AdviceBefore && !($second instanceof AdviceBefore) => -1, + $first instanceof AdviceAround && !($second instanceof AdviceAround) => 1, + $first instanceof AdviceAfter && !($second instanceof AdviceAfter) => $second instanceof AdviceBefore ? 1 : -1, + $first instanceof OrderedAdvice && $second instanceof OrderedAdvice => $first->getAdviceOrder() - $second->getAdviceOrder(), + default => 0, } ); @@ -106,7 +76,9 @@ function (Advice $first, Advice $second) { /** * Replace concrete advices with list of ids * - * @param Advice[][][] $advices List of advices + * @param array>> $advices List of advices + * + * @return array>> Sorted identifier of advices/interceptors */ public static function flatAndSortAdvices(array $advices): array { diff --git a/src/Aop/Framework/AbstractMethodInvocation.php b/src/Aop/Framework/AbstractMethodInvocation.php index 65f61e0f..cd01aeb0 100644 --- a/src/Aop/Framework/AbstractMethodInvocation.php +++ b/src/Aop/Framework/AbstractMethodInvocation.php @@ -12,6 +12,7 @@ namespace Go\Aop\Framework; +use Go\Aop\Intercept\Interceptor; use Go\Aop\Intercept\MethodInvocation; use ReflectionMethod; use function array_merge; @@ -23,20 +24,7 @@ */ abstract class AbstractMethodInvocation extends AbstractInvocation implements MethodInvocation { - /** - * Instance of object for invoking - */ - protected ?object $instance; - - /** - * Instance of reflection method for invocation - */ - protected ReflectionMethod $reflectionMethod; - - /** - * Class name scope for static invocation - */ - protected string $scope = ''; + protected readonly ReflectionMethod $reflectionMethod; /** * This static string variable holds the name of field to use to avoid extra "if" section in the __invoke method @@ -45,30 +33,36 @@ abstract class AbstractMethodInvocation extends AbstractInvocation implements Me */ protected static string $propertyName; + /** + * Stack frames to work with recursive calls or with cross-calls inside object + * + * @var (array&array, object|class-string, int}>) + */ + private array $stackFrames = []; + /** * Constructor for method invocation * - * @param array $advices List of advices for this invocation + * @param array $advices List of advices for this invocation + * @param (string&class-string) $className Class, containing method to invoke + * @param (string&non-empty-string) $methodName Name of the method to invoke */ public function __construct(array $advices, string $className, string $methodName) { parent::__construct($advices); - $this->reflectionMethod = new ReflectionMethod($className, $methodName); + $reflectionMethod = new ReflectionMethod($className, $methodName); + + // If we have method inside AOP proxy class, we would like to use prototype instead + if ($reflectionMethod->hasPrototype()) { + $reflectionMethod = $reflectionMethod->getPrototype(); + } + $this->reflectionMethod = $reflectionMethod; } - /** - * Invokes current method invocation with all interceptors - * - * @param null|object|string $instance Invocation instance (class name for static methods) - * @param array $arguments List of arguments for method invocation - * @param array $variadicArguments Additional list of variadic arguments - * - * @return mixed Result of invocation - */ - final public function __invoke($instance = null, array $arguments = [], array $variadicArguments = []) + final public function __invoke(object|string $instanceOrScope, array $arguments = [], array $variadicArguments = []): mixed { if ($this->level > 0) { - $this->stackFrames[] = [$this->arguments, $this->scope, $this->instance, $this->current]; + $this->stackFrames[] = [$this->arguments, $this->{static::$propertyName}, $this->current]; } if (count($variadicArguments) > 0) { @@ -81,24 +75,21 @@ final public function __invoke($instance = null, array $arguments = [], array $v $this->current = 0; $this->arguments = $arguments; - $this->{static::$propertyName} = $instance; + $this->{static::$propertyName} = $instanceOrScope; return $this->proceed(); } finally { --$this->level; - if ($this->level > 0) { - [$this->arguments, $this->scope, $this->instance, $this->current] = array_pop($this->stackFrames); + if ($this->level > 0 && ($stackFrame = array_pop($this->stackFrames))) { + [$this->arguments, $this->{static::$propertyName}, $this->current] = $stackFrame; } else { - $this->instance = null; + unset($this->{static::$propertyName}); $this->arguments = []; } } } - /** - * Gets the method being called. - */ public function getMethod(): ReflectionMethod { return $this->reflectionMethod; diff --git a/src/Aop/Framework/AfterInterceptor.php b/src/Aop/Framework/AfterInterceptor.php index e3deb8f2..8780c2ec 100644 --- a/src/Aop/Framework/AfterInterceptor.php +++ b/src/Aop/Framework/AfterInterceptor.php @@ -22,10 +22,7 @@ */ final class AfterInterceptor extends AbstractInterceptor implements AdviceAfter { - /** - * @inheritdoc - */ - public function invoke(Joinpoint $joinpoint) + public function invoke(Joinpoint $joinpoint): mixed { try { return $joinpoint->proceed(); diff --git a/src/Aop/Framework/AfterThrowingInterceptor.php b/src/Aop/Framework/AfterThrowingInterceptor.php index 1c1bb5d4..33e61d47 100644 --- a/src/Aop/Framework/AfterThrowingInterceptor.php +++ b/src/Aop/Framework/AfterThrowingInterceptor.php @@ -25,9 +25,9 @@ final class AfterThrowingInterceptor extends AbstractInterceptor implements Advi { /** * @inheritdoc - * @throws Throwable + * @throws Throwable if original joinpoint throws an exception */ - public function invoke(Joinpoint $joinpoint) + public function invoke(Joinpoint $joinpoint): mixed { try { return $joinpoint->proceed(); diff --git a/src/Aop/Framework/AroundInterceptor.php b/src/Aop/Framework/AroundInterceptor.php index 364da67a..89a65307 100644 --- a/src/Aop/Framework/AroundInterceptor.php +++ b/src/Aop/Framework/AroundInterceptor.php @@ -22,10 +22,7 @@ */ final class AroundInterceptor extends AbstractInterceptor implements AdviceAround { - /** - * @inheritdoc - */ - public function invoke(Joinpoint $joinpoint) + public function invoke(Joinpoint $joinpoint): mixed { return ($this->adviceMethod)($joinpoint); } diff --git a/src/Aop/Framework/BeforeInterceptor.php b/src/Aop/Framework/BeforeInterceptor.php index c0d30417..6da1cf78 100644 --- a/src/Aop/Framework/BeforeInterceptor.php +++ b/src/Aop/Framework/BeforeInterceptor.php @@ -22,10 +22,7 @@ */ final class BeforeInterceptor extends AbstractInterceptor implements AdviceBefore { - /** - * @inheritdoc - */ - public function invoke(Joinpoint $joinpoint) + public function invoke(Joinpoint $joinpoint): mixed { ($this->adviceMethod)($joinpoint); diff --git a/src/Aop/Framework/ClassFieldAccess.php b/src/Aop/Framework/ClassFieldAccess.php index 250e0d71..73da2e47 100644 --- a/src/Aop/Framework/ClassFieldAccess.php +++ b/src/Aop/Framework/ClassFieldAccess.php @@ -15,30 +15,35 @@ use Go\Aop\AspectException; use Go\Aop\Intercept\FieldAccess; use Go\Aop\Intercept\FieldAccessType; +use Go\Aop\Intercept\Interceptor; use ReflectionProperty; -use function get_class; /** * Represents a field access joinpoint */ final class ClassFieldAccess extends AbstractJoinpoint implements FieldAccess { + /** + * Stack frames to work with recursive calls or with cross-calls inside object + * + * @phpstan-var array> + */ + private array $stackFrames = []; + /** * Instance of object for accessing */ - protected object $instance; + private object $instance; /** * Instance of reflection property */ - protected ReflectionProperty $reflectionProperty; + private readonly ReflectionProperty $reflectionProperty; /** * New value to set - * - * @var mixed */ - protected $newValue; + private mixed $newValue; /** * Access type for field access @@ -47,20 +52,23 @@ final class ClassFieldAccess extends AbstractJoinpoint implements FieldAccess /** * Copy of the original value of property - * - * @var mixed */ - private $value; + private mixed $value; /** * Constructor for field access * - * @param array $advices List of advices for this invocation + * @param array $advices List of advices for this invocation + * @param (string&class-string) $className */ public function __construct(array $advices, string $className, string $fieldName) { parent::__construct($advices); - + // We should bind our interceptor to the parent class where property is usually declared + $parentClass = get_parent_class($className); + if ($parentClass !== false && property_exists($parentClass, $fieldName)) { + $className = $parentClass; + } $this->reflectionProperty = new ReflectionProperty($className, $fieldName); } @@ -69,28 +77,19 @@ public function getAccessType(): FieldAccessType return $this->accessType; } - /** - * Gets the field being accessed. - */ public function getField(): ReflectionProperty { return $this->reflectionProperty; } - /** - * Gets the current value of property - */ - public function &getValue() + public function &getValue(): mixed { $value = &$this->value; return $value; } - /** - * Gets the value that must be set to the field. - */ - public function &getValueToSet() + public function &getValueToSet(): mixed { $newValue = &$this->newValue; @@ -126,7 +125,7 @@ public function ensureScopeRule(int $stackLevel = 2): void } /** - * Proceed to the next interceptor in the Chain + * @inheritdoc * * @return void Covariant, as for field interceptor there is no return value */ @@ -142,14 +141,11 @@ final public function proceed(): void /** * Invokes current field access with all interceptors * - * @param object $instance Instance of object - * @param int $accessType Type of access: READ or WRITE - * @param mixed $originalValue Original value of property - * @param mixed $newValue New value to set + * @param mixed $originalValue Original value of property, passed by reference * * @return mixed */ - final public function &__invoke(object $instance, FieldAccessType $accessType, &$originalValue, $newValue = NAN) + final public function &__invoke(object $instance, FieldAccessType $accessType, mixed &$originalValue, mixed $newValue = NAN): mixed { if ($this->level > 0) { $this->stackFrames[] = [$this->instance, $this->accessType, &$this->value, &$this->newValue]; @@ -176,14 +172,14 @@ final public function &__invoke(object $instance, FieldAccessType $accessType, & } finally { --$this->level; - if ($this->level > 0) { - [$this->instance, $this->accessType, $this->value, $this->newValue] = array_pop($this->stackFrames); + if ($this->level > 0 && ($stackFrame = array_pop($this->stackFrames))) { + [$this->instance, $this->accessType, $this->value, $this->newValue] = $stackFrame; } } } /** - * Returns the object for which current joinpoint is invoked + * @inheritdoc * * @return object Covariant, always instance of object, can not be null */ @@ -193,23 +189,16 @@ final public function getThis(): object } /** - * Checks if the current joinpoint is dynamic or static - * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call - * - * @see ClassJoinpoint::getThis() + * @return true Covariance, always true for class properties */ - final public function isDynamic(): bool + final public function isDynamic(): true { return true; } - /** - * Returns the static scope name (class name) of this joinpoint. - */ final public function getScope(): string { - return get_class($this->instance); + return $this->instance::class; } /** diff --git a/src/Aop/Framework/DeclareErrorInterceptor.php b/src/Aop/Framework/DeclareErrorInterceptor.php index 6b1a63fa..c2033b80 100644 --- a/src/Aop/Framework/DeclareErrorInterceptor.php +++ b/src/Aop/Framework/DeclareErrorInterceptor.php @@ -13,8 +13,8 @@ namespace Go\Aop\Framework; use Closure; -use Go\Aop\Intercept\FieldAccess; use Go\Aop\Intercept\Joinpoint; +use Stringable; /** * Interceptor to dynamically trigger an user notice/warning/error on method call @@ -24,45 +24,29 @@ */ final class DeclareErrorInterceptor extends AbstractInterceptor { - /** - * Error message to show for this interceptor - */ - protected string $message; - - /** - * Default level of error - */ - protected int $level; - /** * Default constructor for interceptor + * + * @param (string&non-empty-string) $message Error message to show for this interceptor + * @param int&(E_USER_NOTICE|E_USER_WARNING|E_USER_ERROR|E_USER_DEPRECATED) $level Default level of error, only E_USER_* constants */ - public function __construct(string $message, int $errorLevel, string $pointcutExpression) - { - $adviceMethod = self::getDeclareErrorAdvice(); - $this->message = $message; - $this->level = $errorLevel; + public function __construct( + protected string $message, + protected int $level, + string $pointcutExpression + ) { + $adviceMethod = self::declareErrorAdvice(...); parent::__construct($adviceMethod, -256, $pointcutExpression); } - /** - * @inheritdoc - */ public static function unserializeAdvice(array $adviceData): Closure { - return self::getDeclareErrorAdvice(); + return self::declareErrorAdvice(...); } - /** - * @inheritdoc - */ - public function invoke(Joinpoint $joinpoint) + public function invoke(Joinpoint $joinpoint): mixed { - if ($joinpoint instanceof FieldAccess) { - $scope = $joinpoint->getScope(); - $name = $joinpoint->getField()->getName(); - ($this->adviceMethod)($scope, $name, $this->message, $this->level); - } + ($this->adviceMethod)($joinpoint, $this->message, $this->level); return $joinpoint->proceed(); } @@ -70,23 +54,15 @@ public function invoke(Joinpoint $joinpoint) /** * Returns an advice */ - private static function getDeclareErrorAdvice(): Closure + private static function declareErrorAdvice(Stringable $joinPoint, string $message, int $level): void { - static $adviceMethod; - - if ($adviceMethod === null) { - $adviceMethod = function (string $scope, string $property, string $message, int $level = E_USER_NOTICE) { - $message = vsprintf( - '[AOP Declare Error]: %s has an error: "%s"', - [ - $scope . '->' . $property, - $message - ] - ); - trigger_error($message, $level); - }; - } - - return $adviceMethod; + $message = vsprintf( + '[AOP Declare Error]: %s has an error: "%s"', + [ + (string) $joinPoint, + $message + ] + ); + trigger_error($message, $level); } } diff --git a/src/Aop/Framework/DynamicClosureMethodInvocation.php b/src/Aop/Framework/DynamicClosureMethodInvocation.php index 4a3f8eb7..b74bd7eb 100644 --- a/src/Aop/Framework/DynamicClosureMethodInvocation.php +++ b/src/Aop/Framework/DynamicClosureMethodInvocation.php @@ -14,32 +14,40 @@ use Closure; -use function get_class; - /** * Dynamic closure method invocation is responsible to call dynamic methods via closure */ final class DynamicClosureMethodInvocation extends AbstractMethodInvocation { /** - * Closure to use + * For dynamic calls we store given argument as 'instance' property + * + * @see parent::__invoke() method to find out how this optimization works + * @see $instance Property, which is referenced by this static property */ - protected ?Closure $closureToCall = null; + protected static string $propertyName = 'instance'; /** - * Previous instance of invocation + * @var object Instance of object for invoking, should be protected as it's read in parent class + * @see parent::__invoke() where this variable is accessed via {@see $propertyName} value */ - protected ?object $previousInstance; + protected object $instance; /** - * For dynamic calls we store given argument as 'instance' property + * Closure to use */ - protected static string $propertyName = 'instance'; + private ?Closure $closureToCall; /** - * Invokes original method and return result from it + * @var object Previous instance of invocation */ - public function proceed() + private object $closureInstance; + + /** + * @inheritdoc + * @return mixed Covariant, always mixed + */ + public function proceed(): mixed { if (isset($this->advices[$this->current])) { $currentInterceptor = $this->advices[$this->current++]; @@ -48,22 +56,23 @@ public function proceed() } // Fill the closure only once if it's empty - if ($this->closureToCall === null) { - $this->closureToCall = $this->reflectionMethod->getClosure($this->instance); - $this->previousInstance = $this->instance; + if (!isset($this->closureToCall)) { + $this->closureToCall = $this->reflectionMethod->getClosure($this->instance); + $this->closureInstance = $this->instance; } // Rebind the closure if instance was changed since last time - if ($this->previousInstance !== $this->instance) { - $this->closureToCall = $this->closureToCall->bindTo($this->instance, $this->reflectionMethod->class); - $this->previousInstance = $this->instance; + // This code won't work with {@see Closure::call()} as it fails to rebind closure created from method + if ($this->closureInstance !== $this->instance) { + $this->closureToCall = $this->closureToCall?->bindTo($this->instance, $this->reflectionMethod->class); + $this->closureInstance = $this->instance; } - return ($this->closureToCall)(...$this->arguments); + return ($this->closureToCall)?->__invoke(...$this->arguments); } /** - * Returns the object for which current joinpoint is invoked + * @inheritdoc * * @return object Covariance, always instance of object */ @@ -73,24 +82,17 @@ final public function getThis(): object } /** - * Checks if the current joinpoint is dynamic or static - * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call - * - * @see ClassJoinpoint::getThis() + * @return true Covariance, always true for dynamic method calls */ - final public function isDynamic(): bool + final public function isDynamic(): true { return true; } - /** - * Returns the static scope name (class name) of this joinpoint. - */ final public function getScope(): string { // Due to optimization $this->scope won't be filled for each invocation // However, $this->instance always contains an object, so we can take it's name as a scope name - return get_class($this->instance); + return $this->instance::class; } } diff --git a/src/Aop/Framework/DynamicInvocationMatcherInterceptor.php b/src/Aop/Framework/DynamicInvocationMatcherInterceptor.php index f0f3e60a..f66ddc3f 100644 --- a/src/Aop/Framework/DynamicInvocationMatcherInterceptor.php +++ b/src/Aop/Framework/DynamicInvocationMatcherInterceptor.php @@ -24,31 +24,17 @@ * For each invocation interceptor asks the pointcut if it matches the invocation. * Matcher will receive reflection point, object instance and invocation arguments to make a decision */ -class DynamicInvocationMatcherInterceptor implements Interceptor +readonly class DynamicInvocationMatcherInterceptor implements Interceptor { - /** - * Instance of pointcut to dynamically match joinpoints with args - */ - protected PointFilter $pointFilter; - - /** - * Instance of interceptor to invoke - */ - protected Interceptor $interceptor; - /** * Dynamic matcher constructor */ - public function __construct(PointFilter $pointFilter, Interceptor $interceptor) - { - $this->pointFilter = $pointFilter; - $this->interceptor = $interceptor; - } + public function __construct( + private PointFilter $pointFilter, + private Interceptor $interceptor + ){} - /** - * @inheritdoc - */ - final public function invoke(Joinpoint $joinpoint) + final public function invoke(Joinpoint $joinpoint): mixed { if ($joinpoint instanceof MethodInvocation) { $method = $joinpoint->getMethod(); diff --git a/src/Aop/Framework/ReflectionConstructorInvocation.php b/src/Aop/Framework/ReflectionConstructorInvocation.php index b26b2648..a00db717 100644 --- a/src/Aop/Framework/ReflectionConstructorInvocation.php +++ b/src/Aop/Framework/ReflectionConstructorInvocation.php @@ -13,57 +13,53 @@ namespace Go\Aop\Framework; use Go\Aop\Intercept\ConstructorInvocation; -use Go\Core\AspectContainer; +use Go\Aop\Intercept\Interceptor; use ReflectionClass; use ReflectionMethod; /** * Reflection constructor invocation implementation + * + * @template T of object */ class ReflectionConstructorInvocation extends AbstractInvocation implements ConstructorInvocation { /** - * Reflection class + * @var ReflectionClass Reflection of given class */ - protected ReflectionClass $class; + private readonly ReflectionClass $class; /** - * Instance of created class, can be used for Around or After types of advices. + * @var null|(object&T) Instance of created class, can be used for Around or After types of advices. */ - protected ?object $instance = null; + private ?object $instance = null; /** * Instance of reflection constructor for class (if present) */ - private ?ReflectionMethod $constructor; + private readonly ?ReflectionMethod $constructor; /** * Constructor for constructor invocation :) * - * @param array $advices List of advices for this invocation + * @param array $advices List of advices for this invocation + * @phpstan-param class-string $className Name of the class */ public function __construct(array $advices, string $className) { - $originalClass = $className; - if (strpos($originalClass, AspectContainer::AOP_PROXIED_SUFFIX) !== false) { - $originalClass = substr($originalClass, 0, -strlen(AspectContainer::AOP_PROXIED_SUFFIX)); - } - - $this->class = new ReflectionClass($originalClass); + $this->class = new ReflectionClass($className); $this->constructor = $this->class->getConstructor(); parent::__construct($advices); } /** - * Proceed to the next interceptor in the Chain - * - * Typically this method is called inside previous closure, as instance of Joinpoint is passed to callback - * Do not call this method directly, only inside callback closures. + * @inheritdoc * - * @return object Covariant, always new object. + * @return (mixed|T) Covariant, always new object. + * @throws \ReflectionException If class is internal and cannot be created without constructor */ - final public function proceed(): object + final public function proceed(): mixed { if (isset($this->advices[$this->current])) { $currentInterceptor = $this->advices[$this->current]; @@ -73,17 +69,13 @@ final public function proceed(): object } $this->instance = $this->class->newInstanceWithoutConstructor(); - $constructor = $this->getConstructor(); - if ($constructor !== null) { - $constructor->invoke($this->instance, ...$this->arguments); - } + + // Null-safe invocation of constructor with constructor arguments + $this->getConstructor()?->invoke($this->instance, ...$this->arguments); return $this->instance; } - /** - * Gets the constructor being called or null if it is absent. - */ public function getConstructor(): ?ReflectionMethod { return $this->constructor; @@ -92,7 +84,7 @@ public function getConstructor(): ?ReflectionMethod /** * Returns the object for which current joinpoint is invoked * - * @return object|null Instance of object or null if object hasn't been created yet (Before) + * @return (object&T)|null Instance of object or null if object hasn't been created yet (Before) */ public function getThis(): ?object { @@ -101,8 +93,11 @@ public function getThis(): ?object /** * Invokes current constructor invocation with all interceptors + * + * @param array $arguments Arguments for constructor invocation + * @return (mixed|T) Instance of object or anything else from interceptors, eg Around type can replace object */ - final public function __invoke(array $arguments = []): object + final public function __invoke(array $arguments = []): mixed { $this->current = 0; $this->arguments = $arguments; @@ -111,20 +106,13 @@ final public function __invoke(array $arguments = []): object } /** - * Checks if the current joinpoint is dynamic or static - * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call - * - * @see ClassJoinpoint::getThis() + * @return true Covariance, always true for new object creation */ - public function isDynamic(): bool + public function isDynamic(): true { return true; } - /** - * Returns the static scope name (class name) of this joinpoint. - */ public function getScope(): string { return $this->class->getName(); diff --git a/src/Aop/Framework/ReflectionFunctionInvocation.php b/src/Aop/Framework/ReflectionFunctionInvocation.php index adcbf701..8f0ae1d2 100644 --- a/src/Aop/Framework/ReflectionFunctionInvocation.php +++ b/src/Aop/Framework/ReflectionFunctionInvocation.php @@ -13,26 +13,32 @@ namespace Go\Aop\Framework; use Go\Aop\Intercept\FunctionInvocation; +use Go\Aop\Intercept\Interceptor; use ReflectionException; use ReflectionFunction; - -use function array_merge; use function array_pop; /** * Function invocation implementation */ -class ReflectionFunctionInvocation extends AbstractInvocation implements FunctionInvocation +final class ReflectionFunctionInvocation extends AbstractInvocation implements FunctionInvocation { + /** + * Stack frames to work with recursive calls or with cross-calls inside object + * + * @phpstan-var array, int}> + */ + private array $stackFrames = []; + /** * Instance of reflection function */ - protected ReflectionFunction $reflectionFunction; + private readonly ReflectionFunction $reflectionFunction; /** * Constructor for function invocation * - * @param array $advices List of advices for this invocation + * @param array $advices List of advices for this invocation * * @throws ReflectionException */ @@ -43,11 +49,10 @@ public function __construct(array $advices, string $functionName) } /** - * Invokes original function and return result from it - * - * @return mixed + * @inheritdoc + * @return mixed Covariant, always mixed */ - public function proceed() + public function proceed(): mixed { if (isset($this->advices[$this->current])) { $currentInterceptor = $this->advices[$this->current++]; @@ -58,9 +63,6 @@ public function proceed() return $this->reflectionFunction->invokeArgs($this->arguments); } - /** - * Gets the function being called. - */ public function getFunction(): ReflectionFunction { return $this->reflectionFunction; @@ -69,19 +71,17 @@ public function getFunction(): ReflectionFunction /** * Invokes current function invocation with all interceptors * - * @param array $arguments List of arguments for function invocation - * @param array $variadicArguments Additional list of variadic arguments - * - * @return mixed Result of invocation + * @param array $arguments List of arguments for function invocation + * @param array $variadicArguments Additional list of variadic arguments */ - final public function __invoke(array $arguments = [], array $variadicArguments = []) + final public function __invoke(array $arguments = [], array $variadicArguments = []): mixed { if ($this->level > 0) { $this->stackFrames[] = [$this->arguments, $this->current]; } if (!empty($variadicArguments)) { - $arguments = array_merge($arguments, $variadicArguments); + $arguments = [...$arguments, ...$variadicArguments]; } try { @@ -94,8 +94,8 @@ final public function __invoke(array $arguments = [], array $variadicArguments = } finally { --$this->level; - if ($this->level > 0) { - [$this->arguments, $this->current] = array_pop($this->stackFrames); + if ($this->level > 0 && ($stackFrame = array_pop($this->stackFrames))) { + [$this->arguments, $this->current] = $stackFrame; } } diff --git a/src/Aop/Framework/StaticClosureMethodInvocation.php b/src/Aop/Framework/StaticClosureMethodInvocation.php index de4ff219..b09019fd 100644 --- a/src/Aop/Framework/StaticClosureMethodInvocation.php +++ b/src/Aop/Framework/StaticClosureMethodInvocation.php @@ -13,6 +13,7 @@ namespace Go\Aop\Framework; use Closure; +use Go\Aop\AspectException; /** * Static closure method invocation is responsible to call static methods via closure @@ -20,24 +21,33 @@ final class StaticClosureMethodInvocation extends AbstractMethodInvocation { /** - * Closure to use + * For static calls we store given argument as 'scope' property + * + * @see parent::__invoke() method to find out how this optimization works + * @see $scope Property, which is referenced by this static property */ - protected ?Closure $closureToCall = null; + protected static string $propertyName = 'scope'; /** - * Previous scope of invocation + * @var (string&class-string) Class name scope for static invocation */ - protected ?string $previousScope = null; + protected string $scope; /** - * For static calls we store given argument as 'scope' property + * Closure to use */ - protected static string $propertyName = 'scope'; + protected ?Closure $closureToCall; + + /** + * @var null|(string&class-string) Previous scope of invocation or null for first call + */ + protected ?string $previousScope = null; /** - * Proceeds all registered advices for the static method and returns an invocation result + * @inheritdoc + * @return mixed Covariant, always mixed */ - public function proceed() + public function proceed(): mixed { if (isset($this->advices[$this->current])) { $currentInterceptor = $this->advices[$this->current++]; @@ -47,7 +57,7 @@ public function proceed() // Rebind the closure if scope (class name) was changed since last time if ($this->previousScope !== $this->scope) { - if ($this->closureToCall === null) { + if (!isset($this->closureToCall)) { $this->closureToCall = self::getStaticInvoker( $this->reflectionMethod->class, $this->reflectionMethod->name @@ -57,7 +67,7 @@ public function proceed() $this->previousScope = $this->scope; } - return ($this->closureToCall)($this->arguments); + return ($this->closureToCall)?->__invoke($this->arguments); } /** @@ -65,34 +75,33 @@ public function proceed() */ protected static function getStaticInvoker(string $className, string $methodName): Closure { - return fn(array $args) => forward_static_call_array([$className, $methodName], $args); + $staticCallback = [$className, $methodName]; + // We can not check callable fully because of protected static methods, as we will be inside LSB call later + if (!is_callable($staticCallback, true)) { + throw new AspectException("Invalid static callback given {$className}::{$methodName}"); + } + + return fn(array $arguments): mixed => forward_static_call_array($staticCallback, $arguments); } /** - * Checks if the current joinpoint is dynamic or static - * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call - * - * @see ClassJoinpoint::getThis() + * @return false Covariance, always false for static initialization */ - final public function isDynamic(): bool + final public function isDynamic(): false { return false; } /** - * Returns the object for which current joinpoint is invoked + * @inheritdoc * - * @return object Instance of object or null for static call/unavailable context + * @return null Covariance, always null for static invocations */ - final public function getThis(): ?object + final public function getThis(): null { return null; } - /** - * Returns the static scope name (class name) of this joinpoint. - */ final public function getScope(): string { // $this->scope contains the current class scope that was received via static::class diff --git a/src/Aop/Framework/StaticInitializationJoinpoint.php b/src/Aop/Framework/StaticInitializationJoinpoint.php index b944990f..e5a42a7b 100644 --- a/src/Aop/Framework/StaticInitializationJoinpoint.php +++ b/src/Aop/Framework/StaticInitializationJoinpoint.php @@ -13,37 +13,35 @@ namespace Go\Aop\Framework; use Go\Aop\Intercept\ClassJoinpoint; -use Go\Core\AspectContainer; +use Go\Aop\Intercept\Interceptor; use ReflectionClass; -use function strlen; - /** * Static initialization joinpoint is invoked after class is loaded into memory + * + * @template T of object */ class StaticInitializationJoinpoint extends AbstractJoinpoint implements ClassJoinpoint { - - protected ReflectionClass $reflectionClass; + /** + * @var ReflectionClass Reflection of given class + */ + private readonly ReflectionClass $reflectionClass; /** * Constructor for the class static initialization joinpoint * - * @param array $advices List of advices for this invocation + * @param array $advices List of advices for this invocation + * @phpstan-param class-string $className Name of the class */ public function __construct(array $advices, string $className) { - $originalClass = $className; - if (strpos($originalClass, AspectContainer::AOP_PROXIED_SUFFIX)) { - $originalClass = substr($originalClass, 0, -strlen(AspectContainer::AOP_PROXIED_SUFFIX)); - } - $this->reflectionClass = new ReflectionClass($originalClass); + $this->reflectionClass = new ReflectionClass($className); parent::__construct($advices); } /** - * Proceeds to the next interceptor in the chain. - * + * @inheritdoc * @return void Covariant, as static initializtion could not return anything */ public function proceed(): void @@ -64,30 +62,23 @@ final public function __invoke(): void } /** - * Returns the object for which current joinpoint is invoked + * @inheritdoc * - * @return object|null Instance of object or null for static call/unavailable context + * @return null Covariance, always null for static initialization */ - public function getThis(): ?object + public function getThis(): null { return null; } /** - * Checks if the current joinpoint is dynamic or static - * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call - * - * @see ClassJoinpoint::getThis() + * @return false Covariance, always false for static method calls */ - public function isDynamic(): bool + public function isDynamic(): false { return false; } - /** - * Returns the static scope name (class name) of this joinpoint. - */ public function getScope(): string { return $this->reflectionClass->getName(); diff --git a/src/Aop/Framework/TraitIntroductionInfo.php b/src/Aop/Framework/TraitIntroductionInfo.php index fd83ef0c..087f72e9 100644 --- a/src/Aop/Framework/TraitIntroductionInfo.php +++ b/src/Aop/Framework/TraitIntroductionInfo.php @@ -15,40 +15,23 @@ use Go\Aop\IntroductionInfo; /** - * Advice for introduction that holds list of traits and interfaces for the concrete class + * Advice for introduction that holds trait and interface for the concrete class */ -class TraitIntroductionInfo implements IntroductionInfo +readonly class TraitIntroductionInfo implements IntroductionInfo { - /** - * Introduced interface - */ - private string $introducedInterface; - - /** - * Introduced trait - */ - private string $introducedTrait; - /** * Creates a TraitIntroductionInfo with given trait name and interface name. */ - public function __construct(string $introducedTrait, string $introducedInterface) - { - $this->introducedTrait = $introducedTrait; - $this->introducedInterface = $introducedInterface; - } + public function __construct( + private string $introducedTrait, + private string $introducedInterface + ){} - /** - * Returns the additional interface introduced by this Advisor or Advice. - */ public function getInterface(): string { return $this->introducedInterface; } - /** - * Returns the additional trait with realization of introduced interface - */ public function getTrait(): string { return $this->introducedTrait; diff --git a/src/Aop/Intercept/ClassJoinpoint.php b/src/Aop/Intercept/ClassJoinpoint.php index 7b90a66f..76224c84 100644 --- a/src/Aop/Intercept/ClassJoinpoint.php +++ b/src/Aop/Intercept/ClassJoinpoint.php @@ -22,7 +22,7 @@ interface ClassJoinpoint extends Joinpoint /** * Checks if the current joinpoint is dynamic or static * - * Dynamic joinpoint contains a reference to an object that can be received via getThis() method call + * Dynamic joinpoint contains a reference to an object that can be received via {@see getThis()} method call * * @see ClassJoinpoint::getThis() * @@ -31,9 +31,7 @@ interface ClassJoinpoint extends Joinpoint public function isDynamic(): bool; /** - * Returns the object for which current joinpoint is invoked - * - * @return object|null Instance of object or null for static call/unavailable context + * Returns the object for which current joinpoint is invoked or null for static calls * * @api */ @@ -42,6 +40,8 @@ public function getThis(): ?object; /** * Returns the static scope name (class name) of this joinpoint. * + * @return (string&class-string) + * * @api */ public function getScope(): string; diff --git a/src/Aop/Intercept/ConstructorInvocation.php b/src/Aop/Intercept/ConstructorInvocation.php index a90725c1..e4e7a7f9 100644 --- a/src/Aop/Intercept/ConstructorInvocation.php +++ b/src/Aop/Intercept/ConstructorInvocation.php @@ -18,6 +18,8 @@ * Description of an invocation to a constructor, given to an interceptor upon constructor-call. * * A constructor invocation is a joinpoint and can be intercepted by a constructor interceptor. + * + * @api */ interface ConstructorInvocation extends Invocation, ClassJoinpoint { diff --git a/src/Aop/Intercept/FieldAccess.php b/src/Aop/Intercept/FieldAccess.php index d7593c70..d30be99d 100644 --- a/src/Aop/Intercept/FieldAccess.php +++ b/src/Aop/Intercept/FieldAccess.php @@ -35,14 +35,14 @@ public function getField(): ReflectionProperty; * * @api */ - public function &getValue(); + public function &getValue(): mixed; /** * Gets the value that must be set to the field, applicable only for WRITE access type * * @api */ - public function &getValueToSet(); + public function &getValueToSet(): mixed; /** * Returns the access type. diff --git a/src/Aop/Intercept/FunctionInvocation.php b/src/Aop/Intercept/FunctionInvocation.php index f3e08453..6128ee24 100644 --- a/src/Aop/Intercept/FunctionInvocation.php +++ b/src/Aop/Intercept/FunctionInvocation.php @@ -29,10 +29,8 @@ public function getFunction(): ReflectionFunction; /** * Invokes current function invocation with all interceptors * - * @param array $arguments List of arguments for function invocation - * @param array $variadicArguments Additional list of variadic arguments - * - * @return mixed Result of invocation + * @param array $arguments List of arguments for function invocation + * @param array $variadicArguments Additional list of variadic arguments */ - public function __invoke(array $arguments = [], array $variadicArguments = []); + public function __invoke(array $arguments = [], array $variadicArguments = []): mixed; } diff --git a/src/Aop/Intercept/Interceptor.php b/src/Aop/Intercept/Interceptor.php index 29502a48..cffb3018 100644 --- a/src/Aop/Intercept/Interceptor.php +++ b/src/Aop/Intercept/Interceptor.php @@ -23,26 +23,7 @@ * access, exceptions... * * This interface is not used directly. Use the the sub-interfaces - * to intercept specific events. For instance, the following class - * implements some specific interceptors in order to implement a - * debugger: - * - *
- * class DebuggingInterceptor implements Interceptor
- * {
- *     public function invoke(Joinpoint $i)
- *     {
- *         $this->debug($i->getStaticPart(), $i->getThis(), $i->getArgs());
- *
- *         return $i->proceed();
- *     }
- *
- *     protected function debug($accessibleObject, $object, $value)
- *     {
- *         ...
- *     }
- * }
- * 
+ * to intercept specific events. * * @see Joinpoint * @api @@ -50,11 +31,9 @@ interface Interceptor extends Advice { /** - * Performs extra actions before and after the invocation of joinpoint. - * - * @return mixed The result of the call. Uses covariance in children. + * Performs extra actions before, after or around the invocation of joinpoint. * * @api */ - public function invoke(Joinpoint $joinpoint); + public function invoke(Joinpoint $joinpoint): mixed; } diff --git a/src/Aop/Intercept/Invocation.php b/src/Aop/Intercept/Invocation.php index 2f51a7b6..5d971020 100644 --- a/src/Aop/Intercept/Invocation.php +++ b/src/Aop/Intercept/Invocation.php @@ -15,7 +15,8 @@ /** * This interface represents an invocation in the program * - * An invocation is a callable joinpoint and can be intercepted by an interceptor + * An invocation is a callable joinpoint and can be intercepted by an interceptor. All invocation + * instances provide API for accessing/modification of invocation arguments. * * @api */ @@ -24,6 +25,7 @@ interface Invocation extends Joinpoint /** * Get the arguments of invocation as an array. * + * @return array * @api */ public function getArguments(): array; @@ -31,8 +33,7 @@ public function getArguments(): array; /** * Sets the arguments for current invocation * - * @param array $arguments New list of arguments - * + * @param array $arguments New list of arguments * @api */ public function setArguments(array $arguments): void; diff --git a/src/Aop/Intercept/Joinpoint.php b/src/Aop/Intercept/Joinpoint.php index 87ff15b0..c1530dd7 100644 --- a/src/Aop/Intercept/Joinpoint.php +++ b/src/Aop/Intercept/Joinpoint.php @@ -12,21 +12,26 @@ namespace Go\Aop\Intercept; +use Stringable; + /** * This interface represents a generic runtime joinpoint (in the AOP terminology). * * A runtime joinpoint is an event that occurs on a static joinpoint (i.e. a location in a the program). * For instance, an invocation is the runtime joinpoint on a method (static joinpoint). - * The static part of a given joinpoint can be generically retrieved using the getStaticPart() method. * - * In the context of an interception framework, a runtime joinpoint is then the reification of an access to - * an accessible object (a method, a constructor, a field), i.e. the static part of the joinpoint. It is passed - * to the interceptors that are installed on the static joinpoint. + * Joinpoint extends system's {@see Stringable} interface for better representation during debugging. + * + * As Joinpoint represents different places in the code, return type for the {@see self::proceed()} can not be + * specified in the current interface. Instead, all children classes should use covariant return types + * to narrow return type of this method. + * + * @see https://www.php.net/manual/en/language.oop5.variance.php * * @see Interceptor * @api */ -interface Joinpoint +interface Joinpoint extends Stringable { /** * Proceeds to the next interceptor in the chain. @@ -36,9 +41,4 @@ interface Joinpoint * @api */ public function proceed(); - - /** - * Returns a friendly description of current joinpoint - */ - public function __toString(): string; } diff --git a/src/Aop/Intercept/MethodInvocation.php b/src/Aop/Intercept/MethodInvocation.php index 32811009..c6e5f7f9 100644 --- a/src/Aop/Intercept/MethodInvocation.php +++ b/src/Aop/Intercept/MethodInvocation.php @@ -15,9 +15,14 @@ use ReflectionMethod; /** - * Description of an invocation to a method, given to an interceptor upon method-call. + * MethodInvocation interface represents an invocation of given method in the program. * * A method invocation is a joinpoint and can be intercepted by a method interceptor. + * + * Interceptor can read or modify invocation arguments via {@see Invocation} interface or + * receive full information about invoked method via {@see self::getMethod()} method. + * + * @api */ interface MethodInvocation extends Invocation, ClassJoinpoint { @@ -25,19 +30,15 @@ interface MethodInvocation extends Invocation, ClassJoinpoint * Gets the method being called. * * @api - * - * @return ReflectionMethod the method being called. */ public function getMethod(): ReflectionMethod; /** * Invokes current method invocation with all interceptors * - * @param null|object|string $instance Invocation instance (class name for static methods) - * @param array $arguments List of arguments for method invocation - * @param array $variadicArguments Additional list of variadic arguments - * - * @return mixed Result of invocation + * @phpstan-param object|class-string $instanceOrScope Invocation instance (or class name for static methods) + * @phpstan-param list $arguments List of arguments for method invocation + * @phpstan-param list $variadicArguments Additional list of variadic arguments */ - public function __invoke($instance = null, array $arguments = [], array $variadicArguments = []); + public function __invoke(object|string $instanceOrScope, array $arguments = [], array $variadicArguments = []): mixed; } diff --git a/src/Aop/Framework/OrderedAdvice.php b/src/Aop/OrderedAdvice.php similarity index 90% rename from src/Aop/Framework/OrderedAdvice.php rename to src/Aop/OrderedAdvice.php index 35022588..974478c1 100644 --- a/src/Aop/Framework/OrderedAdvice.php +++ b/src/Aop/OrderedAdvice.php @@ -10,9 +10,7 @@ * with this source code in the file LICENSE. */ -namespace Go\Aop\Framework; - -use Go\Aop\Advice; +namespace Go\Aop; /** * Ordered advice can have a custom order to implement sorting diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index e6417b12..8c75e12c 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -143,7 +143,7 @@ public static function injectJoinPoints(string $targetClassName): void $joinPointsProperty = $reflectionClass->getProperty(JoinPointPropertyGenerator::NAME); $advices = $joinPointsProperty->getValue(); - $joinPoints = static::wrapWithJoinPoints($advices, $reflectionClass->getParentClass()->name); + $joinPoints = static::wrapWithJoinPoints($advices, $reflectionClass->name); $joinPointsProperty->setValue(null, $joinPoints); $staticInit = AspectContainer::STATIC_INIT_PREFIX . ':root'; diff --git a/tests/Go/Aop/Framework/AbstractJoinpointTest.php b/tests/Go/Aop/Framework/AbstractJoinpointTest.php index 807bff66..06b638a4 100644 --- a/tests/Go/Aop/Framework/AbstractJoinpointTest.php +++ b/tests/Go/Aop/Framework/AbstractJoinpointTest.php @@ -7,6 +7,7 @@ use Go\Aop\AdviceAfter; use Go\Aop\AdviceAround; use Go\Aop\AdviceBefore; +use Go\Aop\OrderedAdvice; use PHPUnit\Framework\TestCase; class AbstractJoinpointTest extends TestCase diff --git a/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php b/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php index 0d5239bb..390bed09 100644 --- a/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php +++ b/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php @@ -20,7 +20,8 @@ public function setUp(): void public function testInvocationReturnsMethod(): void { - $this->assertEquals(self::class, $this->invocation->getMethod()->class); + // AbstractMethodInvocation uses prototype methods to avoid hard-coded class sufixes + $this->assertEquals(parent::class, $this->invocation->getMethod()->class); $this->assertEquals('setUp', $this->invocation->getMethod()->name); } } diff --git a/tests/Go/Aop/Framework/BaseInterceptorTest.php b/tests/Go/Aop/Framework/BaseInterceptorTest.php index 1da2f636..7685963b 100644 --- a/tests/Go/Aop/Framework/BaseInterceptorTest.php +++ b/tests/Go/Aop/Framework/BaseInterceptorTest.php @@ -43,7 +43,7 @@ public function testCanSerializeInterceptor() $mockClass = get_class($mock); $mockNameLength = strlen($mockClass); $result = serialize($mock); - $expected = 'O:' . $mockNameLength . ':"' . $mockClass . '":1:{s:12:"adviceMethod";a:3:{s:5:"scope";s:6:"aspect";s:6:"method";s:26:"Go\Aop\Framework\{closure}";s:6:"aspect";s:36:"Go\Aop\Framework\BaseInterceptorTest";}}'; + $expected = 'O:' . $mockNameLength . ':"' . $mockClass . '":1:{s:12:"adviceMethod";a:2:{s:4:"name";s:26:"Go\Aop\Framework\{closure}";s:5:"class";s:44:"Go\Aop\Framework\AbstractInterceptorTestCase";}}'; $this->assertEquals($expected, $result); } diff --git a/tests/Go/Aop/Framework/ReflectionConstructorInvocationTest.php b/tests/Go/Aop/Framework/ReflectionConstructorInvocationTest.php index 8b652214..59e3fdaf 100644 --- a/tests/Go/Aop/Framework/ReflectionConstructorInvocationTest.php +++ b/tests/Go/Aop/Framework/ReflectionConstructorInvocationTest.php @@ -23,14 +23,6 @@ public function testCanCreateObjectDuringInvocation(): void $this->assertInstanceOf(\Exception::class, $result); } - public function testKnowsAboutSpecialClassSuffix(): void - { - $specialName = \Exception::class . AspectContainer::AOP_PROXIED_SUFFIX; - $invocation = new ReflectionConstructorInvocation([], $specialName); - $result = $invocation->__invoke(); - $this->assertInstanceOf(\Exception::class, $result); - } - public function testCanExecuteAdvicesDuringConstruct(): void { $sequence = []; diff --git a/tests/Go/Stubs/AbstractInterceptorMock.php b/tests/Go/Stubs/AbstractInterceptorMock.php index 7e4d6544..75efb7eb 100644 --- a/tests/Go/Stubs/AbstractInterceptorMock.php +++ b/tests/Go/Stubs/AbstractInterceptorMock.php @@ -20,30 +20,12 @@ class AbstractInterceptorMock extends AbstractInterceptor { private static Closure $advice; - /** - * {@inheritdoc} - */ public function __construct(Closure $adviceMethod, int $adviceOrder = 0, string $pointcutExpression = '') { self::$advice = $adviceMethod; parent::__construct($adviceMethod, $adviceOrder, $pointcutExpression); } - /** - * {@inheritdoc} - */ - public static function serializeAdvice(Closure $adviceMethod): array - { - return [ - 'scope' => 'aspect', - 'method' => 'Go\Aop\Framework\{closure}', - 'aspect' => 'Go\Aop\Framework\BaseInterceptorTest' - ]; - } - - /** - * {@inheritdoc} - */ public static function unserializeAdvice(array $adviceData): Closure { return self::$advice;