From 5ca44ece1cf192b0935195d37b18a84d6d6aeaef Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 14 Oct 2026 14:55:11 +0200 Subject: [PATCH 1/4] Add `@arguments-by-name`, `@optional-arguments-by-name` and `@stable-parameter-names` support PHPStan now reports a positional argument when the PHPDoc of the called function, method or class requires it to be passed by name. `@stable-parameter-names` is parsed but reports nothing at call sites. --- src/PhpDoc/PhpDocNodeResolver.php | 15 +++ src/PhpDoc/ResolvedPhpDocBlock.php | 33 ++++++ .../BetterReflectionProvider.php | 1 + src/Reflection/FunctionReflectionFactory.php | 2 + src/Reflection/Php/PhpFunctionReflection.php | 7 ++ src/Rules/ArgumentsByNameHelper.php | 65 +++++++++++ src/Rules/AttributesCheck.php | 5 + src/Rules/Classes/InstantiationRule.php | 6 + src/Rules/FunctionCallParametersCheck.php | 27 +++++ src/Rules/Functions/CallCallablesRule.php | 3 + .../CallToFunctionParametersRule.php | 5 + src/Rules/Functions/CallUserFuncRule.php | 3 + src/Rules/Methods/CallMethodsRule.php | 5 + src/Rules/Methods/CallStaticMethodsRule.php | 6 + .../Rules/Classes/ClassAttributesRuleTest.php | 11 ++ .../Rules/Classes/InstantiationRuleTest.php | 19 +++ .../class-attributes-arguments-by-name.php | 28 +++++ .../data/instantiation-arguments-by-name.php | 45 ++++++++ .../CallToFunctionParametersRuleTest.php | 51 +++++++++ .../Functions/data/arguments-by-name.php | 78 +++++++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 44 +++++++ .../Methods/CallStaticMethodsRuleTest.php | 25 ++++ .../Rules/Methods/data/arguments-by-name.php | 108 ++++++++++++++++++ .../data/static-method-arguments-by-name.php | 46 ++++++++ tests/PHPStan/Type/FileTypeMapperTest.php | 26 +++++ 25 files changed, 664 insertions(+) create mode 100644 src/Rules/ArgumentsByNameHelper.php create mode 100644 tests/PHPStan/Rules/Classes/data/class-attributes-arguments-by-name.php create mode 100644 tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php create mode 100644 tests/PHPStan/Rules/Functions/data/arguments-by-name.php create mode 100644 tests/PHPStan/Rules/Methods/data/arguments-by-name.php create mode 100644 tests/PHPStan/Rules/Methods/data/static-method-arguments-by-name.php diff --git a/src/PhpDoc/PhpDocNodeResolver.php b/src/PhpDoc/PhpDocNodeResolver.php index 048031ae7fb..71788f3ead4 100644 --- a/src/PhpDoc/PhpDocNodeResolver.php +++ b/src/PhpDoc/PhpDocNodeResolver.php @@ -759,6 +759,21 @@ public function resolveAcceptsNamedArguments(PhpDocNode $phpDocNode): bool return count($phpDocNode->getTagsByName('@no-named-arguments')) === 0; } + public function resolveRequiresArgumentsByName(PhpDocNode $phpDocNode): bool + { + return count($phpDocNode->getTagsByName('@arguments-by-name')) > 0; + } + + public function resolveRequiresOptionalArgumentsByName(PhpDocNode $phpDocNode): bool + { + return count($phpDocNode->getTagsByName('@optional-arguments-by-name')) > 0; + } + + public function resolveHasStableParameterNames(PhpDocNode $phpDocNode): bool + { + return count($phpDocNode->getTagsByName('@stable-parameter-names')) > 0; + } + private function shouldSkipType(string $tagName, Type $type): bool { if (!str_starts_with($tagName, '@psalm-')) { diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index e1026770cef..ecb4dc85344 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -162,6 +162,12 @@ final class ResolvedPhpDocBlock private ?bool $acceptsNamedArguments = null; + private ?bool $requiresArgumentsByName = null; + + private ?bool $requiresOptionalArgumentsByName = null; + + private ?bool $hasStableParameterNames = null; + private function __construct() { } @@ -255,6 +261,9 @@ public static function createEmpty(): self $self->isAllowedPrivateMutation = false; $self->hasConsistentConstructor = false; $self->acceptsNamedArguments = true; + $self->requiresArgumentsByName = false; + $self->requiresOptionalArgumentsByName = false; + $self->hasStableParameterNames = false; return $self; } @@ -313,6 +322,9 @@ public function merge(ResolvedPhpDocBlock $parent, InheritedPhpDocParameterMappi $result->isAllowedPrivateMutation = $this->isAllowedPrivateMutation(); $result->hasConsistentConstructor = $this->hasConsistentConstructor(); $result->acceptsNamedArguments = $acceptsNamedArguments; + $result->requiresArgumentsByName = $this->requiresArgumentsByName() || $parent->requiresArgumentsByName(); + $result->requiresOptionalArgumentsByName = $this->requiresOptionalArgumentsByName() || $parent->requiresOptionalArgumentsByName(); + $result->hasStableParameterNames = $this->hasStableParameterNames() || $parent->hasStableParameterNames(); return $result; } @@ -835,6 +847,27 @@ public function acceptsNamedArguments(): bool ); } + public function requiresArgumentsByName(): bool + { + return $this->requiresArgumentsByName ??= $this->phpDocNodeResolver->resolveRequiresArgumentsByName( + $this->phpDocNode, + ); + } + + public function requiresOptionalArgumentsByName(): bool + { + return $this->requiresOptionalArgumentsByName ??= $this->phpDocNodeResolver->resolveRequiresOptionalArgumentsByName( + $this->phpDocNode, + ); + } + + public function hasStableParameterNames(): bool + { + return $this->hasStableParameterNames ??= $this->phpDocNodeResolver->resolveHasStableParameterNames( + $this->phpDocNode, + ); + } + public function getTemplateTypeMap(): TemplateTypeMap { return $this->templateTypeMap; diff --git a/src/Reflection/BetterReflection/BetterReflectionProvider.php b/src/Reflection/BetterReflection/BetterReflectionProvider.php index 391e293a2ec..e7536c17d4b 100644 --- a/src/Reflection/BetterReflection/BetterReflectionProvider.php +++ b/src/Reflection/BetterReflection/BetterReflectionProvider.php @@ -375,6 +375,7 @@ private function getCustomFunction(string $functionName): PhpFunctionReflection $this->attributeReflectionFactory->fromNativeReflection($reflectionFunction->getAttributes(), InitializerExprContext::fromFunction($reflectionFunction->getName(), $reflectionFunction->getFileName() !== false ? $reflectionFunction->getFileName() : null)), $phpDocParameterPureUnlessCallableIsImpure, $phpDocParameterPureUnlessParameterPassed, + $resolvedPhpDoc, ); } diff --git a/src/Reflection/FunctionReflectionFactory.php b/src/Reflection/FunctionReflectionFactory.php index 3ff2adcbbbe..fc8e2cdaf7d 100644 --- a/src/Reflection/FunctionReflectionFactory.php +++ b/src/Reflection/FunctionReflectionFactory.php @@ -3,6 +3,7 @@ namespace PHPStan\Reflection; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionFunction; +use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\Reflection\Php\PhpFunctionReflection; use PHPStan\Type\Generic\TemplateTypeMap; use PHPStan\Type\Type; @@ -39,6 +40,7 @@ public function create( array $attributes, array $phpDocParameterPureUnlessCallableIsImpure, array $phpDocParameterPureUnlessParameterPassed, + ?ResolvedPhpDocBlock $resolvedPhpDocBlock, ): PhpFunctionReflection; } diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 1dbee429546..8bea19c1c07 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -6,6 +6,7 @@ use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; use PHPStan\DependencyInjection\GenerateFactory; use PHPStan\Internal\DeprecatedAttributeHelper; +use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\Reflection\Assertions; use PHPStan\Reflection\AttributeReflection; use PHPStan\Reflection\AttributeReflectionFactory; @@ -66,6 +67,7 @@ public function __construct( private array $attributes, private array $phpDocParameterPureUnlessCallableIsImpure, private array $phpDocParameterPureUnlessParameterPassed, + private ?ResolvedPhpDocBlock $resolvedPhpDocBlock, ) { } @@ -275,4 +277,9 @@ public function mustUseReturnValue(): TrinaryLogic return TrinaryLogic::createNo(); } + public function getResolvedPhpDoc(): ?ResolvedPhpDocBlock + { + return $this->resolvedPhpDocBlock; + } + } diff --git a/src/Rules/ArgumentsByNameHelper.php b/src/Rules/ArgumentsByNameHelper.php new file mode 100644 index 00000000000..5b7f52f63d2 --- /dev/null +++ b/src/Rules/ArgumentsByNameHelper.php @@ -0,0 +1,65 @@ +getResolvedPhpDoc(), + $method->getDeclaringClass()->getResolvedPhpDoc(), + ]); + } + + /** + * @return array{bool, bool} + */ + public static function getForFunction(FunctionReflection $function): array + { + if (!$function instanceof PhpFunctionReflection) { + return [false, false]; + } + + return self::getFromPhpDocs([$function->getResolvedPhpDoc()]); + } + + /** + * @param list $phpDocs + * @return array{bool, bool} + */ + private static function getFromPhpDocs(array $phpDocs): array + { + $requiresArgumentsByName = false; + $requiresOptionalArgumentsByName = false; + foreach ($phpDocs as $phpDoc) { + if ($phpDoc === null) { + continue; + } + + $requiresArgumentsByName = $requiresArgumentsByName || $phpDoc->requiresArgumentsByName(); + $requiresOptionalArgumentsByName = $requiresOptionalArgumentsByName || $phpDoc->requiresOptionalArgumentsByName(); + } + + return [$requiresArgumentsByName, $requiresOptionalArgumentsByName]; + } + +} diff --git a/src/Rules/AttributesCheck.php b/src/Rules/AttributesCheck.php index be32556cde5..df9f72ff64d 100644 --- a/src/Rules/AttributesCheck.php +++ b/src/Rules/AttributesCheck.php @@ -132,6 +132,8 @@ public function check( $nodeAttributes = $attribute->getAttributes(); $nodeAttributes['isAttribute'] = true; + [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($attributeConstructor); + $parameterErrors = $this->functionCallParametersCheck->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -144,6 +146,8 @@ public function check( new New_($attribute->name, $attribute->args, $nodeAttributes), 'attribute', $attributeConstructor->acceptsNamedArguments(), + $requiresArgumentsByName, + $requiresOptionalArgumentsByName, 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameter, %d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameters, %d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameter, at least %d required.', @@ -159,6 +163,7 @@ public function check( 'Return type of call to ' . $attributeClassName . ' constructor contains unresolvable type.', '%s of attribute class ' . $attributeClassName . ' constructor contains unresolvable type.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of attribute class ' . $attributeClassName . ' constructor must be passed by name.', 'Constant %s is not allowed for %s of attribute class ' . $attributeClassName . ' constructor.', 'Constants %s cannot be combined for %s of attribute class ' . $attributeClassName . ' constructor.', 'Combining constants with | is not allowed for %s of attribute class ' . $attributeClassName . ' constructor.', diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index f9c19257b04..6c7fa83b138 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -17,6 +17,7 @@ use PHPStan\Reflection\Php\PhpMethodReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Reflection\Type\CalledOnTypeUnresolvedMethodPrototypeReflection; +use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\ClassNameCheck; use PHPStan\Rules\ClassNameNodePair; use PHPStan\Rules\ClassNameUsageLocation; @@ -307,6 +308,8 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N ))->getTransformedMethod(); } + [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($constructorReflection); + return array_merge($messages, $this->check->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -319,6 +322,8 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N $node, 'new', $constructorReflection->acceptsNamedArguments(), + $requiresArgumentsByName, + $requiresOptionalArgumentsByName, 'Class ' . $classDisplayName . ' constructor invoked with %d parameter, %d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameters, %d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameter, at least %d required.', @@ -334,6 +339,7 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N 'Return type of call to ' . $classDisplayName . ' constructor contains unresolvable type.', '%s of class ' . $classDisplayName . ' constructor contains unresolvable type.', 'Class ' . $classDisplayName . ' constructor invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of class ' . $classDisplayName . ' constructor must be passed by name.', 'Constant %s is not allowed for %s of class ' . $classDisplayName . ' constructor.', 'Constants %s cannot be combined for %s of class ' . $classDisplayName . ' constructor.', 'Combining constants with | is not allowed for %s of class ' . $classDisplayName . ' constructor.', diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index aa14e2a9f16..3f606a57e95 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -80,6 +80,8 @@ public function check( Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall, string $nodeType, TrinaryLogic $acceptsNamedArguments, + bool $requiresArgumentsByName, + bool $requiresOptionalArgumentsByName, string $singleInsufficientParameterMessage, string $pluralInsufficientParametersMessage, string $singleInsufficientParameterInVariadicFunctionMessage, @@ -95,6 +97,7 @@ public function check( string $unresolvableReturnTypeMessage, string $unresolvableParameterTypeMessage, string $namedArgumentMessage, + string $positionalArgumentMessage, string $invalidConstantMessage, string $exclusiveConstantsMessage, string $bitmaskNotAllowedMessage, @@ -329,7 +332,11 @@ public function check( return $errors; } + $isAfterUnpackedArgument = false; foreach ($argumentsWithParameters as $i => [$argumentValue, $argumentValueType, $unpack, $argumentName, $argumentLine, $parameter, $originalParameter]) { + $isPositionKnown = !$isAfterUnpackedArgument; + $isAfterUnpackedArgument = $isAfterUnpackedArgument || $unpack; + if ($this->checkArgumentTypes && $unpack) { $iterableTypeResult = $this->ruleLevelHelper->findTypeToCheck( $scope, @@ -387,6 +394,20 @@ public function check( $renamedNamedArgumentParameterData, [$parameter->getName(), $argumentLine], )); + } elseif ( + $argumentName === null + && $isPositionKnown + && !$parameter->isVariadic() + && ($requiresArgumentsByName || ($requiresOptionalArgumentsByName && $parameter->isOptional())) + && (!$unpack || $this->isUnpackedAsPositionalArguments($scope->getType($argumentValue))) + ) { + $errors[] = RuleErrorBuilder::message(sprintf( + $positionalArgumentMessage, + sprintf($requiresArgumentsByName ? 'parameter $%s' : 'optional parameter $%s', $parameter->getName()), + )) + ->identifier('argument.positional') + ->line($argumentLine) + ->build(); } if ($this->checkArgumentTypes) { @@ -789,6 +810,12 @@ private function processArguments( return [$errors, $newArguments]; } + private function isUnpackedAsPositionalArguments(Type $unpackedType): bool + { + return $unpackedType->getIterableKeyType()->isInteger()->yes() + && $unpackedType->isIterableAtLeastOnce()->yes(); + } + private function describeParameter(ParameterReflection $parameter, int|string|null $positionOrNamed): string { $parts = []; diff --git a/src/Rules/Functions/CallCallablesRule.php b/src/Rules/Functions/CallCallablesRule.php index 85c33490b1b..622eceb4e56 100644 --- a/src/Rules/Functions/CallCallablesRule.php +++ b/src/Rules/Functions/CallCallablesRule.php @@ -126,6 +126,8 @@ public function processNode( $node, 'callable', $acceptsNamedArguments, + false, + false, ucfirst($callableDescription) . ' invoked with %d parameter, %d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, at least %d required.', @@ -141,6 +143,7 @@ public function processNode( 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of ' . $callableDescription . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $callableDescription . '.', 'Constants %s cannot be combined for %s of ' . $callableDescription . '.', 'Combining constants with | is not allowed for %s of ' . $callableDescription . '.', diff --git a/src/Rules/Functions/CallToFunctionParametersRule.php b/src/Rules/Functions/CallToFunctionParametersRule.php index 06fe1ce0459..cc277f73c28 100644 --- a/src/Rules/Functions/CallToFunctionParametersRule.php +++ b/src/Rules/Functions/CallToFunctionParametersRule.php @@ -12,6 +12,7 @@ use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; +use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\Rule; use function count; @@ -51,6 +52,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $function->getVariants(), $function->getNamedArgumentsVariants(), ); + [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForFunction($function); return $this->check->check( self::getVariantForArgumentsCount($node->getArgs(), $function->getVariants(), $parametersAcceptor) ?? $parametersAcceptor, @@ -59,6 +61,8 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $node, 'function', $function->acceptsNamedArguments(), + $requiresArgumentsByName, + $requiresOptionalArgumentsByName, 'Function ' . $functionName . ' invoked with %d parameter, %d required.', 'Function ' . $functionName . ' invoked with %d parameters, %d required.', 'Function ' . $functionName . ' invoked with %d parameter, at least %d required.', @@ -74,6 +78,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE 'Return type of call to function ' . $functionName . ' contains unresolvable type.', '%s of function ' . $functionName . ' contains unresolvable type.', 'Function ' . $functionName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of function ' . $functionName . ' must be passed by name.', 'Constant %s is not allowed for %s of function ' . $functionName . '.', 'Constants %s cannot be combined for %s of function ' . $functionName . '.', 'Combining constants with | is not allowed for %s of function ' . $functionName . '.', diff --git a/src/Rules/Functions/CallUserFuncRule.php b/src/Rules/Functions/CallUserFuncRule.php index cfc8f1955be..a0611c74f06 100644 --- a/src/Rules/Functions/CallUserFuncRule.php +++ b/src/Rules/Functions/CallUserFuncRule.php @@ -79,6 +79,8 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $funcCall, 'function', $acceptsNamedArguments, + false, + false, ucfirst($callableDescription) . ' invoked with %d parameter, %d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, at least %d required.', @@ -94,6 +96,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of ' . $callableDescription . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $callableDescription . '.', 'Constants %s cannot be combined for %s of ' . $callableDescription . '.', 'Combining constants with | is not allowed for %s of ' . $callableDescription . '.', diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index 61c7cd620c8..e52c25c704a 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -12,6 +12,7 @@ use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; +use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\NonStringableDynamicAccessCheck; @@ -83,6 +84,7 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $declaringClass = $methodReflection->getDeclaringClass(); $messagesMethodName = SprintfHelper::escapeFormatString($declaringClass->getDisplayName() . '::' . $methodReflection->getName() . '()'); + [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($methodReflection); return array_merge($errors, $this->parametersCheck->check( ParametersAcceptorSelector::selectFromArgs( @@ -96,6 +98,8 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $node, 'method', $methodReflection->acceptsNamedArguments(), + $requiresArgumentsByName, + $requiresOptionalArgumentsByName, 'Method ' . $messagesMethodName . ' invoked with %d parameter, %d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameters, %d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameter, at least %d required.', @@ -111,6 +115,7 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData '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.', + 'Argument for %s of method ' . $messagesMethodName . ' must be passed by name.', 'Constant %s is not allowed for %s of method ' . $messagesMethodName . '.', 'Constants %s cannot be combined for %s of method ' . $messagesMethodName . '.', 'Combining constants with | is not allowed for %s of method ' . $messagesMethodName . '.', diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index 53d797361e5..2f07f633068 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -12,6 +12,7 @@ use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; use PHPStan\Reflection\ParametersAcceptorSelector; +use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\NonStringableDynamicAccessCheck; @@ -93,6 +94,8 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $method->getDeclaringClass()->getDisplayName() . '::' . $method->getName() . '()', )); + [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($method); + $errors = array_merge($errors, $this->parametersCheck->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -105,6 +108,8 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $node, 'staticMethod', $method->acceptsNamedArguments(), + $requiresArgumentsByName, + $requiresOptionalArgumentsByName, $displayMethodName . ' invoked with %d parameter, %d required.', $displayMethodName . ' invoked with %d parameters, %d required.', $displayMethodName . ' invoked with %d parameter, at least %d required.', @@ -120,6 +125,7 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData 'Return type of call to ' . $lowercasedMethodName . ' contains unresolvable type.', '%s of ' . $lowercasedMethodName . ' contains unresolvable type.', $displayMethodName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', + 'Argument for %s of ' . $lowercasedMethodName . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $lowercasedMethodName . '.', 'Constants %s cannot be combined for %s of ' . $lowercasedMethodName . '.', 'Combining constants with | is not allowed for %s of ' . $lowercasedMethodName . '.', diff --git a/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php b/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php index 026661a35e9..eccfcb5c99d 100644 --- a/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php @@ -217,4 +217,15 @@ public function testDeprecatedAttribute(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->analyse([__DIR__ . '/data/class-attributes-arguments-by-name.php'], [ + [ + 'Argument for optional parameter $name of attribute class ClassAttributesArgumentsByName\Route constructor must be passed by name.', + 24, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index e5be65d06a4..4564b48bc3a 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -368,6 +368,25 @@ public function testNamedArguments(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->analyse([__DIR__ . '/data/instantiation-arguments-by-name.php'], [ + [ + 'Argument for optional parameter $groups of class InstantiationArgumentsByName\Configuration constructor must be passed by name.', + 40, + ], + [ + 'Argument for optional parameter $groups of class InstantiationArgumentsByName\ChildConfiguration constructor must be passed by name.', + 42, + ], + [ + 'Argument for parameter $dialect of class InstantiationArgumentsByName\ClassLevel constructor must be passed by name.', + 44, + ], + ]); + } + public function testBug4471(): void { $this->analyse([__DIR__ . '/data/bug-4471.php'], [ diff --git a/tests/PHPStan/Rules/Classes/data/class-attributes-arguments-by-name.php b/tests/PHPStan/Rules/Classes/data/class-attributes-arguments-by-name.php new file mode 100644 index 00000000000..f3138fbc101 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/class-attributes-arguments-by-name.php @@ -0,0 +1,28 @@ += 8.0 + +namespace ClassAttributesArgumentsByName; + +#[\Attribute(\Attribute::TARGET_CLASS)] +class Route +{ + + /** + * @optional-arguments-by-name + */ + public function __construct(string $path, string $name = '', array $methods = []) + { + } + +} + +#[Route('/foo', name: 'foo')] +class Foo +{ + +} + +#[Route('/bar', 'bar')] +class Bar +{ + +} diff --git a/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php b/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php new file mode 100644 index 00000000000..f93b0c05963 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php @@ -0,0 +1,45 @@ += 8.0 + +namespace InstantiationArgumentsByName; + +class Configuration +{ + + /** + * @optional-arguments-by-name + */ + public function __construct( + string $dialect, + array $groups = [], + bool $allowExtraAttributes = true, + ) + { + } + +} + +class ChildConfiguration extends Configuration +{ + +} + +/** + * @arguments-by-name + */ +class ClassLevel +{ + + public function __construct(string $dialect, array $groups = []) + { + } + +} + +function (): void { + new Configuration('openapi-3.1', groups: ['read']); + new Configuration('openapi-3.1', ['read']); + new Configuration(dialect: 'openapi-3.1'); + new ChildConfiguration('openapi-3.1', ['read']); + new ClassLevel(dialect: 'openapi-3.1', groups: ['read']); + new ClassLevel('openapi-3.1'); +}; diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 3ac6c660706..83304da18b2 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2242,6 +2242,57 @@ public function testNoNamedArguments(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->analyse([__DIR__ . '/data/arguments-by-name.php'], [ + [ + 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', + 42, + ], + [ + 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', + 43, + ], + [ + 'Argument for optional parameter $allowExtraAttributes of function ArgumentsByNameFunction\optional must be passed by name.', + 43, + ], + [ + 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', + 46, + ], + [ + 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', + 47, + ], + [ + 'Argument for parameter $group of function ArgumentsByNameFunction\all must be passed by name.', + 47, + ], + [ + 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', + 63, + ], + [ + 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', + 67, + ], + [ + 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', + 71, + ], + [ + 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', + 73, + ], + [ + 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', + 77, + ], + ]); + } + public function testBug11056(): void { $this->checkExplicitMixed = true; diff --git a/tests/PHPStan/Rules/Functions/data/arguments-by-name.php b/tests/PHPStan/Rules/Functions/data/arguments-by-name.php new file mode 100644 index 00000000000..c5589b84909 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/arguments-by-name.php @@ -0,0 +1,78 @@ += 8.0 + +namespace ArgumentsByNameFunction; + +/** + * @optional-arguments-by-name + */ +function optional(string $dialect, string $group = 'default', bool $allowExtraAttributes = true, string ...$rest): void +{ + +} + +/** + * @arguments-by-name Parameter names are stable, their order is not. + */ +function all(string $dialect, string $group = 'default', string ...$rest): void +{ + +} + +/** + * @stable-parameter-names + */ +function stable(string $dialect, string $group = 'default'): void +{ + +} + +/** + * @no-named-arguments + * @arguments-by-name + */ +function conflicting(string $dialect, string $group = 'default'): void +{ + +} + +function (): void { + optional('openapi-3.1'); + optional('openapi-3.1', group: 'read'); + optional(dialect: 'openapi-3.1', allowExtraAttributes: false); + optional('openapi-3.1', 'read'); + optional('openapi-3.1', 'read', false, 'a', 'b'); + + all(dialect: 'openapi-3.1', group: 'read'); + all('openapi-3.1', group: 'read'); + all('openapi-3.1', 'read', 'a'); + + stable('openapi-3.1', 'read'); + stable(dialect: 'openapi-3.1', group: 'read'); + + conflicting('openapi-3.1', 'read'); +}; + +/** + * @param list $list + * @param non-empty-list $nonEmptyList + * @param array $named + * @param array $unknown + */ +function unpacking(array $list, array $nonEmptyList, array $named, array $unknown): void +{ + optional(...['openapi-3.1', 'read']); + optional(...['openapi-3.1', 'group' => 'read']); + optional('openapi-3.1', ...['group' => 'read']); + optional(...$list); + optional('openapi-3.1', ...$nonEmptyList); + optional(...$named); + optional(...$unknown); + + all(...['openapi-3.1']); + all(...['dialect' => 'openapi-3.1']); + all(...$nonEmptyList); + all(...$list); + all(...$named); + all(...$unknown); + all(...$nonEmptyList, ...$nonEmptyList); +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index e03a97fc739..e3713e95c11 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3543,6 +3543,50 @@ public function testNoNamedArguments(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->checkExplicitMixed = true; + + $this->analyse([__DIR__ . '/data/arguments-by-name.php'], [ + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\Foo::optional() must be passed by name.', + 93, + ], + [ + 'Argument for optional parameter $allowExtraAttributes of method ArgumentsByNameMethod\Foo::optional() must be passed by name.', + 93, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\Foo::all() must be passed by name.', + 95, + ], + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\ClassLevel::doFoo() must be passed by name.', + 100, + ], + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\Child::optional() must be passed by name.', + 102, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\Child::all() must be passed by name.', + 103, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\FooInterface::doFoo() must be passed by name.', + 105, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\FooImplementation::doFoo() must be passed by name.', + 106, + ], + ]); + } + public function testTraitMixin(): void { $this->checkThisOnly = false; diff --git a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php index fbed93ba6c9..00f5e5dd533 100644 --- a/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php @@ -450,6 +450,31 @@ public function testNamedArguments(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->checkThisOnly = false; + + $this->analyse([__DIR__ . '/data/static-method-arguments-by-name.php'], [ + [ + 'Argument for optional parameter $group of static method StaticMethodArgumentsByName\Foo::optional() must be passed by name.', + 27, + ], + [ + 'Argument for parameter $dialect of static method StaticMethodArgumentsByName\Foo::all() must be passed by name.', + 29, + ], + [ + 'Argument for parameter $dialect of static method StaticMethodArgumentsByName\ClassLevel::doFoo() must be passed by name.', + 30, + ], + [ + 'Argument for parameter $group of static method StaticMethodArgumentsByName\ClassLevel::doFoo() must be passed by name.', + 30, + ], + ]); + } + public function testBug577(): void { $this->checkThisOnly = false; diff --git a/tests/PHPStan/Rules/Methods/data/arguments-by-name.php b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php new file mode 100644 index 00000000000..b1c4173cd13 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php @@ -0,0 +1,108 @@ += 8.0 + +namespace ArgumentsByNameMethod; + +class Foo +{ + + /** + * @optional-arguments-by-name + */ + public function optional(string $dialect, string $group = 'default', bool $allowExtraAttributes = true): void + { + + } + + /** + * @arguments-by-name + */ + public function all(string $dialect, string $group = 'default'): void + { + + } + + /** + * @stable-parameter-names + */ + public function stable(string $dialect, string $group = 'default'): void + { + + } + + /** + * @no-named-arguments + * @optional-arguments-by-name + */ + public function conflicting(string $dialect, string $group = 'default'): void + { + + } + +} + +/** + * @optional-arguments-by-name + */ +class ClassLevel +{ + + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +class Child extends Foo +{ + + public function optional(string $dialect, string $group = 'default', bool $allowExtraAttributes = true): void + { + + } + + public function all(string $dialect, string $group = 'default'): void + { + + } + +} + +interface FooInterface +{ + + /** + * @arguments-by-name + */ + public function doFoo(string $dialect, string $group = 'default'): void; + +} + +class FooImplementation implements FooInterface +{ + + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +function (Foo $foo, ClassLevel $classLevel, Child $child, FooInterface $interface, FooImplementation $implementation): void { + $foo->optional('openapi-3.1', group: 'read'); + $foo->optional('openapi-3.1', 'read', false); + $foo->all(dialect: 'openapi-3.1', group: 'read'); + $foo->all('openapi-3.1', group: 'read'); + $foo->stable('openapi-3.1', 'read'); + $foo->conflicting('openapi-3.1', 'read'); + + $classLevel->doFoo('openapi-3.1', group: 'read'); + $classLevel->doFoo('openapi-3.1', 'read'); + + $child->optional('openapi-3.1', 'read'); + $child->all('openapi-3.1'); + + $interface->doFoo('openapi-3.1'); + $implementation->doFoo('openapi-3.1'); + $implementation->doFoo(dialect: 'openapi-3.1'); +}; diff --git a/tests/PHPStan/Rules/Methods/data/static-method-arguments-by-name.php b/tests/PHPStan/Rules/Methods/data/static-method-arguments-by-name.php new file mode 100644 index 00000000000..52b46f275aa --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/static-method-arguments-by-name.php @@ -0,0 +1,46 @@ += 8.0 + +namespace StaticMethodArgumentsByName; + +class Foo +{ + + /** + * @optional-arguments-by-name + */ + public static function optional(string $dialect, string $group = 'default'): void + { + + } + + /** + * @arguments-by-name + */ + public static function all(string $dialect, string $group = 'default'): void + { + + } + + public function doBar(): void + { + self::optional('openapi-3.1', group: 'read'); + self::optional('openapi-3.1', 'read'); + self::all(dialect: 'openapi-3.1'); + self::all('openapi-3.1'); + ClassLevel::doFoo('openapi-3.1', 'read'); + } + +} + +/** + * @arguments-by-name + */ +class ClassLevel +{ + + public static function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} diff --git a/tests/PHPStan/Type/FileTypeMapperTest.php b/tests/PHPStan/Type/FileTypeMapperTest.php index 49658c33707..8f0c273dc5f 100644 --- a/tests/PHPStan/Type/FileTypeMapperTest.php +++ b/tests/PHPStan/Type/FileTypeMapperTest.php @@ -15,6 +15,7 @@ use PHPStan\Reflection\ReflectionProvider\ReflectionProviderProvider; use PHPStan\ShouldNotHappenException; use PHPStan\Testing\PHPStanTestCase; +use PHPUnit\Framework\Attributes\DataProvider; use RuntimeException; use function array_values; use function realpath; @@ -116,6 +117,31 @@ public function testGetResolvedPhpDoc(): void $this->assertFalse($paramMultipleTypesWithExtraSpaces->getParameters()['object']->isVariadic()); } + public static function dataArgumentsByNameTags(): iterable + { + yield ['/** @param int $i */', false, false, false]; + yield ['/** @arguments-by-name */', true, false, false]; + yield ['/** @optional-arguments-by-name Optional parameters can be reordered. */', false, true, false]; + yield ['/** @stable-parameter-names */', false, false, true]; + } + + #[DataProvider('dataArgumentsByNameTags')] + public function testArgumentsByNameTags( + string $phpDoc, + bool $requiresArgumentsByName, + bool $requiresOptionalArgumentsByName, + bool $hasStableParameterNames, + ): void + { + /** @var FileTypeMapper $fileTypeMapper */ + $fileTypeMapper = self::getContainer()->getByType(FileTypeMapper::class); + + $resolved = $fileTypeMapper->getResolvedPhpDoc(null, null, null, null, $phpDoc); + $this->assertSame($requiresArgumentsByName, $resolved->requiresArgumentsByName()); + $this->assertSame($requiresOptionalArgumentsByName, $resolved->requiresOptionalArgumentsByName()); + $this->assertSame($hasStableParameterNames, $resolved->hasStableParameterNames()); + } + public function testFileWithDependentPhpDocs(): void { /** @var FileTypeMapper $fileTypeMapper */ From 207b87f70477ed4adfcddbefc4a5824bb4cd367b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 14 Oct 2026 17:21:18 +0200 Subject: [PATCH 2/4] Apply class-level tags to overriding methods, and let method-level tags win A tag on a method now wins over a class-level tag, including a class-level `@no-named-arguments`. Constructors no longer inherit the tags of the parent constructor. The new arguments of `FunctionCallParametersCheck::check()` are now optional and last, so extensions that call it keep working. --- src/PhpDoc/ResolvedPhpDocBlock.php | 9 +- src/Reflection/ArgumentsByNameTagResolver.php | 117 ++++++++++ src/Reflection/Php/PhpMethodReflection.php | 9 +- src/Rules/ArgumentsByNameHelper.php | 65 ------ src/Rules/AttributesCheck.php | 8 +- src/Rules/Classes/InstantiationRule.php | 9 +- src/Rules/FunctionCallParametersCheck.php | 10 +- src/Rules/Functions/CallCallablesRule.php | 3 - .../CallToFunctionParametersRule.php | 8 +- src/Rules/Functions/CallUserFuncRule.php | 3 - src/Rules/Methods/CallMethodsRule.php | 8 +- src/Rules/Methods/CallStaticMethodsRule.php | 9 +- .../Rules/Classes/InstantiationRuleTest.php | 8 + .../data/instantiation-arguments-by-name.php | 59 +++++ .../CallToFunctionParametersRuleTest.php | 10 +- .../Functions/data/arguments-by-name.php | 4 +- .../Rules/Methods/CallMethodsRuleTest.php | 36 +++ .../Rules/Methods/data/arguments-by-name.php | 217 ++++++++++++++++++ 18 files changed, 480 insertions(+), 112 deletions(-) create mode 100644 src/Reflection/ArgumentsByNameTagResolver.php delete mode 100644 src/Rules/ArgumentsByNameHelper.php diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index ecb4dc85344..c220f5dc291 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -276,9 +276,10 @@ public function merge(ResolvedPhpDocBlock $parent, InheritedPhpDocParameterMappi // skip $result->phpDocNode $phpDocNodes = $this->phpDocNodes; $acceptsNamedArguments = $this->acceptsNamedArguments(); + $hasArgumentsByNameTag = $this->requiresArgumentsByName() || $this->requiresOptionalArgumentsByName() || $this->hasStableParameterNames(); foreach ($parent->phpDocNodes as $phpDocNode) { $phpDocNodes[] = $phpDocNode; - $acceptsNamedArguments = $acceptsNamedArguments && $parent->acceptsNamedArguments(); + $acceptsNamedArguments = $acceptsNamedArguments && ($hasArgumentsByNameTag || $parent->acceptsNamedArguments()); } $result->phpDocNodes = $phpDocNodes; $result->phpDocString = $this->phpDocString; @@ -322,9 +323,9 @@ public function merge(ResolvedPhpDocBlock $parent, InheritedPhpDocParameterMappi $result->isAllowedPrivateMutation = $this->isAllowedPrivateMutation(); $result->hasConsistentConstructor = $this->hasConsistentConstructor(); $result->acceptsNamedArguments = $acceptsNamedArguments; - $result->requiresArgumentsByName = $this->requiresArgumentsByName() || $parent->requiresArgumentsByName(); - $result->requiresOptionalArgumentsByName = $this->requiresOptionalArgumentsByName() || $parent->requiresOptionalArgumentsByName(); - $result->hasStableParameterNames = $this->hasStableParameterNames() || $parent->hasStableParameterNames(); + $result->requiresArgumentsByName = $this->requiresArgumentsByName(); + $result->requiresOptionalArgumentsByName = $this->requiresOptionalArgumentsByName(); + $result->hasStableParameterNames = $this->hasStableParameterNames(); return $result; } diff --git a/src/Reflection/ArgumentsByNameTagResolver.php b/src/Reflection/ArgumentsByNameTagResolver.php new file mode 100644 index 00000000000..e621c3ab70e --- /dev/null +++ b/src/Reflection/ArgumentsByNameTagResolver.php @@ -0,0 +1,117 @@ +getResolvedPhpDoc()); + } + + /** + * @return self::*|null + */ + public static function resolveForMethod(ExtendedMethodReflection $method): ?string + { + $phpDoc = $method->getResolvedPhpDoc(); + if ($phpDoc !== null && !$phpDoc->acceptsNamedArguments()) { + return null; + } + + $tag = self::resolveFromPhpDoc($phpDoc); + if ($tag !== null) { + return $tag; + } + + $declaringClass = $method->getDeclaringClass(); + if (strtolower($method->getName()) !== '__construct') { + $tag = self::resolveFromOverriddenMethods($declaringClass, $method->getName()); + if ($tag !== null) { + return $tag; + } + } + + return self::resolveFromPhpDoc($declaringClass->getResolvedPhpDoc()); + } + + /** + * @return self::*|null + */ + private static function resolveFromOverriddenMethods(ClassReflection $class, string $methodName): ?string + { + $parentClass = $class->getParentClass(); + $ancestors = $parentClass !== null ? [$parentClass] : []; + foreach ($class->getImmediateInterfaces() as $interface) { + $ancestors[] = $interface; + } + + foreach ($ancestors as $ancestor) { + if (!$ancestor->hasNativeMethod($methodName)) { + continue; + } + + $overriddenMethod = $ancestor->getNativeMethod($methodName); + if ($overriddenMethod->isPrivate()) { + continue; + } + + $tag = self::resolveForMethod($overriddenMethod); + if ($tag !== null) { + return $tag; + } + } + + return null; + } + + /** + * @return self::*|null + */ + private static function resolveFromPhpDoc(?ResolvedPhpDocBlock $phpDoc): ?string + { + if ($phpDoc === null || !$phpDoc->acceptsNamedArguments()) { + return null; + } + + if ($phpDoc->requiresArgumentsByName()) { + return self::ARGUMENTS_BY_NAME; + } + + if ($phpDoc->requiresOptionalArgumentsByName()) { + return self::OPTIONAL_ARGUMENTS_BY_NAME; + } + + if ($phpDoc->hasStableParameterNames()) { + return self::STABLE_PARAMETER_NAMES; + } + + return null; + } + +} diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index db2f8a8d69c..a053b77183c 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -7,6 +7,7 @@ use PHPStan\DependencyInjection\GenerateFactory; use PHPStan\Internal\DeprecatedAttributeHelper; use PHPStan\PhpDoc\ResolvedPhpDocBlock; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\Assertions; use PHPStan\Reflection\AttributeReflection; use PHPStan\Reflection\AttributeReflectionFactory; @@ -390,8 +391,14 @@ public function getAsserts(): Assertions public function acceptsNamedArguments(): TrinaryLogic { + if (!$this->acceptsNamedArguments) { + return TrinaryLogic::createNo(); + } + + // a tag of the method overrides @no-named-arguments on the class return TrinaryLogic::createFromBoolean( - $this->declaringClass->acceptsNamedArguments() && $this->acceptsNamedArguments, + $this->declaringClass->acceptsNamedArguments() + || ArgumentsByNameTagResolver::resolveForMethod($this) !== null, ); } diff --git a/src/Rules/ArgumentsByNameHelper.php b/src/Rules/ArgumentsByNameHelper.php deleted file mode 100644 index 5b7f52f63d2..00000000000 --- a/src/Rules/ArgumentsByNameHelper.php +++ /dev/null @@ -1,65 +0,0 @@ -getResolvedPhpDoc(), - $method->getDeclaringClass()->getResolvedPhpDoc(), - ]); - } - - /** - * @return array{bool, bool} - */ - public static function getForFunction(FunctionReflection $function): array - { - if (!$function instanceof PhpFunctionReflection) { - return [false, false]; - } - - return self::getFromPhpDocs([$function->getResolvedPhpDoc()]); - } - - /** - * @param list $phpDocs - * @return array{bool, bool} - */ - private static function getFromPhpDocs(array $phpDocs): array - { - $requiresArgumentsByName = false; - $requiresOptionalArgumentsByName = false; - foreach ($phpDocs as $phpDoc) { - if ($phpDoc === null) { - continue; - } - - $requiresArgumentsByName = $requiresArgumentsByName || $phpDoc->requiresArgumentsByName(); - $requiresOptionalArgumentsByName = $requiresOptionalArgumentsByName || $phpDoc->requiresOptionalArgumentsByName(); - } - - return [$requiresArgumentsByName, $requiresOptionalArgumentsByName]; - } - -} diff --git a/src/Rules/AttributesCheck.php b/src/Rules/AttributesCheck.php index df9f72ff64d..e7d5921467e 100644 --- a/src/Rules/AttributesCheck.php +++ b/src/Rules/AttributesCheck.php @@ -11,6 +11,7 @@ use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; use function array_key_exists; @@ -132,8 +133,6 @@ public function check( $nodeAttributes = $attribute->getAttributes(); $nodeAttributes['isAttribute'] = true; - [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($attributeConstructor); - $parameterErrors = $this->functionCallParametersCheck->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -146,8 +145,6 @@ public function check( new New_($attribute->name, $attribute->args, $nodeAttributes), 'attribute', $attributeConstructor->acceptsNamedArguments(), - $requiresArgumentsByName, - $requiresOptionalArgumentsByName, 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameter, %d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameters, %d required.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %d parameter, at least %d required.', @@ -163,11 +160,12 @@ public function check( 'Return type of call to ' . $attributeClassName . ' constructor contains unresolvable type.', '%s of attribute class ' . $attributeClassName . ' constructor contains unresolvable type.', 'Attribute class ' . $attributeClassName . ' constructor invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of attribute class ' . $attributeClassName . ' constructor must be passed by name.', 'Constant %s is not allowed for %s of attribute class ' . $attributeClassName . ' constructor.', 'Constants %s cannot be combined for %s of attribute class ' . $attributeClassName . ' constructor.', 'Combining constants with | is not allowed for %s of attribute class ' . $attributeClassName . ' constructor.', null, + ArgumentsByNameTagResolver::resolveForMethod($attributeConstructor), + 'Argument for %s of attribute class ' . $attributeClassName . ' constructor must be passed by name.', ); foreach ($parameterErrors as $error) { diff --git a/src/Rules/Classes/InstantiationRule.php b/src/Rules/Classes/InstantiationRule.php index 6c7fa83b138..f359cb82458 100644 --- a/src/Rules/Classes/InstantiationRule.php +++ b/src/Rules/Classes/InstantiationRule.php @@ -12,12 +12,12 @@ use PHPStan\DependencyInjection\ExtensionsCollection; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\Php\PhpMethodReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Reflection\Type\CalledOnTypeUnresolvedMethodPrototypeReflection; -use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\ClassNameCheck; use PHPStan\Rules\ClassNameNodePair; use PHPStan\Rules\ClassNameUsageLocation; @@ -308,8 +308,6 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N ))->getTransformedMethod(); } - [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($constructorReflection); - return array_merge($messages, $this->check->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -322,8 +320,6 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N $node, 'new', $constructorReflection->acceptsNamedArguments(), - $requiresArgumentsByName, - $requiresOptionalArgumentsByName, 'Class ' . $classDisplayName . ' constructor invoked with %d parameter, %d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameters, %d required.', 'Class ' . $classDisplayName . ' constructor invoked with %d parameter, at least %d required.', @@ -339,11 +335,12 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope&N 'Return type of call to ' . $classDisplayName . ' constructor contains unresolvable type.', '%s of class ' . $classDisplayName . ' constructor contains unresolvable type.', 'Class ' . $classDisplayName . ' constructor invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of class ' . $classDisplayName . ' constructor must be passed by name.', 'Constant %s is not allowed for %s of class ' . $classDisplayName . ' constructor.', 'Constants %s cannot be combined for %s of class ' . $classDisplayName . ' constructor.', 'Combining constants with | is not allowed for %s of class ' . $classDisplayName . ' constructor.', null, + ArgumentsByNameTagResolver::resolveForMethod($constructorReflection), + 'Argument for %s of class ' . $classDisplayName . ' constructor must be passed by name.', )); } diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 3f606a57e95..c0bb00b7540 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -10,6 +10,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ConstantReflection; use PHPStan\Reflection\ExtendedParameterReflection; use PHPStan\Reflection\ParameterReflection; @@ -71,6 +72,7 @@ public function __construct( /** * @param 'attribute'|'callable'|'method'|'staticMethod'|'function'|'new' $nodeType * @param array{class-string, string}|null $renamedNamedArgumentParameterData + * @param ArgumentsByNameTagResolver::*|null $argumentsByNameTag * @return list */ public function check( @@ -80,8 +82,6 @@ public function check( Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall, string $nodeType, TrinaryLogic $acceptsNamedArguments, - bool $requiresArgumentsByName, - bool $requiresOptionalArgumentsByName, string $singleInsufficientParameterMessage, string $pluralInsufficientParametersMessage, string $singleInsufficientParameterInVariadicFunctionMessage, @@ -97,13 +97,17 @@ public function check( string $unresolvableReturnTypeMessage, string $unresolvableParameterTypeMessage, string $namedArgumentMessage, - string $positionalArgumentMessage, string $invalidConstantMessage, string $exclusiveConstantsMessage, string $bitmaskNotAllowedMessage, ?array $renamedNamedArgumentParameterData, + ?string $argumentsByNameTag = null, + string $positionalArgumentMessage = '', ): array { + $requiresArgumentsByName = $argumentsByNameTag === ArgumentsByNameTagResolver::ARGUMENTS_BY_NAME; + $requiresOptionalArgumentsByName = $argumentsByNameTag === ArgumentsByNameTagResolver::OPTIONAL_ARGUMENTS_BY_NAME; + if ($funcCall instanceof Node\Expr\MethodCall || $funcCall instanceof Node\Expr\StaticCall || $funcCall instanceof Node\Expr\FuncCall) { $funcCallLine = $funcCall->name->getStartLine(); } else { diff --git a/src/Rules/Functions/CallCallablesRule.php b/src/Rules/Functions/CallCallablesRule.php index 622eceb4e56..85c33490b1b 100644 --- a/src/Rules/Functions/CallCallablesRule.php +++ b/src/Rules/Functions/CallCallablesRule.php @@ -126,8 +126,6 @@ public function processNode( $node, 'callable', $acceptsNamedArguments, - false, - false, ucfirst($callableDescription) . ' invoked with %d parameter, %d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, at least %d required.', @@ -143,7 +141,6 @@ public function processNode( 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of ' . $callableDescription . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $callableDescription . '.', 'Constants %s cannot be combined for %s of ' . $callableDescription . '.', 'Combining constants with | is not allowed for %s of ' . $callableDescription . '.', diff --git a/src/Rules/Functions/CallToFunctionParametersRule.php b/src/Rules/Functions/CallToFunctionParametersRule.php index cc277f73c28..23fc70237be 100644 --- a/src/Rules/Functions/CallToFunctionParametersRule.php +++ b/src/Rules/Functions/CallToFunctionParametersRule.php @@ -9,10 +9,10 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\Rule; use function count; @@ -52,7 +52,6 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $function->getVariants(), $function->getNamedArgumentsVariants(), ); - [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForFunction($function); return $this->check->check( self::getVariantForArgumentsCount($node->getArgs(), $function->getVariants(), $parametersAcceptor) ?? $parametersAcceptor, @@ -61,8 +60,6 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $node, 'function', $function->acceptsNamedArguments(), - $requiresArgumentsByName, - $requiresOptionalArgumentsByName, 'Function ' . $functionName . ' invoked with %d parameter, %d required.', 'Function ' . $functionName . ' invoked with %d parameters, %d required.', 'Function ' . $functionName . ' invoked with %d parameter, at least %d required.', @@ -78,11 +75,12 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE 'Return type of call to function ' . $functionName . ' contains unresolvable type.', '%s of function ' . $functionName . ' contains unresolvable type.', 'Function ' . $functionName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of function ' . $functionName . ' must be passed by name.', 'Constant %s is not allowed for %s of function ' . $functionName . '.', 'Constants %s cannot be combined for %s of function ' . $functionName . '.', 'Combining constants with | is not allowed for %s of function ' . $functionName . '.', null, + ArgumentsByNameTagResolver::resolveForFunction($function), + 'Argument for %s of function ' . $functionName . ' must be passed by name.', ); } diff --git a/src/Rules/Functions/CallUserFuncRule.php b/src/Rules/Functions/CallUserFuncRule.php index a0611c74f06..cfc8f1955be 100644 --- a/src/Rules/Functions/CallUserFuncRule.php +++ b/src/Rules/Functions/CallUserFuncRule.php @@ -79,8 +79,6 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $funcCall, 'function', $acceptsNamedArguments, - false, - false, ucfirst($callableDescription) . ' invoked with %d parameter, %d required.', ucfirst($callableDescription) . ' invoked with %d parameters, %d required.', ucfirst($callableDescription) . ' invoked with %d parameter, at least %d required.', @@ -96,7 +94,6 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE 'Return type of call to ' . $callableDescription . ' contains unresolvable type.', '%s of ' . $callableDescription . ' contains unresolvable type.', ucfirst($callableDescription) . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of ' . $callableDescription . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $callableDescription . '.', 'Constants %s cannot be combined for %s of ' . $callableDescription . '.', 'Combining constants with | is not allowed for %s of ' . $callableDescription . '.', diff --git a/src/Rules/Methods/CallMethodsRule.php b/src/Rules/Methods/CallMethodsRule.php index e52c25c704a..f33857de425 100644 --- a/src/Rules/Methods/CallMethodsRule.php +++ b/src/Rules/Methods/CallMethodsRule.php @@ -11,8 +11,8 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ParametersAcceptorSelector; -use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\NonStringableDynamicAccessCheck; @@ -84,7 +84,6 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $declaringClass = $methodReflection->getDeclaringClass(); $messagesMethodName = SprintfHelper::escapeFormatString($declaringClass->getDisplayName() . '::' . $methodReflection->getName() . '()'); - [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($methodReflection); return array_merge($errors, $this->parametersCheck->check( ParametersAcceptorSelector::selectFromArgs( @@ -98,8 +97,6 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $node, 'method', $methodReflection->acceptsNamedArguments(), - $requiresArgumentsByName, - $requiresOptionalArgumentsByName, 'Method ' . $messagesMethodName . ' invoked with %d parameter, %d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameters, %d required.', 'Method ' . $messagesMethodName . ' invoked with %d parameter, at least %d required.', @@ -115,7 +112,6 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData '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.', - 'Argument for %s of method ' . $messagesMethodName . ' must be passed by name.', 'Constant %s is not allowed for %s of method ' . $messagesMethodName . '.', 'Constants %s cannot be combined for %s of method ' . $messagesMethodName . '.', 'Combining constants with | is not allowed for %s of method ' . $messagesMethodName . '.', @@ -123,6 +119,8 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $declaringClass->getName(), $methodReflection->getName(), ] : null, + ArgumentsByNameTagResolver::resolveForMethod($methodReflection), + 'Argument for %s of method ' . $messagesMethodName . ' must be passed by name.', )); } diff --git a/src/Rules/Methods/CallStaticMethodsRule.php b/src/Rules/Methods/CallStaticMethodsRule.php index 2f07f633068..32c2b2580d1 100644 --- a/src/Rules/Methods/CallStaticMethodsRule.php +++ b/src/Rules/Methods/CallStaticMethodsRule.php @@ -11,8 +11,8 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ParametersAcceptorSelector; -use PHPStan\Rules\ArgumentsByNameHelper; use PHPStan\Rules\FunctionCallParametersCheck; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\NonStringableDynamicAccessCheck; @@ -94,8 +94,6 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $method->getDeclaringClass()->getDisplayName() . '::' . $method->getName() . '()', )); - [$requiresArgumentsByName, $requiresOptionalArgumentsByName] = ArgumentsByNameHelper::getForMethod($method); - $errors = array_merge($errors, $this->parametersCheck->check( ParametersAcceptorSelector::selectFromArgs( $scope, @@ -108,8 +106,6 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData $node, 'staticMethod', $method->acceptsNamedArguments(), - $requiresArgumentsByName, - $requiresOptionalArgumentsByName, $displayMethodName . ' invoked with %d parameter, %d required.', $displayMethodName . ' invoked with %d parameters, %d required.', $displayMethodName . ' invoked with %d parameter, at least %d required.', @@ -125,11 +121,12 @@ private function processSingleMethodCall(Scope&NodeCallbackInvoker&CollectedData 'Return type of call to ' . $lowercasedMethodName . ' contains unresolvable type.', '%s of ' . $lowercasedMethodName . ' contains unresolvable type.', $displayMethodName . ' invoked with %s, but it\'s not allowed because of @no-named-arguments.', - 'Argument for %s of ' . $lowercasedMethodName . ' must be passed by name.', 'Constant %s is not allowed for %s of ' . $lowercasedMethodName . '.', 'Constants %s cannot be combined for %s of ' . $lowercasedMethodName . '.', 'Combining constants with | is not allowed for %s of ' . $lowercasedMethodName . '.', null, + ArgumentsByNameTagResolver::resolveForMethod($method), + 'Argument for %s of ' . $lowercasedMethodName . ' must be passed by name.', )); return $errors; diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index 4564b48bc3a..1b028caa0b8 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -384,6 +384,14 @@ public function testArgumentsByName(): void 'Argument for parameter $dialect of class InstantiationArgumentsByName\ClassLevel constructor must be passed by name.', 44, ], + [ + 'Argument for parameter $dialect of class InstantiationArgumentsByName\NoNamedArgumentsByDefault constructor must be passed by name.', + 101, + ], + [ + 'Argument for optional parameter $groups of class InstantiationArgumentsByName\NoNamedArgumentsByDefaultWithOptional constructor must be passed by name.', + 103, + ], ]); } diff --git a/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php b/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php index f93b0c05963..c0c7e611187 100644 --- a/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php +++ b/tests/PHPStan/Rules/Classes/data/instantiation-arguments-by-name.php @@ -43,3 +43,62 @@ function (): void { new ClassLevel(dialect: 'openapi-3.1', groups: ['read']); new ClassLevel('openapi-3.1'); }; + +class ChildWithOwnConstructor extends Configuration +{ + + public function __construct(string $dialect, array $groups = []) + { + parent::__construct($dialect, groups: $groups); + } + +} + +class ChildOfClassLevelWithOwnConstructor extends ClassLevel +{ + + public function __construct(string $dialect, array $groups = []) + { + parent::__construct(dialect: $dialect, groups: $groups); + } + +} + +/** + * @no-named-arguments + */ +class NoNamedArgumentsByDefault +{ + + /** + * @arguments-by-name + */ + public function __construct(string $dialect, array $groups = []) + { + } + +} + +/** + * @no-named-arguments + */ +class NoNamedArgumentsByDefaultWithOptional +{ + + /** + * @optional-arguments-by-name + */ + public function __construct(string $dialect, array $groups = []) + { + } + +} + +function (): void { + new ChildWithOwnConstructor('openapi-3.1', ['read']); + new ChildOfClassLevelWithOwnConstructor('openapi-3.1'); + new NoNamedArgumentsByDefault(dialect: 'openapi-3.1', groups: ['read']); + new NoNamedArgumentsByDefault('openapi-3.1'); + new NoNamedArgumentsByDefaultWithOptional('openapi-3.1', groups: ['read']); + new NoNamedArgumentsByDefaultWithOptional('openapi-3.1', ['read']); +}; diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 83304da18b2..8f1255a3f89 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2272,23 +2272,23 @@ public function testArgumentsByName(): void ], [ 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', - 63, + 64, ], [ 'Argument for optional parameter $group of function ArgumentsByNameFunction\optional must be passed by name.', - 67, + 68, ], [ 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', - 71, + 72, ], [ 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', - 73, + 74, ], [ 'Argument for parameter $dialect of function ArgumentsByNameFunction\all must be passed by name.', - 77, + 78, ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/arguments-by-name.php b/tests/PHPStan/Rules/Functions/data/arguments-by-name.php index c5589b84909..a972701e25c 100644 --- a/tests/PHPStan/Rules/Functions/data/arguments-by-name.php +++ b/tests/PHPStan/Rules/Functions/data/arguments-by-name.php @@ -57,8 +57,9 @@ function (): void { * @param non-empty-list $nonEmptyList * @param array $named * @param array $unknown + * @param non-empty-array $nonEmptyUnknown */ -function unpacking(array $list, array $nonEmptyList, array $named, array $unknown): void +function unpacking(array $list, array $nonEmptyList, array $named, array $unknown, array $nonEmptyUnknown): void { optional(...['openapi-3.1', 'read']); optional(...['openapi-3.1', 'group' => 'read']); @@ -75,4 +76,5 @@ function unpacking(array $list, array $nonEmptyList, array $named, array $unknow all(...$named); all(...$unknown); all(...$nonEmptyList, ...$nonEmptyList); + all(...$nonEmptyUnknown); } diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index e3713e95c11..5888b68c41f 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3584,6 +3584,42 @@ public function testArgumentsByName(): void 'Argument for parameter $dialect of method ArgumentsByNameMethod\FooImplementation::doFoo() must be passed by name.', 106, ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\NoNamedArgumentsByDefault::all() must be passed by name.', + 235, + ], + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\NoNamedArgumentsByDefault::optional() must be passed by name.', + 237, + ], + [ + 'Method ArgumentsByNameMethod\NoNamedArgumentsByDefault::doDefault() invoked with named argument $dialect, but it\'s not allowed because of @no-named-arguments.', + 240, + ], + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\ArgumentsByNameByDefault::optional() must be passed by name.', + 243, + ], + [ + 'Method ArgumentsByNameMethod\ArgumentsByNameByDefault::noNamed() invoked with named argument $dialect, but it\'s not allowed because of @no-named-arguments.', + 245, + ], + [ + 'Argument for optional parameter $group of method ArgumentsByNameMethod\ChildOfClassLevel::doFoo() must be passed by name.', + 247, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\ClassLevelInterfaceImplementation::doFoo() must be passed by name.', + 248, + ], + [ + 'Argument for parameter $dialect of method ArgumentsByNameMethod\ArgumentsByNameChild::doFoo() must be passed by name.', + 250, + ], + [ + 'Method ArgumentsByNameMethod\ConflictingClassLevel::doFoo() invoked with named argument $dialect, but it\'s not allowed because of @no-named-arguments.', + 322, + ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/arguments-by-name.php b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php index b1c4173cd13..7b4219af8d8 100644 --- a/tests/PHPStan/Rules/Methods/data/arguments-by-name.php +++ b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php @@ -106,3 +106,220 @@ function (Foo $foo, ClassLevel $classLevel, Child $child, FooInterface $interfac $implementation->doFoo('openapi-3.1'); $implementation->doFoo(dialect: 'openapi-3.1'); }; + +/** + * @no-named-arguments + */ +class NoNamedArgumentsByDefault +{ + + /** + * @arguments-by-name + */ + public function all(string $dialect, string $group = 'default'): void + { + + } + + /** + * @optional-arguments-by-name + */ + public function optional(string $dialect, string $group = 'default'): void + { + + } + + /** + * @stable-parameter-names + */ + public function stable(string $dialect, string $group = 'default'): void + { + + } + + public function doDefault(string $dialect): void + { + + } + +} + +/** + * @arguments-by-name + */ +class ArgumentsByNameByDefault +{ + + /** + * @optional-arguments-by-name + */ + public function optional(string $dialect, string $group = 'default'): void + { + + } + + /** + * @no-named-arguments + */ + public function noNamed(string $dialect): void + { + + } + +} + +class ChildOfClassLevel extends ClassLevel +{ + + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +/** + * @arguments-by-name + */ +interface ClassLevelInterface +{ + + public function doFoo(string $dialect, string $group = 'default'): void; + +} + +class ClassLevelInterfaceImplementation implements ClassLevelInterface +{ + + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +class NoNamedArgumentsParent +{ + + /** + * @no-named-arguments + */ + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +class ArgumentsByNameChild extends NoNamedArgumentsParent +{ + + /** + * @arguments-by-name + */ + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +function ( + NoNamedArgumentsByDefault $noNamedByDefault, + ArgumentsByNameByDefault $byNameByDefault, + ChildOfClassLevel $childOfClassLevel, + ClassLevelInterfaceImplementation $classLevelImplementation, + ArgumentsByNameChild $byNameChild, +): void { + $noNamedByDefault->all(dialect: 'openapi-3.1', group: 'read'); + $noNamedByDefault->all('openapi-3.1'); + $noNamedByDefault->optional('openapi-3.1', group: 'read'); + $noNamedByDefault->optional('openapi-3.1', 'read'); + $noNamedByDefault->stable(dialect: 'openapi-3.1', group: 'read'); + $noNamedByDefault->stable('openapi-3.1', 'read'); + $noNamedByDefault->doDefault(dialect: 'openapi-3.1'); + + $byNameByDefault->optional('openapi-3.1', group: 'read'); + $byNameByDefault->optional('openapi-3.1', 'read'); + $byNameByDefault->noNamed('openapi-3.1'); + $byNameByDefault->noNamed(dialect: 'openapi-3.1'); + + $childOfClassLevel->doFoo('openapi-3.1', 'read'); + $classLevelImplementation->doFoo('openapi-3.1'); + $byNameChild->doFoo(dialect: 'openapi-3.1'); + $byNameChild->doFoo('openapi-3.1'); +}; + +class ChildOfClassLevelWithNewMethod extends ClassLevel +{ + + public function doBar(string $dialect, string $group = 'default'): void + { + + } + +} + +class PrivateTagged +{ + + /** + * @arguments-by-name + */ + private function doFoo(string $dialect): void + { + + } + +} + +class ChildOfPrivateTagged extends PrivateTagged +{ + + public function doFoo(string $dialect): void + { + + } + +} + +/** + * @no-named-arguments + * @arguments-by-name + */ +class ConflictingClassLevel +{ + + public function doFoo(string $dialect): void + { + + } + +} + +class StableParameterNamesChild extends NoNamedArgumentsParent +{ + + /** + * @stable-parameter-names + */ + public function doFoo(string $dialect, string $group = 'default'): void + { + + } + +} + +function ( + ChildOfClassLevelWithNewMethod $childWithNewMethod, + ChildOfPrivateTagged $childOfPrivateTagged, + ConflictingClassLevel $conflictingClassLevel, + StableParameterNamesChild $stableChild, +): void { + $childWithNewMethod->doBar('openapi-3.1', 'read'); + $childOfPrivateTagged->doFoo('openapi-3.1'); + $conflictingClassLevel->doFoo('openapi-3.1'); + $conflictingClassLevel->doFoo(dialect: 'openapi-3.1'); + $stableChild->doFoo(dialect: 'openapi-3.1', group: 'read'); + $stableChild->doFoo('openapi-3.1', 'read'); +}; From 7d3fd0156e45e6cdfbb68e4d8cbf818d2ce73a26 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 14 Oct 2026 17:21:18 +0200 Subject: [PATCH 3/4] Report overriding methods that rename a parameter covered by a tag Constructors are not checked. --- .../MethodParameterComparisonHelper.php | 23 +++ .../Methods/OverridingMethodRuleTest.php | 28 ++++ .../overriding-method-arguments-by-name.php | 136 ++++++++++++++++++ 3 files changed, 187 insertions(+) create mode 100644 tests/PHPStan/Rules/Methods/data/overriding-method-arguments-by-name.php diff --git a/src/Rules/Methods/MethodParameterComparisonHelper.php b/src/Rules/Methods/MethodParameterComparisonHelper.php index c86a76cacfc..c040c9c89c0 100644 --- a/src/Rules/Methods/MethodParameterComparisonHelper.php +++ b/src/Rules/Methods/MethodParameterComparisonHelper.php @@ -7,6 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Php\PhpVersion; +use PHPStan\Reflection\ArgumentsByNameTagResolver; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection; @@ -24,6 +25,7 @@ use function array_slice; use function count; use function sprintf; +use function strtolower; #[AutowiredService] final class MethodParameterComparisonHelper @@ -49,6 +51,9 @@ public function compare( $prototypeVariant = $prototype->getVariants()[0]; $methodParameters = $method->getParameters(); + $argumentsByNameTag = strtolower($method->getName()) !== '__construct' + ? ArgumentsByNameTagResolver::resolveForMethod($prototype) + : null; $prototypeAfterVariadic = false; foreach ($prototypeVariant->getParameters() as $i => $prototypeParameter) { @@ -73,6 +78,24 @@ public function compare( } $methodParameter = $methodParameters[$i]; + if ( + $argumentsByNameTag !== null + && !$prototypeParameter->isVariadic() + && $prototypeParameter->getName() !== $methodParameter->getName() + && ($argumentsByNameTag !== ArgumentsByNameTagResolver::OPTIONAL_ARGUMENTS_BY_NAME || $prototypeParameter->isOptional()) + ) { + $messages[] = RuleErrorBuilder::message(sprintf( + 'Parameter #%d $%s of method %s::%s() should be named $%s like in method %s::%s() because of %s.', + $i + 1, + $methodParameter->getName(), + $method->getDeclaringClass()->getDisplayName(), + $method->getName(), + $prototypeParameter->getName(), + $prototypeDeclaringClass->getDisplayName(true), + $prototype->getName(), + $argumentsByNameTag, + ))->identifier('parameter.renamed')->build(); + } if ($prototype->acceptsNamedArguments()->yes()) { if ($prototypeParameter->getName() !== $methodParameter->getName()) { $scope->emitCollectedData(OverridingMethodRenamesParameterCollector::class, [ diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index 9ba61a4171f..717eabef5e8 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -881,6 +881,34 @@ public function testBug14457(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testArgumentsByName(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->analyse([__DIR__ . '/data/overriding-method-arguments-by-name.php'], [ + [ + 'Parameter #1 $name of method OverridingMethodArgumentsByName\Bar::all() should be named $dialect like in method OverridingMethodArgumentsByName\Foo::all() because of @arguments-by-name.', + 46, + ], + [ + 'Parameter #2 $category of method OverridingMethodArgumentsByName\Bar::optional() should be named $group like in method OverridingMethodArgumentsByName\Foo::optional() because of @optional-arguments-by-name.', + 50, + ], + [ + 'Parameter #2 $category of method OverridingMethodArgumentsByName\Bar::stable() should be named $group like in method OverridingMethodArgumentsByName\Foo::stable() because of @stable-parameter-names.', + 54, + ], + [ + 'Parameter #2 $category of method OverridingMethodArgumentsByName\FooImplementation::doFoo() should be named $group like in method OverridingMethodArgumentsByName\FooInterface::doFoo() because of @optional-arguments-by-name.', + 81, + ], + [ + 'Parameter #1 $name of method OverridingMethodArgumentsByName\ChildOfClassLevel::doFoo() should be named $dialect like in method OverridingMethodArgumentsByName\ClassLevel::doFoo() because of @arguments-by-name.', + 128, + ], + ]); + } + #[RequiresPhp('>= 8.0.0')] public function testBug14398(): void { diff --git a/tests/PHPStan/Rules/Methods/data/overriding-method-arguments-by-name.php b/tests/PHPStan/Rules/Methods/data/overriding-method-arguments-by-name.php new file mode 100644 index 00000000000..57502b55663 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/overriding-method-arguments-by-name.php @@ -0,0 +1,136 @@ += 8.0 + +namespace OverridingMethodArgumentsByName; + +class Foo +{ + + /** + * @arguments-by-name + */ + public function all(string $dialect, string $group = 'default'): void + { + } + + /** + * @optional-arguments-by-name + */ + public function optional(string $dialect, string $group = 'default', string ...$rest): void + { + } + + /** + * @stable-parameter-names + */ + public function stable(string $dialect, string $group = 'default'): void + { + } + + public function untagged(string $dialect): void + { + } + + /** + * @no-named-arguments + * @arguments-by-name + */ + public function conflicting(string $dialect): void + { + } + +} + +class Bar extends Foo +{ + + public function all(string $name, string $group = 'default'): void + { + } + + public function optional(string $name, string $category = 'default', string ...$others): void + { + } + + public function stable(string $dialect, string $category = 'default'): void + { + } + + public function untagged(string $name): void + { + } + + public function conflicting(string $name): void + { + } + +} + +/** + * @optional-arguments-by-name + */ +interface FooInterface +{ + + public function doFoo(string $dialect, string $group = 'default'): void; + +} + +class FooImplementation implements FooInterface +{ + + public function doFoo(string $name, string $category = 'default'): void + { + } + +} + +abstract class AbstractConstructor +{ + + /** + * @arguments-by-name + */ + abstract public function __construct(string $dialect); + +} + +class Constructor extends AbstractConstructor +{ + + public function __construct(string $name) + { + } + +} + +/** + * @arguments-by-name + */ +class ClassLevel +{ + + public function doFoo(string $dialect): void + { + } + + /** + * @no-named-arguments + */ + public function noNamed(string $dialect): void + { + } + +} + +class ChildOfClassLevel extends ClassLevel +{ + + public function doFoo(string $name): void + { + } + + public function noNamed(string $name): void + { + } + +} From 33f9c64b9e8a7d65bb2e9108237e69933f52844b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 14 Oct 2026 17:54:59 +0200 Subject: [PATCH 4/4] Don't apply class-level tags to private methods A private method is only called by its own class, so how it is called doesn't matter to other packages. --- src/Reflection/ArgumentsByNameTagResolver.php | 7 ++++++- .../Rules/Methods/data/arguments-by-name.php | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Reflection/ArgumentsByNameTagResolver.php b/src/Reflection/ArgumentsByNameTagResolver.php index e621c3ab70e..5e5e068b6a7 100644 --- a/src/Reflection/ArgumentsByNameTagResolver.php +++ b/src/Reflection/ArgumentsByNameTagResolver.php @@ -10,7 +10,7 @@ * Finds which of @arguments-by-name, @optional-arguments-by-name and @stable-parameter-names applies to a function or a method. * * For a method, the tag on the method comes first. Then comes the tag of the method it overrides or implements, - * except for constructors. The class-level tag of the declaring class comes last. + * except for constructors. The class-level tag of the declaring class comes last, except for private methods. * A docblock with @no-named-arguments has none of these tags. */ final class ArgumentsByNameTagResolver @@ -57,6 +57,11 @@ public static function resolveForMethod(ExtendedMethodReflection $method): ?stri } } + // private methods are only called by their class + if ($method->isPrivate()) { + return null; + } + return self::resolveFromPhpDoc($declaringClass->getResolvedPhpDoc()); } diff --git a/tests/PHPStan/Rules/Methods/data/arguments-by-name.php b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php index 7b4219af8d8..ca55f3894bb 100644 --- a/tests/PHPStan/Rules/Methods/data/arguments-by-name.php +++ b/tests/PHPStan/Rules/Methods/data/arguments-by-name.php @@ -323,3 +323,21 @@ function ( $stableChild->doFoo(dialect: 'openapi-3.1', group: 'read'); $stableChild->doFoo('openapi-3.1', 'read'); }; + +/** + * @arguments-by-name + */ +class ClassLevelWithPrivateMethod +{ + + public function doFoo(string $dialect): void + { + $this->doBar($dialect); + } + + private function doBar(string $dialect): void + { + + } + +}