CallToMethodStatementWithoutImpurePointsRule - fix for methods called…

… on unions
phpstan · May 15, 2024 · ee33f1e · ee33f1e
1 parent 9340249
commit ee33f1e
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 27 deletions.
diff --git a/src/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRule.php b/src/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRule.php
@@ -38,19 +38,22 @@ public function processNode(Node $node, Scope $scope): array
 
 		$errors = [];
 		foreach ($node->get(PossiblyPureMethodCallCollector::class) as $filePath => $data) {
-			foreach ($data as [$className, $method, $line]) {
-				$className = strtolower($className);
+			foreach ($data as [$classNames, $method, $line]) {
+				$originalMethodName = null;
+				foreach ($classNames as $className) {
+					$className = strtolower($className);
 
-				if (!array_key_exists($className, $methods)) {
-					continue;
-				}
+					if (!array_key_exists($className, $methods)) {
+						continue 2;
+					}
 
-				$lowerMethod = strtolower($method);
-				if (!array_key_exists($lowerMethod, $methods[$className])) {
-					continue;
-				}
+					$lowerMethod = strtolower($method);
+					if (!array_key_exists($lowerMethod, $methods[$className])) {
+						continue 2;
+					}
 
-				$originalMethodName = $methods[$className][$lowerMethod];
+					$originalMethodName = $methods[$className][$lowerMethod];
+				}
 
 				$errors[] = RuleErrorBuilder::message(sprintf(
 					'Call to method %s() on a separate line has no effect.',

diff --git a/src/Rules/DeadCode/PossiblyPureMethodCallCollector.php b/src/Rules/DeadCode/PossiblyPureMethodCallCollector.php
@@ -6,10 +6,9 @@
 use PhpParser\Node\Stmt\Expression;
 use PHPStan\Analyser\Scope;
 use PHPStan\Collectors\Collector;
-use function count;
 
 /**
- * @implements Collector<Node\Stmt\Expression, array{class-string, string, int}>
+ * @implements Collector<Node\Stmt\Expression, array{non-empty-list<class-string>, string, int}>
  */
 class PossiblyPureMethodCallCollector implements Collector
 {
@@ -34,32 +33,42 @@ public function processNode(Node $node, Scope $scope)
 
 		$methodName = $node->expr->name->toString();
 		$calledOnType = $scope->getType($node->expr->var);
-		$methodReflection = $scope->getMethodReflection($calledOnType, $methodName);
-		if ($methodReflection === null) {
+		if (!$calledOnType->hasMethod($methodName)->yes()) {
 			return null;
 		}
-		if (
-			!$methodReflection->isPrivate()
-			&& !$methodReflection->isFinal()->yes()
-			&& !$methodReflection->getDeclaringClass()->isFinal()
-		) {
-			$typeClassReflections = $calledOnType->getObjectClassReflections();
-			if (count($typeClassReflections) !== 1) {
+
+		$classNames = [];
+		$methodReflection = null;
+		foreach ($calledOnType->getObjectClassReflections() as $classReflection) {
+			if (!$classReflection->hasMethod($methodName)) {
 				return null;
 			}
 
-			if (!$typeClassReflections[0]->isFinal()) {
+			$methodReflection = $classReflection->getMethod($methodName, $scope);
+			if (
+				!$methodReflection->isPrivate()
+				&& !$methodReflection->isFinal()->yes()
+				&& !$methodReflection->getDeclaringClass()->isFinal()
+			) {
+				if (!$classReflection->isFinal()) {
+					return null;
+				}
+			}
+			if (!$methodReflection->isPure()->maybe()) {
 				return null;
 			}
+			if (!$methodReflection->hasSideEffects()->maybe()) {
+				return null;
+			}
+
+			$classNames[] = $methodReflection->getDeclaringClass()->getName();
 		}
-		if (!$methodReflection->isPure()->maybe()) {
-			return null;
-		}
-		if (!$methodReflection->hasSideEffects()->maybe()) {
+
+		if ($methodReflection === null) {
 			return null;
 		}
 
-		return [$methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), $node->getStartLine()];
+		return [$classNames, $methodReflection->getName(), $node->getStartLine()];
 	}
 
 }
diff --git a/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php
@@ -58,6 +58,16 @@ public function testRule(): void
 		]);
 	}
 
+	public function testBug11011(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-11011.php'], [
+			[
+				'Call to method Bug11011\AnotherPureImpl::doFoo() on a separate line has no effect.',
+				32,
+			],
+		]);
+	}
+
 	protected function getCollectors(): array
 	{
 		return [

diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-11011.php b/tests/PHPStan/Rules/DeadCode/data/bug-11011.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace Bug11011;
+
+final class ImpureImpl  {
+	/** @phpstan-impure */
+	public function doFoo() {
+		echo "yes";
+		$_SESSION['ab'] = 1;
+	}
+}
+
+final class PureImpl {
+	public function doFoo(): bool {
+		return true;
+	}
+}
+
+final class AnotherPureImpl {
+	public function doFoo(): bool {
+		return true;
+	}
+}
+
+class User {
+	function doBar(PureImpl|ImpureImpl $f): bool {
+		$f->doFoo();
+		return true;
+	}
+
+	function doBar2(PureImpl|AnotherPureImpl $f): bool {
+		$f->doFoo();
+		return true;
+	}
+}