Skip to content

[Renaming] Add RenameCastRector #7117

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

Merged
merged 1 commit into from
Aug 11, 2025

Conversation

mttsch
Copy link
Contributor

@mttsch mttsch commented Aug 7, 2025

While there is already RealToFloatTypeCastRector to handle the deprecation of the (real) cast, this generalized rector rule will allow to configure the cast renaming and thus also support other types of cast renamings.

@mttsch
Copy link
Contributor Author

mttsch commented Aug 7, 2025

Background: PHP 8.5 will most likely deprecate further non-standard casts, see https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_non-standard_cast_names.

In preparation, I already submitted a patch to nikic/php-parser (nikic/PHP-Parser#1098) to get the kind of cast for bool, string, and int casts so that once that a new version of nikic/php-parser is released with this cast kind information and the cast deprecations in PHP 8.5 are official, this new rector can be used. (The double cast could already now be implemented.)

Additional side information: I have already prepared locally quite a few branches for other changes from this deprecation RFC that are likely to pass. Please inform me whether I could already open PRs for them to signal to other contributors that they were already implemented and you can merge then whenever you want or do you want me to wait until a future point in time to open them?

@calebdw
Copy link
Contributor

calebdw commented Aug 7, 2025

While there is already RealToFloatTypeCastRector to handle the deprecation of the (real) cast,

Why not rename that rule and make it configurable then?

@mttsch
Copy link
Contributor Author

mttsch commented Aug 7, 2025

@calebdw Good question, that is a point I wanted to ask, too, but forgot.

I do not know how rector handles BC with rector rules. Renaming the real cast rector could break existing code. It could be deprecated and the PHP 7.4 set could use this rector instead, I guess.

final readonly class RenameCast
{
public function __construct(
/** @var class-string<Cast> */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering whether this is okay, to directly expose PHP parser class names in value objects. This avoids having to add a mapping layer between the value passed here and the cast class.

While there is already `RealToFloatTypeCastRector` to handle the deprecation of the `(real)` cast, this generalized rector rule will allow to configure the cast renaming and thus also support other types of cast renamings.
@mttsch mttsch force-pushed the feature/rename-cast-rector branch from d0f9def to 16b0c9d Compare August 7, 2025 14:59
@TomasVotruba
Copy link
Member

Great idea, thank you 👍

Could you replace RealToFloatTypeCastRector with this rule in our configs in follow-up PR?

@TomasVotruba TomasVotruba merged commit 7843966 into rectorphp:main Aug 11, 2025
47 checks passed
@mttsch
Copy link
Contributor Author

mttsch commented Aug 11, 2025

@TomasVotruba Yes, see #7136

@mttsch mttsch deleted the feature/rename-cast-rector branch August 11, 2025 10:17
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.

4 participants