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

Rule to convert "@param int|null $param" to "function method(?int $param)" #8623

Closed
MagePsycho opened this issue May 3, 2024 · 7 comments
Closed

Comments

@MagePsycho
Copy link

Hello rector users,

I am struggling to find the rectorphp rule that converts the following code:

   /**
    * @param int|null $storeId
    */
    public function getBaseUrl($storeId = null): string
    {
        return $this->storeManager->getStore($storeId)->getBaseUrl();
    }

to

    public function getBaseUrl(?int $storeId = null): string
    {
        return $this->storeManager->getStore($storeId)->getBaseUrl();
    }	

FYI, I am using LevelSetList::UP_TO_PHP_81

@samsonasik
Copy link
Member

If the getStore() method already param type hinted, it will converted with typeDeclarations set, see https://getrector.com/demo/7ae2d2a6-2697-4c30-b04b-54ed12d824c7

@MagePsycho
Copy link
Author

@samsonasik Not sure how to adjust the piece of code:

RectorConfig::configure()
    // A. whole set
    ->withPreparedSets(typeDeclarations: true)
    // B. or few rules
    ->withRules([
        TypedPropertyFromAssignsRector::class
    ]);

in my rector.php file:

<?php

declare(strict_types=1);

use Rector\CodeQuality\Rector\ClassMethod\ReturnTypeFromStrictScalarReturnExprRector;
use Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictNativeCallRector;
use Rector\Config\RectorConfig;
use Rector\Set\ValueObject\LevelSetList;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->paths([
        __DIR__ . '/app/code',
    ]);

    # register rules
    $rectorConfig->rules([
        \Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector::class,
        \Rector\Php74\Rector\Property\TypedPropertyRector::class,
        \Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector::class,
        \Rector\Visibility\Rector\ClassMethod\ExplicitPublicClassMethodRector::class,
        ReturnTypeFromStrictNativeCallRector::class,
        ReturnTypeFromStrictScalarReturnExprRector::class,
        \Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector::class,
    ]);

    $rectorConfig->skip([
        \Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector::class
    ]);

    #define sets of rules
    $rectorConfig->sets([
        LevelSetList::UP_TO_PHP_81
    ]);
};

@samsonasik
Copy link
Member

You can add type declaration with sets() if you use old config, eg:

    $rectorConfig->sets([
        LevelSetList::UP_TO_PHP_81,
+        \Rector\Set\ValueObject\SetList::TYPE_DECLARATION
    ]);

@MagePsycho
Copy link
Author

MagePsycho commented May 3, 2024

Hi @samsonasik, it's not working.
FYI, I am using Rector v0.14.8

Also, this refactoring is not working:

<?php
class Handler
{
    /**
     * @var string
     */
    protected $fileName = '/var/log/filename.log';

    /**
     * @var int
     */
    protected $loggerType = \Monolog\Logger::INFO;
}

to

<?php
class Handler
{
    protected string $fileName = '/var/log/filename.log';
    protected int $loggerType = \Monolog\Logger::INFO;
}

@samsonasik
Copy link
Member

samsonasik commented May 3, 2024

Update protected property may cause BC break on child classes, that's why it disabled by default, you can either make it private first:

https://getrector.com/demo/1a5cc52e-b860-434e-b4d2-805f57df040e

of use TypedPropertyFromAssignsRector configurable inline public:

https://getrector.com/demo/a55f7384-e2b1-412e-9038-650537961e4f
https://getrector.com/demo/22c90383-a98f-459d-bc34-631429aa17c4

with beware: it may cause bc break. You can learn configurable example on the rule itself

, [\Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector::INLINE_PUBLIC => \false])]);

@MagePsycho
Copy link
Author

@samsonasik
Still, it's not working for me.

I have this class file & rector file:

test.php:

<?php

class MyClass
{
    /**
     * @var MyOtherClass
     */
    private $myOtherClass;

    public function __construct(
        MyOtherClass $myOtherClass
    ) {
        $this->myOtherClass = $myOtherClass;
    }

    /**
     * @param int|null $storeId
     */
    public function getBaseUrl($storeId = null): string
    {
        return $this->myOtherClass->getStore($storeId)->getBaseUrl();
    }
}

And rector.php:

<?php

declare(strict_types=1);

use Rector\Set\ValueObject\SetList;
use Rector\Config\RectorConfig;
use Rector\Set\ValueObject\LevelSetList;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->paths([
        __DIR__ . '/app/code',
    ]);

    # Register rules
    $rectorConfig->rules([
        \Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector::class,
        \Rector\CodeQuality\Rector\Class_\CompleteDynamicPropertiesRector::class,
        \Rector\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector::class,
        \Rector\CodeQuality\Rector\ClassMethod\ReturnTypeFromStrictScalarReturnExprRector::class,

        \Rector\DeadCode\Rector\StaticCall\RemoveParentCallWithoutParentRector::class,

        \Rector\Visibility\Rector\ClassMethod\ExplicitPublicClassMethodRector::class,

        \Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector::class,
        \Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictNativeCallRector::class,
        \Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector::class,
        \Rector\TypeDeclaration\Rector\Param\ParamTypeFromStrictTypedPropertyRector::class,
        \Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector::class,
        \Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictTypedPropertyRector::class,
        \Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector::class,

        \Rector\Php54\Rector\Array_\LongArrayToShortArrayRector::class,

        \Rector\Php71\Rector\List_\ListToArrayDestructRector::class,
        \Rector\Php71\Rector\ClassConst\PublicConstantVisibilityRector::class,
        \Rector\Php74\Rector\Property\TypedPropertyRector::class,

        \Rector\Php80\Rector\FunctionLike\UnionTypesRector::class,
        \Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector::class,
        \Rector\Php80\Rector\ClassMethod\FinalPrivateToPrivateVisibilityRector::class,
        \Rector\Php80\Rector\Class_\StringableForToStringRector::class,
        \Rector\Php80\Rector\Catch_\RemoveUnusedVariableInCatchRector::class,
        \Rector\Php80\Rector\FunctionLike\MixedTypeRector::class,

        \Rector\Php81\Rector\FuncCall\Php81ResourceReturnToObjectRector::class,
        \Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector::class,

    ]);

    $rectorConfig->skip([
        \Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector::class,
        \Rector\Php81\Rector\Property\ReadOnlyPropertyRector::class,
        \Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector::class,
        \Rector\Php81\Rector\ClassConst\FinalizePublicClassConstantRector::class,
        \Rector\CodingStyle\Rector\ClassConst\VarConstantCommentRector::class,
        \Rector\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchExprVariableRector::class,
    ]);

    # define rules with custom configuration
    /*$rectorConfig->ruleWithConfiguration(\Rector\Renaming\Rector\FuncCall\RenameFunctionRector::class, [
        'json_encode' => 'jsonEncode',
        'json_decode' => 'jsonDecode',
    ]);*/

    # define sets of rules
    $rectorConfig->sets([
        LevelSetList::UP_TO_PHP_81,
        SetList::PHP_81,
        SetList::DEAD_CODE,
        SetList::NAMING,
        SetList::TYPE_DECLARATION,
        SetList::TYPE_DECLARATION_STRICT,
        SetList::EARLY_RETURN,
        SetList::PRIVATIZATION,
        SetList::CODE_QUALITY,
        SetList::CODING_STYLE,
    ]);
};

@samsonasik
Copy link
Member

You may have the target getStore() method on MyOtherClass not typed hint, eg: by docblock, we mostly don't convert docblock to type hint as by assumption.

When you want to be typed hinted, the MyOtherClass::getStore() need to have param type hint in the first place so the caller must be typed hint as well.

see again my original example https://getrector.com/demo/7ae2d2a6-2697-4c30-b04b-54ed12d824c7

@rectorphp rectorphp locked as resolved and limited conversation to collaborators May 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants