8000 [PropertyAccess] Throw an UnexpectedTypeException when the type do not match by dunglas · Pull Request #18032 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[PropertyAccess] Throw an UnexpectedTypeException when the type do not match #18032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[PropertyAccess] Throw an UnexpectedTypeException when the type do no…
…t match
  • Loading branch information
dunglas committed Mar 6, 2016
commit 21e0e0798913a73804be7d84555e0cf6212f4385
70 changes: 68 additions & 2 deletions src/Symfony/Component/PropertyAccess/PropertyAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ private function writeIndex(&$array, $index, $value)
*
* @throws NoSuchPropertyException If the property does not exist or is not
* public.
* @throws UnexpectedTypeException
*/
private function writeProperty(&$object, $property, $singular, $value)
{
Expand All @@ -410,7 +411,7 @@ private function writeProperty(&$object, $property, $singular, $value)
$access = $this->getWriteAccessInfo($object, $property, $singular, $value);

if (self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]) {
$object->{$access[self::ACCESS_NAME]}($value);
$this->callMethod($object, $access[self::ACCESS_NAME], $value);
} elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
$object->{$access[self::ACCESS_NAME]} = $value;
} elseif (self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE]) {
Expand Down Expand Up @@ -457,12 +458,77 @@ private function writeProperty(&$object, $property, $singular, $value)

$object->$property = $value;
} elseif (self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]) {
$object->{$access[self::ACCESS_NAME]}($value);
$this->callMethod($object, $access[self::ACCESS_NAME], $value);
} else {
throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
}
}

/**
* Throws a {@see UnexpectedTypeException} as in PHP 7 when using PHP 5.
*
* @param object $object
* @param string $method
* @param mixed $value
*
* @throws UnexpectedTypeException
* @throws \Exception
*/
private function callMethod($object, $method, $value) {
if (PHP_MAJOR_VERSION >= 7) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually prefer comparing against PHP_VERSION_ID

try {
$object->{$method}($value);
} catch (\TypeError $e) {
throw $this->createUnexpectedTypeException($object, $method, $value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check the error message as done for the E_RECOVERABLE_ERROR below to be sure that we deal really with an exception thrown from $method rather than from some other deeply nested call?

}

return;
}

set_error_handler(function ($errno, $errstr) use ($object, $method, $value) {
if (E_RECOVERABLE_ERROR === $errno && false !== strpos($errstr, sprintf('passed to %s::%s() must', get_class($object), $method))) {
throw $this->createUnexpectedTypeException($object, $method, $value);
}

return false;
});

try {
$object->{$method}($value);
restore_error_handler();
} catch (\Exception $e) {
// Cannot use finally in 5.5 because of https://bugs.php.net/bug.php?id=67047
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fact, since autoloading would not happen in the finally block, the bug would not apply if I'm not wrong.
But still, finally doesn't exist on PHP 5.3

restore_error_handler();

throw $e;
}
}

/**
* Creates an UnexpectedTypeException.
*
* @param object $object
* @param string $method
* @param mixed $value
*
* @return UnexpectedTypeException
*/
private function createUnexpectedTypeException($object, $method, $value)
{
$reflectionMethod = new \ReflectionMethod($object, $method);
$parameters = $reflectionMethod->getParameters();

$expectedType = 'unknown';
if (isset($parameters[0])) {
$class = $parameters[0]->getClass();
if (null !== $class) {
$expectedType = $class->getName();
}
}

return new UnexpectedTypeException($value, $expectedType);
}

/**
* Guesses how to write the property value.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ interface PropertyAccessorInterface
*
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then should be: "is not an object"?

* nor array
*/
public function setValue(&$objectOrArray, $propertyPath, $value);

Expand Down
30 changes: 30 additions & 0 deletions src/Symfony/Component/PropertyAccess/Tests/Fixtures/TypeHinted.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\PropertyAccess\Tests\Fixtures;

/**
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class TypeHinted
{
private $date;

public function setDate(\DateTime $date)
{
$this->date = $date;
}

public function getDate()
{
return $this->date;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\PropertyAccess\Tests\Fixtures\Magician;
use Symfony\Component\PropertyAccess\Tests\Fixtures\MagicianCall;
use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted;

class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -403,4 +404,22 @@ public function getValidPropertyPaths()
array(array('root' => array('index' => array())), '[root][index][firstName]', null),
);
}

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException
* @expectedExceptionMessage Expected argument of type "DateTime", "string" given
*/
public function testThrowTypeError()
{
$this->propertyAccessor->setValue(new TypeHinted(), 'date', 'This is a string, \DateTime excepted.');
}

public function testSetTypeHint()
{
$date = new \DateTime();
$object = new TypeHinted();

$this->propertyAccessor->setValue($object, 'date', $date);
$this->assertSame($date, $object->getDate());
}
}
0