PrivatizeLocalGetterToPropertyRector should not apply to public getters

[devbanana]

Feature Request

Not sure if this is intended behavior or not. But I have a class which implements __toString() but also have a asString() method. I have __toString() calling $this->asString() so as to not duplicate logic. But this rule removes that method call.

IMO it should only apply to non-public methods.

See here: https://getrector.org/demo/1ebfe3d7-28b3-6972-a5c2-d38491e8fc13

[TomasVotruba]

Hi, thanks for suggestion. At first sight this behavior looks valid.

Could you share the reason for a proposed change?

[devbanana]

Hi, IMO it should only apply to private methods. If I use a public getter in another method in the class, it just prevents code duplication. If I want to provide both the magic way of converting to string via __toString(), but also an explicit asString(), now I have two places where I need to update that if the string representation of the object changes.

For example if I have a class like this:

<?php

final class Person implements Stringable
{
    public function __construct(private string $firstName)
    {
    }
    
    public function __toString(): string
    {
        return $this->firstName;
    }

    public function asString(): string
    {
        return $this->firstName;
    }
}

But later I want to make the string representation include a last name as well, now I have to remember to include it in both places.

[TomasVotruba]

I understand your potentical use case. Purpose of this rule is to narrow scope to as small context as possible. Public method has mich bigger context than private property. The goal is basically to keep as minimal visibility to internal code as possible.

This change represents the same idea behing this rule:

class SomeClass
{
    public function __construct(private string $name)
    {
    }
    
    function firstMethod(): string
    {
-        return $this->secondMethod();
+        return $this->name;
    }

    function secondMethod(): string
    {
        return $this->thirdMethod();
    }

    function thirdMethod(): string
    {
       return $this->name;
    }
}

But later I want to make the string representation include a last name as well, now I have to remember to include it in both places.

It sounds like YAGNI case. It's also possible I would like to change such method behavior, but only in one method.

Changing this rule behavior to proposed one would kill the whole idea. If you don't like the proposed changes, I recommend not using it/skipping it.

[devbanana]

Thanks, you make good points. 👍🏻