8000 feature #51697 [PropertyInfo] Make isWriteable() more consistent with… · symfony/symfony@83910a9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 83910a9

Browse files
nicolas-grekasnicolas-grekas
committed
feature #51697 [PropertyInfo] Make isWriteable() more consistent with isReadable() when checking snake_case properties (jbtronics)
This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [PropertyInfo] Make isWriteable() more consistent with isReadable() when checking snake_case properties | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Related with issues #29933 and #16889 | License | MIT | Doc PR | Currently PropertyInfo is a bit inconsistent in the behavior of isWriteable() and isReadable() when using it with properties in snake_case format, which are only accessible via a getter and setter. To be readable the getter function has to be in camel_case (e.g. `getSnakeCase()`), while the setter function has to remain in the snake case format (e.g. `setSnake_case()`). In this example class: ```php class Dummy { private string $snake_case; public function getSnakeCase(): string { } public function setSnakeCase(string $snake_case): void { } } ```` the property `snake_case` is considered readable like you would expect, but not writeable, even though the property has a useable setter and the value can be actually be modified fine by something like the PropertyAccess component. To make the property actually considered writeable, the setter would need to be named `setSnake_case`, which is pretty inconsistent with the behavior of isReadable and makes it very hard to use this component on snake_case properties. This inconsistencies are caused by the fact, that `isReadable` in ReflectionExtractor uses the `getReadInfo()` function which does a camelization of the property name internally, while the `isWriteable()` function uses the `getMutatorMethod()` function which just perform a capitalization of the first letter. This PR fixes this inconsistencies between isReadable() and isWriteable() by allowing to use a camelCase style setter to be considered writeable, as this is much more common then to use the mix of snake and camelCase currently required. The getWriteInfo() function is not useable here, because it needs both a add and remove Function on collection setters to give proper infos, while the current `isWriteable()` implementation considers a class with just a add or a remove function as writeable. Therefore the property name just gets camelized before being feed into the `getMutatorMethod()`, which gives the desired result. To maximize backwards compatibility, if no camelCase style setter is found, it is still checked for a snake_case setter, so that classes having these, are still considered writeable after the change. The current behavior is causing some inconsistencies in higher level libraries, which rely on this component. In API Platform for example properties in snake_case are considered read only even though they have a (camel case) setter, because of this. See issue: api-platform/core#5641 and api-platform/core#1554 Commits ------- 7c9e6bc [PropertyInfo] Make isWriteable() more consistent with isReadable() when checking snake_case properties
2 parents 4f6f404 + 7c9e6bc commit 83910a9

File tree

4 files changed

+66
-0
lines changed

4 files changed

+66
-0
lines changed

‎src/Symfony/Component/PropertyInfo/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
6.4
5+
---
6+
7+
* Make properties writable when a setter in camelCase exists, similar to the camelCase getter
8+
49
6.1
510
---
611

‎src/Symfony/Component/PropertyInfo/Extractor/ReflectionExtractor.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,13 @@ public function isWritable(string $class, string $property, array $context = [])
205205
return true;
206206
}
207207

208+
// First test with the camelized property name
209+
[$reflectionMethod] = $this->getMutatorMethod($class, $this->camelize($property));
210+
if (null !== $reflectionMethod) {
211+
return true;
212+
}
213+
214+
// Otherwise check for the old way
208215
[$reflectionMethod] = $this->getMutatorMethod($class, $property);
209216

210217
return null !== $reflectionMethod;

‎src/Symfony/Component/PropertyInfo/Tests/Extractor/ReflectionExtractorTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php7Dummy;
2727
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php7ParentDummy;
2828
use Symfony\Component\PropertyInfo\Tests\Fixtures\Php81Dummy;
29+
use Symfony\Component\PropertyInfo\Tests\Fixtures\SnakeCaseDummy;
2930
use Symfony\Component\PropertyInfo\Type;
3031

3132
/**
@@ -409,6 +410,20 @@ public static function getWritableProperties()
409410
];
410411
}
411412

413+
public function testIsReadableSnakeCase()
414+
{
415+
$this->assertTrue($this->extractor->isReadable(SnakeCaseDummy::class, 'snake_property'));
416+
$this->assertTrue($this->extractor->isReadable(SnakeCaseDummy::class, 'snake_readonly'));
417+
}
418+
419+
public function testIsWriteableSnakeCase()
420+
{
421+
$this->assertTrue($this->extractor->isWritable(SnakeCaseDummy::class, 'snake_property'));
422+
$this->assertFalse($this->extractor->isWritable(SnakeCaseDummy::class, 'snake_readonly'));
423+
// Ensure that it's still possible to write to the property using the (old) snake name
424+
$this->assertTrue($this->extractor->isWritable(SnakeCaseDummy::class, 'snake_method'));
425+
}
426+
412427
public function testSingularize()
413428
{
414429
$this->assertTrue($this->extractor->isWritable(AdderRemoverDummy::class, 'analyses'));

‎src/Symfony/Component/PropertyInfo/Tests/Fixtures/SnakeCaseDummy.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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\PropertyInfo\Tests\Fixtures;
13+
14+
class SnakeCaseDummy
15+
{
16+
private string $snake_property;
17+
private string $snake_readOnly;
18+
private string $snake_method;
19+
20+
public function getSnakeProperty()
21+
{
22+
return $this->snake_property;
23+
}
24+
25+
public function getSnakeReadOnly()
26+
{
27+
return $this->snake_readOnly;
28+
}
29+
30+
public function setSnakeProperty($snake_property)
31+
{
32+
$this->snake_property = $snake_property;
33+
}
34+
35+
public function setSnake_method($snake_method)
36+
{
37+
$this->snake_method = $snake_method;
38+
}
39+
}

0 commit comments

Comments
 (0)
0