Better support typed properties

Also removes compatibility to diverging property paths defined in the form, we only support direct properties which are mapped 1:1 in the form. This seems easiest and anything else would just lead to a headache, although maybe there is a better option sometime in the future or when we go PHP 8 only. The PHP 8 support is preliminary, as it was not tested yet.
squirrelphp · Nov 25, 2020 · 4ff891e · 4ff891e
1 parent 9ecc4cb
commit 4ff891e
Show file tree

Hide file tree

Showing 9 changed files with 375 additions and 18 deletions.
diff --git a/composer.json b/composer.json
@@ -52,7 +52,7 @@
         "phpstan": "vendor/bin/phpstan analyse",
         "phpstan_base": "vendor/bin/phpstan analyse --generate-baseline",
         "phpstan_clear": "vendor/bin/phpstan clear-result-cache",
-        "psalm": "vendor/bin/psalm --show-info=false --diff",
+        "psalm": "vendor/bin/psalm --show-info=false",
         "psalm_full": "vendor/bin/psalm --show-info=false",
         "psalm_base": "vendor/bin/psalm --set-baseline=psalm-baseline.xml",
         "phpunit": "vendor/bin/phpunit --colors=always",

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,8 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="3.11.2@d470903722cfcbc1cd04744c5491d3e6d13ec3d9">
+<files psalm-version="3.18.2@19aa905f7c3c7350569999a93c40ae91ae4e1626">
   <file src="src/Annotation/StringFilterExtension.php">
-    <PossiblyInvalidArgument occurrences="1">
-      <code>$model</code>
-    </PossiblyInvalidArgument>
+    <TypeDoesNotContainType occurrences="2"/>
+    <UndefinedClass occurrences="4">
+      <code>$reflectionPropertyType</code>
+      <code>$reflectionPropertyType</code>
+      <code>\ReflectionUnionType</code>
+      <code>\ReflectionUnionType</code>
+    </UndefinedClass>
   </file>
 </files>
diff --git a/src/Annotation/StringFilterExtension.php b/src/Annotation/StringFilterExtension.php
@@ -7,7 +7,6 @@
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormEvent;
 use Symfony\Component\Form\FormEvents;
-use Symfony\Component\PropertyAccess\PropertyAccess;
 
 /**
  * Apply StringFilter annotations and filter submitted data accordingly
@@ -37,23 +36,53 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
 
             // We only want form elements with a data class and an array of values
             if (\is_array($data) && \strlen($options['data_class']) > 0) {
-                // Property accessor like the one used by the form component
-                $propertyAccessor = PropertyAccess::createPropertyAccessor();
-
                 // Create instance of the form data object, either from empty_data or by instantiating it
                 if (isset($options['empty_data']) && $options['empty_data'] instanceof $options['data_class']) {
                     $model = clone $options['empty_data'];
                 } else {
                     $model = (new $options['data_class']());
                 }
 
-                // Assign values to the model as the form would do it
+                // Assign values to the model only for direct properties
                 foreach ($data as $key => $value) {
-                    if ($form->has($key)) {
-                        $propertyPath = $form->get($key)->getPropertyPath();
+                    if (\property_exists($model, $key)) {
+                        $reflectionProperty = new \ReflectionProperty($model, $key);
+                        $reflectionPropertyType = $reflectionProperty->getType();
+
+                        // @codeCoverageIgnoreStart
+                        if (
+                            PHP_VERSION_ID >= 80000
+                            && $reflectionPropertyType instanceof \ReflectionUnionType
+                        ) {
+                            $reflectionTypes = $reflectionPropertyType->getTypes();
+                        } else {
+                            $reflectionTypes = [$reflectionPropertyType];
+                        }
+                        // @codeCoverageIgnoreEnd
+
+                        $hasSupportedType = false;
+
+                        if (!$reflectionProperty->hasType()) {
+                            $hasSupportedType = true;
+                        }
+
+                        foreach ($reflectionTypes as $reflectionType) {
+                            if (!($reflectionType instanceof \ReflectionNamedType)) {
+                                continue;
+                            }
+
+                            if (
+                                $reflectionType->getName() === 'string'
+                                || $reflectionType->getName() === 'array'
+                            ) {
+                                $hasSupportedType = true;
+                                break;
+                            }
+                        }
 
-                        if (isset($propertyPath) && $propertyAccessor->isWritable($model, $propertyPath)) {
-                            $propertyAccessor->setValue($model, $propertyPath, $value);
+                        if ($hasSupportedType === true) {
+                            $reflectionProperty->setAccessible(true);
+                            $reflectionProperty->setValue($model, $value);
                         }
                     }
                 }
@@ -63,11 +92,44 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
 
                 // Copy back the processed data to the array
                 foreach ($data as $key => $value) {
-                    if ($form->has($key)) {
-                        $propertyPath = $form->get($key)->getPropertyPath();
+                    if (\property_exists($model, $key)) {
+                        $reflectionProperty = new \ReflectionProperty($model, $key);
+                        $reflectionPropertyType = $reflectionProperty->getType();
+
+                        // @codeCoverageIgnoreStart
+                        if (
+                            PHP_VERSION_ID >= 80000
+                            && $reflectionPropertyType instanceof \ReflectionUnionType
+                        ) {
+                            $reflectionTypes = $reflectionPropertyType->getTypes();
+                        } else {
+                            $reflectionTypes = [$reflectionPropertyType];
+                        }
+                        // @codeCoverageIgnoreEnd
+
+                        $hasSupportedType = false;
+
+                        if (!$reflectionProperty->hasType()) {
+                            $hasSupportedType = true;
+                        }
+
+                        foreach ($reflectionTypes as $reflectionType) {
+                            if (!($reflectionType instanceof \ReflectionNamedType)) {
+                                continue;
+                            }
+
+                            if (
+                                $reflectionType->getName() === 'string'
+                                || $reflectionType->getName() === 'array'
+                            ) {
+                                $hasSupportedType = true;
+                                break;
+                            }
+                        }
 
-                        if (isset($propertyPath) && $propertyAccessor->isReadable($model, $propertyPath)) {
-                            $data[$key] = $propertyAccessor->getValue($model, $propertyPath);
+                        if ($hasSupportedType === true) {
+                            $reflectionProperty->setAccessible(true);
+                            $data[$key] = $reflectionProperty->getValue($model);
                         }
                     }
                 }

diff --git a/tests/FormExtensionTest.php b/tests/FormExtensionTest.php
@@ -10,10 +10,15 @@
 use Squirrel\Strings\Filter\TrimFilter;
 use Squirrel\Strings\StringFilterSelector;
 use Squirrel\Strings\Tests\TestClasses\ClassWithPrivateProperties;
+use Squirrel\Strings\Tests\TestClasses\ClassWithPrivateTypedProperties;
 use Squirrel\Strings\Tests\TestClasses\ClassWithPublicProperties;
+use Squirrel\Strings\Tests\TestClasses\ClassWithPublicTypedProperties;
 use Squirrel\Strings\Tests\TestForms\PrivatePropertiesForm;
+use Squirrel\Strings\Tests\TestForms\PrivateTypedPropertiesForm;
 use Squirrel\Strings\Tests\TestForms\PublicPropertiesEmptyDataForm;
 use Squirrel\Strings\Tests\TestForms\PublicPropertiesForm;
+use Squirrel\Strings\Tests\TestForms\PublicTypedPropertiesEmptyDataForm;
+use Squirrel\Strings\Tests\TestForms\PublicTypedPropertiesForm;
 use Symfony\Component\Form\Extension\Core\Type\SubmitType;
 use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationExtension;
 use Symfony\Component\Form\Forms;
@@ -106,6 +111,83 @@ public function testPublicPropertiesEmptyDataSubmit()
         $this->assertEquals('a   II I ADHSAHZUsd', $data->text);
     }
 
+    public function testPublicTypedPropertiesSubmit()
+    {
+        $data = new ClassWithPublicTypedProperties();
+        $data->title = '    JOOOOPPPPPP    ';
+        $data->texts = [
+            '   cOrEcT  ',
+            ' oMundo ',
+        ];
+
+        $request = Request::create(
+            'https://127.0.0.1/', // URI
+            'POST', // method
+            [ // post parameters
+                'public_typed_properties_form' => [
+                    'title' => '  alsdjl DASDAD ',
+                    'texts' => [
+                        '  a   II I ADHSAHZUsd ',
+                        ' oMundo ',
+                    ],
+                ],
+            ],
+            [], // cookies
+            [], // files
+            [], // $_SERVER
+            '' // content
+        );
+
+        $form = $this->formFactory->create(PublicTypedPropertiesForm::class, $data)
+            ->add('save', SubmitType::class, [
+                'label' => 'Save',
+            ]);
+        $form->handleRequest($request);
+
+        $this->assertEquals('alsdjl dasdad', $data->title);
+        $this->assertEquals([
+            'a   II I ADHSAHZUsd',
+            'oMundo',
+        ], $data->texts);
+    }
+
+    public function testPublicTypedPropertiesEmptyDataSubmit()
+    {
+        $request = Request::create(
+            'https://127.0.0.1/', // URI
+            'POST', // method
+            [ // post parameters
+                'public_typed_properties_empty_data_form' => [
+                    'title' => '  alsdjl DASDAD ',
+                    'texts' => [
+                        '  a   II I ADHSAHZUsd ',
+                        ' oMundo ',
+                        '   ladidida ' . "\n",
+                    ],
+                ],
+            ],
+            [], // cookies
+            [], // files
+            [], // $_SERVER
+            '' // content
+        );
+
+        $form = $this->formFactory->create(PublicTypedPropertiesEmptyDataForm::class)
+            ->add('save', SubmitType::class, [
+                'label' => 'Save',
+            ]);
+        $form->handleRequest($request);
+
+        $data = $form->getData();
+
+        $this->assertEquals('alsdjl dasdad', $data->title);
+        $this->assertEquals([
+            'a   II I ADHSAHZUsd',
+            'oMundo',
+            'ladidida',
+        ], $data->texts);
+    }
+
     public function testPrivatePropertiesSubmit()
     {
         $data = new ClassWithPrivateProperties();
@@ -134,4 +216,33 @@ public function testPrivatePropertiesSubmit()
         $this->assertEquals('alsdjl dasdad', $data->getTitle());
         $this->assertEquals('a   II I ADHSAHZUsd', $data->getText());
     }
+
+    public function testPrivateTypedPropertiesSubmit()
+    {
+        $data = new ClassWithPrivateTypedProperties();
+
+        $request = Request::create(
+            'https://127.0.0.1/', // URI
+            'POST', // method
+            [ // post parameters
+                'private_typed_properties_form' => [
+                    'title' => '  alsdjl DASDAD ',
+                    'text' => '  a   II I ADHSAHZUsd ',
+                ],
+            ],
+            [], // cookies
+            [], // files
+            [], // $_SERVER
+            '' // content
+        );
+
+        $form = $this->formFactory->create(PrivateTypedPropertiesForm::class, $data)
+            ->add('save', SubmitType::class, [
+                'label' => 'Save',
+            ]);
+        $form->handleRequest($request);
+
+        $this->assertEquals('alsdjl dasdad', $data->getTitle());
+        $this->assertEquals('a   II I ADHSAHZUsd', $data->getText());
+    }
 }
diff --git a/tests/TestClasses/ClassWithPrivateTypedProperties.php b/tests/TestClasses/ClassWithPrivateTypedProperties.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace Squirrel\Strings\Tests\TestClasses;
+
+use Squirrel\Strings\Annotation\StringFilter;
+
+class ClassWithPrivateTypedProperties
+{
+    /**
+     * @StringFilter({"Lowercase","Trim"})
+     */
+    private string $title = '';
+
+    /**
+     * @StringFilter("Trim")
+     */
+    private string $text = '';
+
+    private $noAnnotation;
+
+    public function getTitle(): string
+    {
+        return $this->title;
+    }
+
+    public function getText(): string
+    {
+        return $this->text;
+    }
+
+    public function setTitle(string $title)
+    {
+        $this->title = $title;
+    }
+
+    public function setText(string $text)
+    {
+        $this->text = $text;
+    }
+}
diff --git a/tests/TestClasses/ClassWithPublicTypedProperties.php b/tests/TestClasses/ClassWithPublicTypedProperties.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace Squirrel\Strings\Tests\TestClasses;
+
+use Squirrel\Strings\Annotation\StringFilter;
+
+class ClassWithPublicTypedProperties
+{
+    /**
+     * @StringFilter({"Lowercase","Trim"})
+     */
+    public string $title = '';
+
+    /**
+     * @StringFilter("Trim")
+     */
+    public array $texts = [
+        '',
+        '',
+    ];
+
+    public $noAnnotation;
+}
diff --git a/tests/TestForms/PrivateTypedPropertiesForm.php b/tests/TestForms/PrivateTypedPropertiesForm.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Squirrel\Strings\Tests\TestForms;
+
+use Squirrel\Strings\Tests\TestClasses\ClassWithPrivateTypedProperties;
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+class PrivateTypedPropertiesForm extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('title', TextType::class, [
+                'label' => false,
+            ])
+            ->add('text', TextType::class, [
+                'label' => false,
+            ])
+        ;
+    }
+
+    /**
+     * Set class where our form data comes from
+     *
+     * @param OptionsResolver $resolver
+     */
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefaults(array(
+            'data_class' => ClassWithPrivateTypedProperties::class,
+        ));
+    }
+}