Use rector to add property typehints

[jack-worman]

Used Rector's TypedPropertyFromAssignsRector rule.

[muglug]

Hey! Thanks for this PR. What impact does this have on performance? Every time PHP writes to a typed property it performs a type-check.

From what I've elsewhere, I think the performance difference might be too miniscule to matter, but I'm definitely curious.

[jack-worman]

@muglug Here is a benchmark:

<?php

declare(strict_types=1);

class PerformanceTest1 {
    public int $var1;
    /** @var int */
    public $var2;
}
$performanceTest1 = new PerformanceTest1();

function getDelta(array $startTime, array $endTime): array {
    $delta = [$endTime[0] - $startTime[0], $endTime[1] - $startTime[1]];
    if ($delta[1] < 0) {
        $delta[0] -= 1;
        $delta[1] += 1_000_000_000;
    }
    return $delta;
}

$startTime = hrtime();
for ($i = 0; $i < 1_000_000_000; ++$i) {
    $performanceTest1->var2 = $i;
}
$endTime = hrtime();
$delta1 = getDelta($startTime, $endTime);

$startTime = hrtime();
for ($i = 0; $i < 1_000_000_000; ++$i) {
    $performanceTest1->var1 = $i;
}
$endTime = hrtime();
$delta2 = getDelta($startTime, $endTime);

var_export($delta1);
echo "\n";
var_export($delta2);
echo "\n";
var_export(getDelta($delta1, $delta2));
echo "\n";

Takes about 1 billion writes to get to a difference over 1 second.

[weirdan]

From what I see, there's less than 0.5% slowdown:

[orklah]

Well, for 0.5, I'd say it's worth it. I'm surprised not all properties were changed though. Maybe rector needs to pass in multiple sweeps for tricky properties?

[jack-worman]

@orklah Yea... rector also screwed up on a bunch of the array types and mangled the docblocks. I had to go through and revert a bunch of stuff.

If this PR goes through I would be happy to go work on the rest of the untyped properties.

[weirdan]

Note that changing the property type for non-internal non-private properties is a BC break.

Basically:

• changing a private property type is ok
• changing any property type on internal classes (Psalm\Internal namespace) is ok

[jack-worman]

Note that changing the property type for non-internal non-private properties is a BC break.

I re-reviewed the PR and can confirm there is no BC

[weirdan]

Yes, this PR is fine. I mean, this is something to keep in mind for your potential future PRs.

[orklah]

Thanks!