Check @no-named-arguments in MethodSignatureRule

Add a check preventing a child method from declaring `@no-named-arguments` when parent method does not
phpstan · Jun 6, 2022 · 826f763 · 826f763
1 parent 64b0907
commit 826f763
Show file tree

Hide file tree

Showing 17 changed files with 132 additions and 0 deletions.
diff --git a/src/Reflection/Annotations/AnnotationMethodReflection.php b/src/Reflection/Annotations/AnnotationMethodReflection.php
@@ -110,6 +110,11 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createMaybe();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;

diff --git a/src/Reflection/Dummy/ChangedTypeMethodReflection.php b/src/Reflection/Dummy/ChangedTypeMethodReflection.php
@@ -89,4 +89,9 @@ public function hasSideEffects(): TrinaryLogic
 		return $this->reflection->hasSideEffects();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return $this->reflection->acceptsNamedArguments();
+	}
+
 }
diff --git a/src/Reflection/Dummy/DummyConstructorReflection.php b/src/Reflection/Dummy/DummyConstructorReflection.php
@@ -91,6 +91,11 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createNo();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;

diff --git a/src/Reflection/Dummy/DummyMethodReflection.php b/src/Reflection/Dummy/DummyMethodReflection.php
@@ -91,6 +91,11 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createMaybe();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;

diff --git a/src/Reflection/MethodReflection.php b/src/Reflection/MethodReflection.php
@@ -30,4 +30,6 @@ public function getThrowType(): ?Type;
 
 	public function hasSideEffects(): TrinaryLogic;
 
+	public function acceptsNamedArguments(): bool;
+
 }
diff --git a/src/Reflection/Mixin/MixinMethodReflection.php b/src/Reflection/Mixin/MixinMethodReflection.php
@@ -85,4 +85,9 @@ public function hasSideEffects(): TrinaryLogic
 		return $this->reflection->hasSideEffects();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return $this->reflection->acceptsNamedArguments();
+	}
+
 }
diff --git a/src/Reflection/Native/NativeMethodReflection.php b/src/Reflection/Native/NativeMethodReflection.php
@@ -137,6 +137,11 @@ public function hasSideEffects(): TrinaryLogic
 		return $this->hasSideEffects;
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 	private function isVoid(): bool
 	{
 		foreach ($this->variants as $variant) {

diff --git a/src/Reflection/Php/ClosureCallMethodReflection.php b/src/Reflection/Php/ClosureCallMethodReflection.php
@@ -118,4 +118,9 @@ public function hasSideEffects(): TrinaryLogic
 		return $this->nativeMethodReflection->hasSideEffects();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return $this->nativeMethodReflection->acceptsNamedArguments();
+	}
+
 }
diff --git a/src/Reflection/Php/EnumCasesMethodReflection.php b/src/Reflection/Php/EnumCasesMethodReflection.php
@@ -105,4 +105,9 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createNo();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 }
diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php
@@ -78,6 +78,7 @@ public function __construct(
 		private bool $isInternal,
 		private bool $isFinal,
 		private ?bool $isPure,
+		private bool $acceptsNamedArguments = true,
 	)
 	{
 	}
@@ -419,4 +420,9 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createMaybe();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return $this->acceptsNamedArguments;
+	}
+
 }
diff --git a/src/Reflection/Php/Soap/SoapClientMethodReflection.php b/src/Reflection/Php/Soap/SoapClientMethodReflection.php
@@ -97,4 +97,9 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::createYes();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return true;
+	}
+
 }
diff --git a/src/Reflection/ResolvedMethodReflection.php b/src/Reflection/ResolvedMethodReflection.php
@@ -115,4 +115,9 @@ public function hasSideEffects(): TrinaryLogic
 		return $this->reflection->hasSideEffects();
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		return $this->reflection->acceptsNamedArguments();
+	}
+
 }
diff --git a/src/Reflection/Type/IntersectionTypeMethodReflection.php b/src/Reflection/Type/IntersectionTypeMethodReflection.php
@@ -147,6 +147,17 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::maxMin(...array_map(static fn (MethodReflection $method): TrinaryLogic => $method->hasSideEffects(), $this->methods));
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		foreach ($this->methods as $method) {
+			if (!$method->acceptsNamedArguments()) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;

diff --git a/src/Reflection/Type/UnionTypeMethodReflection.php b/src/Reflection/Type/UnionTypeMethodReflection.php
@@ -148,6 +148,17 @@ public function hasSideEffects(): TrinaryLogic
 		return TrinaryLogic::extremeIdentity(...array_map(static fn (MethodReflection $method): TrinaryLogic => $method->hasSideEffects(), $this->methods));
 	}
 
+	public function acceptsNamedArguments(): bool
+	{
+		foreach ($this->methods as $method) {
+			if (!$method->acceptsNamedArguments()) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;

diff --git a/src/Rules/Methods/MethodSignatureRule.php b/src/Rules/Methods/MethodSignatureRule.php
@@ -67,6 +67,16 @@ public function processNode(Node $node, Scope $scope): array
 		$errors = [];
 		$declaringClass = $method->getDeclaringClass();
 		foreach ($this->collectParentMethods($methodName, $method->getDeclaringClass()) as $parentMethod) {
+			if ($parentMethod->acceptsNamedArguments() && ! $method->acceptsNamedArguments()) {
+				$errors[] = RuleErrorBuilder::message(sprintf(
+					'Method %s::%s() should accept named arguments as method %s::%s() does',
+					$method->getDeclaringClass()->getDisplayName(),
+					$method->getName(),
+					$parentMethod->getDeclaringClass()->getDisplayName(),
+					$parentMethod->getName(),
+				))->build();
+			}
+
 			$parentVariants = $parentMethod->getVariants();
 			if (count($parentVariants) !== 1) {
 				continue;

diff --git a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php
@@ -374,4 +374,20 @@ public function testOverridenMethodWithConditionalReturnType(): void
 		]);
 	}
 
+	public function testOverridenMethodWithNoNamedArguments(): void
+	{
+		$this->reportMaybes = true;
+		$this->reportStatic = true;
+		$this->analyse([__DIR__ . '/data/no-named-arguments.php'], [
+			[
+				'Method NoNamedArguments\Baz::namedArgumentsInParent() should accept named arguments as method NoNamedArguments\Foo::namedArgumentsInParent() does',
+				10,
+			],
+			[
+				'Method NoNamedArguments\Baz::namedArgumentsInInterface() should accept named arguments as method NoNamedArguments\Bar::namedArgumentsInInterface() does',
+				15,
+			],
+		]);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Methods/data/no-named-arguments.php b/tests/PHPStan/Rules/Methods/data/no-named-arguments.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace NoNamedArguments;
+
+class Baz extends Foo implements Bar
+{
+	/**
+	 * @no-named-arguments
+	 */
+	public function namedArgumentsInParent(float ...$args) {}
+
+	/**
+	 * @no-named-arguments
+	 */
+	public function namedArgumentsInInterface(float ...$args) {}
+}
+
+abstract class Foo
+{
+	abstract public function namedArgumentsInParent(float ...$args);
+}
+
+interface Bar
+{
+	public function namedArgumentsInInterface();
+}