[PHP 8.4] Add PropertyHookRector (optional)

[TomasVotruba]

Closes rectorphp/rector#9152

final class Product {
-    private string $name;
+    public string $name {
+        get { return $this->name; }
+        set { $this->name = ucfirst($value); }
+    }
-
-    public function getName(): string {
-        return $this->name;
-    }
-
-    public function setName(string $name): void {
-        $this->name = ucfirst($name);
-    }
}

Changing getters/setters to property hooks may have alter behavior, if the class is extended and uses getter, see https://3v4l.org/HIHXs
That's why it runs safely on final classes only.

If you're confident there is close to zero of such cases, you can enable option to run on non-final classes too:

use Rector\Config\RectorConfig;

return RectorConfig::configure()
    ->withTreatClassesAsFinal()

[samsonasik]

This need test for readonly property which should be skipped.

https://3v4l.org/2g4ZM

property hook seems remove readonly flag based on the transformation.

[TomasVotruba]

property hook seems remove readonly flag based on the transformation.

👍

[samsonasik]

If class->isReadonly(), all properties become readonly, so seems should be skipped as well.

[TomasVotruba]

@samsonasik What if there is only getter property hook?

[samsonasik]

@TomasVotruba that will error, see https://3v4l.org/aQhYo#v8.4.12

[TomasVotruba]

@samsonasik I see 👍

[samsonasik]

this should check if there is promoted property that is readonly, eg:

$constructClassMethod = $node->getMethod('__construct');

if ($constructClassMethod instanceof ClassMethod) {
       foreach ($constructClassMethod->params as $param) {
            // readonly param must be promoted property already, and can't have property hook
            if ($param->isReadonly()) {
                  return null;
            }
      }
}

[TomasVotruba]

I'm not sure if promoted properties should be covered, as those are usually set in the constructor.

If we change it to property hook, it will start look rather messy. Property will become non-promoted and constructor full of assings. I want to skip these.

---

Before:

final class SomeClass
{
    public function __construct(
        private int $value,
    )
    {
    }

    public function getValue()
    {
        return $this->value;
    }
}

After:

final class SomeClass
{
    public int $value {
        get => $this->value;
    }

    public function __construct(int $value)
    {
        $this->value = $value;
    }
}

[samsonasik]

What I mean if there is another readonly promoted property, it should be skipped as well, same behaviour with normal readonly property.

I will create PR to skip mixed existing property with readonly promoted property.

[samsonasik]

@TomasVotruba I created new PR to skip if class has:

• normal property

• readonly promoted property

• [Php84] Skip has mix between normal property and readonly promoted property on PropertyHookRector #7504

[TomasVotruba]

Let's kick of first version, to try and iterate 👍