Merge pull request #126 from Ocramius/hotfix/non-existing-properties-…

…read-write

Non existing properties read write

src/ProxyManager/Generator/MagicMethodGenerator.php

@@ -38,6 +38,8 @@ public function __construct(ReflectionClass $originalClass, $name, array $parame
             $originalClass->hasMethod($name) ? '{@inheritDoc}' : null
         );
 
+        $this->setReturnsReference(strtolower($name) === '__get');
+
         if ($originalClass->hasMethod($name)) {
             $this->setReturnsReference($originalClass->getMethod($name)->returnsReference());
         }

src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGet.php

@@ -48,7 +48,6 @@ public function __construct(
         $override = $originalClass->hasMethod('__get');
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         if ($override) {
             $callParent = '$returnValue = & parent::__get($name);';

src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicIsset.php

@@ -48,7 +48,6 @@ public function __construct(
         $override = $originalClass->hasMethod('__isset');
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         if ($override) {
             $callParent = '$returnValue = & parent::__isset($name);';

src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicSet.php

@@ -52,7 +52,6 @@ public function __construct(
         $override = $originalClass->hasMethod('__set');
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         if ($override) {
             $callParent = '$returnValue = & parent::__set($name, $value);';

src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicUnset.php

@@ -48,7 +48,6 @@ public function __construct(
         $override = $originalClass->hasMethod('__unset');
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         if ($override) {
             $callParent = '$returnValue = & parent::__unset($name);';

src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicGet.php

@@ -50,7 +50,6 @@ public function __construct(
         $valueHolderName = $valueHolder->getName();
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_GET,

src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicIsset.php

@@ -50,7 +50,6 @@ public function __construct(
         $valueHolderName = $valueHolder->getName();
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_ISSET,

src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicSet.php

@@ -54,7 +54,6 @@ public function __construct(
         $valueHolderName = $valueHolder->getName();
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_SET,

src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/MagicUnset.php

@@ -50,7 +50,6 @@ public function __construct(
         $valueHolderName = $valueHolder->getName();
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         $callParent = PublicScopeSimulator::getPublicAccessSimulationCode(
             PublicScopeSimulator::OPERATION_UNSET,

src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/MagicGet.php

@@ -52,7 +52,6 @@ public function __construct(
         $callParent = '';
 
         $this->setDocblock(($override ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         if (! $publicProperties->isEmpty()) {
             $callParent = 'if (isset(self::$' . $publicProperties->getName() . "[\$name])) {\n"

src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicGet.php

@@ -45,7 +45,6 @@ public function __construct(
         parent::__construct($originalClass, '__get', array(new ParameterGenerator('name')));
 
         $this->setDocblock(($originalClass->hasMethod('__get') ? "{@inheritDoc}\n" : '') . '@param string $name');
-        $this->setReturnsReference(true);
 
         $initializer = $initializerProperty->getName();
         $valueHolder = $valueHolderProperty->getName();

src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/MagicGet.php

@@ -42,8 +42,8 @@ public function __construct(ReflectionClass $originalClass, PropertyGenerator $a
 
         $this->setDocblock('@param string $name');
         $this->setBody(
-            'return $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
-            . ', \'__get\', array($name));'
+            '$return = $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
+            . ', \'__get\', array($name));' . "\n\n" . 'return $return;'
         );
     }
 }

src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/MagicIsset.php

@@ -42,8 +42,9 @@ public function __construct(ReflectionClass $originalClass, PropertyGenerator $a
 
         $this->setDocblock('@param string $name');
         $this->setBody(
-            'return $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
-            . ', \'__isset\', array($name));'
+            '$return = $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
+            . ', \'__isset\', array($name));' . "\n\n"
+            . 'return $return;'
         );
     }
 }

src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/MagicSet.php

@@ -46,8 +46,9 @@ public function __construct(ReflectionClass $originalClass, PropertyGenerator $a
 
         $this->setDocblock('@param string \$name\n@param mixed \$value');
         $this->setBody(
-            'return $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
-            .', \'__set\', array($name, $value));'
+            '$return = $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
+            . ', \'__set\', array($name, $value));' . "\n\n"
+            . 'return $return;'
         );
     }
 }

src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/MagicUnset.php

@@ -43,8 +43,9 @@ public function __construct(ReflectionClass $originalClass, PropertyGenerator $a
 
         $this->setDocblock('@param string $name');
         $this->setBody(
-            'return $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
-            . ', \'__unset\', array($name));'
+            '$return = $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
+            . ', \'__unset\', array($name));' . "\n\n"
+            . 'return $return;'
         );
     }
 }

src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethod.php

@@ -53,8 +53,9 @@ public static function generateMethod(
         }
 
         $method->setBody(
-            'return $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
-            . ', ' . var_export($originalMethod->getName(), true) . ', array('. implode(', ', $list) .'));'
+            '$return = $this->' . $adapterProperty->getName() . '->call(' . var_export($originalClass->getName(), true)
+            . ', ' . var_export($originalMethod->getName(), true) . ', array('. implode(', ', $list) .'));' . "\n\n"
+            . 'return $return;'
         );
 
         return $method;

src/ProxyManager/ProxyGenerator/Util/PublicScopeSimulator.php

@@ -36,6 +36,10 @@ class PublicScopeSimulator
     const OPERATION_UNSET = 'unset';
 
     /**
+     * Generates code for simulating access to a property from the scope that is accessing a proxy.
+     * This is done by introspecting `debug_backtrace()` and then binding a closure to the scope
+     * of the parent caller.
+     *
      * @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
@@ -55,15 +59,53 @@ public static function getPublicAccessSimulationCode(
         PropertyGenerator $valueHolder = null,
         $returnPropertyName = null
     ) {
-        $byRef = self::getByRefReturnValue($operationType);
-        $value = static::OPERATION_SET === $operationType ? ', $value' : '';
+        $byRef  = self::getByRefReturnValue($operationType);
+        $value  = static::OPERATION_SET === $operationType ? ', $value' : '';
+        $target = '$this';
+
+        if ($valueHolder) {
+            $target = '$this->' . $valueHolder->getName();
+        }
 
-        return '$targetObject = ' . self::getTargetObject($valueHolder) . ";\n"
-            . '    $accessor = function ' . $byRef . '() use ($targetObject, $name' . $value . ') {' . "\n"
-            . '        ' . self::getOperation($operationType, $nameParameter, $valueParameter) . ';' . "\n"
-            . "    };\n"
+        return '$realInstanceReflection = new \\ReflectionClass(get_parent_class($this));' . "\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"
+            . '    ' . self::getOperation($operationType, $nameParameter, $valueParameter) . "\n"
+            . "};\n"
             . self::getScopeReBind()
-            . '    ' . ($returnPropertyName ? '$' . $returnPropertyName . ' =' : 'return') . ' $accessor();';
+            . (
+                $returnPropertyName
+                    ? '$' . $returnPropertyName . ' = ' . $byRef . '$accessor();'
+                    : '$returnValue = ' . $byRef . '$accessor();' . "\n\n" . 'return $returnValue;'
+            );
+    }
+
+    /**
+     * This will generate code that triggers a notice if access is attempted on a non-existing property
+     *
+     * @param string $operationType
+     * @param string $nameParameter
+     *
+     * @return string
+     */
+    private static function getUndefinedPropertyNotice($operationType, $nameParameter)
+    {
+        if (static::OPERATION_GET !== $operationType) {
+            return '';
+        }
+
+        //
+        return '    $backtrace = debug_backtrace(false);' . "\n"
+            . '    trigger_error(\'Undefined property: \' . get_parent_class($this) . \'::$\' . $'
+            . $nameParameter
+            . ' . \' in \' . $backtrace[0][\'file\'] . \' on line \' . $backtrace[0][\'line\'], \E_USER_NOTICE);'
+            . "\n";
     }
 
     /**
@@ -95,7 +137,7 @@ private static function getTargetObject(PropertyGenerator $valueHolder = null)
             return '$this->' . $valueHolder->getName();
         }
 
-        return 'unserialize(sprintf(\'O:%d:"%s":0:{}\', strlen(get_parent_class($this)), get_parent_class($this)));';
+        return 'unserialize(sprintf(\'O:%d:"%s":0:{}\', strlen(get_parent_class($this)), get_parent_class($this)))';
     }
 
     /**
@@ -111,17 +153,17 @@ private static function getOperation($operationType, $nameParameter, $valueParam
     {
         switch ($operationType) {
             case static::OPERATION_GET:
-                return 'return $targetObject->$' . $nameParameter;
+                return 'return $targetObject->$' . $nameParameter . ";";
             case static::OPERATION_SET:
                 if (! $valueParameter) {
                     throw new \InvalidArgumentException('Parameter $valueParameter not provided');
                 }
 
-                return 'return $targetObject->$' . $nameParameter . ' = $' . $valueParameter;
+                return 'return $targetObject->$' . $nameParameter . ' = $' . $valueParameter . ';';
             case static::OPERATION_ISSET:
-                return 'return isset($targetObject->$' . $nameParameter . ')';
+                return 'return isset($targetObject->$' . $nameParameter . ');';
             case static::OPERATION_UNSET:
-                return 'unset($targetObject->$' . $nameParameter . ')';
+                return 'unset($targetObject->$' . $nameParameter . ');';
         }
 
         throw new \InvalidArgumentException(sprintf('Invalid operation "%s" provided', $operationType));
@@ -140,7 +182,7 @@ private static function getScopeReBind()
             // @codeCoverageIgnoreEnd
         }
 
-        return '    $backtrace = debug_backtrace(\DEBUG_BACKTRACE_PROVIDE_OBJECT);' . "\n"
+        return '    $backtrace = debug_backtrace(true);' . "\n"
             . '    $scopeObject = isset($backtrace[1][\'object\'])'
             . ' ? $backtrace[1][\'object\'] : new \stdClass();' . "\n"
             . '    $accessor = $accessor->bindTo($scopeObject, get_class($scopeObject));' . "\n";

tests/Bootstrap.php

@@ -16,7 +16,7 @@
  * and is licensed under the MIT license.
  */
 
-ini_set('error_reporting', E_ALL);
+ini_set('error_reporting', E_ALL | E_STRICT);
 
 $files = array(__DIR__ . '/../vendor/autoload.php', __DIR__ . '/../../../autoload.php');
 

tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicGetTest.php

@@ -50,7 +50,7 @@ public function testBodyStructure()
 
         $this->assertSame('__get', $magicGet->getName());
         $this->assertCount(1, $magicGet->getParameters());
-        $this->assertStringMatchesFormat('%a$returnValue = $accessor();%a', $magicGet->getBody());
+        $this->assertStringMatchesFormat('%a$returnValue = & $accessor();%a', $magicGet->getBody());
     }
 
     /**

tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/MagicSetTest.php

@@ -50,7 +50,7 @@ public function testBodyStructure()
 
         $this->assertSame('__set', $magicGet->getName());
         $this->assertCount(2, $magicGet->getParameters());
-        $this->assertStringMatchesFormat('%a$returnValue = $accessor();%a', $magicGet->getBody());
+        $this->assertStringMatchesFormat('%a$returnValue = & $accessor();%a', $magicGet->getBody());
     }
 
     /**

tests/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/MagicGetTest.php

@@ -44,7 +44,8 @@ public function testBodyStructure()
         $this->assertSame('__get', $magicGet->getName());
         $this->assertCount(1, $magicGet->getParameters());
         $this->assertStringMatchesFormat(
-            'return $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__get\', array($name));',
+            '$return = $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__get\', array($name));'
+            . "\n\nreturn \$return;",
             $magicGet->getBody()
         );
     }

tests/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/MagicIssetTest.php

@@ -44,7 +44,8 @@ public function testBodyStructure()
         $this->assertSame('__isset', $magicGet->getName());
         $this->assertCount(1, $magicGet->getParameters());
         $this->assertStringMatchesFormat(
-            'return $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__isset\', array($name));',
+            '$return = $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__isset\', array($name));'
+            . "\n\nreturn \$return;",
             $magicGet->getBody()
         );
     }

tests/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/MagicSetTest.php

@@ -44,7 +44,8 @@ public function testBodyStructure()
         $this->assertSame('__set', $magicGet->getName());
         $this->assertCount(2, $magicGet->getParameters());
         $this->assertStringMatchesFormat(
-            'return $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__set\', array($name, $value));',
+            '$return = $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__set\', array($name, $value));'
+            . "\n\nreturn \$return;",
             $magicGet->getBody()
         );
     }

tests/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/MagicUnsetTest.php

@@ -44,7 +44,8 @@ public function testBodyStructure()
         $this->assertSame('__unset', $magicGet->getName());
         $this->assertCount(1, $magicGet->getParameters());
         $this->assertStringMatchesFormat(
-            'return $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__unset\', array($name));',
+            '$return = $this->foo->call(\'ProxyManagerTestAsset\\\EmptyClass\', \'__unset\', array($name));'
+            . "\n\nreturn \$return;",
             $magicGet->getBody()
         );
     }

tests/ProxyManagerTest/ProxyGenerator/RemoteObject/MethodGenerator/RemoteObjectMethodTest.php

@@ -53,8 +53,9 @@ public function testBodyStructureWithParameters()
         $this->assertSame('publicByReferenceParameterMethod', $method->getName());
         $this->assertCount(2, $method->getParameters());
         $this->assertSame(
-            'return $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'publicByReferenceParameterMethod\', array($param, $byRefParam));',
+            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
+            . '\'publicByReferenceParameterMethod\', array($param, $byRefParam));'
+            . "\n\nreturn \$return;",
             $method->getBody()
         );
     }
@@ -78,8 +79,9 @@ public function testBodyStructureWithArrayParameter()
         $this->assertSame('publicArrayHintedMethod', $method->getName());
         $this->assertCount(1, $method->getParameters());
         $this->assertSame(
-            'return $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'publicArrayHintedMethod\', array($param));',
+            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
+            . '\'publicArrayHintedMethod\', array($param));'
+            . "\n\nreturn \$return;",
             $method->getBody()
         );
     }
@@ -103,8 +105,9 @@ public function testBodyStructureWithoutParameters()
         $this->assertSame('testBodyStructureWithoutParameters', $method->getName());
         $this->assertCount(0, $method->getParameters());
         $this->assertSame(
-            'return $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
-            . '\'testBodyStructureWithoutParameters\', array());',
+            '$return = $this->adapter->call(\'Zend\\\Code\\\Generator\\\PropertyGenerator\', '
+            . '\'testBodyStructureWithoutParameters\', array());'
+            . "\n\nreturn \$return;",
             $method->getBody()
         );
     }

tests/language-feature-scripts/lazy-loading-ghost-allows-inexisting-magic-property-read.phpt

@@ -0,0 +1,25 @@
+--TEST--
+Verifies that generated lazy loading ghost objects disallow reading non-existing properties via direct read
+--FILE--
+<?php
+
+require_once __DIR__ . '/init.php';
+
+class Kitchen
+{
+    private $sweets;
+
+    public function & __get($name)
+    {
+        return $name;
+    }
+}
+
+$factory = new \ProxyManager\Factory\LazyLoadingGhostFactory($configuration);
+
+$proxy = $factory->createProxy('Kitchen', function () {});
+
+echo $proxy->nonExisting;
+?>
+--EXPECTF--
+nonExisting