Enabling the phpset set "php84" does not enable the ExplicitNullableParamTypeRector

[MatthiasKuehneEllerhold]

Bug Report

Subject	Details
Rector version	1.2.2
PHP runtime	8.3.8

I want to make sure my code base is compatible with php 8.4 - although there is just the first RC for it. So I thought I'll enable the php set "php8.4: True" and all rules will be running turning my code php 8.4 compatible ... well as far as rules are already in rector. I was especially interested in the ExplicitNullableParamTypeRector because I suspected a lot of needed changes...

Minimal PHP Code Causing Issue

Use this rector.php:

<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;

return RectorConfig::configure()
    ->withPaths(
        [
           __DIR__ . '/src',
        ]
    )
    ->withPhpSets(
        php84: True
    );

With this class:

<?php

declare(strict_types=1);

class MyTest
{
    protected static string $suffix = '';

    public static function setSuffix(string $suffix = null): void
    {
        self::$suffix = $suffix ?? '';
    }
}

Expected Behaviour

I'd expect rector to refactor the function to something this:

    public static function setSuffix(?string $suffix = null): void

Alas ... it didnt. It just let it through and said everything is OK.

Debugging the code I found that provideMinPhpVersion() of this rector rule returns 80400 and since my php runtime version is 8.3.8 it didnt enable this rule.

I suspect provideMinPhpVersion() is meant to provide the minimum php version the refactored code of this rule will run. This should not be 8.4, but 8.0 I think?

[samsonasik]

You can enforce php version feature via config:

->withPhpVersion(\Rector\ValueObject\PhpVersion::PHP_84);

[MatthiasKuehneEllerhold]

Do I still need the ->withPhpSets(php84: True); ?

[samsonasik]

Yes, that to load the rules from php 8.4 set, I suggest you should not apply rule if it still not match with your php runtime to ensure no bc break :),

[MatthiasKuehneEllerhold]

Thank your for your continued help, but somethings nagging me. Please correct me if I understood it wrong:

• withPhpSets(php84: True) should be used to run rules to migrate the code base to a newer PHP version
  • this will provide currently valid php code, thats future-proof for the given PHP version
• withPhpVersion(\Rector\ValueObject\PhpVersion::PHP_84) should be used if SOMEHOW the php runtime of rector does not match your desired php version
  • this CAN generate code that wont run on the php version youre currently running rector with

Is that correct?

If that is correct, I think my first approach (setting withPhpSet(php84: True)) would be correct, because it SHOULD run rules for PHP8.4 EXCEPT those that generate unusable code for < php 8.4. Looking at the ExplicitNullableParamTypeRector it does generate PHP 8.0 compatible code and therefor should have been run in my case, correct? Because its not a BC break?

[samsonasik]

You can use php83: true, then cherry-pick php 8.4 rule via withRules() that you want that you ensure it not cause bc break, eg:

<?php

return \Rector\Config\RectorConfig::configure()
    ->withPaths([__DIR__ . '/src'])
    ->withPhpSets(php83: true)
    ->withPhpVersion(\Rector\ValueObject\PhpVersion::PHP_84)
    ->withRules([
         \Rector\Php84\Rector\Param\ExplicitNullableParamTypeRector::class,
    ]);

see this @GromNaN blog post for reference https://dev.to/gromnan/fix-php-84-deprecation-implicitly-marking-parameter-as-nullable-is-deprecated-the-explicit-nullable-type-must-be-used-instead-5gp3

[MatthiasKuehneEllerhold]

Thanks, that seems to be what I want!

[github-actions]

This issue has been automatically locked because it has been closed for 150 days. Please open a new issue if you have a similar problem.