diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 27fb82d0267..10e58d49762 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -55,6 +55,7 @@ use function is_int; use function is_string; use function sprintf; +use function str_contains; use const ARRAY_FILTER_USE_BOTH; use const ARRAY_FILTER_USE_KEY; use const CURLOPT_SHARE; @@ -461,7 +462,18 @@ public static function selectFromArgs( if (count($parametersAcceptors) === 1) { $acceptor = $parametersAcceptors[0]; if (!self::hasAcceptorTemplateOrLateResolvableType($acceptor)) { - return $acceptor; + $skipEarlyReturn = false; + if ($namedArgumentsVariants !== null && self::acceptorHasCompoundParameterNames($acceptor)) { + foreach ($args as $arg) { + if ($arg->name !== null) { + $skipEarlyReturn = true; + break; + } + } + } + if (!$skipEarlyReturn) { + return $acceptor; + } } } @@ -868,6 +880,142 @@ public static function combineAcceptors(array $acceptors): ExtendedParametersAcc ); } + /** + * @param ParametersAcceptor[] $acceptors + */ + public static function combineAcceptorsByParameterName(array $acceptors): ExtendedParametersAcceptor + { + if (count($acceptors) === 0) { + throw new ShouldNotHappenException( + 'getVariants() must return at least one variant.', + ); + } + if (count($acceptors) === 1) { + return self::wrapAcceptor($acceptors[0]); + } + + $parametersByName = []; + $parameterNames = []; + foreach ($acceptors as $acceptor) { + foreach ($acceptor->getParameters() as $parameter) { + $name = $parameter->getName(); + if (!isset($parametersByName[$name])) { + $parameterNames[] = $name; + $parametersByName[$name] = []; + } + $parametersByName[$name][] = $parameter; + } + } + + $acceptorCount = count($acceptors); + $parameters = []; + $isVariadic = false; + $returnTypes = []; + $phpDocReturnTypes = []; + $nativeReturnTypes = []; + + foreach ($acceptors as $acceptor) { + $returnTypes[] = $acceptor->getReturnType(); + if ($acceptor instanceof ExtendedParametersAcceptor) { + $phpDocReturnTypes[] = $acceptor->getPhpDocReturnType(); + $nativeReturnTypes[] = $acceptor->getNativeReturnType(); + } + $isVariadic = $isVariadic || $acceptor->isVariadic(); + } + + foreach ($parameterNames as $name) { + $params = $parametersByName[$name]; + $existsInAll = count($params) === $acceptorCount; + + $types = []; + $nativeTypes = []; + $phpDocTypes = []; + $defaultValues = []; + $paramIsVariadic = false; + $outType = null; + $closureThisType = null; + $immediatelyInvokedCallable = TrinaryLogic::createMaybe(); + $attributes = []; + $isOptional = !$existsInAll; + $passedByRef = $params[0]->passedByReference(); + + foreach ($params as $j => $param) { + $types[] = $param->getType(); + $paramIsVariadic = $paramIsVariadic || $param->isVariadic(); + + if (!$isOptional && $param->isOptional()) { + $isOptional = true; + } + + $defaultValue = $param->getDefaultValue(); + if ($defaultValue !== null) { + $defaultValues[] = $defaultValue; + } + + if ($j > 0) { + $passedByRef = $passedByRef->combine($param->passedByReference()); + } + + if ($param instanceof ExtendedParameterReflection) { + $nativeTypes[] = $param->getNativeType(); + $phpDocTypes[] = $param->getPhpDocType(); + $immediatelyInvokedCallable = $param->isImmediatelyInvokedCallable()->or($immediatelyInvokedCallable); + $attributes = array_merge($attributes, $param->getAttributes()); + + if ($param->getOutType() !== null) { + $outType = $outType === null ? $param->getOutType() : TypeCombinator::union($outType, $param->getOutType()); + } else { + $outType = null; + } + + if ($param->getClosureThisType() !== null && $closureThisType !== null) { + $closureThisType = TypeCombinator::union($closureThisType, $param->getClosureThisType()); + } elseif ($closureThisType === null && $param === $params[0] && $param->getClosureThisType() !== null) { + $closureThisType = $param->getClosureThisType(); + } else { + $closureThisType = null; + } + } else { + $nativeTypes[] = new MixedType(); + $phpDocTypes[] = $param->getType(); + } + } + + $combinedDefaultValue = count($defaultValues) === count($params) + ? TypeCombinator::union(...$defaultValues) + : null; + + $parameters[] = new ExtendedDummyParameter( + $name, + TypeCombinator::union(...$types), + $isOptional, + $passedByRef, + $paramIsVariadic, + $combinedDefaultValue, + TypeCombinator::union(...$nativeTypes), + TypeCombinator::union(...$phpDocTypes), + $outType, + $immediatelyInvokedCallable, + $closureThisType, + $attributes, + ); + } + + $returnType = TypeCombinator::union(...$returnTypes); + $phpDocReturnType = $phpDocReturnTypes === [] ? null : TypeCombinator::union(...$phpDocReturnTypes); + $nativeReturnType = $nativeReturnTypes === [] ? null : TypeCombinator::union(...$nativeReturnTypes); + + return new ExtendedFunctionVariant( + TemplateTypeMap::createEmpty(), + null, + $parameters, + $isVariadic, + $returnType, + $phpDocReturnType ?? $returnType, + $nativeReturnType ?? new MixedType(), + ); + } + private static function wrapAcceptor(ParametersAcceptor $acceptor): ExtendedParametersAcceptor { if ($acceptor instanceof ExtendedParametersAcceptor) { @@ -907,6 +1055,16 @@ private static function wrapAcceptor(ParametersAcceptor $acceptor): ExtendedPara ); } + private static function acceptorHasCompoundParameterNames(ParametersAcceptor $acceptor): bool + { + foreach ($acceptor->getParameters() as $parameter) { + if (str_contains($parameter->getName(), '|')) { + return true; + } + } + return false; + } + private static function wrapParameter(ParameterReflection $parameter): ExtendedParameterReflection { return $parameter instanceof ExtendedParameterReflection ? $parameter : new ExtendedDummyParameter( diff --git a/src/Reflection/Type/IntersectionTypeMethodReflection.php b/src/Reflection/Type/IntersectionTypeMethodReflection.php index 7aab93fa82d..b73b53cb960 100644 --- a/src/Reflection/Type/IntersectionTypeMethodReflection.php +++ b/src/Reflection/Type/IntersectionTypeMethodReflection.php @@ -10,6 +10,7 @@ use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Reflection\ExtendedParametersAcceptor; use PHPStan\Reflection\MethodReflection; +use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\Type; @@ -128,9 +129,17 @@ public function getOnlyVariant(): ExtendedParametersAcceptor return $variants[0]; } - public function getNamedArgumentsVariants(): ?array + public function getNamedArgumentsVariants(): array { - return null; + $allVariants = []; + foreach ($this->methods as $method) { + $namedVariants = $method->getNamedArgumentsVariants(); + foreach ($namedVariants ?? $method->getVariants() as $variant) { + $allVariants[] = $variant; + } + } + + return [ParametersAcceptorSelector::combineAcceptorsByParameterName($allVariants)]; } public function isDeprecated(): TrinaryLogic diff --git a/src/Reflection/Type/UnionTypeMethodReflection.php b/src/Reflection/Type/UnionTypeMethodReflection.php index 89557b4e2c0..b4655e6f042 100644 --- a/src/Reflection/Type/UnionTypeMethodReflection.php +++ b/src/Reflection/Type/UnionTypeMethodReflection.php @@ -89,9 +89,17 @@ public function getOnlyVariant(): ExtendedParametersAcceptor return $this->getVariants()[0]; } - public function getNamedArgumentsVariants(): ?array + public function getNamedArgumentsVariants(): array { - return null; + $allVariants = []; + foreach ($this->methods as $method) { + $namedVariants = $method->getNamedArgumentsVariants(); + foreach ($namedVariants ?? $method->getVariants() as $variant) { + $allVariants[] = $variant; + } + } + + return [ParametersAcceptorSelector::combineAcceptorsByParameterName($allVariants)]; } public function isDeprecated(): TrinaryLogic diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index 1f042288f0e..37c8735c2b7 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -9,11 +9,13 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use function array_merge; +use function str_contains; /** * @implements Rule @@ -69,16 +71,51 @@ private function processSingleMethodCall(Scope $scope, MethodCall $node, string return $errors; } + $args = $node->getArgs(); + $selectedAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $args, + $methodReflection->getVariants(), + $methodReflection->getNamedArgumentsVariants(), + ); + + if ($this->shouldCheckPerUnionMember($selectedAcceptor, $args)) { + $callerType = $scope->getType($node->var); + foreach ($callerType->getObjectClassReflections() as $classReflection) { + if (!$classReflection->hasMethod($methodName)) { + continue; + } + $memberMethod = $classReflection->getMethod($methodName, $scope); + $memberAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $args, + $memberMethod->getVariants(), + $memberMethod->getNamedArgumentsVariants(), + ); + $errors = array_merge($errors, $this->checkMethodParameters($memberAcceptor, $scope, $node, $memberMethod)); + } + } else { + $errors = array_merge($errors, $this->checkMethodParameters($selectedAcceptor, $scope, $node, $methodReflection)); + } + + return $errors; + } + + /** + * @return list + */ + private function checkMethodParameters( + \PHPStan\Reflection\ParametersAcceptor $acceptor, + Scope $scope, + MethodCall $node, + ExtendedMethodReflection $methodReflection, + ): array + { $declaringClass = $methodReflection->getDeclaringClass(); $messagesMethodName = SprintfHelper::escapeFormatString($declaringClass->getDisplayName() . '::' . $methodReflection->getName() . '()'); - return array_merge($errors, $this->parametersCheck->check( - ParametersAcceptorSelector::selectFromArgs( - $scope, - $node->getArgs(), - $methodReflection->getVariants(), - $methodReflection->getNamedArgumentsVariants(), - ), + return $this->parametersCheck->check( + $acceptor, $scope, $declaringClass->isBuiltin(), $node, @@ -99,7 +136,33 @@ private function processSingleMethodCall(Scope $scope, MethodCall $node, string 'Return type of call to method ' . $messagesMethodName . ' contains unresolvable type.', '%s of method ' . $messagesMethodName . ' contains unresolvable type.', 'Method ' . $messagesMethodName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', - )); + ); + } + + /** + * @param Node\Arg[] $args + */ + private function shouldCheckPerUnionMember(\PHPStan\Reflection\ParametersAcceptor $acceptor, array $args): bool + { + $hasCompoundName = false; + foreach ($acceptor->getParameters() as $parameter) { + if (str_contains($parameter->getName(), '|')) { + $hasCompoundName = true; + break; + } + } + + if (!$hasCompoundName) { + return false; + } + + foreach ($args as $arg) { + if ($arg->name !== null) { + return false; + } + } + + return true; } } diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index d7a4d6766d8..9e3b3ef6db8 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -4106,4 +4106,46 @@ public function testBug14596(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug14661(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/bug-14661.php'], [ + [ + 'Parameter $a of method Bug14661\A::differentTypes() expects int, string given.', + 76, + ], + [ + 'Parameter $b of method Bug14661\A::differentTypes() expects string, int given.', + 76, + ], + [ + 'Parameter $a of method Bug14661\A::differentTypes() expects int, string given.', + 77, + ], + [ + 'Parameter $b of method Bug14661\A::differentTypes() expects string, int given.', + 77, + ], + [ + 'Parameter #1 $a of method Bug14661\A::differentTypes() expects int, string given.', + 82, + ], + [ + 'Parameter #2 $b of method Bug14661\A::differentTypes() expects string, int given.', + 82, + ], + [ + 'Parameter #1 $b of method Bug14661\B::differentTypes() expects string, int given.', + 83, + ], + [ + 'Parameter #2 $a of method Bug14661\B::differentTypes() expects int, string given.', + 83, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index 47a70c9d02a..54e03490124 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -1033,4 +1033,11 @@ public function testBug14596(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug14661(): void + { + $this->checkThisOnly = false; + $this->analyse([__DIR__ . '/data/bug-14661-static.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-14661-static.php b/tests/PHPStan/Rules/Methods/data/bug-14661-static.php new file mode 100644 index 00000000000..6d1970c6023 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-14661-static.php @@ -0,0 +1,31 @@ += 8.0 + +declare(strict_types=1); + +namespace Bug14661Static; + +class A +{ + public static function mixedOrder( + ?string $other = null, + ?string $target = null, + ): void {} +} + +class B +{ + public static function mixedOrder( + ?string $target = null, + ?string $other = null, + ): void {} +} + +/** + * @param class-string|class-string $class + */ +function staticMixedOrder(string $class): void +{ + $class::mixedOrder(target: 'value'); + $class::mixedOrder(other: 'a', target: 'b'); + $class::mixedOrder(target: 'b', other: 'a'); +} diff --git a/tests/PHPStan/Rules/Methods/data/bug-14661.php b/tests/PHPStan/Rules/Methods/data/bug-14661.php new file mode 100644 index 00000000000..75e1a4d2205 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-14661.php @@ -0,0 +1,90 @@ += 8.0 + +declare(strict_types=1); + +namespace Bug14661; + +class A +{ + public function mixedOrder( + ?string $other = null, + ?string $target = null, + ): void {} + + public function sameOrder( + ?string $other = null, + ?string $target = null, + ): void {} + + public function differentTypes( + int $a, + string $b, + ): void {} +} + +class B +{ + public function mixedOrder( + ?string $target = null, + ?string $other = null, + ): void {} + + public function sameOrder( + ?string $other = null, + ?string $target = null, + ): void {} + + public function differentTypes( + string $b, + int $a, + ): void {} +} + +class C +{ + public function mixedOrder( + ?string $target = null, + ?string $extra = null, + ?string $other = null, + ): void {} +} + +function mixedOrder(A|B $obj): void +{ + $obj->mixedOrder(target: 'value'); +} + +function sameOrder(A|B $obj): void +{ + $obj->sameOrder(target: 'value'); +} + +function mixedOrderBothArgs(A|B $obj): void +{ + $obj->mixedOrder(other: 'a', target: 'b'); + $obj->mixedOrder(target: 'b', other: 'a'); +} + +function differentTypes(A|B $obj): void +{ + $obj->differentTypes(a: 1, b: 'hello'); + $obj->differentTypes(b: 'hello', a: 1); +} + +function differentTypesErrors(A|B $obj): void +{ + $obj->differentTypes(a: 'hello', b: 1); + $obj->differentTypes(b: 1, a: 'hello'); +} + +function differentTypesPositional(A|B $obj): void +{ + $obj->differentTypes('hello', 1); + $obj->differentTypes(1, 'hello'); +} + +function threeWayUnion(A|B|C $obj): void +{ + $obj->mixedOrder(target: 'value'); + $obj->mixedOrder(other: 'a', target: 'b'); +}