diff --git a/src/Analyser/ExprHandler/NewHandler.php b/src/Analyser/ExprHandler/NewHandler.php index 0d48d1cfabc..67cc17797d5 100644 --- a/src/Analyser/ExprHandler/NewHandler.php +++ b/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 $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); } } diff --git a/src/Analyser/InternalScopeFactory.php b/src/Analyser/InternalScopeFactory.php index 1d95299a693..3f32562ca40 100644 --- a/src/Analyser/InternalScopeFactory.php +++ b/src/Analyser/InternalScopeFactory.php @@ -15,7 +15,7 @@ interface InternalScopeFactory * @param array $expressionTypes * @param array $nativeExpressionTypes * @param array $conditionalExpressions - * @param list $inClosureBindScopeClasses + * @param list $inClosureBindScopeClasses * @param array $currentlyAssignedExpressions * @param array $currentlyAllowedUndefinedExpressions * @param list $inFunctionCallsStack diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index e60242af43d..84875b15218 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -162,7 +162,7 @@ class MutatingScope implements Scope, NodeCallbackInvoker * @param callable(Node $node, Scope $scope): void|null $nodeCallback * @param array $expressionTypes * @param array $conditionalExpressions - * @param list $inClosureBindScopeClasses + * @param list $inClosureBindScopeClasses * @param array $currentlyAssignedExpressions * @param array $currentlyAllowedUndefinedExpressions * @param array $nativeExpressionTypes @@ -1820,7 +1820,7 @@ public function enterNamespace(string $namespaceName): self } /** - * @param list $scopeClasses + * @param list $scopeClasses */ public function enterClosureBind(?Type $thisType, ?Type $nativeThisType, array $scopeClasses): self { diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ce853bd980c..369f8e8eda2 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1878,7 +1878,6 @@ public function processStmtNode( // explicit only $onlyExplicitIsThrow = true; - $hasDirectExplicitNonThrowMatch = false; if (count($matchingThrowPoints) === 0) { foreach ($throwPoints as $throwPointIndex => $throwPoint) { foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) { @@ -1896,17 +1895,15 @@ public function processStmtNode( && !($throwNode instanceof Node\Stmt\Expression && $throwNode->expr instanceof Expr\Throw_) ) { $onlyExplicitIsThrow = false; - if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->yes()) { - $hasDirectExplicitNonThrowMatch = true; - } } + $matchingThrowPoints[$throwPointIndex] = $throwPoint; } } } // implicit only - if (count($matchingThrowPoints) === 0 || $onlyExplicitIsThrow || !$hasDirectExplicitNonThrowMatch) { + if (count($matchingThrowPoints) === 0 || $onlyExplicitIsThrow) { foreach ($throwPoints as $throwPointIndex => $throwPoint) { if ($throwPoint->isExplicit()) { continue; diff --git a/src/Analyser/Scope.php b/src/Analyser/Scope.php index 626e9db7ffa..6f616acd802 100644 --- a/src/Analyser/Scope.php +++ b/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; diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index a5db7514764..a05ca24efd7 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/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.', + 24, + ], + [ + 'Variable $command might not be defined.', + 50, + ], + [ + 'Variable $command might not be defined.', + 119, + ], + ]); + } + #[RequiresPhp('>= 8.0')] public function testBug14274(): void { diff --git a/tests/PHPStan/Rules/Variables/data/bug-14323.php b/tests/PHPStan/Rules/Variables/data/bug-14323.php new file mode 100644 index 00000000000..18427b8f030 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-14323.php @@ -0,0 +1,212 @@ + $a */ + 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(); +} + + +class Process2 { + /** + * @param array $a + * @throws ProcessFailedException + */ + public function __construct(array $a) {} +} + +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(); +} + +class Process3 { + /** + * @param array $a + * @throws void + */ + public function __construct(array $a) {} +} + +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(); +} + +class Process4 { + /** + * @param array $a + * + * @throws \LogicException + */ + public function __construct(array $a) {} + + /** + * @throws ProcessFailedException + * @throws \LogicException + */ + public function mustRun(): int {} +} + +abstract class DbCommand5 +{ + /** + * @return int + */ + public function handle() + { + try { + (new Process4( + array_merge([$command = $this->getCommand()]), + ))->mustRun(); + } catch (ProcessFailedException $e) { + echo ("{$command} not found in path."); + + return 1; + } + + return 0; + } + + /** + * @return string + */ + abstract public function getCommand(); +} + +class Process5 { + /** + * @param array $a + * + * @throws \LogicException + */ + public function __construct(array $a) {} + + /** + * @throws ProcessFailedException + */ + public function mustRun(): int {} +} + +abstract class DbCommand6 +{ + /** + * @return int + */ + public function handle() + { + try { + (new Process5( + array_merge([$command = $this->getCommand()]), + ))->mustRun(); + } catch (ProcessFailedException $e) { + echo ("{$command} not found in path."); + + return 1; + } + + return 0; + } + + /** + * @return string + */ + abstract public function getCommand(); +} diff --git a/tests/PHPStan/Rules/Variables/data/bug-9349.php b/tests/PHPStan/Rules/Variables/data/bug-9349.php index 44ed0202118..31dcfc1e2b0 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-9349.php +++ b/tests/PHPStan/Rules/Variables/data/bug-9349.php @@ -6,7 +6,7 @@ class HelloWorld { public function test(): void { - global $pdo; + $pdo = new \PDO('123'); try { $this->maybeThrows(); @@ -36,7 +36,7 @@ class HelloWorld2 { public function test2(): void { - global $pdo; + $pdo = new \PDO('123'); try { $this->maybeThrows2(); @@ -65,7 +65,7 @@ class HelloWorld3 { public function test3(): void { - global $pdo; + $pdo = new \PDO('123'); try { $this->maybeThrows3();