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

AnnotationToAttributeRector not applied #5517

Closed
vichanse opened this issue Feb 12, 2021 · 20 comments · Fixed by #5926
Closed

AnnotationToAttributeRector not applied #5517

vichanse opened this issue Feb 12, 2021 · 20 comments · Fixed by #5926

Comments

@vichanse
Copy link

vichanse commented Feb 12, 2021

Question

I can't get the AnnotationToAttributeRector being applied on my code
I reproduced the behaviours here
https://getrector.org/demo/138439e7-e2cf-4fed-94de-4b83bbe51f77

What am i doing wrong?

I'm using rector-prefixed as i'm unable to run rector with php8. I have the Rector SymfonyConsole class not found error

@TomasVotruba
Copy link
Member

Hi,

this one is a bit tricky, because it requires symfony/framework-bundle of version 5.2 to be installed.

The demo only uses code that you paste in it. The @Route class is not be present, so the result is expected.

You have to either complete it, or better provide example on Github repository

@7system7
Copy link

7system7 commented Feb 15, 2021

Hello,

I have the same issue. It is a freshly created symfony 5.2.x project, I have a User entity in src/Entity:

<?php
...

/**
 * Class User
 *
 * @ApiResource
 * @ORM\Entity(repositoryClass=UserRepository::class)
 * @Gedmo\Loggable
 */
class User implements UserInterface, Timestampable

...

I have a rector php file:

<?php

declare(strict_types=1);

use Rector\Core\Configuration\Option;
use Rector\Php80\Rector\Class_\AnnotationToAttributeRector;
use Rector\Set\ValueObject\SetList;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    // get parameters
    $parameters = $containerConfigurator->parameters();

    // paths to refactor; solid alternative to CLI arguments
    $parameters->set(Option::PATHS, [
        __DIR__ . '/src',
    ]);

    // Define what rule sets will be applied
    $parameters
        ->set(Option::SETS, [
            SetList::PHP_80,
            SetList::SYMFONY_52,
            SetList::SYMFONY_AUTOWIRE,
            SetList::SYMFONY_CODE_QUALITY,
        ])
        ->set(Option::AUTOLOAD_PATHS, [
            __DIR__ . '/vendor/autoload.php',
        ]);

    // get services (needed for register a single rule)
    $services = $containerConfigurator->services();

    $services->set(AnnotationToAttributeRector::class);
};

I run rector w/ this $ php8.0 vendor/bin/rector process (I have multiple php versions on the system.)

After that nothing happened w/ the User entity's annotations. What am i doing wrong?

Rector 0.9.28
Config file: rector.php

 20/20 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

                                                                                                                        
 [OK] Rector is done!                                                                                                   
                                                  

@TomasVotruba
Copy link
Member

Hi, whas is the expected behavior?
We'll need a failing repository on Github to verify

@7system7
Copy link

7system7 commented Feb 15, 2021

I think the entity should look like this, after the process:

/**
 * Class User
 */
#[ApiResource]
#[ORM\Entity(repositoryClass: UserRepository::class)]
#[Gedmo\Loggable]
class User implements UserInterface, Timestampable
{
...
}

... if I am not wrong. :) But ok, I will create one a bit later.

@TomasVotruba
Copy link
Member

I see :) Could you provide links to Attribute classes on Github?

AFAIK Doctrine does not provide attributes yet.

If that's wrong, we'll add support for them 👍 But we need exact existing classes with #[Attribute]

@aless673
Copy link

Hi, here the diff produced by Rector with AnnotationToAttributeRector rule :

      * @ORM\Column(name="email", type="string", length=180, unique=true, nullable=true)
      * @Assert\Type("string")
      * @Assert\Length(max="180")
-     * @Assert\Email(groups={"registration"})
      * @Assert\NotNull(groups={"registration"})
      */
+    #[\Symfony\Component\Validator\Constraints\Email(groups: ['registration'])]
     private $email;

This is not working correctly, as you can see only the assert Email annotation is handled and nothing else.

@TomasVotruba
Copy link
Member

I see. We'll need a failing fixture for AnnotationToAttributeRectorTest to be able to fix it.
Here is how you can add it: https://github.com/rectorphp/rector/blob/master/docs/how_to_add_test_for_rector_rule.md

@OskarStark
Copy link
Contributor

I have the same problem with the following entity:

<?php

declare(strict_types=1);

namespace App\Entity;

use App\Domain\Identifier\UserId;
use App\Repository\UserRepository;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @ORM\Entity(repositoryClass=UserRepository::class)
 * @ORM\Table(name="`users`")
 * @UniqueEntity(fields={"email"}, message="Es existiert bereits ein Account mit dieser Email")
 */
class User implements UserInterface
{

So @UniqueEntity should be replaced by an Attribute

I already use Symfony 5.2 which includes:

namespace Symfony\Bridge\Doctrine\Validator\Constraints;

use Symfony\Component\Validator\Constraint;

/**
 * Constraint for the Unique Entity validator.
 *
 * @Annotation
 * @Target({"CLASS", "ANNOTATION"})
 *
 * @author Benjamin Eberlei <kontakt@beberlei.de>
 */
#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
class UniqueEntity extends Constraint
{

@OskarStark
Copy link
Contributor

AFAIK Doctrine does not provide attributes yet.

@TomasVotruba In this case the example should not show ORM as example in:
https://github.com/rectorphp/rector/blob/master/docs/rector_rules_overview.md#annotationtoattributerector

😃

@TomasVotruba
Copy link
Member

@OskarStark I have the same problem with the following entity:

We'll need a test case for that.

@TomasVotruba
Copy link
Member

@TomasVotruba In this case the example should not show ORM as example in:

Good point 🙂 could you sent PR with fix?

@OskarStark
Copy link
Contributor

Sure, shall I update the README file or is it located somewhere else and generated afterwards?

@TomasVotruba
Copy link
Member

In the rule itself, there is a code sample that docs is generated from

@TomasVotruba
Copy link
Member

@OskarStark
Copy link
Contributor

What do you mean? Which class should be skipped? 🤔

@TomasVotruba
Copy link
Member

The linked line:

    /**
     * @var string
     */
-   public const CLASS_NAME = \RectorPrefix20210227\Symfony\Component\Routing\Annotation\Route::class;
+   public const CLASS_NAME = \Symfony\Component\Routing\Annotation\Route::class;

Scoper is now prefixing it, but it should be skipped

@OskarStark
Copy link
Contributor

Ah, yes that makes sense

@TomasVotruba
Copy link
Member

TomasVotruba commented Mar 21, 2021

The thing is, the route class must be present there, so doctrine/annotation parser can load it.

I've updated the demo link and the migration now works as expected:
https://getrector.org/demo/9b0bc64e-38f4-4fa9-8925-09e3fc456738

Also note, the SYMFONY_52 set must be used, as both framework and PHP versoin is relevant. PHP 8 on Symfony 5.1 would not work.



I've also added test workflow to rector-prefixed and it works there too ✔️

image

@OskarStark
Copy link
Contributor

Nice thank you for the clarification 👌🏻

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

Successfully merging a pull request may close this issue.

5 participants