Skip to content

Commit 43c2d27

Browse files
ondrejmirtesondrejmirtes
committed
ImpossibleInstanceOfRule - add PossiblyImpureTipHelper and ConstantConditionInTraitHelper
1 parent 4a9202b commit 43c2d27

File tree

4 files changed

+209
-20
lines changed

4 files changed

+209
-20
lines changed

‎src/Rules/Classes/ImpossibleInstanceOfRule.php‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@
33
namespace PHPStan\Rules\Classes;
44

55
use PhpParser\Node;
6+
use PHPStan\Analyser\CollectedDataEmitter;
7+
use PHPStan\Analyser\NodeCallbackInvoker;
68
use PHPStan\Analyser\Scope;
79
use PHPStan\DependencyInjection\AutowiredParameter;
810
use PHPStan\DependencyInjection\RegisteredRule;
911
use PHPStan\Parser\LastConditionVisitor;
12+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
13+
use PHPStan\Rules\Comparison\PossiblyImpureTipHelper;
1014
use PHPStan\Rules\Rule;
1115
use PHPStan\Rules\RuleErrorBuilder;
1216
use PHPStan\Rules\RuleLevelHelper;
@@ -29,6 +33,8 @@ final class ImpossibleInstanceOfRule implements Rule
2933

3034
public function __construct(
3135
private RuleLevelHelper $ruleLevelHelper,
36+
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
37+
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
3238
#[AutowiredParameter]
3339
private bool $treatPhpDocTypesAsCertain,
3440
#[AutowiredParameter]
@@ -44,7 +50,7 @@ public function getNodeType(): string
4450
return Node\Expr\Instanceof_::class;
4551
}
4652

47-
public function processNode(Node $node, Scope $scope): array
53+
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
4854
{
4955
if ($node->class instanceof Node\Name) {
5056
$className = $scope->resolveName($node->class);
@@ -74,40 +80,48 @@ public function processNode(Node $node, Scope $scope): array
7480

7581
$instanceofType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node) : $scope->getNativeType($node);
7682
if (!$instanceofType instanceof ConstantBooleanType) {
83+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $node);
7784
return [];
7885
}
7986

8087
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder {
8188
if (!$this->treatPhpDocTypesAsCertain) {
82-
return $ruleErrorBuilder;
89+
return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder);
8390
}
8491

8592
$instanceofTypeWithoutPhpDocs = $scope->getNativeType($node);
8693
if ($instanceofTypeWithoutPhpDocs instanceof ConstantBooleanType) {
87-
return $ruleErrorBuilder;
94+
return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder);
8895
}
8996

9097
if (!$this->treatPhpDocTypesAsCertainTip) {
91-
return $ruleErrorBuilder;
98+
return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder);
9299
}
93100

94-
return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
101+
$ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
102+
103+
return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder);
95104
};
96105

97106
if (!$instanceofType->getValue()) {
98107
$exprType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->expr) : $scope->getNativeType($node->expr);
99108

100-
return [
101-
$addTip(RuleErrorBuilder::message(sprintf(
102-
'Instanceof between %s and %s will always evaluate to false.',
103-
$exprType->describe(VerbosityLevel::typeOnly()),
104-
$classType->describe(VerbosityLevel::getRecommendedLevelByType($classType)),
105-
)))->identifier('instanceof.alwaysFalse')->build(),
106-
];
109+
$ruleError = $addTip(RuleErrorBuilder::message(sprintf(
110+
'Instanceof between %s and %s will always evaluate to false.',
111+
$exprType->describe(VerbosityLevel::typeOnly()),
112+
$classType->describe(VerbosityLevel::getRecommendedLevelByType($classType)),
113+
)))->identifier('instanceof.alwaysFalse')->build();
114+
if ($scope->isInTrait()) {
115+
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $node, false, $ruleError);
116+
return [];
117+
}
118+
119+
return [$ruleError];
107120
}
108121

109122
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
110123
if ($isLast === true && !$this->reportAlwaysTrueInLastCondition) {
124+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $node);
111125
return [];
112126
}
113127

@@ -123,7 +137,13 @@ public function processNode(Node $node, Scope $scope): array
123137

124138
$errorBuilder->identifier('instanceof.alwaysTrue');
125139

126-
return [$errorBuilder->build()];
140+
$ruleError = $errorBuilder->build();
141+
if ($scope->isInTrait()) {
142+
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $node, true, $ruleError);
143+
return [];
144+
}
145+
146+
return [$ruleError];
127147
}
128148

129149
}

‎tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@
22

33
namespace PHPStan\Rules\Classes;
44

5+
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
6+
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
7+
use PHPStan\Rules\Comparison\PossiblyImpureTipHelper;
58
use PHPStan\Rules\Rule;
69
use PHPStan\Rules\RuleLevelHelper;
10+
use PHPStan\Testing\CompositeRule;
711
use PHPStan\Testing\RuleTestCase;
812
use PHPUnit\Framework\Attributes\DataProvider;
913
use PHPUnit\Framework\Attributes\RequiresPhp;
1014
use function sprintf;
1115
use const PHP_VERSION_ID;
1216

1317
/**
14-
* @extends RuleTestCase<ImpossibleInstanceOfRule>
18+
* @extends RuleTestCase<CompositeRule>
1519
*/
1620
class ImpossibleInstanceOfRuleTest extends RuleTestCase
1721
{
@@ -33,12 +37,18 @@ protected function getRule(): Rule
3337
discoveringSymbolsTip: true,
3438
);
3539

36-
return new ImpossibleInstanceOfRule(
37-
$ruleLevelHelper,
38-
treatPhpDocTypesAsCertain: $this->treatPhpDocTypesAsCertain,
39-
reportAlwaysTrueInLastCondition: $this->reportAlwaysTrueInLastCondition,
40-
treatPhpDocTypesAsCertainTip: true,
41-
);
40+
// @phpstan-ignore argument.type
41+
return new CompositeRule([
42+
new ImpossibleInstanceOfRule(
43+
$ruleLevelHelper,
44+
new PossiblyImpureTipHelper(true),
45+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
46+
treatPhpDocTypesAsCertain: $this->treatPhpDocTypesAsCertain,
47+
reportAlwaysTrueInLastCondition: $this->reportAlwaysTrueInLastCondition,
48+
treatPhpDocTypesAsCertainTip: true,
49+
),
50+
new ConstantConditionInTraitRule(),
51+
]);
4252
}
4353

4454
protected function shouldTreatPhpDocTypesAsCertain(): bool
@@ -602,4 +612,42 @@ public function testBug13975(string $file): void
602612
$this->analyse([$file], []);
603613
}
604614

615+
public function testPossiblyImpureTip(): void
616+
{
617+
$this->treatPhpDocTypesAsCertain = true;
618+
$learnMore = ' Learn more: <fg=cyan>https://phpstan.org/blog/remembering-and-forgetting-returned-values</>';
619+
$this->analyse([__DIR__ . '/data/possibly-impure-instanceof-tip.php'], [
620+
// maybe-impure: tip expected
621+
[
622+
'Instanceof between PossiblyImpureInstanceofTip\Cat and PossiblyImpureInstanceofTip\Cat will always evaluate to true.',
623+
41,
624+
'If PossiblyImpureInstanceofTip\Holder::maybeImpureMethod() is impure, add <fg=cyan>@phpstan-impure</> PHPDoc tag above its declaration.' . $learnMore,
625+
],
626+
// pure: no tip, error explained by type
627+
[
628+
'Instanceof between PossiblyImpureInstanceofTip\Cat and PossiblyImpureInstanceofTip\Cat will always evaluate to true.',
629+
53,
630+
],
631+
// impure: no error - $holder invalidated
632+
]);
633+
}
634+
635+
public function testInTrait(): void
636+
{
637+
$this->treatPhpDocTypesAsCertain = true;
638+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
639+
$this->analyse([__DIR__ . '/data/impossible-instanceof-in-trait.php'], [
640+
[
641+
'Instanceof between ImpossibleInstanceofInTrait\Cat and stdClass will always evaluate to false.',
642+
25,
643+
$tipText,
644+
],
645+
[
646+
'Instanceof between ImpossibleInstanceofInTrait\Dog and stdClass will always evaluate to false.',
647+
25,
648+
$tipText,
649+
],
650+
]);
651+
}
652+
605653
}

‎tests/PHPStan/Rules/Classes/data/impossible-instanceof-in-trait.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
namespace ImpossibleInstanceofInTrait;
4+
5+
class Dog {}
6+
class Cat {}
7+
8+
trait FooTrait
9+
{
10+
11+
/** @var Dog|Cat */
12+
protected $animal;
13+
14+
public function doFoo(): void
15+
{
16+
// sometimes true, sometimes false
17+
if ($this->animal instanceof Dog) {
18+
19+
}
20+
}
21+
22+
public function doFoo2(): void
23+
{
24+
// always false
25+
if ($this->animal instanceof \stdClass) {
26+
27+
}
28+
}
29+
30+
}
31+
32+
class Foo
33+
{
34+
35+
/** @use FooTrait */
36+
use FooTrait;
37+
38+
/** @var Dog */
39+
protected $animal;
40+
41+
}
42+
43+
class FooAnother
44+
{
45+
46+
/** @use FooTrait */
47+
use FooTrait;
48+
49+
/** @var Cat */
50+
protected $animal;
51+
52+
}

‎tests/PHPStan/Rules/Classes/data/possibly-impure-instanceof-tip.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PossiblyImpureInstanceofTip;
4+
5+
class Dog {}
6+
class Cat extends Dog {}
7+
8+
class Holder
9+
{
10+
/** @phpstan-pure */
11+
public function getAnimal(): Dog
12+
{
13+
return new Dog();
14+
}
15+
16+
public function maybeImpureMethod(): int
17+
{
18+
return rand(1, 100);
19+
}
20+
21+
/** @phpstan-pure */
22+
public function pureMethod(): int
23+
{
24+
return 42;
25+
}
26+
27+
/** @phpstan-impure */
28+
public function impureMethod(): int
29+
{
30+
echo 'hello';
31+
return rand(1, 100);
32+
}
33+
}
34+
35+
function testMaybeImpure(Holder $holder): void
36+
{
37+
if ($holder->getAnimal() instanceof Cat) {
38+
$holder->maybeImpureMethod();
39+
40+
// tip expected: maybeImpureMethod() might have changed the object
41+
if ($holder->getAnimal() instanceof Cat) {
42+
return;
43+
}
44+
}
45+
}
46+
47+
function testPure(Holder $holder): void
48+
{
49+
if ($holder->getAnimal() instanceof Cat) {
50+
$holder->pureMethod();
51+
52+
// no tip - pureMethod() cannot change anything
53+
if ($holder->getAnimal() instanceof Cat) {
54+
return;
55+
}
56+
}
57+
}
58+
59+
function testImpure(Holder $holder): void
60+
{
61+
if ($holder->getAnimal() instanceof Cat) {
62+
$holder->impureMethod();
63+
64+
// no error - $holder invalidated by impure call
65+
if ($holder->getAnimal() instanceof Cat) {
66+
return;
67+
}
68+
}
69+
}

0 commit comments

Comments
 (0)