8000 bug #18259 [PropertyAccess] Backport fixes from 2.7 (nicolas-grekas) · symfony/symfony@e065741 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit e065741

Browse files
nicolas-grekasnicolas-grekas
committed
bug #18259 [PropertyAccess] Backport fixes from 2.7 (nicolas-grekas)
This PR was merged into the 2.3 branch. Discussion ---------- [PropertyAccess] Backport fixes from 2.7 | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - While merging 2.3 into 2.7, I noticed that many fixes were merged into 2.7 that should have been merged into 2.3. This PR backports them, with a few other fixes applied when merging 2.3 into 2.7. Commits ------- cb1c87a [PropertyAccess] Backport fixes from 2.7
2 parents d01a106 + cb1c87a commit e065741

File tree

3 files changed

+131
-45
lines changed

3 files changed

+131
-45
lines changed

‎src/Symfony/Component/PropertyAccess/Exception/InvalidArgumentException.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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\Exception;
13+
14+
/**
15+
* Base InvalidArgumentException for the PropertyAccess component.
16+
*
17+
* @author Bernhard Schussek <bschussek@gmail.com>
18+
*/
19+
class InvalidArgumentException extends \InvalidArgumentException implements ExceptionInterface
20+
{
21+
}

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

Lines changed: 109 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\PropertyAccess;
1313

14+
use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException;
1415
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
1516
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
1617

@@ -32,6 +33,11 @@ class PropertyAccessor implements PropertyAccessorInterface
3233
*/
3334
const REF = 1;
3435

36+
/**
37+
* @internal
38+
*/
39+
const IS_REF_CHAINED = 2;
40+
3541
/**
3642
* @internal
3743
*/
@@ -87,16 +93,29 @@ class PropertyAccessor implements PropertyAccessorInterface
8793
*/
8894
const ACCESS_TYPE_NOT_FOUND = 4;
8995

96+
/**
97+
* @var bool
98+
*/
9099
private $magicCall;
100+
101+
/**
102+
* @var array
103+
*/
91104
private $readPropertyCache = array();
105+
106+
/**
107+
* @var array
108+
*/
92109
private $writePropertyCache = array();
93-
private static $previousErrorHandler;
110+
private static $previousErrorHandler = false;
94111
private static $errorHandler = array(__CLASS__, 'handleError');
95112
private static $resultProto = array(self::VALUE => null);
96113

97114
/**
98115
* Should not be used by application code. Use
99116
* {@link PropertyAccess::createPropertyAccessor()} instead.
117+
*
118+
* @param bool $magicCall
100119
*/
101120
public function __construct($magicCall = false)
102121
{
@@ -137,14 +156,25 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
137156
$overwrite = true;
138157

139158
try {
140-
if (PHP_VERSION_ID < 70000) {
159+
if (PHP_VERSION_ID < 70000 && false === self::$previousErrorHandler) {
141160
self::$previousErrorHandler = set_error_handler(self::$errorHandler);
142161
}
143162

144163
for ($i = count($propertyValues) - 1; 0 <= $i; --$i) {
145164
$zval = $propertyValues[$i];
146165
unset($propertyValues[$i]);
147166

167+
// You only need set value for current element if:
168+
// 1. it's the parent of the last index element
169+
// OR
170+
// 2. its child is not passed by reference
171+
//
172+
// This may avoid uncessary value setting process for array elements.
173+
// For example:
174+
// '[a][b][c]' => 'old-value'
175+
// If you want to change its value to 'new-value',
176+
// you only need set value for '[a][b][c]' and it's safe to ignore '[a][b]' and '[a]'
177+
//
148178
if ($overwrite) {
149179
$property = $propertyPath->getElement($i);
150180

@@ -159,22 +189,31 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
159189
} else {
160190
$this->writeProperty($zval, $property, $value);
161191
}
192+
193+
// if current element is an object
194+
// OR
195+
// if current element's reference chain is not broken - current element
196+
// as well as all its ancients in the property path are all passed by reference,
197+
// then there is no need to continue the value setting process
198+
if (is_object($zval[self::VALUE]) || isset($zval[self::IS_REF_CHAINED])) {
199+
return;
200+
}
162201
}
163202

164203
$value = $zval[self::VALUE];
165204
}
166205
} catch (\TypeError $e) {
167206
try {
168-
self::throwUnexpectedTypeException($e->getMessage(), $e->getTrace(), 0);
169-
} catch (UnexpectedTypeException $e) {
207+
self::throwInvalidArgumentException($e->getMessage(), $e->getTrace(), 0);
208+
} catch (InvalidArgumentException $e) {
170209
}
171210
} catch (\Exception $e) {
172211
} catch (\Throwable $e) {
173212
}
174213

175-
if (PHP_VERSION_ID < 70000) {
214+
if (PHP_VERSION_ID < 70000 && false !== self::$previousErrorHandler) {
176215
restore_error_handler();
177-
self::$previousErrorHandler = null;
216+
self::$previousErrorHandler = false;
178217
}
179218
if (isset($e)) {
180219
throw $e;
@@ -187,19 +226,21 @@ public function setValue(&$objectOrArray, $propertyPath, $value)
187226
public static function handleError($type, $message, $file, $line, $context)
188227
{
189228
if (E_RECOVERABLE_ERROR === $type) {
190-
self::throwUnexpectedTypeException($message, debug_backtrace(false), 1);
229+
self::throwInvalidArgumentException($message, debug_backtrace(false), 1);
191230
}
192231

193232
return null !== self::$previousErrorHandler && false !== call_user_func(self::$previousErrorHandler, $type, $message, $file, $line, $context);
194233
}
195234

196-
private static function throwUnexpectedTypeException($message, $trace, $i)
235+
private static function throwInvalidArgumentException($message, $trace, $i)
197236
{
198237
if (isset($trace[$i]['file']) && __FILE__ === $trace[$i]['file']) {
199238
$pos = strpos($message, $delim = 'must be of the type ') ?: strpos($message, $delim = 'must be an instance of ');
200239
$pos += strlen($delim);
240+
$type = $trace[$i]['args'][0];
241+
$type = is_object($type) ? get_class($type) : gettype($type);
201242

202-
throw new UnexpectedTypeException($trace[$i]['args'][0], substr($message, $pos, strpos($message, ',', $pos) - $pos));
243+
throw new InvalidArgumentException(sprintf('Expected argument of type "%s", "%s" given', substr($message, $pos, strpos($message, ',', $pos) - $pos), $type));
203244
}
204245
}
205246

@@ -229,14 +270,15 @@ private function readPropertiesUntil($zval, PropertyPathInterface $propertyPath,
229270

230271
if ($isIndex) {
231272
// Create missing nested arrays on demand
232-
if ($i + 1 < $propertyPath->getLength() && (
233-
($zval[self::VALUE] instanceof \ArrayAccess && !$zval[self::VALUE]->offsetExists($property)) ||
273+
if (($zval[self::VALUE] instanceof \ArrayAccess && !$zval[self::VALUE]->offsetExists($property)) ||
234274
(is_array($zval[self::VALUE]) && !isset($zval[self::VALUE][$property]) && !array_key_exists($property, $zval[self::VALUE]))
235-
)) {
236-
$zval[self::VALUE][$property] = array();
275+
) {
276+
if ($i + 1 < $propertyPath->getLength()) {
277+
$zval[self::VALUE][$property] = array();
237278

238-
if (isset($zval[self::REF])) {
239-
$zval[self::REF] = $zval[self::VALUE];
279+
if (isset($zval[self::REF])) {
280+
$zval[self::REF] = $zval[self::VALUE];
281+
}
240282
}
241283
}
242284

@@ -250,6 +292,15 @@ private function readPropertiesUntil($zval, PropertyPathInterface $propertyPath,
250292
throw new UnexpectedTypeException($zval[self::VALUE], 'object or array');
251293
}
252294

295+
if (isset($zval[self::REF]) &&am F438 p; (0 === $i || isset($propertyValues[$i - 1][self::IS_REF_CHAINED]))) {
296+
// Set the IS_REF_CHAINED flag to true if:
297+
// current property is passed by reference and
298+
// it is the first element in the property path or
299+
// the IS_REF_CHAINED flag of its parent element is true
300+
// Basically, this flag is true only when the reference chain from the top element to current element is not broken
301+
$zval[self::IS_REF_CHAINED] = true;
302+
}
303+
253304
$propertyValues[] = $zval;
254305
}
255306

@@ -412,7 +463,7 @@ private function getReadAccessInfo($class, $property)
412463
}
413464

414465
/**
415-
* Sets the value of the property at the given index in the path.
466+
* Sets the value of an index in a given array-accessible value.
416467
*
417468
* @param array $zval The array containing the array or \ArrayAccess object to write to
418469
* @param string|int $index The index to write at
@@ -430,7 +481,7 @@ private function writeIndex($zval, $index, $value)
430481
}
431482

432483
/**
433-
* Sets the value of the property at the given index in the path.
484+
* Sets the value of a property in the given object.
434485
*
435486
* @param array $zval The array containing the object to write to
436487
* @param string $property The property to write
@@ -452,32 +503,7 @@ private function writeProperty($zval, $property, $value)
452503
} elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
453504
$object->{$access[self::ACCESS_NAME]} = $value;
454505
} elseif (self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE]) {
455-
// At this point the add and remove methods have been found
456-
$previousValue = $this->readProperty($zval, $property);
457-
$previousValue = $previousValue[self::VALUE];
458-
459-
if ($previousValue instanceof \Traversable) {
460-
$previousValue = iterator_to_array($previousValue);
461-
}
462-
if ($previousValue && is_array($previousValue)) {
463-
if (is_object($value)) {
464-
$value = iterator_to_array($value);
465-
}
466-
foreach ($previousValue as $key => $item) {
467-
if (!in_array($item, $value, true)) {
468-
unset($previousValue[$key]);
469-
$object->{$access[self::ACCESS_REMOVER]}($item);
470-
}
471-
}
472-
} else {
473-
$previousValue = false;
474-
}
475-
476-
foreach ($value as $item) {
477-
if (!$previousValue || !in_array($item, $previousValue, true)) {
478-
$object->{$access[self::ACCESS_ADDER]}($item);
479-
}
480-
}
506+
$this->writeCollection($zval, $property, $value, $access[self::ACCESS_ADDER], $access[self::ACCESS_REMOVER]);
481507
} elseif (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) {
482508
// Needed to support \stdClass instances. We need to explicitly
483509
// exclude $classHasProperty, otherwise if in the previous clause
@@ -493,6 +519,45 @@ private function writeProperty($zval, $property, $value)
493519
}
494520
}
495521

522+
/**
523+
* Adjusts a collection-valued property by calling add*() and remove*() methods.
524+
*
525+
* @param array $zval The array containing the object to write to
526+
* @param string $property The property to write
527+
* @param array|\Traversable $collection The collection to write
528+
* @param string $addMethod The add*() method
529+
* @param string $removeMethod The remove*() method
530+
*/
531+
private function writeCollection($zval, $property, $collection, $addMethod, $removeMethod)
532+
{
533+
// At this point the add and remove methods have been found
534+
$previousValue = $this->readProperty($zval, $property);
535+
$previousValue = $previousValue[self::VALUE];
536+
537+
if ($previousValue instanceof \Traversable) {
538+
$previousValue = iterator_to_array($previousValue);
539+
}
540+
if ($previousValue && is_array($previousValue)) {
541+
if (is_object($collection)) {
542+
$collection = iterator_to_array($collection);
543+
}
544+
foreach ($previousValue as $key => $item) {
545+
if (!in_array($item, $collection, true)) {
546+
unset($previousValue[$key]);
547+
$zval[self::VALUE]->{$removeMethod}($item);
548+
}
549+
}
550+
} else {
551+
$previousValue = false;
552+
}
553+
554+
foreach ($collection as $item) {
555+
if (!$previousValue || !in_array($item, $previousValue, true)) {
556+
$zval[self::VALUE]->{$addMethod}($item);
557+
}
558+
}
559+
}
560+
496561
/**
497562
* Guesses how to write the D6C9 property value.
498563
*
@@ -618,7 +683,7 @@ private function findAdderAndRemover(\ReflectionClass $reflClass, array $singula
618683
}
619684

620685
/**
621-
* Returns whether a method is public and has a specific number of required parameters.
686+
* Returns whether a method is public and has the number of required parameters.
622687
*
623688
* @param \ReflectionClass $class The class of the method
624689
* @param string $methodName The method name

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ public function getValidPropertyPaths()
406406
}
407407

408408
/**
409-
* @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException
409+
* @expectedException \Symfony\Component\PropertyAccess\Exception\InvalidArgumentException
410410
* @expectedExceptionMessage Expected argument of type "DateTime", "string" given
411411
*/
412412
public function testThrowTypeError()

0 commit comments

Comments
 (0)
0