From 6d20cfcdb8b2464e9d47dba93f6f26de1e3d4346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=81abno?= <64330419+jakublabno@users.noreply.github.com> Date: Tue, 19 Apr 2022 16:23:56 +0200 Subject: [PATCH 1/5] [FIX] Library generates non-nullable return type for nullable return methods Library version 3.0.0 For below method proxy is generating non-null return ``` /** * @AopAnnotation() * @return null|int */ public function getSomeRandomValueAllowOnNull(): ?int { return random_int(0, 666); } ``` result: ``` /** * @AopAnnotation() * @return null|int */ public function getSomeRandomValueAllowOnNull() : int { return self::$__joinPoints['method:getSomeRandomValueAllowOnNull']->__invoke($this); } ``` --- src/Proxy/Part/InterceptedMethodGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Proxy/Part/InterceptedMethodGenerator.php b/src/Proxy/Part/InterceptedMethodGenerator.php index 44e1b35b..b8b26b46 100644 --- a/src/Proxy/Part/InterceptedMethodGenerator.php +++ b/src/Proxy/Part/InterceptedMethodGenerator.php @@ -39,7 +39,7 @@ public function __construct(ReflectionMethod $reflectionMethod, string $body, bo if ($reflectionMethod->hasReturnType()) { $reflectionReturnType = $reflectionMethod->getReturnType(); if ($reflectionReturnType instanceof ReflectionNamedType) { - $returnTypeName = $reflectionReturnType->getName(); + $returnTypeName = ($reflectionReturnType->allowsNull() ? '?' : '') . $reflectionReturnType->getName(); } else { $returnTypeName = (string)$reflectionReturnType; } From 32b897bbc259afe7e35f81edb98b409ccb108e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Malte=20D=C3=B6lker?= Date: Wed, 26 Apr 2023 12:43:58 +0200 Subject: [PATCH 2/5] Migrate to doctrine/annotations 2.x --- composer.json | 2 +- src/Core/GoAspectContainer.php | 7 ++++--- src/Instrument/ClassLoading/AopComposerLoader.php | 8 -------- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 841d6b36..1e193a36 100644 --- a/composer.json +++ b/composer.json @@ -9,7 +9,7 @@ "require": { "php": "^7.4.0", - "doctrine/annotations": "^1.11.1", + "doctrine/annotations": "^2.0", "doctrine/cache": "^1.10", "goaop/parser-reflection": "^3.0.1", "jakubledl/dissect": "~1.0", diff --git a/src/Core/GoAspectContainer.php b/src/Core/GoAspectContainer.php index f565d471..9c7a37fc 100644 --- a/src/Core/GoAspectContainer.php +++ b/src/Core/GoAspectContainer.php @@ -13,7 +13,7 @@ namespace Go\Core; use Doctrine\Common\Annotations\AnnotationReader; -use Doctrine\Common\Annotations\CachedReader; +use Doctrine\Common\Annotations\PsrCachedReader; use Doctrine\Common\Cache as DoctrineCache; use Go\Aop\Advisor; use Go\Aop\Aspect; @@ -104,10 +104,11 @@ public function __construct() $this->share('aspect.annotation.reader', function (Container $container) { $options = $container->get('kernel.options'); + $annotationCache = $container->get('aspect.annotation.cache'); - return new CachedReader( + return new PsrCachedReader( new AnnotationReader(), - $container->get('aspect.annotation.cache'), + new DoctrineCache\Psr6\CacheAdapter($annotationCache), $options['debug'] ?? false ); }); diff --git a/src/Instrument/ClassLoading/AopComposerLoader.php b/src/Instrument/ClassLoading/AopComposerLoader.php index 691b8bfe..2491bbc5 100644 --- a/src/Instrument/ClassLoading/AopComposerLoader.php +++ b/src/Instrument/ClassLoading/AopComposerLoader.php @@ -18,7 +18,6 @@ use Go\Instrument\PathResolver; use Go\Instrument\Transformer\FilterInjectorTransformer; use Composer\Autoload\ClassLoader; -use Doctrine\Common\Annotations\AnnotationRegistry; /** * AopComposerLoader class is responsible to use a weaver for classes instead of original one @@ -92,13 +91,6 @@ public static function init(array $options, AspectContainer $container): bool foreach ($loaders as &$loader) { $loaderToUnregister = $loader; if (is_array($loader) && ($loader[0] instanceof ClassLoader)) { - $originalLoader = $loader[0]; - // Configure library loader for doctrine annotation loader - AnnotationRegistry::registerLoader(function($class) use ($originalLoader) { - $originalLoader->loadClass($class); - - return class_exists($class, false); - }); $loader[0] = new AopComposerLoader($loader[0], $container, $options); self::$wasInitialized = true; } From e60fbd18b772c3e3e012c20761f585e3bfcf037f Mon Sep 17 00:00:00 2001 From: Daniel Schreiber Date: Thu, 25 May 2023 20:52:47 +0200 Subject: [PATCH 3/5] fix: prevent generating "namespace ;" for classes without declared namespace --- composer.json | 3 ++- src/Proxy/ClassProxyGenerator.php | 2 +- tests/Go/Proxy/ClassProxyGeneratorTest.php | 1 + tests/Go/Stubs/ClassWithoutNamespace.php | 11 +++++++++++ 4 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 tests/Go/Stubs/ClassWithoutNamespace.php diff --git a/composer.json b/composer.json index 841d6b36..97f9050e 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,8 @@ "Go\\Tests\\TestProject\\": "tests/Fixtures/project/src/" }, "files": [ - "tests/functions.php" + "tests/functions.php", + "tests/Go/Stubs/ClassWithoutNamespace.php" ] }, diff --git a/src/Proxy/ClassProxyGenerator.php b/src/Proxy/ClassProxyGenerator.php index b151d146..2e31cef8 100644 --- a/src/Proxy/ClassProxyGenerator.php +++ b/src/Proxy/ClassProxyGenerator.php @@ -109,7 +109,7 @@ public function __construct( $this->generator = new ClassGenerator( $originalClass->getShortName(), - $originalClass->getNamespaceName(), + !empty($originalClass->getNamespaceName()) ? $originalClass->getNamespaceName() : null, $originalClass->isFinal() ? ClassGenerator::FLAG_FINAL : null, $parentClassName, $introducedInterfaces, diff --git a/tests/Go/Proxy/ClassProxyGeneratorTest.php b/tests/Go/Proxy/ClassProxyGeneratorTest.php index 9c2eee17..d13158f7 100644 --- a/tests/Go/Proxy/ClassProxyGeneratorTest.php +++ b/tests/Go/Proxy/ClassProxyGeneratorTest.php @@ -93,6 +93,7 @@ public function dataGenerator(): array [First::class, 'publicMethod'], [First::class, 'protectedMethod'], [First::class, 'passByReference'], + [\ClassWithoutNamespace::class, 'publicMethod'], ]; } } diff --git a/tests/Go/Stubs/ClassWithoutNamespace.php b/tests/Go/Stubs/ClassWithoutNamespace.php new file mode 100644 index 00000000..d14eef67 --- /dev/null +++ b/tests/Go/Stubs/ClassWithoutNamespace.php @@ -0,0 +1,11 @@ + Date: Fri, 26 May 2023 12:30:10 +0200 Subject: [PATCH 4/5] fix: prevent error for classes without namespace: Call to a member function toString() on null fixes #420 --- .../Transformer/SelfValueVisitor.php | 2 +- .../Transformer/SelfValueTransformerTest.php | 10 +++ ...ile-with-self-no-namespace-transformed.php | 61 +++++++++++++++++++ .../_files/file-with-self-no-namespace.php | 61 +++++++++++++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/Go/Instrument/Transformer/_files/file-with-self-no-namespace-transformed.php create mode 100644 tests/Go/Instrument/Transformer/_files/file-with-self-no-namespace.php diff --git a/src/Instrument/Transformer/SelfValueVisitor.php b/src/Instrument/Transformer/SelfValueVisitor.php index 3e8c7924..a8339a6b 100644 --- a/src/Instrument/Transformer/SelfValueVisitor.php +++ b/src/Instrument/Transformer/SelfValueVisitor.php @@ -81,7 +81,7 @@ public function beforeTraverse(array $nodes) public function enterNode(Node $node) { if ($node instanceof Namespace_) { - $this->namespace = $node->name->toString(); + $this->namespace = !empty($node->name) ? $node->name->toString() : null; } elseif ($node instanceof Class_) { if ($node->name !== null) { $this->className = new Name($node->name->toString()); diff --git a/tests/Go/Instrument/Transformer/SelfValueTransformerTest.php b/tests/Go/Instrument/Transformer/SelfValueTransformerTest.php index 89bb6d0e..98f4b991 100644 --- a/tests/Go/Instrument/Transformer/SelfValueTransformerTest.php +++ b/tests/Go/Instrument/Transformer/SelfValueTransformerTest.php @@ -72,4 +72,14 @@ public function testTransformerReplacesAllSelfPlaces(): void $expected = file_get_contents(__DIR__ . '/_files/file-with-self-transformed.php'); $this->assertSame($expected, (string) $metadata->source); } + + public function testTransformerReplacesAllSelfPlacesWithoutNamespace(): void + { + $testFile = fopen(__DIR__ . '/_files/file-with-self-no-namespace.php', 'rb'); + $content = stream_get_contents($testFile); + $metadata = new StreamMetaData($testFile, $content); + $this->transformer->transform($metadata); + $expected = file_get_contents(__DIR__ . '/_files/file-with-self-no-namespace-transformed.php'); + $this->assertSame($expected, (string) $metadata->source); + } } diff --git a/tests/Go/Instrument/Transformer/_files/file-with-self-no-namespace-transformed.php b/tests/Go/Instrument/Transformer/_files/file-with-self-no-namespace-transformed.php new file mode 100644 index 00000000..25324abd --- /dev/null +++ b/tests/Go/Instrument/Transformer/_files/file-with-self-no-namespace-transformed.php @@ -0,0 +1,61 @@ + Date: Sun, 11 Jun 2023 21:43:18 +0200 Subject: [PATCH 5/5] fix: prevent "AbstractMethodInvocation::$instance must not be accessed before initialization" for recursive calls see #481 --- src/Aop/Framework/AbstractInterceptor.php | 2 +- .../Framework/AbstractMethodInvocation.php | 2 +- .../AbstractMethodInvocationTest.php | 37 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/Aop/Framework/AbstractInterceptor.php b/src/Aop/Framework/AbstractInterceptor.php index 069ad44e..be2ff5ca 100644 --- a/src/Aop/Framework/AbstractInterceptor.php +++ b/src/Aop/Framework/AbstractInterceptor.php @@ -65,7 +65,7 @@ abstract class AbstractInterceptor implements Interceptor, OrderedAdvice, Serial /** * Advice order */ - private int $adviceOrder; + private int $adviceOrder = 0; /** * Default constructor for interceptor diff --git a/src/Aop/Framework/AbstractMethodInvocation.php b/src/Aop/Framework/AbstractMethodInvocation.php index 932c7589..a711685d 100644 --- a/src/Aop/Framework/AbstractMethodInvocation.php +++ b/src/Aop/Framework/AbstractMethodInvocation.php @@ -27,7 +27,7 @@ abstract class AbstractMethodInvocation extends AbstractInvocation implements Me /** * Instance of object for invoking */ - protected ?object $instance; + protected ?object $instance = null; /** * Instance of reflection method for invocation diff --git a/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php b/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php index 6ffc80a2..e6a54adb 100644 --- a/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php +++ b/tests/Go/Aop/Framework/AbstractMethodInvocationTest.php @@ -29,4 +29,41 @@ public function testProvidesAccessToAnnotations(): void { $this->assertInstanceOf(AnnotationAccess::class, $this->invocation->getMethod()); } + + public function testInstanceIsInitialized(): void + { + $this->expectNotToPerformAssertions(); + $o = new class extends AbstractMethodInvocation { + + protected static string $propertyName = 'scope'; + public function __construct() + { + parent::__construct([new AroundInterceptor(function () {})], '\Go\Aop\Framework\AbstractMethodInvocationTest', 'testInstanceIsInitialized'); + } + + public function isDynamic(): bool + { + return false; + } + + public function getThis(): ?object + { + return null; + } + + public function getScope(): string + { + return 'testScope'; + } + + public function proceed() + { + if ($this->level < 3) { + $this->__invoke('testInstance'); + } + } + }; + + $o->__invoke('testInstance'); + } }