From 30fc41df0eae427962892ab34caf899abcd45c0f Mon Sep 17 00:00:00 2001 From: Robert Lemke Date: Tue, 21 Oct 2025 11:47:42 +0200 Subject: [PATCH 1/2] BUGFIX: Support never return types in AOP proxies This adjusts proxy class building for AOP proxies so that the generate code correctly advises methods which have a "never" return type. Resolves #3451 --- .../AbstractMethodInterceptorBuilder.php | 12 ++- .../AdvisedConstructorInterceptorBuilder.php | 2 +- .../AdvisedMethodInterceptorBuilder.php | 5 +- .../Fixtures/NeverReturnTypeTestingAspect.php | 80 +++++++++++++++++++ .../TargetClassWithNeverReturnType.php | 37 +++++++++ .../Tests/Functional/Aop/FrameworkTest.php | 33 ++++++++ 6 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 Neos.Flow/Tests/Functional/Aop/Fixtures/NeverReturnTypeTestingAspect.php create mode 100644 Neos.Flow/Tests/Functional/Aop/Fixtures/TargetClassWithNeverReturnType.php diff --git a/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php b/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php index b3b4d987e3..e2a7eb2c3a 100644 --- a/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php +++ b/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php @@ -156,7 +156,7 @@ protected function buildSavedConstructorParametersCode(?string $className = null * @param string|null $declaringClassName Name of the declaring class. This is usually the same as the $targetClassName. However, it is the introduction interface for introduced methods. * @return string PHP code to be used in the method interceptor */ - protected function buildAdvicesCode(array $groupedAdvices, ?string $methodName = null, ?string $targetClassName = null, ?string $declaringClassName = null): string + protected function buildAdvicesCode(array $groupedAdvices, ?string $methodName, ?string $targetClassName, ?string $declaringClassName, ?string $declaredReturnType): string { $advicesCode = $this->buildMethodArgumentsArrayCode($declaringClassName, $methodName, ($methodName === '__construct')); @@ -184,9 +184,17 @@ protected function buildAdvicesCode(array $groupedAdvices, ?string $methodName = $adviceChain = $adviceChains[\'Neos\Flow\Aop\Advice\AroundAdvice\']; $adviceChain->rewind(); $joinPoint = new \Neos\Flow\Aop\JoinPoint($this, \'' . $targetClassName . '\', \'' . $methodName . '\', $methodArguments, $adviceChain); +'; + if ($declaredReturnType === 'never') { + $advicesCode .= ' + $adviceChain->proceed($joinPoint); +'; + } else { + $advicesCode .= ' $result = $adviceChain->proceed($joinPoint); $methodArguments = $joinPoint->getMethodArguments(); '; + } } else { $advicesCode .= ' $joinPoint = new \Neos\Flow\Aop\JoinPoint($this, \'' . $targetClassName . '\', \'' . $methodName . '\', $methodArguments); @@ -195,7 +203,7 @@ protected function buildAdvicesCode(array $groupedAdvices, ?string $methodName = '; } - if (isset($groupedAdvices[\Neos\Flow\Aop\Advice\AfterReturningAdvice::class])) { + if (isset($groupedAdvices[\Neos\Flow\Aop\Advice\AfterReturningAdvice::class]) && $declaredReturnType !== 'never') { $advicesCode .= ' if (isset($this->Flow_Aop_Proxy_targetMethodsAndGroupedAdvices[\'' . $methodName . '\'][\'Neos\Flow\Aop\Advice\AfterReturningAdvice\'])) { $advices = $this->Flow_Aop_Proxy_targetMethodsAndGroupedAdvices[\'' . $methodName . '\'][\'Neos\Flow\Aop\Advice\AfterReturningAdvice\']; diff --git a/Neos.Flow/Classes/Aop/Builder/AdvisedConstructorInterceptorBuilder.php b/Neos.Flow/Classes/Aop/Builder/AdvisedConstructorInterceptorBuilder.php index baaa0c188e..43f892a9f7 100644 --- a/Neos.Flow/Classes/Aop/Builder/AdvisedConstructorInterceptorBuilder.php +++ b/Neos.Flow/Classes/Aop/Builder/AdvisedConstructorInterceptorBuilder.php @@ -41,7 +41,7 @@ public function build(string $methodName, array $methodMetaInformation, string $ $proxyMethod = $this->compiler->getProxyClass($targetClassName)->getConstructor(); $groupedAdvices = $methodMetaInformation[$methodName]['groupedAdvices']; - $advicesCode = $this->buildAdvicesCode($groupedAdvices, $methodName, $targetClassName, $declaringClassName); + $advicesCode = $this->buildAdvicesCode($groupedAdvices, $methodName, $targetClassName, $declaringClassName, null); $proxyMethod->addPreParentCallCode(<<Flow_Aop_Proxy_methodIsInAdviceMode['{$methodName}'])) { diff --git a/Neos.Flow/Classes/Aop/Builder/AdvisedMethodInterceptorBuilder.php b/Neos.Flow/Classes/Aop/Builder/AdvisedMethodInterceptorBuilder.php index e0f7107500..e181df07aa 100644 --- a/Neos.Flow/Classes/Aop/Builder/AdvisedMethodInterceptorBuilder.php +++ b/Neos.Flow/Classes/Aop/Builder/AdvisedMethodInterceptorBuilder.php @@ -39,6 +39,7 @@ public function build(string $methodName, array $methodMetaInformation, string $ } $declaringClassName = $methodMetaInformation[$methodName]['declaringClassName']; + $declaredReturnType = ($declaringClassName !== null) ? $this->reflectionService->getMethodDeclaredReturnType($declaringClassName, $methodName) : null; $proxyMethod = $this->compiler->getProxyClass($targetClassName)->getMethod($methodName); if ($proxyMethod->getVisibility() === ProxyMethodGenerator::VISIBILITY_PRIVATE) { throw new Exception(sprintf('The %s cannot build interceptor code for private method %s::%s(). Please change the scope to at least protected or adjust the pointcut expression in the corresponding aspect.', __CLASS__, $targetClassName, $methodName), 1593070574); @@ -49,7 +50,8 @@ public function build(string $methodName, array $methodMetaInformation, string $ } $groupedAdvices = $methodMetaInformation[$methodName]['groupedAdvices']; - $advicesCode = $this->buildAdvicesCode($groupedAdvices, $methodName, $targetClassName, $declaringClassName); + $advicesCode = $this->buildAdvicesCode($groupedAdvices, $methodName, $targetClassName, $declaringClassName, $declaredReturnType); + $neverThrowCode = $declaredReturnType === 'never' ? 'throw new \RuntimeException(\'Possible bug in around advice proxy code for method ' . $targetClassName . '::' . $methodName . '() with return type "never". This point should never be reached. 👻\', 1761038455);' : ''; $proxyMethod->addPreParentCallCode(<<Flow_Aop_Proxy_methodIsInAdviceMode['{$methodName}'])) { @@ -65,6 +67,7 @@ public function build(string $methodName, array $methodMetaInformation, string $ } unset(\$this->Flow_Aop_Proxy_methodIsInAdviceMode['{$methodName}']); } + {$neverThrowCode} PHP); } } diff --git a/Neos.Flow/Tests/Functional/Aop/Fixtures/NeverReturnTypeTestingAspect.php b/Neos.Flow/Tests/Functional/Aop/Fixtures/NeverReturnTypeTestingAspect.php new file mode 100644 index 0000000000..c0fc3c9899 --- /dev/null +++ b/Neos.Flow/Tests/Functional/Aop/Fixtures/NeverReturnTypeTestingAspect.php @@ -0,0 +1,80 @@ +methodThatThrows())") + * @param JoinPointInterface $joinPoint + * @return void + */ + public function beforeNeverReturningMethod(JoinPointInterface $joinPoint): void + { + $proxy = $joinPoint->getProxy(); + assert($proxy instanceof TargetClassWithNeverReturnType); + $proxy->beforeAdviceWasInvoked = true; + } + + /** + * An after throwing advice should work with never return type + * + * @Flow\AfterThrowing("method(Neos\Flow\Tests\Functional\Aop\Fixtures\TargetClassWithNeverReturnType->methodThatThrows())") + * @param JoinPointInterface $joinPoint + * @return void + */ + public function afterThrowingNeverReturningMethod(JoinPointInterface $joinPoint): void + { + $proxy = $joinPoint->getProxy(); + assert($proxy instanceof TargetClassWithNeverReturnType); + $proxy->afterThrowingAdviceWasInvoked = true; + } + + /** + * + * @Flow\Around("method(Neos\Flow\Tests\Functional\Aop\Fixtures\TargetClassWithNeverReturnType->aroundAdvicedMethodThatThrows())") + * @param JoinPointInterface $joinPoint + * @return void + */ + public function aroundNeverReturningMethod(JoinPointInterface $joinPoint): void + { + $proxy = $joinPoint->getProxy(); + assert($proxy instanceof TargetClassWithNeverReturnType); + try { + $joinPoint->getAdviceChain()->proceed($joinPoint); + } catch (\RuntimeException $exception) { + $proxy->aroundAdviceWasInvoked = true; + throw $exception; + } + } + + /** + * An after returning advice makes no sense for never return type - but let's test what happens + * + * @Flow\AfterReturning("method(Neos\Flow\Tests\Functional\Aop\Fixtures\TargetClassWithNeverReturnType->methodThatExits())") + * @return void + */ + public function afterReturningNeverReturningMethod(): void + { + throw new \LogicException('AfterReturning advice should not be invoked for never-returning methods'); + } +} diff --git a/Neos.Flow/Tests/Functional/Aop/Fixtures/TargetClassWithNeverReturnType.php b/Neos.Flow/Tests/Functional/Aop/Fixtures/TargetClassWithNeverReturnType.php new file mode 100644 index 0000000000..a75cf1e844 --- /dev/null +++ b/Neos.Flow/Tests/Functional/Aop/Fixtures/TargetClassWithNeverReturnType.php @@ -0,0 +1,37 @@ +expectExceptionCode(1686132896); $targetClass->alwaysNever(); } + + /** + * @test + */ + public function methodWithNeverReturnTypeCanBeAdvised(): void + { + $targetClass = new TargetClassWithNeverReturnType(); + + try { + $targetClass->methodThatThrows(); + } catch (\Exception) { + } finally { + self::assertTrue($targetClass->beforeAdviceWasInvoked, 'Before advice should be invoked for never-returning method methodThatThrows()'); + self::assertTrue($targetClass->afterThrowingAdviceWasInvoked, 'AfterThrowing advice should be invoked for never-returning method methodThatThrows()'); + } + } + + /** + * @test + */ + public function methodWithNeverReturnTypeCanBeAroundAdvised(): void + { + $target = new TargetClassWithNeverReturnType(); + + try { + $target->aroundAdvicedMethodThatThrows(); + } catch (\Exception $e) { + self::assertSame(1761036724, $e->getCode()); + } finally { + self::assertTrue($target->aroundAdviceWasInvoked, 'Around advice should be invoked for never-returning method aroundAdvicedMethodThatThrows()'); + } + } } From 946a855194586b76ef8122be7b7186ecfadf35df Mon Sep 17 00:00:00 2001 From: Robert Lemke Date: Thu, 23 Oct 2025 14:11:48 +0200 Subject: [PATCH 2/2] Fix missing $methodArguments in AbstractMethodInterceptorBuilder --- .../Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php b/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php index e2a7eb2c3a..61c9011d45 100644 --- a/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php +++ b/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php @@ -188,6 +188,7 @@ protected function buildAdvicesCode(array $groupedAdvices, ?string $methodName, if ($declaredReturnType === 'never') { $advicesCode .= ' $adviceChain->proceed($joinPoint); + $methodArguments = $joinPoint->getMethodArguments(); '; } else { $advicesCode .= '