Skip to content

Commit c85b24b

Browse files
committed
Do not check the body of an impure method against an inherited @pure-unless-* tag
1 parent d679562 commit c85b24b

3 files changed

Lines changed: 116 additions & 2 deletions

File tree

‎src/Rules/Pure/FunctionPurityCheck.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,14 @@ public function check(
154154
}
155155

156156
$errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription));
157-
} elseif ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== []) {
157+
} elseif (!$isPure->no() && ($pureUnlessCallableParamNames !== [] || $pureUnlessParameterPassedParamNames !== [])) {
158158
// A function declared @pure-unless-callable-is-impure is pure except
159159
// for the flagged callables, so its body is checked for purity while
160160
// the flagged callables' own invocations are exempt. The same holds for
161161
// @pure-unless-parameter-passed: writing through the flagged by-ref
162162
// parameter is not an impure point, so the rest of the body still has
163-
// to be pure for the conditional verdict to hold.
163+
// to be pure for the conditional verdict to hold. A method marked
164+
// @phpstan-impure that inherits either tag makes no such promise.
164165
$errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription));
165166
} elseif ($isPure->no()) {
166167
if (

‎tests/PHPStan/Rules/Pure/PureMethodRuleTest.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,4 +438,27 @@ public function testPureUnlessParameterPassed(): void
438438
]);
439439
}
440440

441+
public function testPureUnlessImpureOverride(): void
442+
{
443+
$this->treatPhpDocTypesAsCertain = true;
444+
$this->analyse([__DIR__ . '/data/pure-unless-impure-override.php'], [
445+
[
446+
'Method PureUnlessImpureOverride\ImpureReplacerWithoutSideEffects::replace() is marked as impure but does not have any side effects.',
447+
57,
448+
],
449+
[
450+
'Method PureUnlessImpureOverride\ImpureReplacerWithoutSideEffects::map() is marked as impure but does not have any side effects.',
451+
65,
452+
],
453+
[
454+
'Impure echo in pure method PureUnlessImpureOverride\InheritingReplacer::replace().',
455+
77,
456+
],
457+
[
458+
'Impure echo in pure method PureUnlessImpureOverride\InheritingReplacer::map().',
459+
85,
460+
],
461+
]);
462+
}
463+
441464
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PureUnlessImpureOverride;
4+
5+
interface Replacer
6+
{
7+
8+
/**
9+
* @param-out int $count
10+
* @pure-unless-parameter-passed $count
11+
*/
12+
public function replace(string $subject, int &$count = 0): string;
13+
14+
/**
15+
* @param callable(string): string $cb
16+
* @pure-unless-callable-is-impure $cb
17+
*/
18+
public function map(callable $cb, string $subject): string;
19+
20+
}
21+
22+
final class ImpureReplacer implements Replacer
23+
{
24+
25+
/**
26+
* The method is marked impure, so its body is not checked for purity
27+
* although it inherits @pure-unless-parameter-passed.
28+
*
29+
* @phpstan-impure
30+
*/
31+
public function replace(string $subject, int &$count = 0): string
32+
{
33+
echo $subject;
34+
$count = 1;
35+
36+
return $subject;
37+
}
38+
39+
/**
40+
* @phpstan-impure
41+
*/
42+
public function map(callable $cb, string $subject): string
43+
{
44+
echo $subject;
45+
46+
return $cb($subject);
47+
}
48+
49+
}
50+
51+
final class ImpureReplacerWithoutSideEffects implements Replacer
52+
{
53+
54+
/**
55+
* @phpstan-impure
56+
*/
57+
public function replace(string $subject, int &$count = 0): string
58+
{
59+
return $subject;
60+
}
61+
62+
/**
63+
* @phpstan-impure
64+
*/
65+
public function map(callable $cb, string $subject): string
66+
{
67+
return $subject;
68+
}
69+
70+
}
71+
72+
final class InheritingReplacer implements Replacer
73+
{
74+
75+
public function replace(string $subject, int &$count = 0): string
76+
{
77+
echo $subject;
78+
$count = 1;
79+
80+
return $subject;
81+
}
82+
83+
public function map(callable $cb, string $subject): string
84+
{
85+
echo $subject;
86+
87+
return $cb($subject);
88+
}
89+
90+
}

0 commit comments

Comments
 (0)