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

ReturnUnionTypeRector now skips nullable types, to prepare space for new nullable rule as off PHP 7.1+ #6105

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Jul 2, 2024

@TomasVotruba TomasVotruba marked this pull request as draft July 2, 2024 15:37
@TomasVotruba TomasVotruba changed the title tv nullable returns ReturnUnionTypeRector now return only non-nullable types, to prepare space for new nullable rule as off PHP 7.1+ Jul 2, 2024
@@ -26,7 +26,7 @@ namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnUnionTypeRector\
final class FalseBoolDocblock
{
/**
* @return array|false some description
* @return array|bool some description
Copy link
Member

Choose a reason for hiding this comment

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

new rule to update sub type by docblock seems needed, to ensure not introduce phpstan notice

https://phpstan.org/r/49cbf30f-03e8-4b82-b47e-b649cadc7a34

Copy link
Member Author

@TomasVotruba TomasVotruba Jul 2, 2024

Choose a reason for hiding this comment

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

Docblock changes are out of our scope since Rector 0.15. This will have to be dealt manualy.

Copy link
Member

Choose a reason for hiding this comment

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

That will ok for 1 or 2 files, but for many files, imo, that's worth it, I think coding-style or code-quality set will match that

Copy link
Member Author

Choose a reason for hiding this comment

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

That's beyond our scope. I'd leave that for manual review or custom set 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, this seems rather rare case.

@TomasVotruba TomasVotruba marked this pull request as ready for review July 2, 2024 15:55
@TomasVotruba TomasVotruba merged commit 00a7c2a into main Jul 2, 2024
33 checks passed
@TomasVotruba TomasVotruba deleted the tv-nullable-returns branch July 2, 2024 16:00
@TomasVotruba TomasVotruba changed the title ReturnUnionTypeRector now return only non-nullable types, to prepare space for new nullable rule as off PHP 7.1+ ReturnUnionTypeRector now handles only non-nullable types, to prepare space for new nullable rule as off PHP 7.1+ Jul 3, 2024
@TomasVotruba TomasVotruba changed the title ReturnUnionTypeRector now handles only non-nullable types, to prepare space for new nullable rule as off PHP 7.1+ ReturnUnionTypeRector now skips nullable types, to prepare space for new nullable rule as off PHP 7.1+ Jul 3, 2024
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 this pull request may close these issues.

2 participants