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

Add failing test fixture for AnnotationToAttributeRector #2577

Closed
wants to merge 2 commits into from

Conversation

javaDeveloperKid
Copy link
Contributor

Failing Test for AnnotationToAttributeRector

Based on https://getrector.org/demo/4753f273-2649-4a87-bf0f-5ed01e1246e5

@javaDeveloperKid
Copy link
Contributor Author

javaDeveloperKid commented Jun 26, 2022

BTW. This https://github.com/rectorphp/rector-src/blob/main/rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/Fixture/Doctrine/doctrine_join_table.php.inc test case is wrong in my opinion. First rector does not leave annotation for @JoinTable - it does what link in my description shows, second you cannot use both annotations and attributes at the time with doctrine (ofc one can write custom chain-like driver but this is not a case rector should consider).

private $users;
}

?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how this file differs from https://github.com/rectorphp/rector-src/blob/main/rules-tests/Php80/Rector/Class_/AnnotationToAttributeRector/Fixture/Doctrine/doctrine_join_table.php.inc
They seem have the same input example.

It would be better update that one instead adding a duplicate.

Comment on lines +50 to +53
#[ORM\ManyToMany(targetEntity: 'User')]
#[ORM\JoinTable(name: 'user_group_role')]
#[ORM\JoinColumn(name: 'user_group_id', referencedColumnName: 'id')]
#[ORM\InverseJoinColumn(name: 'user_id', referencedColumnName: 'id')]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought JoinTable supports nested attributes, but it does not:

#[JoinTable(name: "user_groups")]
#[JoinColumn(name: "user_id", referencedColumnName: "id")]
#[InverseJoinColumn(name: "group_id", referencedColumnName: "id")]
private $groups;

Ref: https://www.doctrine-project.org/projects/doctrine-orm/en/2.11/reference/attributes-reference.html#jointable


It's quite confusing that ManyToOne supports nested attributes 🤔 ...ah, they were joined together by commad, it's actually a new attribute. Thanks god for ambiguous syntax :D

#[ManyToOne(targetEntity: Country:class)]
#[JoinColumn(name: "country_code", referencedColumnName: "country_code")]

Ref: https://www.doctrine-project.org/2022/01/11/orm-2.11.html

@javaDeveloperKid
Copy link
Contributor Author

I can't find issue that lead me to creating this PR so can't find if it was resolved. However it still doesn't work in 0.13.10.

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 18, 2022

I think this will be solved now by #2781 🙂

Please rebase on dev-main, so we can see if it works propperly

@TomasVotruba
Copy link
Member

TomasVotruba commented Aug 18, 2022

I see... it needed move to the NestedAnnotationToAttributeRector and test only the nested items.
It moved it for you, to save you the hustle: #2785

@TomasVotruba
Copy link
Member

It passes the CI ✔️ #2785

This fixture is already in there, so I close both PR to avoid duplications.

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