Skip to content

Commit

Permalink
Check @no-named-arguments in MethodSignatureRule
Browse files Browse the repository at this point in the history
Add a check preventing a child method from declaring `@no-named-arguments` when parent method does not

Co-authored-by: Ondrej Mirtes <ondrej@mirtes.cz>
  • Loading branch information
jrmajor and ondrejmirtes committed Jun 28, 2022
1 parent 45d951a commit 7e58d45
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/Reflection/Annotations/AnnotationMethodReflection.php
Expand Up @@ -110,6 +110,11 @@ public function hasSideEffects(): TrinaryLogic
return TrinaryLogic::createMaybe();
}

public function acceptsNamedArguments(): bool
{
return true;
}

public function getDocComment(): ?string
{
return null;
Expand Down
2 changes: 2 additions & 0 deletions src/Reflection/ExtendedMethodReflection.php
Expand Up @@ -16,4 +16,6 @@
interface ExtendedMethodReflection extends MethodReflection
{

public function acceptsNamedArguments(): bool;

}
5 changes: 5 additions & 0 deletions src/Reflection/Native/NativeMethodReflection.php
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions src/Reflection/Php/EnumCasesMethodReflection.php
Expand Up @@ -105,4 +105,9 @@ public function hasSideEffects(): TrinaryLogic
return TrinaryLogic::createNo();
}

public function acceptsNamedArguments(): bool
{
return true;
}

}
2 changes: 2 additions & 0 deletions src/Reflection/Php/PhpClassReflectionExtension.php
Expand Up @@ -705,6 +705,7 @@ private function createMethod(
$isInternal = $resolvedPhpDoc->isInternal();
$isFinal = $resolvedPhpDoc->isFinal();
$isPure = $resolvedPhpDoc->isPure();
$acceptsNamedArguments = $resolvedPhpDoc->acceptsNamedArguments();

return $this->methodReflectionFactory->create(
$declaringClass,
Expand All @@ -719,6 +720,7 @@ private function createMethod(
$isInternal,
$isFinal,
$isPure,
$acceptsNamedArguments,
);
}

Expand Down
6 changes: 6 additions & 0 deletions src/Reflection/Php/PhpMethodReflection.php
Expand Up @@ -78,6 +78,7 @@ public function __construct(
private bool $isInternal,
private bool $isFinal,
private ?bool $isPure,
private bool $acceptsNamedArguments,
)
{
}
Expand Down Expand Up @@ -419,4 +420,9 @@ public function hasSideEffects(): TrinaryLogic
return TrinaryLogic::createMaybe();
}

public function acceptsNamedArguments(): bool
{
return $this->acceptsNamedArguments;
}

}
1 change: 1 addition & 0 deletions src/Reflection/Php/PhpMethodReflectionFactory.php
Expand Up @@ -26,6 +26,7 @@ public function create(
bool $isInternal,
bool $isFinal,
?bool $isPure = null,
bool $acceptsNamedArguments = true,
): PhpMethodReflection;

}
5 changes: 5 additions & 0 deletions src/Reflection/WrappedExtendedMethodReflection.php
Expand Up @@ -82,4 +82,9 @@ public function hasSideEffects(): TrinaryLogic
return $this->method->hasSideEffects();
}

public function acceptsNamedArguments(): bool
{
return true;
}

}
10 changes: 10 additions & 0 deletions src/Rules/Methods/MethodSignatureRule.php
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php
Expand Up @@ -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,
],
]);
}

}
26 changes: 26 additions & 0 deletions 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();
}

0 comments on commit 7e58d45

Please sign in to comment.