Fix phpstan/phpstan#14323: Weird variable might not be defined behavior

src/Analyser/ExprHandler/NewHandler.php

@@ -81,6 +81,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 	{
 		$parametersAcceptor = null;
 		$constructorReflection = null;
+		$classReflection = null;
 		$hasYield = false;
 		$throwPoints = [];
 		$impurePoints = [];
@@ -89,8 +90,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		if ($expr->class instanceof Name) {
 			$className = $scope->resolveName($expr->class);
 
-			[$constructorReflection, $parametersAcceptor, $constructorThrowPoints, $constructorImpurePoints] = $this->processConstructorReflection($className, $expr, $scope);
-			$throwPoints = array_merge($throwPoints, $constructorThrowPoints);
+			[$constructorReflection, $classReflection, $parametersAcceptor, $constructorImpurePoints] = $this->processConstructorReflection($className, $expr, $scope);
 			$impurePoints = array_merge($impurePoints, $constructorImpurePoints);
 
 			if ($parametersAcceptor !== null) {
@@ -136,19 +136,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 					}
 				} else {
 					$nodeScopeResolver->processStmtNode($expr->class, $scope, $storage, $nodeCallback, StatementContext::createTopLevel());
-					$declaringClass = $constructorReflection->getDeclaringClass();
-					$constructorThrowPoint = $this->getConstructorThrowPoint($constructorReflection, $parametersAcceptor, $classReflection, $expr, new Name\FullyQualified($declaringClass->getName()), $expr->getArgs(), $scope);
-					if ($constructorThrowPoint !== null) {
-						$throwPoints[] = $constructorThrowPoint;
-					}
-
 					if (!$constructorReflection->hasSideEffects()->no()) {
 						$certain = $constructorReflection->isPure()->no();
 						$impurePoints[] = new ImpurePoint(
 							$scope,
 							$expr,
 							'new',
-							sprintf('instantiation of class %s', $declaringClass->getDisplayName()),
+							sprintf('instantiation of class %s', $constructorReflection->getDeclaringClass()->getDisplayName()),
 							$certain,
 						);
 					}
@@ -176,11 +170,9 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 			$throwPoints = array_merge($throwPoints, $additionalThrowPoints);
 
 			if ($className !== null) {
-				[$constructorReflection, $parametersAcceptor, $constructorThrowPoints, $constructorImpurePoints] = $this->processConstructorReflection($className, $expr, $scope);
-				$throwPoints = array_merge($throwPoints, $constructorThrowPoints);
+				[$constructorReflection, $classReflection, $parametersAcceptor, $constructorImpurePoints] = $this->processConstructorReflection($className, $expr, $scope);
 				$impurePoints = array_merge($impurePoints, $constructorImpurePoints);
 			} else {
-				$throwPoints[] = InternalThrowPoint::createImplicit($scope, $expr);
 				$impurePoints[] = new ImpurePoint(
 					$scope,
 					$expr,
@@ -202,6 +194,16 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		$impurePoints = array_merge($impurePoints, $argsResult->getImpurePoints());
 		$isAlwaysTerminating = $isAlwaysTerminating || $argsResult->isAlwaysTerminating();
 
+		if ($constructorReflection !== null && $parametersAcceptor !== null) {
+			$className ??= $constructorReflection->getDeclaringClass()->getName();
+			$constructorThrowPoint = $this->getConstructorThrowPoint($constructorReflection, $parametersAcceptor, $expr, new Name\FullyQualified($className), $expr->getArgs(), $scope);
+			if ($constructorThrowPoint !== null) {
+				$throwPoints[] = $constructorThrowPoint;
+			}
+		} elseif ($classReflection === null) {
+			$throwPoints[] = InternalThrowPoint::createImplicit($scope, $expr);
+		}
+
 		return new ExpressionResult(
 			$scope,
 			hasYield: $hasYield,
@@ -212,13 +214,12 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 	}
 
 	/**
-	 * @return array{?MethodReflection, ?ParametersAcceptor, InternalThrowPoint[], ImpurePoint[]}
+	 * @return array{?MethodReflection, ?ClassReflection, ?ParametersAcceptor, ImpurePoint[]}
 	 */
 	private function processConstructorReflection(string $className, New_ $expr, MutatingScope $scope): array
 	{
 		$constructorReflection = null;
 		$parametersAcceptor = null;
-		$throwPoints = [];
 		$impurePoints = [];
 
 		$classReflection = null;
@@ -232,13 +233,7 @@ private function processConstructorReflection(string $className, New_ $expr, Mut
 					$constructorReflection->getVariants(),
 					$constructorReflection->getNamedArgumentsVariants(),
 				);
-				$constructorThrowPoint = $this->getConstructorThrowPoint($constructorReflection, $parametersAcceptor, $classReflection, $expr, new Name\FullyQualified($className), $expr->getArgs(), $scope);
-				if ($constructorThrowPoint !== null) {
-					$throwPoints[] = $constructorThrowPoint;
-				}
 			}
-		} else {
-			$throwPoints[] = InternalThrowPoint::createImplicit($scope, $expr);
 		}
 
 		if ($constructorReflection !== null) {
@@ -262,13 +257,13 @@ private function processConstructorReflection(string $className, New_ $expr, Mut
 			);
 		}
 
-		return [$constructorReflection, $parametersAcceptor, $throwPoints, $impurePoints];
+		return [$constructorReflection, $classReflection, $parametersAcceptor, $impurePoints];
 	}
 
 	/**
 	 * @param list<Node\Arg> $args
 	 */
-	private function getConstructorThrowPoint(MethodReflection $constructorReflection, ParametersAcceptor $parametersAcceptor, ClassReflection $classReflection, New_ $new, Name $className, array $args, MutatingScope $scope): ?InternalThrowPoint
+	private function getConstructorThrowPoint(MethodReflection $constructorReflection, ParametersAcceptor $parametersAcceptor, New_ $new, Name $className, array $args, MutatingScope $scope): ?InternalThrowPoint
 	{
 		$methodCall = new StaticCall($className, $constructorReflection->getName(), $args);
 		$normalizedMethodCall = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $methodCall);
@@ -293,7 +288,7 @@ private function getConstructorThrowPoint(MethodReflection $constructorReflectio
 				return InternalThrowPoint::createExplicit($scope, $throwType, $new, true);
 			}
 		} elseif ($this->implicitThrows) {
-			if (!$classReflection->is(Throwable::class)) {
+			if (!$constructorReflection->getDeclaringClass()->is(Throwable::class)) {
 				return InternalThrowPoint::createImplicit($scope, $methodCall);
 			}
 		}

src/Analyser/InternalScopeFactory.php

@@ -15,7 +15,7 @@ interface InternalScopeFactory
 	 * @param array<string, ExpressionTypeHolder> $expressionTypes
 	 * @param array<string, ExpressionTypeHolder> $nativeExpressionTypes
 	 * @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
-	 * @param list<string> $inClosureBindScopeClasses
+	 * @param list<non-empty-string> $inClosureBindScopeClasses
 	 * @param array<string, true> $currentlyAssignedExpressions
 	 * @param array<string, true> $currentlyAllowedUndefinedExpressions
 	 * @param list<array{FunctionReflection|MethodReflection|null, ParameterReflection|null}> $inFunctionCallsStack

src/Analyser/MutatingScope.php

@@ -162,7 +162,7 @@ class MutatingScope implements Scope, NodeCallbackInvoker
 	 * @param callable(Node $node, Scope $scope): void|null $nodeCallback
 	 * @param array<string, ExpressionTypeHolder> $expressionTypes
 	 * @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
-	 * @param list<string> $inClosureBindScopeClasses
+	 * @param list<non-empty-string> $inClosureBindScopeClasses
 	 * @param array<string, true> $currentlyAssignedExpressions
 	 * @param array<string, true> $currentlyAllowedUndefinedExpressions
 	 * @param array<string, ExpressionTypeHolder> $nativeExpressionTypes
@@ -1820,7 +1820,7 @@ public function enterNamespace(string $namespaceName): self
 	}
 
 	/**
-	 * @param list<string> $scopeClasses
+	 * @param list<non-empty-string> $scopeClasses
 	 */
 	public function enterClosureBind(?Type $thisType, ?Type $nativeThisType, array $scopeClasses): self
 	{

src/Analyser/Scope.php

@@ -173,6 +173,8 @@ public function getScopeNativeType(Expr $expr): Type;
 	 * (they should already be fully qualified by the PHP parser's name resolver).
 	 *
 	 * Inside a Closure::bind() context, `self`/`static` resolve to the bound class.
+	 *
+	 * @return non-empty-string
 	 */
 	public function resolveName(Name $name): string;
 

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -1432,6 +1432,29 @@ public function testBug14318(): void
 		$this->analyse([__DIR__ . '/data/bug-14318.php'], []);
 	}
 
+	public function testBug14323(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = false;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+
+		$this->analyse([__DIR__ . '/data/bug-14323.php'], [
+			[
+				'Variable $command might not be defined.',
+				40,
+			],
+			[
+				'Variable $command might not be defined.',
+				66,
+			],
+			[
+				'Variable $command might not be defined.',
+				118,
+			],
+		]);
+	}
+
 	#[RequiresPhp('>= 8.0')]
 	public function testBug14274(): void
 	{

tests/PHPStan/Rules/Variables/data/bug-14323.php

@@ -0,0 +1,130 @@
+<?php declare(strict_types = 1);
+
+namespace Bug14323;
+
+class ProcessFailedException extends \RuntimeException {}
+
+class Process {
+	/** @param array<string> $a */
+	public function __construct(array $a) {}
+}
+
+class Process2 {
+	/**
+	 * @param array<string> $a
+	 * @throws ProcessFailedException
+	 */
+	public function __construct(array $a) {}
+}
+
+class Process3 {
+	/**
+	 * @param array<string> $a
+	 * @throws void
+	 */
+	public function __construct(array $a) {}
+}
+
+abstract class DbCommand
+{
+    /**
+     * @return int
+     */
+    public function handle()
+    {
+        try {
+            new Process(
+                array_merge([$command = $this->getCommand()])
+            );
+        } catch (ProcessFailedException $e) {
+            echo ("{$command} not found in path.");
+
+            return 1;
+        }
+
+        return 0;
+    }
+
+    /**
+     * @return string
+     */
+    abstract public function getCommand();
+}
+
+abstract class DbCommand2
+{
+    /**
+     * @return int
+     */
+    public function handle()
+    {
+        try {
+            new Process(
+                [$command = $this->getCommand()]
+            );
+        } catch (ProcessFailedException $e) {
+            echo ("{$command} not found in path.");
+
+            return 1;
+        }
+
+        return 0;
+    }
+
+    /**
+     * @return string
+     */
+    abstract public function getCommand();
+}
+
+abstract class DbCommand3
+{
+    /**
+     * @return int
+     */
+    public function handle()
+    {
+        try {
+            new Process2(
+                array_merge([$command = $this->getCommand()])
+            );
+        } catch (ProcessFailedException $e) {
+            echo ("{$command} not found in path.");
+
+            return 1;
+        }
+
+        return 0;
+    }
+
+    /**
+     * @return string
+     */
+    abstract public function getCommand();
+}
+
+abstract class DbCommand4
+{
+    /**
+     * @return int
+     */
+    public function handle()
+    {
+        try {
+            new Process3(
+                array_merge([$command = $this->getCommand()])
+            );
+        } catch (ProcessFailedException $e) {
+            echo ("{$command} not found in path.");
+
+            return 1;
+        }
+
+        return 0;
+    }
+
+    /**
+     * @return string
+     */
+    abstract public function getCommand();
+}