parent::__construct() resets $this state

ondrejmirtes · ondrejmirtes · commit c8174d59ccc9 · 2021-01-12T15:42:04.000+01:00
diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -78,6 +78,7 @@
 use PHPStan\PhpDoc\ResolvedPhpDocBlock;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\FunctionReflection;
+use PHPStan\Reflection\MethodReflection;
 use PHPStan\Reflection\Native\NativeMethodReflection;
 use PHPStan\Reflection\ParametersAcceptor;
 use PHPStan\Reflection\ParametersAcceptorSelector;
@@ -1799,6 +1800,21 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
 			}
 			$result = $this->processArgs($methodReflection, $parametersAcceptor, $expr->args, $scope, $nodeCallback, $context, $closureBindScope ?? null);
 			$scope = $result->getScope();
+			$scopeFunction = $scope->getFunction();
+			if (
+				$methodReflection !== null
+				&& !$methodReflection->isStatic()
+				&& $methodReflection->hasSideEffects()->yes()
+				&& $scopeFunction instanceof MethodReflection
+				&& !$scopeFunction->isStatic()
+				&& $scope->isInClass()
+				&& (
+					$scope->getClassReflection()->getName() === $methodReflection->getDeclaringClass()->getName()
+					|| $scope->getClassReflection()->isSubclassOf($methodReflection->getDeclaringClass()->getName())
+				)
+			) {
+				$scope = $scope->invalidateExpression(new Variable('this'), true);
+			}
 			$hasYield = $hasYield || $result->hasYield();
 		} elseif ($expr instanceof PropertyFetch) {
 			$result = $this->processExprNode($expr->var, $scope, $nodeCallback, $context->enterDeep());
diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php
@@ -452,17 +452,22 @@ public function getThrowType(): ?Type
 	public function hasSideEffects(): TrinaryLogic
 	{
 		$name = strtolower($this->getName());
+		$isVoid = $this->getReturnType() instanceof VoidType;
 
 		if (
 			$name !== '__construct'
-			&& $this->getReturnType() instanceof VoidType
+			&& $isVoid
 		) {
 			return TrinaryLogic::createYes();
 		}
 		if ($this->isPure !== null) {
 			return TrinaryLogic::createFromBoolean(!$this->isPure);
 		}
 
+		if ($isVoid) {
+			return TrinaryLogic::createYes();
+		}
+
 		return TrinaryLogic::createMaybe();
 	}
 
diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -10705,6 +10705,11 @@ public function dataImpureMethod(): array
 		return $this->gatherAssertTypes(__DIR__ . '/data/impure-method.php');
 	}
 
+	public function dataBug4351(): array
+	{
+		return $this->gatherAssertTypes(__DIR__ . '/data/bug-4351.php');
+	}
+
 	/**
 	 * @param string $file
 	 * @return array<string, mixed[]>
@@ -10911,6 +10916,7 @@ private function gatherAssertTypes(string $file): array
 	 * @dataProvider dataBug4339
 	 * @dataProvider dataBug4343
 	 * @dataProvider dataImpureMethod
+	 * @dataProvider dataBug4351
 	 * @param string $assertType
 	 * @param string $file
 	 * @param mixed ...$args
diff --git a/tests/PHPStan/Analyser/data/bug-4351.php b/tests/PHPStan/Analyser/data/bug-4351.php
@@ -0,0 +1,71 @@
+<?php
+
+namespace Bug4351;
+
+use function PHPStan\Analyser\assertType;
+
+class Thing
+{
+	public function doSomething(): void
+	{
+	}
+}
+
+class ParentC
+{
+	/** @var Thing|null */
+	protected $thing;
+
+	protected function __construct()
+	{
+		$this->thing = null;
+	}
+}
+
+class HelloWorld extends ParentC
+{
+	public function __construct(Thing $thing)
+	{
+		assertType('Bug4351\Thing|null', $this->thing);
+		$this->thing = $thing;
+		assertType('Bug4351\Thing', $this->thing);
+
+		parent::__construct();
+		assertType('Bug4351\Thing|null', $this->thing);
+	}
+
+	public function doFoo(Thing $thing)
+	{
+		assertType('Bug4351\Thing|null', $this->thing);
+		$this->thing = $thing;
+		assertType('Bug4351\Thing', $this->thing);
+
+		UnrelatedClass::doFoo();
+		assertType('Bug4351\Thing', $this->thing);
+	}
+
+	public function doBar(Thing $thing)
+	{
+		assertType('Bug4351\Thing|null', $this->thing);
+		$this->thing = $thing;
+		assertType('Bug4351\Thing', $this->thing);
+
+		UnrelatedClass::doStaticFoo();
+		assertType('Bug4351\Thing', $this->thing);
+	}
+}
+
+class UnrelatedClass
+{
+
+	public function doFoo(): void
+	{
+
+	}
+
+	public static function doStaticFoo(): void
+	{
+
+	}
+
+}

Original file line number	Diff line number	Diff line change
`@@ -452,17 +452,22 @@ public function getThrowType(): ?Type`
`452`	`452`	`public function hasSideEffects(): TrinaryLogic`
`453`	`453`	`{`
`454`	`454`	`$name = strtolower($this->getName());`
	`455`	`+ $isVoid = $this->getReturnType() instanceof VoidType;`
`455`	`456`
`456`	`457`	`if (`
`457`	`458`	`$name !== '__construct'`
`458`		`- && $this->getReturnType() instanceof VoidType`
	`459`	`+ && $isVoid`
`459`	`460`	`) {`
`460`	`461`	`return TrinaryLogic::createYes();`
`461`	`462`	`}`
`462`	`463`	`if ($this->isPure !== null) {`
`463`	`464`	`return TrinaryLogic::createFromBoolean(!$this->isPure);`
`464`	`465`	`}`
`465`	`466`
	`467`	`+ if ($isVoid) {`
	`468`	`+ return TrinaryLogic::createYes();`
	`469`	`+ }`
	`470`	`+`
`466`	`471`	`return TrinaryLogic::createMaybe();`
`467`	`472`	`}`
`468`	`473`