Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Php80] Handle AnnotationToAttributeRector + ClassPropertyAssignToConstructorPromotionRector #1712

Merged
merged 6 commits into from
Jan 21, 2022

Conversation

samsonasik
Copy link
Member

Given the following code:

use OpenApi\Annotations as OA;

class TestResponse
{
    /**
     * @OA\Property(
     *     type="array",
     *     @OA\Items(ref=@Model(type=TestItem::class))
     * )
     */
    public array $items;

    public function __construct(array $items = [])
    {
        $this->items = $items;
    }
}

It currently produce:

-    /**
-     * @OA\Property(
-     *     type="array",
-     *     @OA\Items(ref=@Model(type=TestItem::class))
-     * )
-     */
-    public array $items;
-
-    public function __construct(array $items = [])
+    public function __construct(public array $items = [])
     {
-        $this->items = $items;
     }

which the annotation should not removed. Applied rules:

Rector\Php80\Rector\Class_\AnnotationToAttributeRector
Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector

This PR try to fixes it.

Closes #1702
Fixes rectorphp/rector#6949

@samsonasik samsonasik force-pushed the handle-constructor-promo-annotation-to-attribute branch 2 times, most recently from 9766da2 to cd915a2 Compare January 21, 2022 17:07
@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

It seems cause error in test for @var:

 1) Rector\Tests\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\ClassPropertyAssignToConstructorPromotionRectorTest::test with data set #6 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/typed_with_var_type.php.inc
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 class TypeWithVarType
 {
-    public function __construct(private SomeTypedPropertyClass $someTypedPropertyClass)
+    public function __construct(
+        /**
+         * @var SomeTypedPropertyClass
+         */
+        private SomeTypedPropertyClass $someTypedPropertyClass
+    )

@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik samsonasik force-pushed the handle-constructor-promo-annotation-to-attribute branch from 57b08ef to 8ca8b98 Compare January 21, 2022 18:11
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba TomasVotruba merged commit c8e5b8a into main Jan 21, 2022
@TomasVotruba TomasVotruba deleted the handle-constructor-promo-annotation-to-attribute branch January 21, 2022 22:59
@TomasVotruba
Copy link
Member

👍

@berezuev
Copy link
Contributor

Thank you, @samsonasik

@bendavies
Copy link
Contributor

this desn't seem to work i'm afraid. i'm using only ClassPropertyAssignToConstructorPromotionRector and annotations are still removed:

<?php

declare(strict_types=1);

use Symfony\Component\Serializer\Annotation\SerializedName;

class Address
{
    /**
     * @var string[]
     * @SerializedName("owsy/derived-facts")
     */
    private array $derivedFacts;

    public function __construct(
        array $derivedFacts = [],
    ) {
        $this->derivedFacts = $derivedFacts;
    }
}

becomes:

<?php

declare(strict_types=1);

use Symfony\Component\Serializer\Annotation\SerializedName;

class Address
{
    public function __construct(
        private array $derivedFacts = []
    )
    {
    }
}

@samsonasik
Copy link
Member Author

@bendavies I can't reproduce, ref https://getrector.org/demo/1ec880a6-0fbe-6808-aca7-534e7c93c383

Please provide failing test case for it, thanks.

@bendavies
Copy link
Contributor

@samsonasik apologies, the issue is when used with AUTO_IMPORT_NAMES:
https://getrector.org/demo/1ec880cf-40cb-697c-8793-8bad936652d9

@bendavies
Copy link
Contributor

bendavies commented Feb 7, 2022

@samsonasik also, in your example the @var string[] should not be removed, as this is used by static analysis tools, and by symfony serializer if the @var is an array of objects.

@samsonasik
Copy link
Member Author

@bendavies on that case, @var should be changed to @param on moving to parameter doc, I will try look into it.

@bendavies
Copy link
Contributor

thankyou!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants