Skip to content

Commit cf070ae

Browse files
ondrejmirtesondrejmirtes
authored
Tip about adding @phpstan-impure where applicable
1 parent 0c740e3 commit cf070ae

File tree

50 files changed

+803
-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

+803
-91
lines changed

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ parameters:
109109
tips:
110110
discoveringSymbols: true
111111
treatPhpDocTypesAsCertain: true
112+
possiblyImpure: true
112113
tipsOfTheDay: true
113114
reportMagicMethods: false
114115
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: 100 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,7 +154,9 @@
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;
159+
use function assert;
157160
use function count;
158161
use function explode;
159162
use function get_class;
@@ -778,6 +781,100 @@ public function getMaybeDefinedVariables(): array
778781
return $variables;
779782
}
780783

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

‎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 = [];
@@ -2935,6 +2938,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
29352938
$scope = $scope->invalidateExpression(new Variable('this'), true);
29362939
}
29372940

2941+
if (
2942+
$functionReflection !== null
2943+
&& $this->rememberPossiblyImpureFunctionValues
2944+
&& $parametersAcceptor !== null
2945+
&& $functionReflection->hasSideEffects()->maybe()
2946+
&& !$functionReflection->isBuiltin()
2947+
) {
2948+
$scope = $scope->assignExpression(
2949+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName())),
2950+
$parametersAcceptor->getReturnType(),
2951+
new MixedType(),
2952+
);
2953+
}
2954+
29382955
if (
29392956
$functionReflection !== null
29402957
&& in_array($functionReflection->getName(), ['json_encode', 'json_decode'], true)
@@ -3233,6 +3250,12 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
32333250
if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
32343251
$this->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
32353252
$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
3253+
} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) {
3254+
$scope = $scope->assignExpression(
3255+
new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())),
3256+
$parametersAcceptor->getReturnType(),
3257+
new MixedType(),
3258+
);
32363259
}
32373260
if ($parametersAcceptor !== null && !$methodReflection->isStatic()) {
32383261
$selfOutType = $methodReflection->getSelfOutType();
@@ -3443,6 +3466,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
34433466
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
34443467
) {
34453468
$scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass());
3469+
} elseif (
3470+
$methodReflection !== null
3471+
&& $this->rememberPossiblyImpureFunctionValues
3472+
&& $parametersAcceptor !== null
3473+
&& $scope->isInClass()
3474+
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
3475+
&& $methodReflection->hasSideEffects()->maybe()
3476+
&& !$methodReflection->getDeclaringClass()->isBuiltin()
3477+
) {
3478+
$scope = $scope->assignExpression(
3479+
new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())),
3480+
$parametersAcceptor->getReturnType(),
3481+
new MixedType(),
3482+
);
34463483
}
34473484

34483485
if (

‎src/Node/Expr/PossiblyImpureCallExpr.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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+
9+
final class PossiblyImpureCallExpr extends Expr implements VirtualNode
10+
{
11+
12+
public function __construct(
13+
public Expr $callExpr,
14+
public Expr $impactedExpr,
15+
private string $callDescription,
16+
)
17+
{
18+
parent::__construct([]);
19+
}
20+
21+
public function getCallDescription(): string
22+
{
23+
return $this->callDescription;
24+
}
25+
26+
#[Override]
27+
public function getType(): string
28+
{
29+
return 'PHPStan_Node_PossiblyImpureCallExpr';
30+
}
31+
32+
/**
33+
* @return string[]
34+
*/
35+
#[Override]
36+
public function getSubNodeNames(): array
37+
{
38+
return ['callExpr', 'impactedExpr'];
39+
}
40+
41+
}

‎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)