Skip to content

Commit f75a266

Browse files
phpstan-botVincentLangletclaude
authored
Report impure method overriding pure parent method in MethodSignatureRule (#5584)
Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ab0b1ae commit f75a266

9 files changed

Lines changed: 446 additions & 2 deletions

File tree

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,4 @@ parameters:
1717
assignToByRefForeachExpr: true
1818
curlSetOptArrayTypes: true
1919
checkDateIntervalConstructor: true
20+
reportMethodPurityOverride: true

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ parameters:
4444
assignToByRefForeachExpr: false
4545
curlSetOptArrayTypes: false
4646
checkDateIntervalConstructor: false
47+
reportMethodPurityOverride: false
4748
fileExtensions:
4849
- php
4950
checkAdvancedIsset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ parametersSchema:
4646
assignToByRefForeachExpr: bool()
4747
curlSetOptArrayTypes: bool()
4848
checkDateIntervalConstructor: bool()
49+
reportMethodPurityOverride: bool()
4950
])
5051
fileExtensions: listOf(string())
5152
checkAdvancedIsset: bool()

‎conf/services.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ services:
9898
arguments:
9999
reportMaybes: %reportMaybesInMethodSignatures%
100100
reportStatic: %reportStaticMethodSignatures%
101+
reportMethodPurityOverride: %featureToggles.reportMethodPurityOverride%
101102

102103
phpstanDiagnoseExtension:
103104
class: PHPStan\Diagnose\PHPStanDiagnoseExtension

‎src/Rules/Methods/MethodSignatureRule.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public function __construct(
4141
private ParentMethodHelper $parentMethodHelper,
4242
private bool $reportMaybes,
4343
private bool $reportStatic,
44+
private bool $reportMethodPurityOverride,
4445
)
4546
{
4647
}
@@ -66,6 +67,16 @@ public function processNode(Node $node, Scope $scope): array
6667
$errors = [];
6768
$declaringClass = $method->getDeclaringClass();
6869
foreach ($this->parentMethodHelper->collectParentMethods($methodName, $method->getDeclaringClass()) as [$parentMethod, $parentMethodDeclaringClass]) {
70+
if ($this->reportMethodPurityOverride && $method->isPure()->no() && $parentMethod->isPure()->yes()) {
71+
$errors[] = RuleErrorBuilder::message(sprintf(
72+
'Impure method %s::%s() overrides pure method %s::%s().',
73+
$method->getDeclaringClass()->getDisplayName(),
74+
$method->getName(),
75+
$parentMethodDeclaringClass->getDisplayName(),
76+
$parentMethod->getName(),
77+
))->identifier('method.impure')->build();
78+
}
79+
6980
$parentVariants = $parentMethod->getVariants();
7081
if (count($parentVariants) !== 1) {
7182
continue;

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

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ class MethodSignatureRuleTest extends RuleTestCase
1919

2020
private bool $reportStatic;
2121

22+
private bool $reportMethodPurityOverride = false;
23+
2224
protected function getRule(): Rule
2325
{
2426
$phpVersion = new PhpVersion(PHP_VERSION_ID);
@@ -27,7 +29,7 @@ protected function getRule(): Rule
2729

2830
return new OverridingMethodRule(
2931
$phpVersion,
30-
new MethodSignatureRule(new ParentMethodHelper($phpClassReflectionExtension), $this->reportMaybes, $this->reportStatic),
32+
new MethodSignatureRule(new ParentMethodHelper($phpClassReflectionExtension), $this->reportMaybes, $this->reportStatic, $this->reportMethodPurityOverride),
3133
true,
3234
new MethodParameterComparisonHelper($phpVersion),
3335
new MethodVisibilityComparisonHelper(),
@@ -565,4 +567,67 @@ public function testBug14320(): void
565567
$this->analyse([__DIR__ . '/data/bug-14320.php'], []);
566568
}
567569

570+
public function testBug14563(): void
571+
{
572+
$this->reportMaybes = true;
573+
$this->reportStatic = true;
574+
$this->reportMethodPurityOverride = true;
575+
$this->analyse([__DIR__ . '/data/bug-14563.php'], [
576+
[
577+
'Impure method Bug14563\ChildImpureOverridesPure::pure() overrides pure method Bug14563\Foo::pure().',
578+
31,
579+
],
580+
[
581+
'Impure method Bug14563\ImpureImplementation::pureMethod() overrides pure method Bug14563\PureInterface::pureMethod().',
582+
93,
583+
],
584+
[
585+
'Impure method Bug14563\ImpureChildOfAllMethodsPure::method() overrides pure method Bug14563\AllMethodsPureParent::method().',
586+
126,
587+
],
588+
[
589+
'Impure method Bug14563\ChildImpureOverridesPureExtended::pure() overrides pure method Bug14563\Foo::pure().',
590+
137,
591+
],
592+
[
593+
'Impure method Bug14563\GrandchildImpureOverridesPure::pure() overrides pure method Bug14563\ChildPureOverridesPure::pure().',
594+
148,
595+
],
596+
[
597+
'Impure method Bug14563\ImpureMultipleInterfaces::sharedMethod() overrides pure method Bug14563\PureInterfaceA::sharedMethod().',
598+
186,
599+
],
600+
[
601+
'Impure method Bug14563\ImpureMultipleInterfaces::sharedMethod() overrides pure method Bug14563\PureInterfaceB::sharedMethod().',
602+
186,
603+
],
604+
[
605+
'Impure method Bug14563\ChildImpureOverridesPureVoid::pureVoid() overrides pure method Bug14563\VoidFoo::pureVoid().',
606+
211,
607+
],
608+
[
609+
'Impure method Bug14563\StaticChildImpureOverridesPure::pure() overrides pure method Bug14563\StaticFoo::pure().',
610+
284,
611+
],
612+
[
613+
'Impure method Bug14563\StaticImpureImplementation::pureMethod() overrides pure method Bug14563\StaticPureInterface::pureMethod().',
614+
335,
615+
],
616+
]);
617+
}
618+
619+
#[RequiresPhp('>= 8.0.0')]
620+
public function testBug14563Trait(): void
621+
{
622+
$this->reportMaybes = true;
623+
$this->reportStatic = true;
624+
$this->reportMethodPurityOverride = true;
625+
$this->analyse([__DIR__ . '/data/bug-14563-trait.php'], [
626+
[
627+
'Impure method Bug14563Trait\ImpureTraitUser::pureTraitMethod() overrides pure method Bug14563Trait\PureTrait::pureTraitMethod().',
628+
19,
629+
],
630+
]);
631+
}
632+
568633
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ protected function getRule(): Rule
3030

3131
return new OverridingMethodRule(
3232
$phpVersion,
33-
new MethodSignatureRule(new ParentMethodHelper($phpClassReflectionExtension), true, true),
33+
new MethodSignatureRule(new ParentMethodHelper($phpClassReflectionExtension), true, true, false),
3434
false,
3535
new MethodParameterComparisonHelper($phpVersion),
3636
new MethodVisibilityComparisonHelper(),

‎tests/PHPStan/Rules/Methods/data/bug-14563-trait.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14563Trait;
4+
5+
trait PureTrait
6+
{
7+
8+
/** @phpstan-pure */
9+
abstract public function pureTraitMethod(): int;
10+
11+
}
12+
13+
class ImpureTraitUser
14+
{
15+
16+
use PureTrait;
17+
18+
/** @phpstan-impure */
19+
public function pureTraitMethod(): int
20+
{
21+
return random_int(0, 1);
22+
}
23+
24+
}

0 commit comments

Comments
 (0)