merged branch bschussek/issue4450 (PR #4806)

Commits
-------

1345360 [Form] Fixed PropertyPath handling of offsetGet() that returns a constant value
6e1462e [Form] Fixed PropertyPath handling of __get() method that returns a constant

Discussion
----------

[Form] Fixed "Indirect modification.." exceptions in PropertyPath

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4450, #4535?, #4612
Todo: -

src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php

@@ -96,6 +96,45 @@ abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase
 {
     abstract protected function getCollection(array $array);
 
+    public function testGetValueReadsArrayAccess()
+    {
+        $object = $this->getCollection(array('firstName' => 'Bernhard'));
+
+        $path = new PropertyPath('[firstName]');
+
+        $this->assertEquals('Bernhard', $path->getValue($object));
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
+     */
+    public function testGetValueThrowsExceptionIfArrayAccessExpected()
+    {
+        $path = new PropertyPath('[firstName]');
+
+        $path->getValue(new \stdClass());
+    }
+
+    public function testSetValueUpdatesArrayAccess()
+    {
+        $object = $this->getCollection(array());
+
+        $path = new PropertyPath('[firstName]');
+        $path->setValue($object, 'Bernhard');
+
+        $this->assertEquals('Bernhard', $object['firstName']);
+    }
+
+    /**
+     * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
+     */
+    public function testSetValueThrowsExceptionIfArrayAccessExpected()
+    {
+        $path = new PropertyPath('[firstName]');
+
+        $path->setValue(new \stdClass(), 'Bernhard');
+    }
+
     public function testSetValueCallsAdderAndRemoverForCollections()
     {
         $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth', 4 => 'fifth'));

src/Symfony/Component/Form/Tests/Util/PropertyPathTest.php

@@ -123,26 +123,6 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('Bernhard', $path->getValue($object));
     }
 
-    public function testGetValueReadsArrayAccess()
-    {
-        $object = new \ArrayObject();
-        $object['firstName'] = 'Bernhard';
-
-        $path = new PropertyPath('[firstName]');
-
-        $this->assertEquals('Bernhard', $path->getValue($object));
-    }
-
-    /**
-     * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
-     */
-    public function testGetValueThrowsExceptionIfArrayAccessExpected()
-    {
-        $path = new PropertyPath('[firstName]');
-
-        $path->getValue(new Author());
-    }
-
     /**
      * @expectedException Symfony\Component\Form\Exception\PropertyAccessDeniedException
      */
@@ -213,6 +193,18 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $this->assertSame('foobar', $path->getValue($object));
     }
 
+    /*
+     * https://github.com/symfony/symfony/pull/4450
+     */
+    public function testGetValueReadsMagicGetThatReturnsConstant()
+    {
+        $path = new PropertyPath('magicProperty');
+
+        $object = new Magician();
+
+        $this->assertNull($path->getValue($object));
+    }
+
     /**
      * @expectedException Symfony\Component\Form\Exception\PropertyAccessDeniedException
      */
@@ -316,16 +308,6 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('Bernhard', $object->child['index']->firstName);
     }
 
-    public function testSetValueUpdatesArrayAccess()
-    {
-        $object = new \ArrayObject();
-
-        $path = new PropertyPath('[firstName]');
-        $path->setValue($object, 'Bernhard');
-
-        $this->assertEquals('Bernhard', $object['firstName']);
-    }
-
     public function testSetValueUpdateMagicSet()
     {
         $object = new Magician();
@@ -336,16 +318,6 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('foobar', $object->__get('magicProperty'));
     }
 
-    /**
-     * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
-     */
-    public function testSetValueThrowsExceptionIfArrayAccessExpected()
-    {
-        $path = new PropertyPath('[firstName]');
-
-        $path->setValue(new Author(), 'Bernhard');
-    }
-
     public function testSetValueUpdatesSetters()
     {
         $object = new Author();

src/Symfony/Component/Form/Util/PropertyPath.php

@@ -365,6 +365,8 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
      */
     protected function &readProperty(&$objectOrArray, $property, $isIndex)
     {
+        // The local variable (instead of immediate returns) is necessary
+        // because we want to return by reference!
         $result = null;
 
         if ($isIndex) {
@@ -373,7 +375,11 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
             }
 
             if (isset($objectOrArray[$property])) {
-                $result =& $objectOrArray[$property];
+                if (is_array($objectOrArray)) {
+                    $result =& $objectOrArray[$property];
+                } else {
+                    $result = $objectOrArray[$property];
+                }
             }
         } elseif (is_object($objectOrArray)) {
             $camelProp = $this->camelize($property);
@@ -402,7 +408,7 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
                 $result = $objectOrArray->$hasser();
             } elseif ($reflClass->hasMethod('__get')) {
                 // needed to support magic method __get
-                $result =& $objectOrArray->$property;
+                $result = $objectOrArray->$property;
             } elseif ($reflClass->hasProperty($property)) {
                 if (!$reflClass->getProperty($property)->isPublic()) {
                     throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "%s()" or "%s()"?', $property, $reflClass->name, $getter, $isser));