Skip to content

Commit 8fdeba5

Browse files
ondrejmirtesondrejmirtes
committed
MethodSignatureRule - do not report duplicates already reported by OverridingMethodRule
1 parent fe72e26 commit 8fdeba5

7 files changed

Lines changed: 71 additions & 18 deletions

File tree

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"nette/utils": "^3.1.1",
2222
"nikic/php-parser": "^4.5.0",
2323
"ondram/ci-detector": "^3.1",
24-
"ondrejmirtes/better-reflection": "^4.3.1",
24+
"ondrejmirtes/better-reflection": "^4.3.2",
2525
"phpdocumentor/type-resolver": "1.0.1",
2626
"phpstan/phpdoc-parser": "^0.4.7",
2727
"react/child-process": "^0.6.1",

‎conf/config.level0.neon‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ rules:
3131
- PHPStan\Rules\Methods\AbstractMethodInNonAbstractClassRule
3232
- PHPStan\Rules\Methods\ExistingClassesInTypehintsRule
3333
- PHPStan\Rules\Methods\MissingMethodImplementationRule
34-
- PHPStan\Rules\Methods\OverridingMethodRule
3534
- PHPStan\Rules\Properties\AccessPropertiesInAssignRule
3635
- PHPStan\Rules\Properties\AccessStaticPropertiesInAssignRule
3736
- PHPStan\Rules\Variables\ThisVariableRule
@@ -78,6 +77,13 @@ services:
7877
checkFunctionNameCase: %checkFunctionNameCase%
7978
reportMagicMethods: %reportMagicMethods%
8079

80+
-
81+
class: PHPStan\Rules\Methods\OverridingMethodRule
82+
arguments:
83+
checkPhpDocMethodSignatures: %checkPhpDocMethodSignatures%
84+
tags:
85+
- phpstan.rules.rule
86+
8187
-
8288
class: PHPStan\Rules\Missing\MissingClosureNativeReturnTypehintRule
8389
arguments:

‎conf/config.level3.neon‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ rules:
1818
- PHPStan\Rules\Variables\ThrowTypeRule
1919
- PHPStan\Rules\Variables\VariableCloningRule
2020

21+
parameters:
22+
checkPhpDocMethodSignatures: true
23+
2124
services:
2225
-
2326
class: PHPStan\Rules\Arrays\AppendedArrayKeyTypeRule
@@ -60,11 +63,3 @@ services:
6063
reportMaybes: %reportMaybes%
6164
tags:
6265
- phpstan.rules.rule
63-
64-
-
65-
class: PHPStan\Rules\Methods\MethodSignatureRule
66-
arguments:
67-
reportMaybes: %reportMaybesInMethodSignatures%
68-
reportStatic: %reportStaticMethodSignatures%
69-
tags:
70-
- phpstan.rules.rule

‎conf/config.neon‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ parameters:
2929
checkUnionTypes: false
3030
checkExplicitMixedMissingReturn: false
3131
checkPhpDocMissingReturn: false
32+
checkPhpDocMethodSignatures: false
3233
checkExtraArguments: false
3334
checkMissingClosureNativeReturnTypehintRule: false
3435
checkMissingTypehints: false
@@ -160,6 +161,7 @@ parametersSchema:
160161
checkUnionTypes: bool()
161162
checkExplicitMixedMissingReturn: bool()
162163
checkPhpDocMissingReturn: bool()
164+
checkPhpDocMethodSignatures: bool()
163165
checkExtraArguments: bool()
164166
checkMissingClosureNativeReturnTypehintRule: bool()
165167
checkMissingTypehints: bool()
@@ -591,6 +593,13 @@ services:
591593
-
592594
class: PHPStan\Rules\IssetCheck
593595

596+
-
597+
# checked as part of OverridingMethodRule
598+
class: PHPStan\Rules\Methods\MethodSignatureRule
599+
arguments:
600+
reportMaybes: %reportMaybesInMethodSignatures%
601+
reportStatic: %reportStaticMethodSignatures%
602+
594603
-
595604
class: PHPStan\Rules\MissingTypehintCheck
596605
arguments:

‎src/Rules/Methods/OverridingMethodRule.php‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PHPStan\Reflection\ParametersAcceptorSelector;
1313
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
1414
use PHPStan\Rules\Rule;
15+
use PHPStan\Rules\RuleError;
1516
use PHPStan\Rules\RuleErrorBuilder;
1617
use PHPStan\Type\ArrayType;
1718
use PHPStan\Type\IterableType;
@@ -29,9 +30,19 @@ class OverridingMethodRule implements Rule
2930

3031
private PhpVersion $phpVersion;
3132

32-
public function __construct(PhpVersion $phpVersion)
33+
private MethodSignatureRule $methodSignatureRule;
34+
35+
private bool $checkPhpDocMethodSignatures;
36+
37+
public function __construct(
38+
PhpVersion $phpVersion,
39+
MethodSignatureRule $methodSignatureRule,
40+
bool $checkPhpDocMethodSignatures
41+
)
3342
{
3443
$this->phpVersion = $phpVersion;
44+
$this->methodSignatureRule = $methodSignatureRule;
45+
$this->checkPhpDocMethodSignatures = $checkPhpDocMethodSignatures;
3546
}
3647

3748
public function getNodeType(): string
@@ -108,13 +119,13 @@ public function processNode(Node $node, Scope $scope): array
108119

109120
if (strtolower($method->getName()) === '__construct') {
110121
if (!$prototype->isAbstract()) {
111-
return $messages;
122+
return $this->addErrors($messages, $node->getOriginalNode(), $scope);
112123
}
113124
}
114125

115126
$prototypeVariants = $prototype->getVariants();
116127
if (count($prototypeVariants) !== 1) {
117-
return $messages;
128+
return $this->addErrors($messages, $node->getOriginalNode(), $scope);
118129
}
119130

120131
$prototypeVariant = $prototypeVariants[0];
@@ -277,7 +288,7 @@ public function processNode(Node $node, Scope $scope): array
277288
$methodReturnType = $methodVariant->getNativeReturnType();
278289

279290
if (!$prototypeVariant instanceof FunctionVariantWithPhpDocs) {
280-
return $messages;
291+
return $this->addErrors($messages, $node->getOriginalNode(), $scope);
281292
}
282293

283294
$prototypeReturnType = $prototypeVariant->getNativeReturnType();
@@ -306,7 +317,7 @@ public function processNode(Node $node, Scope $scope): array
306317
}
307318
}
308319

309-
return $messages;
320+
return $this->addErrors($messages, $node->getOriginalNode(), $scope);
310321
}
311322

312323
private function isTypeCompatible(Type $methodParameterType, Type $prototypeParameterType, bool $supportsContravariance): bool
@@ -339,4 +350,25 @@ private function isTypeCompatible(Type $methodParameterType, Type $prototypePara
339350
return $methodParameterType->isSuperTypeOf($prototypeParameterType)->yes();
340351
}
341352

353+
/**
354+
* @param RuleError[] $errors
355+
* @return RuleError[]
356+
*/
357+
private function addErrors(
358+
array $errors,
359+
Node\Stmt\ClassMethod $classMethod,
360+
Scope $scope
361+
): array
362+
{
363+
if (count($errors) > 0) {
364+
return $errors;
365+
}
366+
367+
if (!$this->checkPhpDocMethodSignatures) {
368+
return $errors;
369+
}
370+
371+
return $this->methodSignatureRule->processNode($classMethod, $scope);
372+
}
373+
342374
}

‎tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
namespace PHPStan\Rules\Methods;
44

5+
use PHPStan\Php\PhpVersion;
6+
use const PHP_VERSION_ID;
7+
58
/**
6-
* @extends \PHPStan\Testing\RuleTestCase<MethodSignatureRule>
9+
* @extends \PHPStan\Testing\RuleTestCase<OverridingMethodRule>
710
*/
811
class MethodSignatureRuleTest extends \PHPStan\Testing\RuleTestCase
912
{
@@ -16,7 +19,11 @@ class MethodSignatureRuleTest extends \PHPStan\Testing\RuleTestCase
1619

1720
protected function getRule(): \PHPStan\Rules\Rule
1821
{
19-
return new MethodSignatureRule($this->reportMaybes, $this->reportStatic);
22+
return new OverridingMethodRule(
23+
new PhpVersion(PHP_VERSION_ID),
24+
new MethodSignatureRule($this->reportMaybes, $this->reportStatic),
25+
true
26+
);
2027
}
2128

2229
public function testReturnTypeRule(): void

‎tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ class OverridingMethodRuleTest extends RuleTestCase
1717

1818
protected function getRule(): Rule
1919
{
20-
return new OverridingMethodRule(new PhpVersion($this->phpVersionId));
20+
return new OverridingMethodRule(
21+
new PhpVersion($this->phpVersionId),
22+
new MethodSignatureRule(true, true),
23+
false
24+
);
2125
}
2226

2327
public function dataOverridingFinalMethod(): array

0 commit comments

Comments
 (0)