Merge pull request #654 from Ocramius/2.10.x-merge-up-into-2.11.x_5fec8fce435468.86689243

Ocramius · web-flow · commit 15440d560210 · 2020-12-30T15:34:33.000+01:00
Merge release 2.10.1 into 2.11.x
diff --git a/infection.json.dist b/infection.json.dist
@@ -10,6 +10,6 @@
             "branch": "master"
         }
     },
-    "minMsi": 84,
+    "minMsi": 84.5,
     "minCoveredMsi": 86
 }
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGet.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGet.php
@@ -38,7 +38,8 @@ public function __construct(
                 'name',
                 null,
                 null,
-                'returnValue'
+                'returnValue',
+                $originalClass
             );
         }
 
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicIsset.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicIsset.php
@@ -38,7 +38,8 @@ public function __construct(
                 'name',
                 null,
                 null,
-                'returnValue'
+                'returnValue',
+                $originalClass
             );
         }
 
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicSet.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicSet.php
@@ -42,7 +42,8 @@ public function __construct(
                 'name',
                 'value',
                 null,
-                'returnValue'
+                'returnValue',
+                $originalClass
             );
         }
 
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicUnset.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicUnset.php
@@ -38,7 +38,8 @@ public function __construct(
                 'name',
                 null,
                 null,
-                'returnValue'
+                'returnValue',
+                $originalClass
             );
         }
 
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicGet.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicGet.php
@@ -39,9 +39,10 @@ public function __construct(
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_GET,
             'name',
-            'value',
+            null,
             $valueHolder,
-            'returnValue'
+            'returnValue',
+            $originalClass
         );
 
         if (! $publicProperties->isEmpty()) {
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicIsset.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicIsset.php
@@ -39,9 +39,10 @@ public function __construct(
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_ISSET,
             'name',
-            'value',
+            null,
             $valueHolder,
-            'returnValue'
+            'returnValue',
+            $originalClass
         );
 
         if (! $publicProperties->isEmpty()) {
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicSet.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicSet.php
@@ -45,7 +45,8 @@ public function __construct(
             'name',
             'value',
             $valueHolder,
-            'returnValue'
+            'returnValue',
+            $originalClass
         );
 
         if (! $publicProperties->isEmpty()) {
diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicUnset.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicUnset.php
@@ -39,9 +39,10 @@ public function __construct(
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_UNSET,
             'name',
-            'value',
+            null,
             $valueHolder,
-            'returnValue'
+            'returnValue',
+            $originalClass
         );
 
         if (! $publicProperties->isEmpty()) {
diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicGet.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicGet.php
@@ -55,7 +55,9 @@ public function __construct(
                 PublicScopeSimulator::OPERATION_GET,
                 'name',
                 null,
-                $valueHolderProperty
+                $valueHolderProperty,
+                null,
+                $originalClass
             )
         );
     }
diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicIsset.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicIsset.php
@@ -44,7 +44,9 @@ public function __construct(
             PublicScopeSimulator::OPERATION_ISSET,
             'name',
             null,
-            $valueHolderProperty
+            $valueHolderProperty,
+            null,
+            $originalClass
         );
 
         $this->setBody(
diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicSet.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicSet.php
@@ -51,7 +51,9 @@ public function __construct(
                 PublicScopeSimulator::OPERATION_SET,
                 'name',
                 'value',
-                $valueHolderProperty
+                $valueHolderProperty,
+                null,
+                $originalClass
             );
 
         $this->setBody(
diff --git a/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicUnset.php b/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicUnset.php
@@ -47,7 +47,9 @@ public function __construct(
                 PublicScopeSimulator::OPERATION_UNSET,
                 'name',
                 null,
-                $valueHolderProperty
+                $valueHolderProperty,
+                null,
+                $originalClass
             );
 
         $this->setBody(
diff --git a/src/ProxyManager/ProxyGenerator/Util/PublicScopeSimulator.php b/src/ProxyManager/ProxyGenerator/Util/PublicScopeSimulator.php
@@ -6,8 +6,13 @@
 
 use InvalidArgumentException;
 use Laminas\Code\Generator\PropertyGenerator;
+use ReflectionClass;
 
+use function array_filter;
+use function array_map;
+use function implode;
 use function sprintf;
+use function var_export;
 
 /**
  * Generates code necessary to simulate a fatal error in case of unauthorized
@@ -28,50 +33,68 @@ class PublicScopeSimulator
      *
      * @param string            $operationType      operation to execute: one of 'get', 'set', 'isset' or 'unset'
      * @param string            $nameParameter      name of the `name` parameter of the magic method
-     * @param string|null       $valueParameter     name of the `value` parameter of the magic method
+     * @param string|null       $valueParameter     name of the `value` parameter of the magic method, only to be
+     *                                              used with $operationType 'set'
      * @param PropertyGenerator $valueHolder        name of the property containing the target object from which
      *                                              to read the property. `$this` if none provided
      * @param string|null       $returnPropertyName name of the property to which we want to assign the result of
      *                                              the operation. Return directly if none provided
+     * @param string|null       $interfaceName      name of the proxified interface if any
      *
      * @throws InvalidArgumentException
+     *
+     * @psalm-param $operationType self::OPERATION_*
      */
     public static function getPublicAccessSimulationCode(
         string $operationType,
         string $nameParameter,
         ?string $valueParameter = null,
         ?PropertyGenerator $valueHolder = null,
-        ?string $returnPropertyName = null
+        ?string $returnPropertyName = null,
+        ?ReflectionClass $originalClass = null
     ): string {
         $byRef  = self::getByRefReturnValue($operationType);
-        $value  = $operationType === self::OPERATION_SET ? ', $value' : '';
         $target = '$this';
 
         if ($valueHolder) {
             $target = '$this->' . $valueHolder->getName();
         }
 
-        return '$realInstanceReflection = new \\ReflectionClass(get_parent_class($this));' . "\n\n"
+        $originalClassReflection = $originalClass === null
+            ? 'new \\ReflectionClass(get_parent_class($this))'
+            : 'new \\ReflectionClass(' . var_export($originalClass->getName(), true) . ')';
+
+        $accessorEvaluation = $returnPropertyName
+            ? '$' . $returnPropertyName . ' = ' . $byRef . '$accessor();'
+            : '$returnValue = ' . $byRef . '$accessor();' . "\n\n" . 'return $returnValue;';
+
+        if ($operationType === self::OPERATION_UNSET) {
+            $accessorEvaluation = '$accessor();';
+        }
+
+        return '$realInstanceReflection = ' . $originalClassReflection . ';' . "\n\n"
             . 'if (! $realInstanceReflection->hasProperty($' . $nameParameter . ')) {' . "\n"
             . '    $targetObject = ' . $target . ';' . "\n\n"
             . self::getUndefinedPropertyNotice($operationType, $nameParameter)
             . '    ' . self::getOperation($operationType, $nameParameter, $valueParameter) . "\n"
-            . "    return;\n"
             . '}' . "\n\n"
             . '$targetObject = ' . self::getTargetObject($valueHolder) . ";\n"
-            . '$accessor = function ' . $byRef . '() use ($targetObject, $name' . $value . ') {' . "\n"
+            . '$accessor = function ' . $byRef . '() use ('
+            . implode(', ', array_map(
+                static fn (string $parameterName): string => '$' . $parameterName,
+                array_filter(['targetObject', $nameParameter, $valueParameter])
+            ))
+            . ') {' . "\n"
             . '    ' . self::getOperation($operationType, $nameParameter, $valueParameter) . "\n"
             . "};\n"
-            . self::getScopeReBind()
-            . (
-                $returnPropertyName
-                    ? '$' . $returnPropertyName . ' = ' . $byRef . '$accessor();'
-                    : '$returnValue = ' . $byRef . '$accessor();' . "\n\n" . 'return $returnValue;'
-            );
+            . self::generateScopeReBind()
+            . $accessorEvaluation;
     }
 
     /**
      * This will generate code that triggers a notice if access is attempted on a non-existing property
+     *
+     * @psalm-param $operationType self::OPERATION_*
      */
     private static function getUndefinedPropertyNotice(string $operationType, string $nameParameter): string
     {
@@ -83,7 +106,7 @@ private static function getUndefinedPropertyNotice(string $operationType, string
             . '    trigger_error(' . "\n"
             . '        sprintf(' . "\n"
             . '            \'Undefined property: %s::$%s in %s on line %s\',' . "\n"
-            . '            get_parent_class($this),' . "\n"
+            . '            $realInstanceReflection->getName(),' . "\n"
             . '            $' . $nameParameter . ',' . "\n"
             . '            $backtrace[0][\'file\'],' . "\n"
             . '            $backtrace[0][\'line\']' . "\n"
@@ -98,6 +121,8 @@ private static function getUndefinedPropertyNotice(string $operationType, string
      * Note: if the object is a wrapper, the wrapped instance is accessed directly. If the object
      * is a ghost or the proxy has no wrapper, then an instance of the parent class is created via
      * on-the-fly unserialization
+     *
+     * @psalm-param $operationType self::OPERATION_*
      */
     private static function getByRefReturnValue(string $operationType): string
     {
@@ -118,25 +143,43 @@ private static function getTargetObject(?PropertyGenerator $valueHolder = null):
 
     /**
      * @throws InvalidArgumentException
+     *
+     * @psalm-param $operationType self::OPERATION_*
      */
     private static function getOperation(string $operationType, string $nameParameter, ?string $valueParameter): string
     {
+        if ($valueParameter !== null && $operationType !== self::OPERATION_SET) {
+            throw new InvalidArgumentException(
+                'Parameter $valueParameter should be provided (only) when $operationType === "' . self::OPERATION_SET . '"'
+                . self::class
+                . '::OPERATION_SET'
+            );
+        }
+
         switch ($operationType) {
             case self::OPERATION_GET:
                 return 'return $targetObject->$' . $nameParameter . ';';
 
             case self::OPERATION_SET:
                 if ($valueParameter === null) {
-                    throw new InvalidArgumentException('Parameter $valueParameter not provided');
+                    throw new InvalidArgumentException(
+                        'Parameter $valueParameter should be provided (only) when $operationType === "' . self::OPERATION_SET . '"'
+                        . self::class
+                        . '::OPERATION_SET'
+                    );
                 }
 
-                return 'return $targetObject->$' . $nameParameter . ' = $' . $valueParameter . ';';
+                return '$targetObject->$' . $nameParameter . ' = $' . $valueParameter . ';'
+                    . "\n\n"
+                    . '    return $targetObject->$' . $nameParameter . ';';
 
             case self::OPERATION_ISSET:
                 return 'return isset($targetObject->$' . $nameParameter . ');';
 
             case self::OPERATION_UNSET:
-                return 'unset($targetObject->$' . $nameParameter . ');';
+                return 'unset($targetObject->$' . $nameParameter . ');'
+                    . "\n\n"
+                    . '    return;';
         }
 
         throw new InvalidArgumentException(sprintf('Invalid operation "%s" provided', $operationType));
@@ -145,11 +188,13 @@ private static function getOperation(string $operationType, string $nameParamete
     /**
      * Generates code to bind operations to the parent scope
      */
-    private static function getScopeReBind(): string
+    private static function generateScopeReBind(): string
     {
-        return '$backtrace = debug_backtrace(true, 2);' . "\n"
-            . '$scopeObject = isset($backtrace[1][\'object\'])'
-            . ' ? $backtrace[1][\'object\'] : new \ProxyManager\Stub\EmptyClassStub();' . "\n"
-            . '$accessor = $accessor->bindTo($scopeObject, get_class($scopeObject));' . "\n";
+        return <<<'PHP'
+$backtrace = debug_backtrace(true, 2);
+$scopeObject = isset($backtrace[1]['object']) ? $backtrace[1]['object'] : new \ProxyManager\Stub\EmptyClassStub();
+$accessor = $accessor->bindTo($scopeObject, get_class($scopeObject));
+
+PHP;
     }
 }
diff --git a/tests/ProxyManagerTest/Functional/PublicScopeSimulatorFunctionalTest.php b/tests/ProxyManagerTest/Functional/PublicScopeSimulatorFunctionalTest.php
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ProxyManagerTest\ProxyGenerator\Functional;
+
+use PHPUnit\Framework\TestCase;
+use ProxyManager\ProxyGenerator\Util\PublicScopeSimulator;
+use ProxyManagerTestAsset\ClassWithMixedProperties;
+
+use function sprintf;
+
+/**
+ * @covers \ProxyManager\ProxyGenerator\Util\PublicScopeSimulator
+ * @coversNothing
+ */
+final class PublicScopeSimulatorFunctionalTest extends TestCase
+{
+    /**
+     * @group #632
+     * @group #645
+     * @group #646
+     */
+    public function testAccessingUndefinedPropertiesDoesNotLeadToInvalidByRefAccess(): void
+    {
+        /** @psalm-var ClassWithMixedProperties $sut */
+        $sut = eval(sprintf(
+            <<<'PHP'
+return new class() extends %s {
+    public function doGet($prop) : string { %s }
+    public function doSet($prop, $val) : string { %s }
+    public function doIsset($prop) : bool { %s }
+    public function doUnset($prop) : void { %s }
+};
+PHP
+            ,
+            ClassWithMixedProperties::class,
+            PublicScopeSimulator::getPublicAccessSimulationCode(PublicScopeSimulator::OPERATION_GET, 'prop'),
+            PublicScopeSimulator::getPublicAccessSimulationCode(PublicScopeSimulator::OPERATION_SET, 'prop', 'val'),
+            PublicScopeSimulator::getPublicAccessSimulationCode(PublicScopeSimulator::OPERATION_ISSET, 'prop'),
+            PublicScopeSimulator::getPublicAccessSimulationCode(PublicScopeSimulator::OPERATION_UNSET, 'prop')
+        ));
+
+        self::assertSame('publicProperty0', $sut->doGet('publicProperty0'));
+        self::assertSame('bar', $sut->doSet('publicProperty0', 'bar'));
+        self::assertTrue($sut->doIsset('publicProperty0'));
+        self::assertNull($sut->doUnset('publicProperty0'));
+    }
+}
diff --git a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGetTest.php b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGetTest.php
@@ -16,12 +16,10 @@
  * Tests for {@see \ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicGet}
  *
  * @group Coverage
+ * @covers \ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicGet
  */
 final class MagicGetTest extends TestCase
 {
-    /**
-     * @covers \ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicGet::__construct
-     */
     public function testBodyStructure(): void
     {
         $reflection         = new ReflectionClass(EmptyClass::class);
@@ -42,9 +40,6 @@ public function testBodyStructure(): void
         self::assertStringMatchesFormat('%a$returnValue = & $accessor();%a', $magicGet->getBody());
     }
 
-    /**
-     * @covers \ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicGet::__construct
-     */
     public function testBodyStructureWithInheritedMethod(): void
     {
         $reflection         = new ReflectionClass(ClassWithMagicMethods::class);
diff --git a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicUnsetTest.php b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicUnsetTest.php
diff --git a/tests/ProxyManagerTest/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicUnsetTest.php b/tests/ProxyManagerTest/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicUnsetTest.php
diff --git a/tests/ProxyManagerTest/ProxyGenerator/Util/PublicScopeSimulatorTest.php b/tests/ProxyManagerTest/ProxyGenerator/Util/PublicScopeSimulatorTest.php
diff --git a/tests/language-feature-scripts/lazy-loading-value-holder-denies-interface-property-read.phpt b/tests/language-feature-scripts/lazy-loading-value-holder-denies-interface-property-read.phpt
diff --git a/tests/language-feature-scripts/lazy-loading-value-holder-internal-php-classes.phpt b/tests/language-feature-scripts/lazy-loading-value-holder-internal-php-classes.phpt

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,6 @@`
`10`	`10`	`"branch": "master"`
`11`	`11`	`}`
`12`	`12`	`},`
`13`		`- "minMsi": 84,`
	`13`	`+ "minMsi": 84.5,`
`14`	`14`	`"minCoveredMsi": 86`
`15`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,8 @@ public function __construct(`
`38`	`38`	`'name',`
`39`	`39`	`null,`
`40`	`40`	`null,`
`41`		`- 'returnValue'`
	`41`	`+ 'returnValue',`
	`42`	`+ $originalClass`
`42`	`43`	`);`
`43`	`44`	`}`
`44`	`45`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,8 @@ public function __construct(`
`42`	`42`	`'name',`
`43`	`43`	`'value',`
`44`	`44`	`null,`
`45`		`- 'returnValue'`
	`45`	`+ 'returnValue',`
	`46`	`+ $originalClass`
`46`	`47`	`);`
`47`	`48`	`}`
`48`	`49`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,9 @@ public function __construct(`
`55`	`55`	`PublicScopeSimulator::OPERATION_GET,`
`56`	`56`	`'name',`
`57`	`57`	`null,`
`58`		`- $valueHolderProperty`
	`58`	`+ $valueHolderProperty,`
	`59`	`+ null,`
	`60`	`+ $originalClass`
`59`	`61`	`)`
`60`	`62`	`);`
`61`	`63`	`}`