Renamed trait constructor promotes properties when called

[peter17]

Constructor property promotion is not limited to methods named __construct. A trait constructor imported under a different name keeps promoting its parameters:

trait T {
      public function __construct(public readonly string $value) {}
}

class C {
      use T { __construct as protected init; }

      public function __construct(string $value) {
              $this->init($value);          // really does initialize $this->value
              if (!$this->isValid()) { throw new \Exception(); }
      }

      private function isValid(): bool {
              return !empty($this->value);  // reported: access to an uninitialized readonly property
      }
}

StaticCallHandler already marks a parent's promoted properties as initialized after parent::__construct(), but MethodCallHandler had no counterpart, so the scope after $this->init($value) still considered $value uninitialized. ClassPropertiesNode::getUninitializedProperties() derives the initialized-property map for each method called from the constructor from that caller scope, so isValid() inherited the stale state and the read was reported as premature access — as was a direct echo $this->value in the constructor itself.

MethodCallHandler now mirrors the parent::__construct() handling for $this->method() calls whose callee is declared in the current class. Detection is by parameter promotion rather than by method name, so no trait-specific special case is needed, and a per-method cache keeps the native-reflection lookup off the hot path.

Still reported, and pinned in the new test data: reading the property before the alias call, calling the alias conditionally, never calling it at all (Class … has an uninitialized readonly property), and calling it on a different instance. The same fix also clears the identical false positive for non-readonly promoted properties (property.uninitialized).

One knock-on: isset($this->value) after the alias call now reports isset.initializedProperty ("not nullable nor uninitialized") where it previously reported isset.property ("not nullable") — the property is genuinely initialized at that point, so the new message is the accurate one.

Not covered here: a trait constructor that assigns a non-promoted readonly property in its body still reports property.readOnlyAssignNotInConstructor. That needs ConstructorsHelper to treat renamed trait constructors as constructors as well, which is a separate change.

Closes phpstan/phpstan#9789

[staabm]

isn't this code running on every method call?
we are only interessted in method calls from within __construct, right?

[SanderMuller]

Reviewed at ff4b31b17. The fix is right and the detection is well chosen; two code-level suggestions and one uncovered case worth naming.

The premise holds — I checked it in PHP rather than assuming. Calling an aliased trait constructor really does promote, and reflection separates it cleanly from a real constructor:

trait T { public function __construct(public readonly string $value) {} }
class C { use T { __construct as protected init; } public function __construct(string $v) { $this->init($v); } }
// (new C('hello'))->value === 'hello'
// ReflectionMethod(C::class, 'init')->getParameters()[0]->isPromoted()        === true
// ReflectionMethod(C::class, '__construct')->getParameters()[0]->isPromoted() === false

So keying on promotion rather than the method name is both sufficient and precise, and the "no trait-specific special case" claim in the description is accurate.

Worth adding to the description: $other->init() is safe by construction, not only by test. MutatingScope::assignInitializedProperty() returns early when TypeUtils::findThisType() is null, so a non-$this receiver cannot mark anything.

Suggestion, hot path. The block computes $scope->getType($normalizedExpr->var) before asking whether the method has promoted parameters at all, and it reassigns $calledOnType, which line 110 already holds. Analysing src/Rules + src/Type/Php at level 8, the block is entered 1233 times and the promoted list is non-empty 0 times - 1233 redundant getType() calls. (Counts, not timings; I am not claiming a measurable wall-clock difference.)

foreach ($this->getPromotedParameterNames($methodReflection) as $propertyName) {
	$scope = $scope->assignInitializedProperty($calledOnType, $propertyName);
}

Reusing the existing $calledOnType is safe - it is the value that resolved the method on lines 113/153, and evaluating arguments cannot turn a non-$this receiver into $this - and it also avoids the same name meaning "before args" at 110 and "after args" at 216.

One more uncovered case, alongside the non-promoted one you list: the alias inherited from a parent class.

class P { use T { __construct as protected init; } }
class DChild extends P {
	public function __construct(string $v) { $this->init($v); }   // still reported
}

The guard requires the declaring class to be the current class, so this keeps reporting - identical output on your branch and on its parent, I checked. Fine as a limitation, just worth a line so it does not read as covered.

Verification on my side: the two new tests fail without MethodCallHandler.php; expectations are complete (blanking them yields exactly the 6 errors expected); the isset knock-on is better than advertised - that shape emitted three errors on the parent, including two false property.uninitializedReadonly, and one accurate isset.initializedProperty now. Full suite 21328, self-analysis and phpcs clean; the only red check is the Benchmark job, which fails on 2.2.x itself. MethodCallHandler is not #[ShadowedByTurboExtension], so no native mirror is needed.

[peter17]

@staabm — yes it runs on every method call, but __construct would be too narrow

The block is reached for every $this->method() call whose callee is declared in the current class. What it does per call is now just two bool checks, two string comparisons and one array_key_exists() on the per-method cache: I dropped the $scope->getType($normalizedExpr->var) line and reuse the $calledOnType from line 110, as @SanderMuller suggested. On their workload (src/Rules + src/Type/Php, level 8):

	before	after
block entered	1233	1233
getType() calls	1233	0
native-reflection lookups	316	316
properties assigned	0	0

The 316 are cache misses — one per distinct Class::method actually called on $this — for the whole run; everything else is an array hit.

Gating on __construct would lose configured additional constructors, which legitimately call the alias:

class TestCase {
      use T { __construct as protected init; }

      protected function setUp(): void          // registered in additionalConstructors
      {
              $this->init('x');
              echo $this->check();
      }

      private function check(): string { return $this->value; }
}

With the gate, $this->value is reported as an uninitialized readonly property again. That shape is now WithAdditionalConstructor in data/bug-9789.php, registered in the test's additionalConstructors next to the existing Bug10523/Bug12253 entries: green as it stands, and it fails with 151: Access to an uninitialized readonly property Bug9789\WithAdditionalConstructor::$value. the moment the gate is added — so the restriction cannot be reintroduced unnoticed.

Two smaller reasons to keep it ungated: isset($this->value) narrowing benefits in any method, and non-readonly promoted properties can be re-initialized through the alias outside a constructor.

@SanderMuller

$calledOnType reuse — done, and it is not only cheaper: the pre-arguments receiver is the object the call actually goes to, since evaluating arguments cannot change the callee. So $obj->init($obj = somethingElse) marks based on the object that was $this, which is what PHP does, and the name no longer means two different things at 110 and 216. Added a comment saying so.

$other->init() safe by construction — agreed, going into the description: MutatingScope::assignInitializedProperty() returns early when TypeUtils::findThisType() is null, so a non-$this receiver cannot mark anything regardless of the guard.

The parent-inherited alias — I probed it and it is narrower than "still reported". When the parent has its own constructor, nothing is reported on either side: the property's declaring class is the parent, so ClassPropertiesNode already treats it as initialized in the child.

class P {
      use T { __construct as protected init; }

      public function __construct(string $v) { $this->init($v); }
}

class DChild extends P {
      public function __construct(string $v) { $this->init($v); echo $this->value; }  // clean, both sides
}

The residual false positive is on the parent's declaration when the parent never initializes the property, including when it is abstract and never instantiated:

abstract class AbstractP {
      use T { __construct as protected init; }
}

class Child extends AbstractP {
      public function __construct(string $v) { $this->init($v); echo $this->value; }
}
// Class AbstractP has an uninitialized readonly property $value.   <- identical on this branch and its parent

That one comes from the uninitialized-properties path rather than premature access, so a fix at the call site cannot reach it — the parent has no constructor whose scope could be corrected. Named in the description as a limitation, next to the non-promoted one.

[SanderMuller]

Checked 0e43751e5 - everything you claimed holds, verified rather than taken on trust.

• $calledOnType reuse: the getType() line is gone and the comment explains why the pre-arguments receiver is the right one. Re-measured on the same workload (src/Rules + src/Type/Php, level 8): 1233 block entries, 316 native-reflection lookups, 0 occurrences of getType($normalizedExpr->var) in the file. Your table is accurate.
• The __construct gate: adding && $methodReflection->getName() === '__construct' fails the tests with exactly 151: Access to an uninitialized readonly property Bug9789\WithAdditionalConstructor::$value., as you said. Worth adding for @staabm that it also brings back 23 and 37 - the original issue's shape - so the gate would not narrow the change, it would undo it. The WithAdditionalConstructor case guards the additional-constructors half specifically.
• The parent-inherited alias: your refinement is right and my phrasing was too coarse. With the parent carrying its own constructor, DChild reading the property after $this->init($v) is clean on both sides; the only error in that shape is on the parent's declaration when the parent never initializes it, identical on your branch and on 0d109aab9. Nothing at the call site could reach that one.

Gates on my side: 38 tests in the two rule classes, full suite 21328, self-analysis clean, phpcs clean. CI's four reds are the ones red for everyone today (two Symplify integrations, Turbo/macos make phpstan, Benchmark).

One loose end: the two description additions you mentioned - the findThisType() note for $other->init(), and the parent/abstract case next to the non-promoted limitation - are not in the body yet. Both are only prose, so nothing blocking.