Passing a variable by reference to a function and method has side eff…

…ects
phpstan · Jan 8, 2023 · 1070788 · 1070788
1 parent 1403c03
commit 1070788
Show file tree

Hide file tree

Showing 10 changed files with 91 additions and 0 deletions.
diff --git a/src/Reflection/Annotations/AnnotationMethodReflection.php b/src/Reflection/Annotations/AnnotationMethodReflection.php
@@ -113,6 +113,14 @@ public function hasSideEffects(): TrinaryLogic
 			return TrinaryLogic::createYes();
 		}
 
+		foreach ($this->getVariants() as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
+
 		return TrinaryLogic::createMaybe();
 	}
 

diff --git a/src/Reflection/Native/NativeFunctionReflection.php b/src/Reflection/Native/NativeFunctionReflection.php
@@ -81,6 +81,13 @@ public function hasSideEffects(): TrinaryLogic
 		if ($this->isVoid()) {
 			return TrinaryLogic::createYes();
 		}
+		foreach ($this->variants as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
 
 		return $this->hasSideEffects;
 	}

diff --git a/src/Reflection/Native/NativeMethodReflection.php b/src/Reflection/Native/NativeMethodReflection.php
@@ -136,6 +136,13 @@ public function hasSideEffects(): TrinaryLogic
 		) {
 			return TrinaryLogic::createYes();
 		}
+		foreach ($this->variants as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
 
 		return $this->hasSideEffects;
 	}

diff --git a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php
@@ -199,6 +199,13 @@ public function hasSideEffects(): TrinaryLogic
 		if ($this->isPure !== null) {
 			return TrinaryLogic::createFromBoolean(!$this->isPure);
 		}
+		foreach ($this->getVariants() as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
 
 		return TrinaryLogic::createMaybe();
 	}

diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php
@@ -245,6 +245,13 @@ public function hasSideEffects(): TrinaryLogic
 		if ($this->isPure !== null) {
 			return TrinaryLogic::createFromBoolean(!$this->isPure);
 		}
+		foreach ($this->getVariants() as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
 
 		return TrinaryLogic::createMaybe();
 	}

diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php
@@ -416,6 +416,13 @@ public function hasSideEffects(): TrinaryLogic
 		if ($this->isPure !== null) {
 			return TrinaryLogic::createFromBoolean(!$this->isPure);
 		}
+		foreach ($this->getVariants() as $variant) {
+			foreach ($variant->getParameters() as $parameter) {
+				if ($parameter->passedByReference()->yes()) {
+					return TrinaryLogic::createYes();
+				}
+			}
+		}
 
 		return TrinaryLogic::createMaybe();
 	}

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -1170,6 +1170,7 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8635.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8625.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8621.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8084.php');
 	}
 
 	/**

diff --git a/tests/PHPStan/Analyser/data/bug-8084.php b/tests/PHPStan/Analyser/data/bug-8084.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Bug8084;
+
+use Exception;
+use function array_shift;
+use function PHPStan\Testing\assertType;
+
+class Bug8084
+{
+	/**
+	 * @param array{a: 0, b?: 1} $arr
+	 * @throws Exception
+	 */
+	public function run(array $arr): void
+	{
+		assertType('0', array_shift($arr) ?? throw new Exception());
+		assertType('1|null', array_shift($arr));
+	}
+}
diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php
@@ -360,4 +360,12 @@ public function testBug7968(): void
 		$this->analyse([__DIR__ . '/data/bug-7968.php'], []);
 	}
 
+	public function testBug8084(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->strictUnnecessaryNullsafePropertyFetch = true;
+
+		$this->analyse([__DIR__ . '/data/bug-8084.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Variables/data/bug-8084.php b/tests/PHPStan/Rules/Variables/data/bug-8084.php
@@ -0,0 +1,19 @@
+<?php // lint >= 8.0
+
+namespace Bug8084;
+
+use Exception;
+use function array_shift;
+use function PHPStan\Testing\assertType;
+
+class Bug8084
+{
+	/**
+	 * @param array{a?: 0} $arr
+	 * @throws Exception
+	 */
+	public function run(array $arr): void
+	{
+		assertType('0|null', array_shift($arr) ?? throw new Exception());
+	}
+}