Skip to content

Do not inherit @pure-unless-* tags into a method marked @phpstan-pure or @phpstan-impure - #6669

Open
zonuexe wants to merge 2 commits into
phpstan:2.3.xfrom
zonuexe:feature/impure-override-skips-conditional-body-check
Open

zonuexe wants to merge 2 commits into
phpstan:2.3.xfrom
zonuexe:feature/impure-override-skips-conditional-body-check

Conversation

@zonuexe

@zonuexe zonuexe commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

A method that declares its own @phpstan-pure or @phpstan-impure inherited the parent's @pure-unless-callable-is-impure / @pure-unless-parameter-passed tags through ResolvedPhpDocBlock::merge(), and FunctionPurityCheck followed the inherited tag instead of the method's own one:

interface Replacer
{
	/**
	 * @param callable(string): string $cb
	 * @pure-unless-callable-is-impure $cb
	 */
	public function map(callable $cb, string $subject): string;
}

final class ImpureReplacer implements Replacer
{
	/** @phpstan-impure */
	public function map(callable $cb, string $subject): string
	{
		echo $subject; // Impure echo in pure method ImpureReplacer::map().

		return $cb($subject);
	}
}

final class PureReplacer implements Replacer
{
	/** @phpstan-pure */
	public function map(callable $cb, string $subject): string
	{
		return $cb($subject); // not reported, while callers treat the method as pure
	}
}

merge() lets a method's own @phpstan-pure / @phpstan-impure win over the parent's (mergePureTags()), but it merged the @pure-unless-* tags next to the method's own purity tag. It now inherits them only into a method that declares neither @phpstan-pure nor @phpstan-impure. A method with its own purity tag keeps the @pure-unless-* tags written in its own docblock.

Effect

  • An @phpstan-impure override of a conditionally pure method is impure throughout. PHPStan no longer reports its body as "Impure ... in pure method" and applies the impure checks instead, so a final method without side effects gets impureMethod.pure. With bleeding edge, MethodSignatureRule reports the override as method.impureOverridePureUnlessCallable / method.impureOverridePureUnlessParameterPassed. Without bleeding edge, PHPStan doesn't report the override, the same as an @phpstan-impure override of a @phpstan-pure method today.
  • A @phpstan-pure override of a conditionally pure method is pure throughout. Callers treated it as pure before as well, since SimpleImpurePoint doesn't consult the conditional tags of a method whose hasSideEffects() is no, but its body could call $cb() and write to the by-ref parameter without an error. PHPStan reports those now (possiblyImpure.functionCall, pureMethod.parameterByRef). Code that relied on the exemption should drop @phpstan-pure and inherit the parent's tag, or write the tag on the method.
  • An override without a purity tag behaves as before: it inherits the tags from the parent class, interface, trait or stub, with parameter-name remapping, and PHPStan checks its body as conditionally pure.
  • Call sites behave as before: a call through the interface stays conditionally pure, and a call through the impure class stays impure.

Together with #6667, the precedence is: the method's own @phpstan-pure / @phpstan-impure, then a @pure-unless-* tag written on or inherited by the method, then the class-level @phpstan-all-methods-pure / @phpstan-all-methods-impure.

Not covered here: a docblock with both @phpstan-impure and a @pure-unless-* tag keeps both, and PHPStan still reports its body as "Impure ... in pure method". That docblock contradicts itself and should get its own error in a follow-up.

Tests

PureMethodRuleTest::testPureUnlessImpureOverride covers both tags with an impure override with and without side effects, a pure override (new errors on its body), and an override without a purity tag (body still checked).

@VincentLanglet VincentLanglet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think your example is valid because if I call

Replacer::replace() I'll expect that the call is pure, which is invalid with your implementation.

@zonuexe zonuexe changed the title Do not check the body of an impure method against an inherited @pure-unless-* tag Do not inherit @pure-unless-* tags into a method marked @phpstan-pure or @phpstan-impure Oct 4, 2026
@zonuexe

zonuexe commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

@VincentLanglet Agreed: ImpureReplacer::replace() breaks the promise of Replacer::replace(), and the example is a contract violation on purpose. The question is which error PHPStan should report for it. "Impure echo in pure method ImpureReplacer::replace()" calls the method pure while its docblock says @phpstan-impure and isPure() is no for every caller, and the error disappears once the body has no visible impure point. The break is in the signature, and MethodSignatureRule reports it as method.impureOverridePureUnlessParameterPassed (#6666).

That rule is behind reportMethodPurityOverride, so without bleeding edge this change leaves the override unreported. An @phpstan-impure override of a @phpstan-pure method behaves the same today: the child's own tag wins in ResolvedPhpDocBlock::merge(), PHPStan doesn't check the body for purity, and method.impure needs bleeding edge. Call sites don't change: a call through Replacer stays conditionally pure, and a call through ImpureReplacer is impure.

I reworked the fix. The first version skipped the body check in FunctionPurityCheck, but I traced the cause to the merged docblock: merge() put the @pure-unless-* tags next to the child's own @phpstan-pure or @phpstan-impure. The same leak let a @phpstan-pure child inherit @pure-unless-callable-is-impure, so its body could call $cb() without an error while callers treated it as pure. The fix is in ResolvedPhpDocBlock::merge() now: a method with its own @phpstan-pure or @phpstan-impure doesn't inherit the parent's @pure-unless-* tags. The test covers both directions, and the @phpstan-pure child gets errors on its body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants