Check only the right side of ??= as the value written to its target

src/Analyser/ExprHandler/AssignHandler.php

@@ -760,7 +760,7 @@ private function doPrepareTarget(
 			// 1b. build the write chain (Set*OffsetValueTypeExpr nesting) after
 			// the root walk, so a property base's holder and current types are
 			// read from its stored result instead of pricing the unwalked fetch
-			$assignedPropertyExpr = $assignedExpr;
+			$assignedPropertyExpr = $this->getWrittenExpr($originalVar, $assignedExpr);
 			$chainVar = $originalVar;
 			while ($chainVar instanceof ArrayDimFetch) {
 				$varForSetOffsetValue = $chainVar->var;
@@ -1374,7 +1374,7 @@ public function applyWrite(
 					$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult);
 				}
 
-				$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
+				$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $this->getWrittenExpr($var, $assignedExpr)), $scopeBeforeAssignEval, $storage);
 
 				$nativeType = $this->readAssignedValueType($nodeScopeResolver, $storedAssignedExprResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain());
 				$scope = $scope->assignVariable(
@@ -1596,7 +1596,7 @@ public function applyWrite(
 				if ($templateArgumentFrame !== null) {
 					$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectSend($propertyReflection->getWritableType(), $assignedExprType));
 				}
-				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
+				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($var, $assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
 				if ($propertyReflection->canChangeTypeAfterAssignment()) {
 					if ($propertyReflection->hasNativeType()) {
 						$propertyNativeType = $propertyReflection->getNativeType();
@@ -1657,7 +1657,7 @@ public function applyWrite(
 			} else {
 				// fallback
 				$assignedExprType = $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope);
-				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
+				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($var, $assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
 				$scope = $scope->assignExpression($var, $assignedExprType, $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()));
 				// simulate dynamic property assign by __set to get throw points
 				if (!$propertyHolderType->hasMethod('__set')->no()) {
@@ -1693,7 +1693,7 @@ public function applyWrite(
 						$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectSend($propertyReflection->getWritableType(), $assignedExprType));
 					}
 				}
-				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
+				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($var, $assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
 				if ($propertyReflection !== null && $propertyReflection->canChangeTypeAfterAssignment()) {
 					if ($propertyReflection->hasNativeType()) {
 						$propertyNativeType = $propertyReflection->getNativeType();
@@ -1724,7 +1724,7 @@ public function applyWrite(
 			} else {
 				// fallback
 				$assignedExprType = $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope);
-				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
+				$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($var, $assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
 				$scope = $scope->assignExpression($var, $assignedExprType, $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()));
 			}
 		} elseif ($kind === PreparedAssignTarget::KIND_LIST) {
@@ -2049,6 +2049,18 @@ private function assignDynamicVariable(MutatingScope $scope, ExpressionResult $n
 		return $scope;
 	}
 
+	/**
+	 * `$var ??= $value` evaluates to `$var ?? $value`, but writes only `$value` to `$var`.
+	 */
+	private function getWrittenExpr(Expr $var, Expr $assignedExpr): Expr
+	{
+		if ($assignedExpr instanceof Expr\AssignOp\Coalesce && $assignedExpr->var === $var) {
+			return $assignedExpr->expr;
+		}
+
+		return $assignedExpr;
+	}
+
 	private function unwrapAssign(Expr $expr): Expr
 	{
 		if ($expr instanceof Assign) {

src/Analyser/ExprHandler/AssignOpHandler.php

@@ -26,9 +26,11 @@
 use PHPStan\Dependency\Dependencies;
 use PHPStan\DependencyInjection\AutowiredService;
 use PHPStan\Node\CoalesceExpressionNode;
+use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
 use PHPStan\Node\Variable\VariableWrite;
 use PHPStan\Reflection\InitializerExprTypeResolver;
 use PHPStan\ShouldNotHappenException;
+use PHPStan\TrinaryLogic;
 use PHPStan\Turbo\ShadowedByTurboExtension;
 use PHPStan\Type\Constant\ConstantIntegerType;
 use PHPStan\Type\MixedType;
@@ -271,6 +273,29 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 			$context,
 		);
 		$scope = $assignResult->getScope();
+		if ($expr instanceof Expr\AssignOp\Coalesce && $expr->var instanceof Expr\Variable && is_string($expr->var->name)) {
+			if ($condResult === null) {
+				throw new ShouldNotHappenException();
+			}
+
+			// the write only happens when the left side is null - a by-ref
+			// parameter may still hold its original value afterwards (a
+			// closure's copy of an outer parameter carries no native marker)
+			$originalValueExpr = new ParameterVariableOriginalValueExpr($expr->var->name);
+			$nativeValueBeforeScope = $valueBeforeScope->doNotTreatPhpDocTypesAsCertain();
+			if (
+				!$condResult->getType()->isNull()->yes()
+				&& !$valueBeforeScope->hasExpressionType($originalValueExpr)->no()
+				&& !$nativeValueBeforeScope->hasExpressionType($originalValueExpr)->no()
+			) {
+				$scope = $scope->specifyExpressionType(
+					$originalValueExpr,
+					$valueBeforeScope->getTrackedExpressionType($originalValueExpr),
+					$nativeValueBeforeScope->getTrackedExpressionType($originalValueExpr),
+					TrinaryLogic::createMaybe(),
+				);
+			}
+		}
 		$throwPoints = $assignResult->getThrowPoints();
 		$impurePoints = $assignResult->getImpurePoints();
 		if (

tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php

@@ -15,6 +15,8 @@ class TypesAssignedToPropertiesRuleTest extends RuleTestCase
 
 	private bool $checkNullables = true;
 
+	private bool $checkUnionTypes = true;
+
 	private bool $checkExplicitMixed = false;
 
 	private bool $checkImplicitMixed = false;
@@ -26,7 +28,7 @@ protected function getRule(): Rule
 				self::createReflectionProvider(),
 				checkNullables: $this->checkNullables,
 				checkThisOnly: false,
-				checkUnionTypes: true,
+				checkUnionTypes: $this->checkUnionTypes,
 				checkExplicitMixed: $this->checkExplicitMixed,
 				checkImplicitMixed: $this->checkImplicitMixed,
 				checkBenevolentUnionTypes: false,
@@ -1131,6 +1133,58 @@ public function testBug9096(): void
 		$this->analyse([__DIR__ . '/data/bug-9096.php'], []);
 	}
 
+	public function testBug15228(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15228.php'], $this->getBug15228Errors());
+	}
+
+	public function testBug15228WithoutUnionTypes(): void
+	{
+		$this->checkUnionTypes = false;
+		$this->analyse([__DIR__ . '/data/bug-15228.php'], $this->getBug15228Errors());
+	}
+
+	/**
+	 * @return list<array{0: string, 1: int, 2?: string|null}>
+	 */
+	private function getBug15228Errors(): array
+	{
+		return [
+			[
+				'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
+				26,
+			],
+			[
+				'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
+				32,
+			],
+			[
+				'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
+				41,
+			],
+			[
+				'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
+				47,
+			],
+			[
+				'Static property Bug15228\\Adjacent::$s (int|null) does not accept string.',
+				64,
+			],
+			[
+				'Property Bug15228\\Adjacent::$a (int|null) does not accept string.',
+				70,
+			],
+			[
+				'Property Bug15228\\Adjacent::$uninitialized (int) does not accept string.',
+				75,
+			],
+			[
+				'Property Bug15228\\Offsets::$m (array<string, int>) does not accept array<string, int|string>.',
+				102,
+			],
+		];
+	}
+
 	public function testIntersectionWithTemplateMixed(): void
 	{
 		$this->analyse([__DIR__ . '/data/intersection-template-mixed-property.php'], [

tests/PHPStan/Rules/Properties/data/bug-15228.php

@@ -0,0 +1,109 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15228;
+
+use function PHPStan\dumpType;
+use function PHPStan\Testing\assertType;
+
+class CoalesceAssignDemo
+{
+	public ?int $a = null;
+
+	/** @var list<int>|null */
+	public $b;
+
+	/** @return list<string> */
+	private function strings(): array
+	{
+		return ['x'];
+	}
+
+	// === Symptom 1: false negative below level 7 ===
+	// Both methods write a string into ?int - a guaranteed TypeError.
+
+	public function coalesceInvalid(): void
+	{
+		$this->a ??= 'boom';           // (NOT reported at levels 0-6.)
+	}
+
+	public function ifInvalid(): void
+	{
+		if (null === $this->a) {
+			$this->a = 'boom';         // (Reported from level 0. Correct.)
+		}
+	}
+
+	// === Symptom 2: false positive at level 7+ ===
+	// Both methods write list<int>, which the property accepts.
+
+	public function coalesceValid(): void
+	{
+		$this->b = $this->strings();   // (Genuine error, both methods.)
+		$this->b ??= [1, 2];           // VALID write - reported at 7+ anyway.
+	}
+
+	public function ifValid(): void
+	{
+		$this->b = $this->strings();   // (Genuine error, both methods.)
+		if (null === $this->b) {
+			$this->b = [1, 2];         // VALID write - correctly not reported.
+		}
+	}
+}
+
+class Adjacent
+{
+	public static ?int $s = null;
+
+	public ?int $a = null;
+
+	public int $uninitialized;
+
+	public function staticProperty(): void
+	{
+		self::$s ??= 'boom';
+	}
+
+	public function neverNull(): void
+	{
+		$this->a = 1;
+		$this->a ??= 'never written';
+	}
+
+	public function maybeUninitialized(): void
+	{
+		$this->uninitialized ??= 'boom';
+	}
+
+	public function valid(): void
+	{
+		$this->a ??= 5;
+		self::$s ??= 5;
+	}
+}
+
+class RightSideNarrowing
+{
+	public ?string $q = null;
+
+	public function rightSideSeesNullLeftSide(): void
+	{
+		$this->q ??= $this->q === null ? 'x' : 1;
+	}
+}
+
+class Offsets
+{
+	/** @var array<string, int> */
+	public array $m = [];
+
+	public function invalid(): void
+	{
+		$this->m['k'] ??= 'boom';
+	}
+
+	public function valid(): void
+	{
+		$this->m['k'] ??= 5;
+	}
+}

tests/PHPStan/Rules/TooWideTypehints/TooWidePropertyTypeRuleTest.php

@@ -144,4 +144,15 @@ public function testBug11844(): void
 		$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-11844.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15228(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15228.php'], [
+			[
+				'Property Bug15228\\OnlyCoalesceAssigned::$x (int|string|null) is never assigned string so it can be removed from the property type.',
+				9,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/TooWideTypehints/data/bug-15228.php

@@ -0,0 +1,20 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace Bug15228;
+
+final class OnlyCoalesceAssigned
+{
+	private int|string|null $x = null;
+
+	public function set(): void
+	{
+		$this->x ??= 1;
+	}
+
+	public function get(): int|string|null
+	{
+		return $this->x;
+	}
+}

tests/PHPStan/Rules/Variables/ParameterOutAssignedTypeRuleTest.php

@@ -145,4 +145,19 @@ public function testBug15428(): void
 		$this->analyse([__DIR__ . '/data/bug-15428.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15228(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15228.php'], [
+			[
+				'Parameter &$p @param-out type of function Bug15228\\writesString() expects int, string given.',
+				16,
+			],
+			[
+				'Parameter &$p @param-out type of function Bug15228\\validWrite() expects list<int>|null, list<string> given.',
+				22,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Variables/ParameterOutExecutionEndTypeRuleTest.php

@@ -86,4 +86,23 @@ public function testBug15066(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15228(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15228.php'], [
+			[
+				'Parameter &$p @param-out type of function Bug15228\\writesString() expects int, int|string given.',
+				16,
+			],
+			[
+				'Parameter &$p @param-out type of function Bug15228\\keepsOriginalValue() expects string, int|string given.',
+				29,
+			],
+			[
+				'Parameter &$p @param-out type of function Bug15228\\keepsOriginalValueTwice() expects string, int|string given.',
+				36,
+			],
+		]);
+	}
+
 }