Skip to content

Commit 6820560

Browse files
ondrejmirtesondrejmirtes
committed
Tip about adding @phpstan-impure where applicable
1 parent 1f4be0e commit 6820560

File tree

50 files changed

+810
-91
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+810
-91
lines changed

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ parameters:
108108
tips:
109109
discoveringSymbols: true
110110
treatPhpDocTypesAsCertain: true
111+
possiblyImpure: true
111112
tipsOfTheDay: true
112113
reportMagicMethods: false
113114
reportMagicProperties: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ parametersSchema:
8181
tips: structure([
8282
discoveringSymbols: bool()
8383
treatPhpDocTypesAsCertain: bool()
84+
possiblyImpure: bool()
8485
])
8586
tipsOfTheDay: bool()
8687
reportMaybes: bool()

‎src/Analyser/MutatingScope.php‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use PHPStan\Node\Expr\OriginalForeachKeyExpr;
4747
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
4848
use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
49+
use PHPStan\Node\Expr\PossiblyImpureCallExpr;
4950
use PHPStan\Node\Expr\PropertyInitializationExpr;
5051
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
5152
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
@@ -153,6 +154,7 @@
153154
use function array_merge;
154155
use function array_pop;
155156
use function array_slice;
157+
use function array_unique;
156158
use function array_values;
157159
use function count;
158160
use function explode;
@@ -778,6 +780,100 @@ public function getMaybeDefinedVariables(): array
778780
return $variables;
779781
}
780782

783+
/**
784+
* @return list<string>
785+
*/
786+
public function findPossiblyImpureCallDescriptions(Expr $expr): array
787+
{
788+
$nodeFinder = new NodeFinder();
789+
$descriptions = [];
790+
$foundCallExprMatch = false;
791+
foreach ($this->expressionTypes as $holder) {
792+
$holderExpr = $holder->getExpr();
793+
if (!$holderExpr instanceof PossiblyImpureCallExpr) {
794+
continue;
795+
}
796+
797+
$callExprKey = $this->getNodeKey($holderExpr->callExpr);
798+
799+
$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($callExprKey): bool {
800+
if (!$node instanceof Expr) {
801+
return false;
802+
}
803+
804+
return $this->getNodeKey($node) === $callExprKey;
805+
});
806+
807+
if ($found === null) {
808+
continue;
809+
}
810+
811+
$foundCallExprMatch = true;
812+
813+
// Only show the tip when the scope's type for the call expression
814+
// differs from the declared return type, meaning control flow
815+
// narrowing affected the type (the cached value was narrowed).
816+
assert($found instanceof Expr);
817+
$scopeType = $this->getType($found);
818+
$declaredReturnType = $holderExpr->getDeclaredReturnType();
819+
if ($declaredReturnType->isSuperTypeOf($scopeType)->yes() && $scopeType->isSuperTypeOf($declaredReturnType)->yes()) {
820+
continue;
821+
}
822+
823+
$descriptions[] = $holderExpr->getCallDescription();
824+
}
825+
826+
if (count($descriptions) > 0) {
827+
return array_values(array_unique($descriptions));
828+
}
829+
830+
// If the first pass found a callExpr in the error expression but
831+
// filtered it out (return type wasn't narrowed), the error is
832+
// explained by the return type alone - skip the fallback.
833+
if ($foundCallExprMatch) {
834+
return [];
835+
}
836+
837+
// Fallback: match by impactedExpr for cases where a maybe-impure method
838+
// on an object didn't invalidate it, but a different method's return
839+
// value was narrowed on that object.
840+
// Skip when the expression itself is a direct method/static call -
841+
// those are passed by ImpossibleCheckType rules where the error is
842+
// about the call's arguments, not about object state.
843+
if ($expr instanceof Expr\MethodCall || $expr instanceof Expr\StaticCall) {
844+
return [];
845+
}
846+
foreach ($this->expressionTypes as $holder) {
847+
$holderExpr = $holder->getExpr();
848+
if (!$holderExpr instanceof PossiblyImpureCallExpr) {
849+
continue;
850+
}
851+
852+
$impactedExprKey = $this->getNodeKey($holderExpr->impactedExpr);
853+
854+
// Skip if impactedExpr is the same as callExpr (function calls)
855+
if ($impactedExprKey === $this->getNodeKey($holderExpr->callExpr)) {
856+
continue;
857+
}
858+
859+
$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($impactedExprKey): bool {
860+
if (!$node instanceof Expr) {
861+
return false;
862+
}
863+
864+
return $this->getNodeKey($node) === $impactedExprKey;
865+
});
866+
867+
if ($found === null) {
868+
continue;
869+
}
870+
871+
$descriptions[] = $holderExpr->getCallDescription();
872+
}
873+
874+
return array_values(array_unique($descriptions));
875+
}
876+
781877
private function isGlobalVariable(string $variableName): bool
782878
{
783879
return in_array($variableName, self::SUPERGLOBAL_VARIABLES, true);

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
use PHPStan\Node\Expr\NativeTypeExpr;
9393
use PHPStan\Node\Expr\OriginalForeachKeyExpr;
9494
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
95+
use PHPStan\Node\Expr\PossiblyImpureCallExpr;
9596
use PHPStan\Node\Expr\PropertyInitializationExpr;
9697
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
9798
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
@@ -291,6 +292,8 @@ public function __construct(
291292
private readonly bool $implicitThrows,
292293
#[AutowiredParameter]
293294
private readonly bool $treatPhpDocTypesAsCertain,
295+
#[AutowiredParameter]
296+
private readonly bool $rememberPossiblyImpureFunctionValues,
294297
)
295298
{
296299
$earlyTerminatingMethodNames = [];
@@ -2918,6 +2921,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
29182921
$scope = $scope->invalidateExpression(new Variable('this'), true);
29192922
}
29202923

2924+
if (
2925+
$functionReflection !== null
2926+
&& $this->rememberPossiblyImpureFunctionValues
2927+
&& $functionReflection->hasSideEffects()->maybe()
2928+
&& !$functionReflection->isBuiltin()
2929+
&& $parametersAcceptor !== null
2930+
) {
2931+
$scope = $scope->assignExpression(
2932+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName()), $parametersAcceptor->getReturnType()),
2933+
new MixedType(),
2934+
new MixedType(),
2935+
);
2936+
}
2937+
29212938
if (
29222939
$functionReflection !== null
29232940
&& in_array($functionReflection->getName(), ['json_encode', 'json_decode'], true)
@@ -3216,6 +3233,12 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
32163233
if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
32173234
$this->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
32183235
$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
3236+
} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) {
3237+
$scope = $scope->assignExpression(
3238+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()),
3239+
new MixedType(),
3240+
new MixedType(),
3241+
);
32193242
}
32203243
if ($parametersAcceptor !== null && !$methodReflection->isStatic()) {
32213244
$selfOutType = $methodReflection->getSelfOutType();
@@ -3426,6 +3449,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
34263449
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
34273450
) {
34283451
$scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass());
3452+
} elseif (
3453+
$methodReflection !== null
3454+
&& $this->rememberPossiblyImpureFunctionValues
3455+
&& $methodReflection->hasSideEffects()->maybe()
3456+
&& !$methodReflection->getDeclaringClass()->isBuiltin()
3457+
&& $parametersAcceptor !== null
3458+
&& $scope->isInClass()
3459+
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
3460+
) {
3461+
$scope = $scope->assignExpression(
3462+
new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()),
3463+
new MixedType(),
3464+
new MixedType(),
3465+
);
34293466
}
34303467

34313468
if (

‎src/Node/Expr/PossiblyImpureCallExpr.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node\Expr;
4+
5+
use Override;
6+
use PhpParser\Node\Expr;
7+
use PHPStan\Node\VirtualNode;
8+
use PHPStan\Type\Type;
9+
10+
final class PossiblyImpureCallExpr extends Expr implements VirtualNode
11+
{
12+
13+
public function __construct(
14+
public Expr $callExpr,
15+
public Expr $impactedExpr,
16+
private string $callDescription,
17+
private Type $declaredReturnType,
18+
)
19+
{
20+
parent::__construct([]);
21+
}
22+
23+
public function getCallDescription(): string
24+
{
25+
return $this->callDescription;
26+
}
27+
28+
public function getDeclaredReturnType(): Type
29+
{
30+
return $this->declaredReturnType;
31+
}
32+
33+
#[Override]
34+
public function getType(): string
35+
{
36+
return 'PHPStan_Node_PossiblyImpureCallExpr';
37+
}
38+
39+
/**
40+
* @return string[]
41+
*/
42+
#[Override]
43+
public function getSubNodeNames(): array
44+
{
45+
return ['callExpr', 'impactedExpr'];
46+
}
47+
48+
}

‎src/Node/Printer/Printer.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use PHPStan\Node\Expr\OriginalForeachKeyExpr;
1818
use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
1919
use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
20+
use PHPStan\Node\Expr\PossiblyImpureCallExpr;
2021
use PHPStan\Node\Expr\PropertyInitializationExpr;
2122
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
2223
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
@@ -88,6 +89,11 @@ protected function pPHPStan_Node_AlwaysRememberedExpr(AlwaysRememberedExpr $expr
8889
return sprintf('__phpstanRemembered(%s)', $this->p($expr->getExpr()));
8990
}
9091

92+
protected function pPHPStan_Node_PossiblyImpureCallExpr(PossiblyImpureCallExpr $expr): string // phpcs:ignore
93+
{
94+
return sprintf('__phpstanPossiblyImpure(%s, %s)', $this->p($expr->callExpr), $this->p($expr->impactedExpr));
95+
}
96+
9197
protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializationExpr $expr): string // phpcs:ignore
9298
{
9399
return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName());

‎src/Rules/Comparison/BooleanAndConstantConditionRule.php‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ final class BooleanAndConstantConditionRule implements Rule
2323

2424
public function __construct(
2525
private ConstantConditionRuleHelper $helper,
26+
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
2627
#[AutowiredParameter]
2728
private bool $treatPhpDocTypesAsCertain,
2829
#[AutowiredParameter]
@@ -51,18 +52,20 @@ public function processNode(
5152
if ($leftType instanceof ConstantBooleanType) {
5253
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder {
5354
if (!$this->treatPhpDocTypesAsCertain) {
54-
return $ruleErrorBuilder;
55+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder);
5556
}
5657

5758
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->left);
5859
if ($booleanNativeType instanceof ConstantBooleanType) {
59-
return $ruleErrorBuilder;
60+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder);
6061
}
6162
if (!$this->treatPhpDocTypesAsCertainTip) {
62-
return $ruleErrorBuilder;
63+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder);
6364
}
6465

65-
return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
66+
$ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
67+
68+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder);
6669
};
6770

6871
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
@@ -89,21 +92,23 @@ public function processNode(
8992
if ($rightType instanceof ConstantBooleanType && !$scope->isInFirstLevelStatement()) {
9093
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode): RuleErrorBuilder {
9194
if (!$this->treatPhpDocTypesAsCertain) {
92-
return $ruleErrorBuilder;
95+
return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder);
9396
}
9497

9598
$booleanNativeType = $this->helper->getNativeBooleanType(
9699
$rightScope,
97100
$originalNode->right,
98101
);
99102
if ($booleanNativeType instanceof ConstantBooleanType) {
100-
return $ruleErrorBuilder;
103+
return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder);
101104
}
102105
if (!$this->treatPhpDocTypesAsCertainTip) {
103-
return $ruleErrorBuilder;
106+
return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder);
104107
}
105108

106-
return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
109+
$ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
110+
111+
return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder);
107112
};
108113

109114
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
@@ -127,18 +132,20 @@ public function processNode(
127132
if ($nodeType instanceof ConstantBooleanType) {
128133
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder {
129134
if (!$this->treatPhpDocTypesAsCertain) {
130-
return $ruleErrorBuilder;
135+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder);
131136
}
132137

133138
$booleanNativeType = $scope->getNativeType($originalNode);
134139
if ($booleanNativeType instanceof ConstantBooleanType) {
135-
return $ruleErrorBuilder;
140+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder);
136141
}
137142
if (!$this->treatPhpDocTypesAsCertainTip) {
138-
return $ruleErrorBuilder;
143+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder);
139144
}
140145

141-
return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
146+
$ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
147+
148+
return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder);
142149
};
143150

144151
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);

‎src/Rules/Comparison/BooleanNotConstantConditionRule.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ final class BooleanNotConstantConditionRule implements Rule
2121

2222
public function __construct(
2323
private ConstantConditionRuleHelper $helper,
24+
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
2425
#[AutowiredParameter]
2526
private bool $treatPhpDocTypesAsCertain,
2627
#[AutowiredParameter]
@@ -45,18 +46,20 @@ public function processNode(
4546
if ($exprType instanceof ConstantBooleanType) {
4647
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder {
4748
if (!$this->treatPhpDocTypesAsCertain) {
48-
return $ruleErrorBuilder;
49+
return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder);
4950
}
5051

5152
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->expr);
5253
if ($booleanNativeType instanceof ConstantBooleanType) {
53-
return $ruleErrorBuilder;
54+
return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder);
5455
}
5556
if (!$this->treatPhpDocTypesAsCertainTip) {
56-
return $ruleErrorBuilder;
57+
return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder);
5758
}
5859

59-
return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
60+
$ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip();
61+
62+
return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder);
6063
};
6164

6265
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);

0 commit comments

Comments
 (0)