8000 bug #18210 [PropertyAccess] Throw an UnexpectedTypeException when the… · symfony/symfony@11bb865 · GitHub
[go: up one dir, main page]
Skip to content

Commit 11bb865

Browse files
fabpotfabpot
committed
bug #18210 [PropertyAccess] Throw an UnexpectedTypeException when the type do not match (dunglas, nicolas-grekas)
This PR was merged into the 2.3 branch. Discussion ---------- [PropertyAccess] Throw an UnexpectedTypeException when the type do not match | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17738, #18032 | License | MIT | Doc PR | - Made in coordination with @dunglas, Diff best viewed [ignoring whitspaces](https://github.com/symfony/symfony/pull/18210/files?w=1). Quoting #18032: > it appears that the current implementation is error prone because it throws a `\TypeError` that is an `Error` in PHP 7 but a regular `Exception` in PHP 5 because it uses the Symfony polyfill. Programs wrote in PHP 5 and catching all exceptions will catch this "custom" `\TypeError ` but not those wrote in PHP 7. It can be very hard to debug. > In this alternative implementation: > * catching type mismatch is considered a bug fix and applied to Symfony 2.3 > * for every PHP versions, a domain exception is thrown Commits ------- 5fe2b06 [PropertyAccess] Reduce overhead of UnexpectedTypeException tracking 10c8d5e [PropertyAccess] Throw an UnexpectedTypeException when the type do not match
2 parents a5d9414 + 5fe2b06 commit 11bb865

File tree

4 files changed

+107
-16
lines changed

4 files changed

+107
-16
lines changed

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ class PropertyAccessor implements PropertyAccessorInterface
8989
private $magicCall;
9090
private $readPropertyCache = array();
9191
private $writePropertyCache = array();
92+
private static $previousErrorHandler;
9293

9394
/**
9495
* Should not be used by application code. Use
@@ -131,23 +132,66 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
131132
self::IS_REF => true,
132133
));
133134

134-
for ($i = count($propertyValues) - 1; $i >= 0; --$i) {
135-
$objectOrArray = &$propertyValues[$i][self::VALUE];
135+
try {
136+
if (PHP_VERSION_ID < 70000) {
137+
self::$previousErrorHandler = set_error_handler(array(__CLASS__, 'handleError'));
138+
}
136139

137-
if ($overwrite) {
138-
$property = $propertyPath->getElement($i);
139-
//$singular = $propertyPath->singulars[$i];
140-
$singular = null;
140+
for ($i = count($propertyValues) - 1; $i >= 0; --$i) {
141+
$objectOrArray = &$propertyValues[$i][self::VALUE];
141142

142-
if ($propertyPath->isIndex($i)) {
143-
$this->writeIndex($objectOrArray, $property, $value);
144-
} else {
145-
$this->writeProperty($objectOrArray, $property, $singular, $value);
143+
if ($overwrite) {
144+
$property = $propertyPath->getElement($i);
145+
//$singular = $propertyPath->singulars[$i];
146+
$singular = null;
147+
148+
if ($propertyPath->isIndex($i)) {
149+
$this->writeIndex($objectOrArray, $property, $value);
150+
} else {
151+
$this->writeProperty($objectOrArray, $property, $singular, $value);
152+
}
146153
}
154+
155+
$value = &$objectOrArray;
156+
$overwrite = !$propertyValues[$i][self::IS_REF];
157+
}
158+
} catch (\TypeError $e) {
159+
try {
160+
self::throwUnexpectedTypeException($e->getMessage(), $e->getTrace(), 0);
161+
} catch (UnexpectedTypeException $e) {
147162
}
163+
} catch (\Exception $e) {
164+
} catch (\Throwable $e) {
165+
}
148166

149-
$value = &$objectOrArray;
150-
$overwrite = !$propertyValues[$i][self::IS_REF];
167+
if (PHP_VERSION_ID < 70000) {
168+
restore_error_handler();
169+
self::$previousErrorHandler = null;
170+
}
171+
if (isset($e)) {
172+
throw $e;
173+
}
174+
}
175+
176+
/**
177+
* @internal
178+
*/
179+
public static function handleError($type, $message, $file, $line, $context)
180+
{
181+
if (E_RECOVERABLE_ERROR === $type) {
182+
self::throwUnexpectedTypeException($message, debug_backtrace(false), 1);
183+
}
184+
185+
return null !== self::$previousErrorHandler && false !== call_user_func(self::$previousErrorHandler, $type, $message, $file, $line, $context);
186+
}
187+
188+
private static function throwUnexpectedTypeException($message, $trace, $i)
189+
{
190+
if (isset($trace[$i]['file']) && __FILE__ === $trace[$i]['file']) {
191+
$pos = strpos($message, $delim = 'must be of the type ') ?: strpos($message, $delim = 'must be an instance of ');
192+
$pos += strlen($delim);
193+
194+
throw new UnexpectedTypeException($trace[$i]['args'][0], substr($message, $pos, strpos($message, ',', $pos) - $pos));
151195
}
152196
}
153197

@@ -398,8 +442,7 @@ private function writeIndex(&$array, $index, $value)
398442
* @param string|null $singular The singular form of the property name or null
399443
* @param mixed $value The value to write
400444
*
401-
* @throws NoSuchPropertyException If the property does not exist or is not
402-
* public.
445+
* @throws NoSuchPropertyException If the property does not exist or is not public.
403446
*/
404447
private function writeProperty(&$object, $property, $singular, $value)
405448
{

‎src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ interface PropertyAccessorInterface
4444
* @param mixed $value The value to set at the end of the property path
4545
*
4646
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
47-
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
48-
* nor array
47+
* @throws Exception\UnexpectedTypeException If a value within the path is neither object nor array.
4948
*/
5049
public function setValue(&$objectOrArray, $propertyPath, $value);
5150

‎src/Symfony/Component/PropertyAccess/Tests/Fixtures/TypeHinted.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Tests\Fixtures;
13+
14+
/**
15+
* @author Kévin Dunglas <dunglas@gmail.com>
16+
*/
17+
class TypeHinted
18+
{
19+
private $date;
20+
21+
public function setDate(\DateTime $date)
22+
{
23+
$this->date = $date;
24+
}
25+
26+
public function getDate()
27+
{
28+
return $this->date;
29+
}
30+
}

‎src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\PropertyAccess\Tests\Fixtures\Magician;
1717
use Symfony\Component\PropertyAccess\Tests\Fixtures\MagicianCall;
1818
use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object;
19+
use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted;
1920

2021
class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
2122
{
@@ -403,4 +404,22 @@ public function getValidPropertyPaths()
403404
array(array('root' => array('index' => array())), '[root][index][firstName]', null),
404405
);
405406
}
407+
408+
/**
409+
* @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException
410+
* @expectedExceptionMessage Expected argument of type "DateTime", "string" given
411+
*/
412+
public function testThrowTypeError()
413+
{
414+
$this->propertyAccessor->setValue(new TypeHinted(), 'date', 'This is a string, \DateTime excepted.');
415+
}
416+
417+
public function testSetTypeHint()
418+
{
419+
$date = new \DateTime();
420+
$object = new TypeHinted();
421+
422+
$this->propertyAccessor->setValue($object, 'date', $date);
423+
$this->assertSame($date, $object->getDate());
424+
}
406425
}

0 commit comments

Comments
 (0)
0