Skip to content

Commit 3d4fc68

Browse files
iquitoiquito
committed
Initialize model via reflection instead of new
Mandatory arguments lead to type errors so far because the string filter extension tried to create a new class via new instead of via reflection. Now with reflection we ignore uninitialized properties of the class in our string filter processor to be less error-prone.
1 parent 68e1931 commit 3d4fc68

File tree

9 files changed

+114
-7
lines changed

9 files changed

+114
-7
lines changed

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
"phpunit": "vendor/bin/phpunit --colors=always",
5858
"phpunit_clover": "vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml",
5959
"coverage": "XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html tests/_reports",
60-
"phpcs": "vendor/bin/phpcs --standard=ruleset.xml --extensions=php --cache=.phpcs-cache --ignore=src/Attribute/StringFilter.php --colors src tests",
60+
"phpcs": "vendor/bin/phpcs --standard=ruleset.xml --extensions=php --cache=.phpcs-cache --ignore=src/Attribute/StringFilter.php,tests/TestClasses/ClassWithPublicPropertyPromotion.php --colors src tests",
6161
"phpcsfix": "vendor/bin/phpcbf --standard=ruleset.xml --extensions=php --cache=.phpcs-cache --ignore=src/Attribute/StringFilter.php src tests",
6262
"binupdate": "@composer bin all update --ansi",
6363
"bininstall": "@composer bin all install --ansi"

‎phpstan-baseline.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,8 @@ parameters:
55
count: 1
66
path: src/Attribute/StringFilterProcessor.php
77

8+
-
9+
message: "#^Property Squirrel\\\\Strings\\\\Condense\\\\ConverterUnicode\\:\\:\\$stringToNumber \\(array\\<string, int\\>\\) does not accept array\\<int\\|string, \\(int\\|string\\)\\>\\.$#"
10+
count: 1
11+
path: src/Condense/ConverterUnicode.php
12+

‎psalm-baseline.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<files psalm-version="4.7.3@38c452ae584467e939d55377aaf83b5a26f19dd1"/>
2+
<files psalm-version="4.9.3@4c262932602b9bbab5020863d1eb22d49de0dbf4"/>

‎src/Attribute/StringFilterExtension.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,15 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
3434
if (isset($options['empty_data']) && $options['empty_data'] instanceof $options['data_class']) {
3535
$model = clone $options['empty_data'];
3636
} else {
37-
$model = (new $options['data_class']());
37+
$model = (new \ReflectionClass($options['data_class']))->newInstanceWithoutConstructor();
3838
}
3939

4040
// Assign values to the model only for direct properties
4141
foreach ($data as $key => $value) {
4242
if (\property_exists($model, $key)) {
4343
$reflectionProperty = new \ReflectionProperty($model, $key);
44+
$reflectionProperty->setAccessible(true);
45+
4446
$reflectionPropertyType = $reflectionProperty->getType();
4547

4648
// @codeCoverageIgnoreStart
@@ -74,7 +76,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
7476
}
7577

7678
if ($hasSupportedType === true) {
77-
$reflectionProperty->setAccessible(true);
7879
$reflectionProperty->setValue($model, $value);
7980
}
8081
}
@@ -87,6 +88,14 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
8788
foreach ($data as $key => $value) {
8889
if (\property_exists($model, $key)) {
8990
$reflectionProperty = new \ReflectionProperty($model, $key);
91+
$reflectionProperty->setAccessible(true);
92+
93+
// @codeCoverageIgnoreStart
94+
if (!$reflectionProperty->isInitialized($model)) {
95+
continue;
96+
}
97+
// @codeCoverageIgnoreEnd
98+
9099
$reflectionPropertyType = $reflectionProperty->getType();
91100

92101
// @codeCoverageIgnoreStart
@@ -120,7 +129,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
120129
}
121130

122131
if ($hasSupportedType === true) {
123-
$reflectionProperty->setAccessible(true);
124132
$data[$key] = $reflectionProperty->getValue($model);
125133
}
126134
}

‎src/Attribute/StringFilterProcessor.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,14 @@ public function process(object $class): void
3232
continue;
3333
}
3434

35-
// Make it possible to change private properties
35+
// Make it possible to access and change private properties
3636
$property->setAccessible(true);
3737

38+
// If the property is not initialized, we skip it
39+
if (!$property->isInitialized($class)) {
40+
continue;
41+
}
42+
3843
// Get the property value via reflection
3944
$propertyValue = $property->getValue($class);
4045

‎tests/FormExtensionTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
use Squirrel\Strings\Tests\TestClasses\ClassWithPrivateProperties;
1111
use Squirrel\Strings\Tests\TestClasses\ClassWithPrivateTypedProperties;
1212
use Squirrel\Strings\Tests\TestClasses\ClassWithPublicProperties;
13+
use Squirrel\Strings\Tests\TestClasses\ClassWithPublicPropertyPromotion;
1314
use Squirrel\Strings\Tests\TestClasses\ClassWithPublicTypedProperties;
1415
use Squirrel\Strings\Tests\TestForms\PrivatePropertiesForm;
1516
use Squirrel\Strings\Tests\TestForms\PrivateTypedPropertiesForm;
1617
use Squirrel\Strings\Tests\TestForms\PublicPropertiesEmptyDataForm;
1718
use Squirrel\Strings\Tests\TestForms\PublicPropertiesForm;
19+
use Squirrel\Strings\Tests\TestForms\PublicPropertyPromotionForm;
1820
use Squirrel\Strings\Tests\TestForms\PublicTypedPropertiesEmptyDataForm;
1921
use Squirrel\Strings\Tests\TestForms\PublicTypedPropertiesForm;
2022
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
@@ -241,4 +243,37 @@ public function testPrivateTypedPropertiesSubmit(): void
241243
$this->assertEquals('alsdjl dasdad', $data->getTitle());
242244
$this->assertEquals('a II I ADHSAHZUsd', $data->getText());
243245
}
246+
247+
public function testPublicTypedPropertyPromotionSubmit(): void
248+
{
249+
$data = new ClassWithPublicPropertyPromotion(
250+
title: ' JOOOOPPPPPP ',
251+
text: ' cOrEcT ',
252+
other: '',
253+
);
254+
255+
$request = Request::create(
256+
'https://127.0.0.1/', // URI
257+
'POST', // method
258+
[ // post parameters
259+
'public_property_promotion_form' => [
260+
'title' => ' alsdjl DASDAD ',
261+
'text' => ' a II I ADHSAHZUsd ',
262+
],
263+
],
264+
[], // cookies
265+
[], // files
266+
[], // $_SERVER
267+
'', // content
268+
);
269+
270+
$form = $this->formFactory->create(PublicPropertyPromotionForm::class, $data)
271+
->add('save', SubmitType::class, [
272+
'label' => 'Save',
273+
]);
274+
$form->handleRequest($request);
275+
276+
$this->assertEquals('alsdjl dasdad', $data->title);
277+
$this->assertEquals('a II I ADHSAHZUsd', $data->text);
278+
}
244279
}

‎tests/TestClasses/ClassWithPublicPropertyPromotion.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
namespace Squirrel\Strings\Tests\TestClasses;
4+
5+
use Squirrel\Strings\Attribute\StringFilter;
6+
7+
class ClassWithPublicPropertyPromotion
8+
{
9+
public function __construct(
10+
#[StringFilter("Lowercase", "Trim")]
11+
public string $title,
12+
13+
#[StringFilter("Trim")]
14+
public string $text,
15+
16+
#[StringFilter("Lowercase", "Trim")]
17+
public string $other,
18+
) {
19+
}
20+
}

‎tests/TestForms/PublicPropertyPromotionForm.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Squirrel\Strings\Tests\TestForms;
4+
5+
use Squirrel\Strings\Tests\TestClasses\ClassWithPublicPropertyPromotion;
6+
use Symfony\Component\Form\AbstractType;
7+
use Symfony\Component\Form\Extension\Core\Type\TextType;
8+
use Symfony\Component\Form\FormBuilderInterface;
9+
use Symfony\Component\OptionsResolver\OptionsResolver;
10+
11+
class PublicPropertyPromotionForm extends AbstractType
12+
{
13+
public function buildForm(FormBuilderInterface $builder, array $options): void
14+
{
15+
$builder
16+
->add('title', TextType::class, [
17+
'label' => false,
18+
])
19+
->add('text', TextType::class, [
20+
'label' => false,
21+
])
22+
;
23+
}
24+
25+
/**
26+
* Set class where our form data comes from
27+
*/
28+
public function configureOptions(OptionsResolver $resolver): void
29+
{
30+
$resolver->setDefaults(array(
31+
'data_class' => ClassWithPublicPropertyPromotion::class,
32+
));
33+
}
34+
}

‎vendor-bin/psalm/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
"require": {
33
"vimeo/psalm": "^4.0",
44
"psalm/plugin-phpunit": "^0.16",
5-
"psalm/plugin-mockery": "^0.7"
5+
"psalm/plugin-mockery": "^0.9"
66
}
77
}

0 commit comments

Comments
 (0)