Report impure method overriding pure parent method in `MethodSignatureRule` by phpstan-bot · Pull Request #5584 · phpstan/phpstan-src

phpstan-bot · 2026-05-02T18:12:01Z

Summary

When a parent method is marked @phpstan-pure, child classes could override it with @phpstan-impure without any error being reported. This violated the Liskov Substitution Principle: callers expecting a pure method would get an impure one. This PR adds a purity compatibility check to MethodSignatureRule.

Changes

src/Rules/Methods/MethodSignatureRule.php: Added a check at the beginning of the parent method loop — when the child method's isPure() is no and the parent method's isPure() is yes, report an error with identifier method.impureOverridePure
tests/PHPStan/Rules/Methods/data/bug-14563.php: New test data file covering all parent method sources and edge cases
tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php: Added testBug14563() test method

Root cause

MethodSignatureRule checked return type covariance and parameter type contravariance for overriding methods, but had no check for purity compatibility. An impure child method could silently override a pure parent method, breaking the purity contract that callers rely on.

Test

The regression test covers these cases (all producing errors):

Class extends class: child has @phpstan-impure, parent has @phpstan-pure
Interface implementation: implementing class marks method @phpstan-impure while interface declares it @phpstan-pure
@phpstan-all-methods-pure on parent class: child class marks method @phpstan-impure
Abstract trait method: trait declares @phpstan-pure abstract method, using class marks it @phpstan-impure
Grandchild inheritance: grandchild overrides with @phpstan-impure a method pure in the parent chain
Multiple interfaces: both interfaces declare the method @phpstan-pure, implementing class marks it @phpstan-impure — error reported for each interface

And these non-error cases (verifying no false positives):

Child with no annotation overriding pure parent (inherits purity)
Child with @phpstan-pure overriding pure parent
Child with @phpstan-pure overriding impure parent (safe — more restrictive)
Child with @phpstan-impure overriding unannotated parent
Child in @phpstan-all-methods-impure class overriding pure parent (PHPDoc inheritance of @phpstan-pure takes precedence over class-level annotation)

Fixes phpstan/phpstan#14563

VincentLanglet

Add test for static method too.

phpstan-bot · 2026-05-02T19:02:40Z

Done. Added static method test cases covering:

Error case: Impure static method overriding a pure static parent method (StaticChildImpureOverridesPure extends StaticFoo)
Error case: Impure static method implementing a pure static interface method (StaticImpureImplementation implements StaticPureInterface)
No-error cases: Static child with no annotation overriding pure parent, static child with @phpstan-pure overriding pure parent, and static child with @phpstan-impure overriding unannotated parent

All tests pass and make phpstan reports no errors.

VincentLanglet

Why tests are failing on PHP 7.4 ?

phpstan-bot · 2026-05-02T19:36:40Z

The PHP 7.4 test failure is fixed. The issue was that collectParentMethods doesn't detect abstract trait methods on PHP 7.4 — this is a known limitation, consistent with other trait-related tests in MethodSignatureRuleTest that use #[RequiresPhp('>= 8.0.0')] (like testTraits, testBug10166, testBug10184, testOverridenAbstractTraitMethodPhpDoc).

The fix makes the trait abstract method error expectation conditional on PHP_VERSION_ID >= 80000. The other integration test failures (PMMP, doctrine/orm, shopware) are unrelated to this PR — they're caused by other recent changes to main.

VincentLanglet · 2026-05-05T13:44:27Z

+					$method->getName(),
+					$parentMethodDeclaringClass->getDisplayName(),
+					$parentMethod->getName(),
+				))->identifier('method.purity')->build();


Identifier must be method.impure.

Also this new error should be behind a dedicated toggle which is enabled behind bleedingedge

Both review comments addressed and pushed:

Identifier changed: method.purity → method.impure

Toggle added: New checkMethodPurityOverride feature toggle (defaults to false, enabled in bleedingEdge.neon). The check in MethodSignatureRule is now gated behind $this->checkPurity, which is wired to %featureToggles.checkMethodPurityOverride% in services.neon.

All 11,987 tests pass and make phpstan reports no errors.

VincentLanglet · 2026-05-05T18:16:22Z

 		reportNestedTooWideType: bool()
 		assignToByRefForeachExpr: bool()
 		curlSetOptArrayTypes: bool()
+		checkMethodPurityOverride: bool()


Change for reportMethodPurityOverride

Renamed checkMethodPurityOverride to reportMethodPurityOverride across all four config files (bleedingEdge.neon, config.neon, parametersSchema.neon, services.neon). All 11,987 tests pass and make phpstan reports no errors.

VincentLanglet · 2026-05-05T18:23:06Z

 			reportMaybes: %reportMaybesInMethodSignatures%
 			reportStatic: %reportStaticMethodSignatures%
-			checkPurity: %featureToggles.checkMethodPurityOverride%
+			checkPurity: %featureToggles.reportMethodPurityOverride%


This should be also reportMethodPurityOverride

Renamed the checkPurity constructor parameter and service argument to reportMethodPurityOverride for consistency with the config toggle name. All tests pass (43/43) and make phpstan reports no errors.

…eRule` - Add purity compatibility check in MethodSignatureRule::processNode() - When a parent method has @phpstan-pure and the child method has @phpstan-impure, report an error with identifier method.impureOverridePure - The check runs for all parent sources: parent classes, interfaces, and abstract trait methods - Covers @phpstan-all-methods-pure on parent class (purity is inherited per-method) - Also covers grandchild inheritance and multiple interface implementation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Trait abstract method detection in collectParentMethods does not work on PHP 7.4, consistent with other trait-related tests in this file that use #[RequiresPhp('>= 8.0.0')]. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…bled in bleedingEdge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…tency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

staabm · 2026-05-05T19:04:05Z

+class ChildImpureOverridesPure extends Foo
+{
+
+	/** @phpstan-impure */


we should also test the case of a method returning :void (implicit impure) beeing overridden by a pure/impure method

Done. Added test cases for :void return type methods covering:

Error case: Child with @phpstan-impure overriding a @phpstan-pure void parent method (ChildImpureOverridesPureVoid)

No-error cases:

Child with @phpstan-pure overriding a pure void parent

Child with no annotation overriding a pure void parent

Child with @phpstan-impure overriding an unannotated void parent

Child with @phpstan-pure overriding an unannotated void parent

All 11,993 tests pass and make phpstan reports no errors.

Test that methods returning :void (implicitly impure) are handled correctly when overriding pure/impure parent methods and when being overridden by pure/impure child methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet reviewed May 2, 2026

View reviewed changes

VincentLanglet self-assigned this May 2, 2026

VincentLanglet reviewed May 2, 2026

View reviewed changes

Comment thread src/Rules/Methods/MethodSignatureRule.php Outdated

VincentLanglet reviewed May 2, 2026

View reviewed changes

Comment thread tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php

VincentLanglet mentioned this pull request May 2, 2026

Impure method should not be able to override pure method phpstan/phpstan#14563

Open

VincentLanglet force-pushed the create-pull-request/patch-zcama98 branch from 53f49ca to ea28b6b Compare May 5, 2026 09:39

VincentLanglet reviewed May 5, 2026

View reviewed changes

VincentLanglet approved these changes May 5, 2026

View reviewed changes

VincentLanglet and others added 9 commits May 5, 2026 20:37

Add static method test cases for impure-overrides-pure check

ea16277

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Skip trait abstract method purity test on PHP < 8.0

8e78545

Trait abstract method detection in collectParentMethods does not work on PHP 7.4, consistent with other trait-related tests in this file that use #[RequiresPhp('>= 8.0.0')]. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Change error identifier to method.purity

9951d24

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Split trait purity test into dedicated file with RequiresPhp 8.0

83d0462

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Change error identifier to method.impure

97c4a7f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Put purity override check behind checkMethodPurityOverride toggle ena…

9a80fb6

…bled in bleedingEdge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Rename checkMethodPurityOverride toggle to reportMethodPurityOverride

6d81c6b

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Rename checkPurity parameter to reportMethodPurityOverride for consis…

e0144c9

…tency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet force-pushed the create-pull-request/patch-zcama98 branch from 84fb578 to e0144c9 Compare May 5, 2026 18:39

staabm reviewed May 5, 2026

View reviewed changes

staabm approved these changes May 5, 2026

View reviewed changes

Conversation

phpstan-bot commented May 2, 2026

Summary

Changes

Root cause

Test

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented May 2, 2026

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented May 2, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants