From ef8d03e7c7f2263f8f42a01772173701ffc67496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Ostroluck=C3=BD?= Date: Sun, 20 Feb 2022 12:29:44 +0100 Subject: [PATCH 1/4] Support reference params in interceptors --- .../Util/InterceptorGenerator.php | 3 +- .../Util/InterceptorGeneratorTest.php | 58 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGenerator.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGenerator.php index f7bf74833..e41b1ce3e 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGenerator.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGenerator.php @@ -66,7 +66,8 @@ public static function createInterceptedMethodBody( foreach ($method->getParameters() as $parameter) { $parameterName = $parameter->getName(); - $params[] = var_export($parameterName, true) . ' => $' . $parameter->getName(); + $symbol = $parameter->getPassedByReference() ? '&$' : '$'; + $params[] = var_export($parameterName, true) . ' => ' . $symbol . $parameterName; } $paramsString = 'array(' . implode(', ', $params) . ')'; diff --git a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGeneratorTest.php b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGeneratorTest.php index bef655ed0..b3025fdc7 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGeneratorTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/Util/InterceptorGeneratorTest.php @@ -175,6 +175,64 @@ public function testInterceptorGeneratorWithNonVoidOriginalMethod(): void } } +return $returnValue; +PHP; + // @codingStandardsIgnoreEnd + + self::assertSame( + $expected, + InterceptorGenerator::createInterceptedMethodBody( + '$returnValue = "foo";', + $method, + $valueHolder, + $prefixInterceptors, + $suffixInterceptors, + new ReflectionMethod(BaseClass::class, 'publicMethod') + ) + ); + } + + public function testInterceptorGeneratorWithReferences(): void + { + $method = $this->createMock(MethodGenerator::class); + $bar = $this->createMock(ParameterGenerator::class); + $baz = $this->createMock(ParameterGenerator::class); + $valueHolder = $this->createMock(PropertyGenerator::class); + $prefixInterceptors = $this->createMock(PropertyGenerator::class); + $suffixInterceptors = $this->createMock(PropertyGenerator::class); + + $bar->method('getName')->willReturn('bar'); + $bar->method('getPassedByReference')->willReturn(false); + $baz->method('getName')->willReturn('baz'); + $baz->method('getPassedByReference')->willReturn(true); + $method->method('getName')->willReturn('fooMethod'); + $method->method('getParameters')->will(self::returnValue([$bar, $baz])); + $valueHolder->method('getName')->willReturn('foo'); + $prefixInterceptors->method('getName')->willReturn('pre'); + $suffixInterceptors->method('getName')->willReturn('post'); + + // @codingStandardsIgnoreStart + $expected = <<<'PHP' +if (isset($this->pre['fooMethod'])) { + $returnEarly = false; + $prefixReturnValue = $this->pre['fooMethod']->__invoke($this, $this->foo, 'fooMethod', array('bar' => $bar, 'baz' => &$baz), $returnEarly); + + if ($returnEarly) { + return $prefixReturnValue; + } +} + +$returnValue = "foo"; + +if (isset($this->post['fooMethod'])) { + $returnEarly = false; + $suffixReturnValue = $this->post['fooMethod']->__invoke($this, $this->foo, 'fooMethod', array('bar' => $bar, 'baz' => &$baz), $returnValue, $returnEarly); + + if ($returnEarly) { + return $suffixReturnValue; + } +} + return $returnValue; PHP; // @codingStandardsIgnoreEnd From f8f50d20c727bc93c8338840f6a3a137f003c676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Ostroluck=C3=BD?= Date: Tue, 22 Feb 2022 20:22:51 +0100 Subject: [PATCH 2/4] Add access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt --- ...tercepting-keeps-parameter-references.phpt | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/language-feature-scripts/access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt diff --git a/tests/language-feature-scripts/access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt b/tests/language-feature-scripts/access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt new file mode 100644 index 000000000..bcb9fcf6f --- /dev/null +++ b/tests/language-feature-scripts/access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt @@ -0,0 +1,58 @@ +--TEST-- +Verifies that generated value holder proxy object doesn't loose track of references inside its interceptors +--FILE-- +createProxy( + new RefClass(), + [ + 'ref1' => static function( + object $proxy, + RefClass $instance, + string $method, + array $args, + bool &$returnEarly + ) { + $returnEarly = true; + $instance->ref1($args['ref']); + }, + ], + [ + 'ref2' => static function( + object $proxy, + RefClass $instance, + string $method, + array $args, + ) { + $instance->ref2($args['ref']); + }, + ], +); + +$ref = 0; + +$proxy->ref1($ref); +var_dump($ref); + +$proxy->ref2($ref); +var_dump($ref); + +--EXPECTF-- +int(1) +int(-1) From 3b653b8ddb8f4460f75914030decc0a3c0fd4614 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 28 Feb 2022 14:35:23 +0100 Subject: [PATCH 3/4] Converted #736 test case into an integration test While the `.phpt` test was valid, it does not produce coverage nor mutation test output, and requires to run in a separate test: converting it back to a more typical PHPUnit test makes it a bit quicker to run, and also puts it under `vimeo/psalm`'s watchful eyes. --- ...ssInterceptorValueHolderFunctionalTest.php | 74 +++++++++++++++++++ .../ReferenceIncrementDecrementClass.php | 13 ++++ ...tercepting-keeps-parameter-references.phpt | 58 --------------- 3 files changed, 87 insertions(+), 58 deletions(-) create mode 100644 tests/ProxyManagerTestAsset/ReferenceIncrementDecrementClass.php delete mode 100644 tests/language-feature-scripts/access-interceptor-value-holder-intercepting-keeps-parameter-references.phpt diff --git a/tests/ProxyManagerTest/Functional/AccessInterceptorValueHolderFunctionalTest.php b/tests/ProxyManagerTest/Functional/AccessInterceptorValueHolderFunctionalTest.php index 364ebe365..3516b73f2 100644 --- a/tests/ProxyManagerTest/Functional/AccessInterceptorValueHolderFunctionalTest.php +++ b/tests/ProxyManagerTest/Functional/AccessInterceptorValueHolderFunctionalTest.php @@ -25,6 +25,7 @@ use ProxyManagerTestAsset\ClassWithSelfHint; use ProxyManagerTestAsset\EmptyClass; use ProxyManagerTestAsset\OtherObjectAccessClass; +use ProxyManagerTestAsset\ReferenceIncrementDecrementClass; use ProxyManagerTestAsset\VoidCounter; use ReflectionClass; use ReflectionProperty; @@ -696,6 +697,79 @@ public function testWillInterceptAndReturnEarlyOnVoidMethod(): void self::assertSame($increment + $addMore + 1, $object->counter); } + public function testByReferencePassedArgumentsAreGivenAsReferenceToInterceptorCallbacks(): void + { + $proxy = (new AccessInterceptorValueHolderFactory())->createProxy( + new ReferenceIncrementDecrementClass(), + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + bool &$returnEarly + ): void { + self::assertSame(0, $args['reference']); + + $returnEarly = true; + $args['reference'] = 5; + }, + ] + ); + + $number = 0; + + $proxy->incrementReference($number); + + self::assertSame(5, $number, 'Number was changed by interceptor'); + } + + public function testByReferenceArgumentsAreForwardedThroughInterceptorsAndSubject(): void + { + $proxy = (new AccessInterceptorValueHolderFactory())->createProxy( + new ReferenceIncrementDecrementClass(), + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + bool &$returnEarly + ): void { + self::assertSame(0, $args['reference']); + + $returnEarly = false; + $args['reference'] = 5; + }, + ], + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + mixed $returnValue, + bool &$returnEarly + ): void { + self::assertIsInt($args['reference']); + + $returnEarly = false; + $args['reference'] *= 2; + }, + ] + ); + + $number = 0; + + $proxy->incrementReference($number); + + self::assertSame( + 12, + $number, + 'Number was changed by prefix interceptor, then incremented, then doubled by suffix interceptor' + ); + } + private static function assertByRefVariableValueSame(mixed $expected, mixed & $actual): void { self::assertSame($expected, $actual); diff --git a/tests/ProxyManagerTestAsset/ReferenceIncrementDecrementClass.php b/tests/ProxyManagerTestAsset/ReferenceIncrementDecrementClass.php new file mode 100644 index 000000000..1a999781d --- /dev/null +++ b/tests/ProxyManagerTestAsset/ReferenceIncrementDecrementClass.php @@ -0,0 +1,13 @@ +createProxy( - new RefClass(), - [ - 'ref1' => static function( - object $proxy, - RefClass $instance, - string $method, - array $args, - bool &$returnEarly - ) { - $returnEarly = true; - $instance->ref1($args['ref']); - }, - ], - [ - 'ref2' => static function( - object $proxy, - RefClass $instance, - string $method, - array $args, - ) { - $instance->ref2($args['ref']); - }, - ], -); - -$ref = 0; - -$proxy->ref1($ref); -var_dump($ref); - -$proxy->ref2($ref); -var_dump($ref); - ---EXPECTF-- -int(1) -int(-1) From 50f315bd281e70860b2299118a9b9f6939d53979 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 28 Feb 2022 14:45:24 +0100 Subject: [PATCH 4/4] Corrected by-ref parameters also for scope-localizer interceptor proxies --- .../Util/InterceptorGenerator.php | 6 +- ...nterceptorScopeLocalizerFunctionalTest.php | 74 +++++++++++++++++++ .../Util/InterceptorGeneratorTest.php | 55 ++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGenerator.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGenerator.php index 6cb122152..fb20a1b30 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGenerator.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGenerator.php @@ -67,7 +67,11 @@ public static function createInterceptedMethodBody( '{{$suffixInterceptorsName}}' => $suffixInterceptors->getName(), '{{$suffixEarlyReturnExpression}}' => ProxiedMethodReturnExpression::generate('$suffixReturnValue', $originalMethod), '{{$returnExpression}}' => ProxiedMethodReturnExpression::generate('$returnValue', $originalMethod), - '{{$paramsString}}' => 'array(' . implode(', ', array_map(static fn (ParameterGenerator $parameter): string => var_export($parameter->getName(), true) . ' => $' . $parameter->getName(), $method->getParameters())) . ')', + '{{$paramsString}}' => 'array(' . implode(', ', array_map( + static fn (ParameterGenerator $parameter): string => var_export($parameter->getName(), true) . ' => ' . ($parameter->getPassedByReference() ? '&$' : '$') . $parameter->getName(), + $method->getParameters() + )) + . ')', ]; return str_replace( diff --git a/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php b/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php index 7eefe2f76..a7b9a8576 100644 --- a/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php +++ b/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php @@ -24,6 +24,7 @@ use ProxyManagerTestAsset\ClassWithPublicStringNullableTypedProperty; use ProxyManagerTestAsset\ClassWithSelfHint; use ProxyManagerTestAsset\EmptyClass; +use ProxyManagerTestAsset\ReferenceIncrementDecrementClass; use ProxyManagerTestAsset\VoidCounter; use ReflectionClass; use stdClass; @@ -562,6 +563,79 @@ public function testWillRefuseToGenerateReferencesToTypedPropertiesWithoutDefaul $factory->createProxy($instance); } + public function testByReferencePassedArgumentsAreGivenAsReferenceToInterceptorCallbacks(): void + { + $proxy = (new AccessInterceptorScopeLocalizerFactory())->createProxy( + new ReferenceIncrementDecrementClass(), + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + bool &$returnEarly + ): void { + self::assertSame(0, $args['reference']); + + $returnEarly = true; + $args['reference'] = 5; + }, + ] + ); + + $number = 0; + + $proxy->incrementReference($number); + + self::assertSame(5, $number, 'Number was changed by interceptor'); + } + + public function testByReferenceArgumentsAreForwardedThroughInterceptorsAndSubject(): void + { + $proxy = (new AccessInterceptorScopeLocalizerFactory())->createProxy( + new ReferenceIncrementDecrementClass(), + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + bool &$returnEarly + ): void { + self::assertSame(0, $args['reference']); + + $returnEarly = false; + $args['reference'] = 5; + }, + ], + [ + 'incrementReference' => static function ( + object $proxy, + ReferenceIncrementDecrementClass $instance, + string $method, + array $args, + mixed $returnValue, + bool &$returnEarly + ): void { + self::assertIsInt($args['reference']); + + $returnEarly = false; + $args['reference'] *= 2; + }, + ] + ); + + $number = 0; + + $proxy->incrementReference($number); + + self::assertSame( + 12, + $number, + 'Number was changed by prefix interceptor, then incremented, then doubled by suffix interceptor' + ); + } + /** * @psalm-param ExpectedType $expected * diff --git a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGeneratorTest.php b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGeneratorTest.php index bb0cbf6b1..5fea44ebc 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGeneratorTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/Util/InterceptorGeneratorTest.php @@ -167,6 +167,61 @@ public function testInterceptorGeneratorWithExistingNonVoidMethod(): void } } +return $returnValue; +PHP; + // @codingStandardsIgnoreEnd + + self::assertSame( + $expected, + InterceptorGenerator::createInterceptedMethodBody( + '$returnValue = "foo";', + $method, + $prefixInterceptors, + $suffixInterceptors, + new ReflectionMethod(BaseClass::class, 'publicMethod') + ) + ); + } + + public function testInterceptorGeneratorWithReferences(): void + { + $method = $this->createMock(MethodGenerator::class); + $bar = $this->createMock(ParameterGenerator::class); + $baz = $this->createMock(ParameterGenerator::class); + $prefixInterceptors = $this->createMock(PropertyGenerator::class); + $suffixInterceptors = $this->createMock(PropertyGenerator::class); + + $bar->method('getName')->willReturn('bar'); + $bar->method('getPassedByReference')->willReturn(false); + $baz->method('getName')->willReturn('baz'); + $baz->method('getPassedByReference')->willReturn(true); + $method->method('getName')->willReturn('fooMethod'); + $method->method('getParameters')->will(self::returnValue([$bar, $baz])); + $prefixInterceptors->method('getName')->willReturn('pre'); + $suffixInterceptors->method('getName')->willReturn('post'); + + // @codingStandardsIgnoreStart + $expected = <<<'PHP' +if (isset($this->pre['fooMethod'])) { + $returnEarly = false; + $prefixReturnValue = $this->pre['fooMethod']->__invoke($this, $this, 'fooMethod', array('bar' => $bar, 'baz' => &$baz), $returnEarly); + + if ($returnEarly) { + return $prefixReturnValue; + } +} + +$returnValue = "foo"; + +if (isset($this->post['fooMethod'])) { + $returnEarly = false; + $suffixReturnValue = $this->post['fooMethod']->__invoke($this, $this, 'fooMethod', array('bar' => $bar, 'baz' => &$baz), $returnValue, $returnEarly); + + if ($returnEarly) { + return $suffixReturnValue; + } +} + return $returnValue; PHP; // @codingStandardsIgnoreEnd