diff --git a/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php b/Neos.Flow/Classes/Aop/Builder/AbstractMethodInterceptorBuilder.php index b3b4d987e3..61c9011d45 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,18 @@ 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); + $methodArguments = $joinPoint->getMethodArguments(); +'; + } else { + $advicesCode .= ' $result = $adviceChain->proceed($joinPoint); $methodArguments = $joinPoint->getMethodArguments(); '; + } } else { $advicesCode .= ' $joinPoint = new \Neos\Flow\Aop\JoinPoint($this, \'' . $targetClassName . '\', \'' . $methodName . '\', $methodArguments); @@ -195,7 +204,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()'); + } + } }