From 68205607a6247b39c4f88b9648ae2314a0a3f793 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 22 Feb 2026 22:32:07 +0100 Subject: [PATCH 1/4] Tip about adding `@phpstan-impure` where applicable --- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + src/Analyser/MutatingScope.php | 96 ++++++ src/Analyser/NodeScopeResolver.php | 37 +++ src/Node/Expr/PossiblyImpureCallExpr.php | 48 +++ src/Node/Printer/Printer.php | 6 + .../BooleanAndConstantConditionRule.php | 31 +- .../BooleanNotConstantConditionRule.php | 11 +- .../BooleanOrConstantConditionRule.php | 31 +- .../ConstantLooseComparisonRule.php | 11 +- .../DoWhileLoopConstantConditionRule.php | 11 +- .../ElseIfConstantConditionRule.php | 11 +- .../Comparison/IfConstantConditionRule.php | 11 +- .../ImpossibleCheckTypeFunctionCallRule.php | 11 +- .../ImpossibleCheckTypeMethodCallRule.php | 11 +- ...mpossibleCheckTypeStaticMethodCallRule.php | 11 +- .../LogicalXorConstantConditionRule.php | 21 +- src/Rules/Comparison/MatchExpressionRule.php | 18 +- ...mparisonOperatorsConstantConditionRule.php | 11 +- .../Comparison/PossiblyImpureTipHelper.php | 47 +++ .../StrictComparisonOfDifferentTypesRule.php | 14 +- .../TernaryOperatorConstantConditionRule.php | 11 +- .../WhileLoopAlwaysFalseConditionRule.php | 11 +- .../WhileLoopAlwaysTrueConditionRule.php | 11 +- src/Rules/RuleErrorBuilder.php | 19 ++ src/Testing/RuleTestCase.php | 1 + src/Testing/TypeInferenceTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 1 + .../Fiber/FiberNodeScopeResolverRuleTest.php | 1 + .../Fiber/FiberNodeScopeResolverTest.php | 1 + .../BooleanAndConstantConditionRuleTest.php | 1 + .../BooleanNotConstantConditionRuleTest.php | 1 + .../BooleanOrConstantConditionRuleTest.php | 1 + .../ConstantLooseComparisonRuleTest.php | 1 + .../DoWhileLoopConstantConditionRuleTest.php | 1 + .../ElseIfConstantConditionRuleTest.php | 1 + .../IfConstantConditionRuleTest.php | 1 + ...mpossibleCheckTypeFunctionCallRuleTest.php | 1 + ...sibleCheckTypeGenericOverwriteRuleTest.php | 1 + ...sibleCheckTypeMethodCallRuleEqualsTest.php | 1 + .../ImpossibleCheckTypeMethodCallRuleTest.php | 1 + ...sibleCheckTypeStaticMethodCallRuleTest.php | 1 + .../LogicalXorConstantConditionRuleTest.php | 1 + .../Comparison/MatchExpressionRuleTest.php | 1 + ...isonOperatorsConstantConditionRuleTest.php | 1 + ...rictComparisonOfDifferentTypesRuleTest.php | 81 +++++ ...rnaryOperatorConstantConditionRuleTest.php | 1 + .../WhileLoopAlwaysFalseConditionRuleTest.php | 1 + .../WhileLoopAlwaysTrueConditionRuleTest.php | 1 + .../Comparison/data/possibly-impure-tip.php | 295 ++++++++++++++++++ 50 files changed, 810 insertions(+), 91 deletions(-) create mode 100644 src/Node/Expr/PossiblyImpureCallExpr.php create mode 100644 src/Rules/Comparison/PossiblyImpureTipHelper.php create mode 100644 tests/PHPStan/Rules/Comparison/data/possibly-impure-tip.php diff --git a/conf/config.neon b/conf/config.neon index 19056af1ab..145eee14bd 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -108,6 +108,7 @@ parameters: tips: discoveringSymbols: true treatPhpDocTypesAsCertain: true + possiblyImpure: true tipsOfTheDay: true reportMagicMethods: false reportMagicProperties: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 269dc27c83..cbd1c68b27 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -81,6 +81,7 @@ parametersSchema: tips: structure([ discoveringSymbols: bool() treatPhpDocTypesAsCertain: bool() + possiblyImpure: bool() ]) tipsOfTheDay: bool() reportMaybes: bool() diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 7829231448..02c8f40713 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -46,6 +46,7 @@ use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr; +use PHPStan\Node\Expr\PossiblyImpureCallExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; @@ -153,6 +154,7 @@ use function array_merge; use function array_pop; use function array_slice; +use function array_unique; use function array_values; use function count; use function explode; @@ -778,6 +780,100 @@ public function getMaybeDefinedVariables(): array return $variables; } + /** + * @return list + */ + public function findPossiblyImpureCallDescriptions(Expr $expr): array + { + $nodeFinder = new NodeFinder(); + $descriptions = []; + $foundCallExprMatch = false; + foreach ($this->expressionTypes as $holder) { + $holderExpr = $holder->getExpr(); + if (!$holderExpr instanceof PossiblyImpureCallExpr) { + continue; + } + + $callExprKey = $this->getNodeKey($holderExpr->callExpr); + + $found = $nodeFinder->findFirst([$expr], function (Node $node) use ($callExprKey): bool { + if (!$node instanceof Expr) { + return false; + } + + return $this->getNodeKey($node) === $callExprKey; + }); + + if ($found === null) { + continue; + } + + $foundCallExprMatch = true; + + // Only show the tip when the scope's type for the call expression + // differs from the declared return type, meaning control flow + // narrowing affected the type (the cached value was narrowed). + assert($found instanceof Expr); + $scopeType = $this->getType($found); + $declaredReturnType = $holderExpr->getDeclaredReturnType(); + if ($declaredReturnType->isSuperTypeOf($scopeType)->yes() && $scopeType->isSuperTypeOf($declaredReturnType)->yes()) { + continue; + } + + $descriptions[] = $holderExpr->getCallDescription(); + } + + if (count($descriptions) > 0) { + return array_values(array_unique($descriptions)); + } + + // If the first pass found a callExpr in the error expression but + // filtered it out (return type wasn't narrowed), the error is + // explained by the return type alone - skip the fallback. + if ($foundCallExprMatch) { + return []; + } + + // Fallback: match by impactedExpr for cases where a maybe-impure method + // on an object didn't invalidate it, but a different method's return + // value was narrowed on that object. + // Skip when the expression itself is a direct method/static call - + // those are passed by ImpossibleCheckType rules where the error is + // about the call's arguments, not about object state. + if ($expr instanceof Expr\MethodCall || $expr instanceof Expr\StaticCall) { + return []; + } + foreach ($this->expressionTypes as $holder) { + $holderExpr = $holder->getExpr(); + if (!$holderExpr instanceof PossiblyImpureCallExpr) { + continue; + } + + $impactedExprKey = $this->getNodeKey($holderExpr->impactedExpr); + + // Skip if impactedExpr is the same as callExpr (function calls) + if ($impactedExprKey === $this->getNodeKey($holderExpr->callExpr)) { + continue; + } + + $found = $nodeFinder->findFirst([$expr], function (Node $node) use ($impactedExprKey): bool { + if (!$node instanceof Expr) { + return false; + } + + return $this->getNodeKey($node) === $impactedExprKey; + }); + + if ($found === null) { + continue; + } + + $descriptions[] = $holderExpr->getCallDescription(); + } + + return array_values(array_unique($descriptions)); + } + private function isGlobalVariable(string $variableName): bool { return in_array($variableName, self::SUPERGLOBAL_VARIABLES, true); diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 0c708a4068..7fbec1a825 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -92,6 +92,7 @@ use PHPStan\Node\Expr\NativeTypeExpr; use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; +use PHPStan\Node\Expr\PossiblyImpureCallExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; @@ -291,6 +292,8 @@ public function __construct( private readonly bool $implicitThrows, #[AutowiredParameter] private readonly bool $treatPhpDocTypesAsCertain, + #[AutowiredParameter] + private readonly bool $rememberPossiblyImpureFunctionValues, ) { $earlyTerminatingMethodNames = []; @@ -2918,6 +2921,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $scope = $scope->invalidateExpression(new Variable('this'), true); } + if ( + $functionReflection !== null + && $this->rememberPossiblyImpureFunctionValues + && $functionReflection->hasSideEffects()->maybe() + && !$functionReflection->isBuiltin() + && $parametersAcceptor !== null + ) { + $scope = $scope->assignExpression( + new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName()), $parametersAcceptor->getReturnType()), + new MixedType(), + new MixedType(), + ); + } + if ( $functionReflection !== null && in_array($functionReflection->getName(), ['json_encode', 'json_decode'], true) @@ -3216,6 +3233,12 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) { $this->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass()); + } elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) { + $scope = $scope->assignExpression( + new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()), + new MixedType(), + new MixedType(), + ); } if ($parametersAcceptor !== null && !$methodReflection->isStatic()) { $selfOutType = $methodReflection->getSelfOutType(); @@ -3426,6 +3449,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) ) { $scope = $scope->invalidateExpression(new Variable('this'), true, $methodReflection->getDeclaringClass()); + } elseif ( + $methodReflection !== null + && $this->rememberPossiblyImpureFunctionValues + && $methodReflection->hasSideEffects()->maybe() + && !$methodReflection->getDeclaringClass()->isBuiltin() + && $parametersAcceptor !== null + && $scope->isInClass() + && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) + ) { + $scope = $scope->assignExpression( + new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()), + new MixedType(), + new MixedType(), + ); } if ( diff --git a/src/Node/Expr/PossiblyImpureCallExpr.php b/src/Node/Expr/PossiblyImpureCallExpr.php new file mode 100644 index 0000000000..bf61d64e7b --- /dev/null +++ b/src/Node/Expr/PossiblyImpureCallExpr.php @@ -0,0 +1,48 @@ +callDescription; + } + + public function getDeclaredReturnType(): Type + { + return $this->declaredReturnType; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_PossiblyImpureCallExpr'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return ['callExpr', 'impactedExpr']; + } + +} diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index 261e8b0a62..f9452cf45c 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -17,6 +17,7 @@ use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr; +use PHPStan\Node\Expr\PossiblyImpureCallExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; use PHPStan\Node\Expr\SetOffsetValueTypeExpr; @@ -88,6 +89,11 @@ protected function pPHPStan_Node_AlwaysRememberedExpr(AlwaysRememberedExpr $expr return sprintf('__phpstanRemembered(%s)', $this->p($expr->getExpr())); } + protected function pPHPStan_Node_PossiblyImpureCallExpr(PossiblyImpureCallExpr $expr): string // phpcs:ignore + { + return sprintf('__phpstanPossiblyImpure(%s, %s)', $this->p($expr->callExpr), $this->p($expr->impactedExpr)); + } + protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializationExpr $expr): string // phpcs:ignore { return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName()); diff --git a/src/Rules/Comparison/BooleanAndConstantConditionRule.php b/src/Rules/Comparison/BooleanAndConstantConditionRule.php index d9945748b3..1554682590 100644 --- a/src/Rules/Comparison/BooleanAndConstantConditionRule.php +++ b/src/Rules/Comparison/BooleanAndConstantConditionRule.php @@ -23,6 +23,7 @@ final class BooleanAndConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -51,18 +52,20 @@ public function processNode( if ($leftType instanceof ConstantBooleanType) { $addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->left); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); @@ -89,7 +92,7 @@ public function processNode( if ($rightType instanceof ConstantBooleanType && !$scope->isInFirstLevelStatement()) { $addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType( @@ -97,13 +100,15 @@ public function processNode( $originalNode->right, ); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); @@ -127,18 +132,20 @@ public function processNode( if ($nodeType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } $booleanNativeType = $scope->getNativeType($originalNode); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); diff --git a/src/Rules/Comparison/BooleanNotConstantConditionRule.php b/src/Rules/Comparison/BooleanNotConstantConditionRule.php index 3a462cc836..0f62704ebd 100644 --- a/src/Rules/Comparison/BooleanNotConstantConditionRule.php +++ b/src/Rules/Comparison/BooleanNotConstantConditionRule.php @@ -21,6 +21,7 @@ final class BooleanNotConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -45,18 +46,20 @@ public function processNode( if ($exprType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->expr); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->expr, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); diff --git a/src/Rules/Comparison/BooleanOrConstantConditionRule.php b/src/Rules/Comparison/BooleanOrConstantConditionRule.php index 199410f197..8d2e1b8610 100644 --- a/src/Rules/Comparison/BooleanOrConstantConditionRule.php +++ b/src/Rules/Comparison/BooleanOrConstantConditionRule.php @@ -23,6 +23,7 @@ final class BooleanOrConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -51,18 +52,20 @@ public function processNode( if ($leftType instanceof ConstantBooleanType) { $addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->left); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->left, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); @@ -89,7 +92,7 @@ public function processNode( if ($rightType instanceof ConstantBooleanType && !$scope->isInFirstLevelStatement()) { $addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType( @@ -97,13 +100,15 @@ public function processNode( $originalNode->right, ); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($rightScope, $originalNode->right, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); @@ -127,18 +132,20 @@ public function processNode( if ($nodeType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } $booleanNativeType = $scope->getNativeType($originalNode); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); diff --git a/src/Rules/Comparison/ConstantLooseComparisonRule.php b/src/Rules/Comparison/ConstantLooseComparisonRule.php index cc9a2ebdd7..dde16af74d 100644 --- a/src/Rules/Comparison/ConstantLooseComparisonRule.php +++ b/src/Rules/Comparison/ConstantLooseComparisonRule.php @@ -20,6 +20,7 @@ final class ConstantLooseComparisonRule implements Rule { public function __construct( + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -48,18 +49,20 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $instanceofTypeWithoutPhpDocs = $scope->getNativeType($node); if ($instanceofTypeWithoutPhpDocs->isTrue()->yes() || $instanceofTypeWithoutPhpDocs->isFalse()->yes()) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; if ($nodeType->isFalse()->yes()) { diff --git a/src/Rules/Comparison/DoWhileLoopConstantConditionRule.php b/src/Rules/Comparison/DoWhileLoopConstantConditionRule.php index 7087fbd006..3740754e29 100644 --- a/src/Rules/Comparison/DoWhileLoopConstantConditionRule.php +++ b/src/Rules/Comparison/DoWhileLoopConstantConditionRule.php @@ -24,6 +24,7 @@ final class DoWhileLoopConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -65,18 +66,20 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->getCond(), $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->getCond()); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->getCond(), $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->getCond(), $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->getCond(), $ruleErrorBuilder); }; return [ diff --git a/src/Rules/Comparison/ElseIfConstantConditionRule.php b/src/Rules/Comparison/ElseIfConstantConditionRule.php index bcf9ba5dde..10df54ef12 100644 --- a/src/Rules/Comparison/ElseIfConstantConditionRule.php +++ b/src/Rules/Comparison/ElseIfConstantConditionRule.php @@ -21,6 +21,7 @@ final class ElseIfConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -45,18 +46,20 @@ public function processNode( if ($exprType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->cond); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); }; $isLast = $node->cond->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); diff --git a/src/Rules/Comparison/IfConstantConditionRule.php b/src/Rules/Comparison/IfConstantConditionRule.php index a4b08ef043..19ee79df13 100644 --- a/src/Rules/Comparison/IfConstantConditionRule.php +++ b/src/Rules/Comparison/IfConstantConditionRule.php @@ -20,6 +20,7 @@ final class IfConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -42,18 +43,20 @@ public function processNode( if ($exprType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->cond); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); }; return [ diff --git a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php index 9abb0b528c..61e3b57c30 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeFunctionCallRule.php @@ -20,6 +20,7 @@ final class ImpossibleCheckTypeFunctionCallRule implements Rule public function __construct( private ImpossibleCheckTypeHelper $impossibleCheckTypeHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -49,18 +50,20 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); if ($isAlways !== null) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; if (!$isAlways) { diff --git a/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php index 9935a164f3..bc8284d111 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeMethodCallRule.php @@ -23,6 +23,7 @@ final class ImpossibleCheckTypeMethodCallRule implements Rule public function __construct( private ImpossibleCheckTypeHelper $impossibleCheckTypeHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -51,18 +52,20 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); if ($isAlways !== null) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; if (!$isAlways) { diff --git a/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php b/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php index 3919824104..3c24b38176 100644 --- a/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php +++ b/src/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRule.php @@ -23,6 +23,7 @@ final class ImpossibleCheckTypeStaticMethodCallRule implements Rule public function __construct( private ImpossibleCheckTypeHelper $impossibleCheckTypeHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -51,18 +52,20 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $isAlways = $this->impossibleCheckTypeHelper->doNotTreatPhpDocTypesAsCertain()->findSpecifiedType($scope, $node); if ($isAlways !== null) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; if (!$isAlways) { diff --git a/src/Rules/Comparison/LogicalXorConstantConditionRule.php b/src/Rules/Comparison/LogicalXorConstantConditionRule.php index 751280bf87..3618a9f964 100644 --- a/src/Rules/Comparison/LogicalXorConstantConditionRule.php +++ b/src/Rules/Comparison/LogicalXorConstantConditionRule.php @@ -22,6 +22,7 @@ final class LogicalXorConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -44,18 +45,20 @@ public function processNode(Node $node, Scope $scope): array if ($leftType instanceof ConstantBooleanType) { $addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->left, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->left); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->left, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->left, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->left, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); @@ -77,7 +80,7 @@ public function processNode(Node $node, Scope $scope): array if ($rightType instanceof ConstantBooleanType) { $addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->right, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType( @@ -85,13 +88,15 @@ public function processNode(Node $node, Scope $scope): array $node->right, ); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->right, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->right, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->right, $ruleErrorBuilder); }; $isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME); diff --git a/src/Rules/Comparison/MatchExpressionRule.php b/src/Rules/Comparison/MatchExpressionRule.php index 8c31b92242..c922a1ae98 100644 --- a/src/Rules/Comparison/MatchExpressionRule.php +++ b/src/Rules/Comparison/MatchExpressionRule.php @@ -30,6 +30,7 @@ final class MatchExpressionRule implements Rule public function __construct( private ConstantConditionRuleHelper $constantConditionRuleHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, ) @@ -95,11 +96,13 @@ public function processNode(Node $node, Scope $scope): array $armLine = $armCondition->getLine(); if (!$armConditionResult->getValue()) { - $errors[] = RuleErrorBuilder::message(sprintf( + $errorBuilder = RuleErrorBuilder::message(sprintf( 'Match arm comparison between %s and %s is always false.', $armConditionScope->getType($matchCondition)->describe(VerbosityLevel::value()), $armConditionScope->getType($armCondition->getCondition())->describe(VerbosityLevel::value()), - ))->line($armLine)->identifier('match.alwaysFalse')->build(); + ))->line($armLine)->identifier('match.alwaysFalse'); + $this->possiblyImpureTipHelper->addTip($armConditionScope, $armConditionExpr, $errorBuilder); + $errors[] = $errorBuilder->build(); continue; } @@ -112,11 +115,14 @@ public function processNode(Node $node, Scope $scope): array $armConditionScope->getType($matchCondition)->describe(VerbosityLevel::value()), $armConditionScope->getType($armCondition->getCondition())->describe(VerbosityLevel::value()), ); - $errors[] = RuleErrorBuilder::message($message) + $errorBuilder = RuleErrorBuilder::message($message) ->line($armLine) - ->identifier('match.alwaysTrue') - ->tip('Remove remaining cases below this one and this error will disappear too.') - ->build(); + ->identifier('match.alwaysTrue'); + if (!$errorBuilder->hasTips()) { + $errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.'); + } + $this->possiblyImpureTipHelper->addTip($armConditionScope, $armConditionExpr, $errorBuilder); + $errors[] = $errorBuilder->build(); } } diff --git a/src/Rules/Comparison/NumberComparisonOperatorsConstantConditionRule.php b/src/Rules/Comparison/NumberComparisonOperatorsConstantConditionRule.php index 14bf2c5a6d..5cd0ab417e 100644 --- a/src/Rules/Comparison/NumberComparisonOperatorsConstantConditionRule.php +++ b/src/Rules/Comparison/NumberComparisonOperatorsConstantConditionRule.php @@ -23,6 +23,7 @@ final class NumberComparisonOperatorsConstantConditionRule implements Rule { public function __construct( + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -54,18 +55,20 @@ public function processNode( if ($exprType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $booleanNativeType = $scope->getNativeType($node); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; switch (get_class($node)) { diff --git a/src/Rules/Comparison/PossiblyImpureTipHelper.php b/src/Rules/Comparison/PossiblyImpureTipHelper.php new file mode 100644 index 0000000000..25fcaccb9c --- /dev/null +++ b/src/Rules/Comparison/PossiblyImpureTipHelper.php @@ -0,0 +1,47 @@ +possiblyImpureTip) { + return $ruleErrorBuilder; + } + if ($ruleErrorBuilder->hasTips()) { + return $ruleErrorBuilder; + } + if (!$scope instanceof MutatingScope) { + return $ruleErrorBuilder; + } + $descriptions = $scope->findPossiblyImpureCallDescriptions($conditionExpr); + if (count($descriptions) === 0) { + return $ruleErrorBuilder; + } + + return $ruleErrorBuilder->possiblyImpureTip($descriptions); + } + +} diff --git a/src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php b/src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php index f0e71a0ee9..b8b0ca09db 100644 --- a/src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php +++ b/src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php @@ -27,6 +27,7 @@ final class StrictComparisonOfDifferentTypesRule implements Rule public function __construct( private RicherScopeGetTypeHelper $richerScopeGetTypeHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter] @@ -67,22 +68,25 @@ public function processNode(Node $node, Scope $scope): array $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $nodeTypeResult): RuleErrorBuilder { $reasons = $nodeTypeResult->reasons; if (count($reasons) > 0) { - return $ruleErrorBuilder->acceptsReasonsTip($reasons); + $ruleErrorBuilder = $ruleErrorBuilder->acceptsReasonsTip($reasons); + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } $instanceofTypeWithoutPhpDocs = $scope->getNativeType($node); if ($instanceofTypeWithoutPhpDocs instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node, $ruleErrorBuilder); }; $verbosity = VerbosityLevel::value(); diff --git a/src/Rules/Comparison/TernaryOperatorConstantConditionRule.php b/src/Rules/Comparison/TernaryOperatorConstantConditionRule.php index b6c515a4f3..273405ee36 100644 --- a/src/Rules/Comparison/TernaryOperatorConstantConditionRule.php +++ b/src/Rules/Comparison/TernaryOperatorConstantConditionRule.php @@ -20,6 +20,7 @@ final class TernaryOperatorConstantConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -42,18 +43,20 @@ public function processNode( if ($exprType instanceof ConstantBooleanType) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->cond); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); }; return [ $addTip(RuleErrorBuilder::message(sprintf( diff --git a/src/Rules/Comparison/WhileLoopAlwaysFalseConditionRule.php b/src/Rules/Comparison/WhileLoopAlwaysFalseConditionRule.php index 1057d4d3d7..77b9b7543b 100644 --- a/src/Rules/Comparison/WhileLoopAlwaysFalseConditionRule.php +++ b/src/Rules/Comparison/WhileLoopAlwaysFalseConditionRule.php @@ -20,6 +20,7 @@ final class WhileLoopAlwaysFalseConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -42,18 +43,20 @@ public function processNode( if ($exprType->isFalse()->yes()) { $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->cond); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $node->cond, $ruleErrorBuilder); }; return [ diff --git a/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php b/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php index 3f251f8366..9a08905283 100644 --- a/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php +++ b/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php @@ -24,6 +24,7 @@ final class WhileLoopAlwaysTrueConditionRule implements Rule public function __construct( private ConstantConditionRuleHelper $helper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, #[AutowiredParameter] private bool $treatPhpDocTypesAsCertain, #[AutowiredParameter(ref: '%tips.treatPhpDocTypesAsCertain%')] @@ -76,18 +77,20 @@ public function processNode( $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder { if (!$this->treatPhpDocTypesAsCertain) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->cond, $ruleErrorBuilder); } $booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->cond); if ($booleanNativeType instanceof ConstantBooleanType) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->cond, $ruleErrorBuilder); } if (!$this->treatPhpDocTypesAsCertainTip) { - return $ruleErrorBuilder; + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->cond, $ruleErrorBuilder); } - return $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + $ruleErrorBuilder = $ruleErrorBuilder->treatPhpDocTypesAsCertainTip(); + + return $this->possiblyImpureTipHelper->addTip($scope, $originalNode->cond, $ruleErrorBuilder); }; return [ diff --git a/src/Rules/RuleErrorBuilder.php b/src/Rules/RuleErrorBuilder.php index c8547fa4fd..760f086985 100644 --- a/src/Rules/RuleErrorBuilder.php +++ b/src/Rules/RuleErrorBuilder.php @@ -227,6 +227,25 @@ public function treatPhpDocTypesAsCertainTip(): self return $this->tip('Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.'); } + /** + * @param list $callDescriptions + * @phpstan-this-out self + * @return self + */ + public function possiblyImpureTip(array $callDescriptions): self + { + foreach ($callDescriptions as $callDescription) { + $this->addTip(sprintf('If %s is impure, add @phpstan-impure PHPDoc tag above its declaration.', $callDescription)); + } + + return $this; + } + + public function hasTips(): bool + { + return count($this->tips) > 0; + } + /** * Sets an error identifier. * diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 9ce1c5004f..524a5cba12 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -119,6 +119,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver [], self::getContainer()->getParameter('exceptions')['implicitThrows'], $this->shouldTreatPhpDocTypesAsCertain(), + self::getContainer()->getParameter('rememberPossiblyImpureFunctionValues'), ); } diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index b90da3b1c8..7dcecc1b26 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -94,6 +94,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver static::getEarlyTerminatingFunctionCalls(), $container->getParameter('exceptions')['implicitThrows'], $container->getParameter('treatPhpDocTypesAsCertain'), + $container->getParameter('rememberPossiblyImpureFunctionValues'), ); } diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 5cd268dfea..d4887caae6 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -835,6 +835,7 @@ private function createAnalyser(): Analyser [], true, $this->shouldTreatPhpDocTypesAsCertain(), + true, ); $lexer = new Lexer(); $fileAnalyser = new FileAnalyser( diff --git a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php index 5f0fe26c4a..055471055e 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php @@ -139,6 +139,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver [], self::getContainer()->getParameter('exceptions')['implicitThrows'], $this->shouldTreatPhpDocTypesAsCertain(), + self::getContainer()->getParameter('rememberPossiblyImpureFunctionValues'), ); } diff --git a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php index c47ffc5830..d528371443 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php @@ -72,6 +72,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver static::getEarlyTerminatingFunctionCalls(), $container->getParameter('exceptions')['implicitThrows'], $container->getParameter('treatPhpDocTypesAsCertain'), + $container->getParameter('rememberPossiblyImpureFunctionValues'), ); } diff --git a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php index aa55dbe441..b332d01adb 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php @@ -28,6 +28,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php index 8f2e2ea4d1..74bf802f0a 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php @@ -28,6 +28,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php index 1006feee88..55cdc0c4e1 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php @@ -29,6 +29,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php index e1a9a3db03..bb6aa370ac 100644 --- a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php @@ -22,6 +22,7 @@ class ConstantLooseComparisonRuleTest extends RuleTestCase protected function getRule(): Rule { return new ConstantLooseComparisonRule( + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php index 519413d54e..b582e494d7 100644 --- a/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/DoWhileLoopConstantConditionRuleTest.php @@ -23,6 +23,7 @@ protected function getRule(): Rule ), $this->shouldTreatPhpDocTypesAsCertain(), ), + new PossiblyImpureTipHelper(true), $this->shouldTreatPhpDocTypesAsCertain(), true, ); diff --git a/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php index 9eec3deec1..cd927d3d58 100644 --- a/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php @@ -29,6 +29,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php index 2bf3017098..abfaa57fe8 100644 --- a/tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php @@ -26,6 +26,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, true, ); diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index b6f79eeeeb..6b484a1385 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -31,6 +31,7 @@ protected function getRule(): Rule [stdClass::class], $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeGenericOverwriteRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeGenericOverwriteRuleTest.php index aff6f8d5df..0190415a98 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeGenericOverwriteRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeGenericOverwriteRuleTest.php @@ -20,6 +20,7 @@ public function getRule(): Rule [], true, ), + new PossiblyImpureTipHelper(true), true, false, true, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php index 2eec07890a..178b414895 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleEqualsTest.php @@ -20,6 +20,7 @@ public function getRule(): Rule [], true, ), + new PossiblyImpureTipHelper(true), true, false, true, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php index fb64511aff..885764f45c 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php @@ -26,6 +26,7 @@ public function getRule(): Rule [], $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php index 8a68085379..96134cbb28 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php @@ -26,6 +26,7 @@ public function getRule(): Rule [], $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, diff --git a/tests/PHPStan/Rules/Comparison/LogicalXorConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/LogicalXorConstantConditionRuleTest.php index 7f9146b058..14b97daaca 100644 --- a/tests/PHPStan/Rules/Comparison/LogicalXorConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/LogicalXorConstantConditionRuleTest.php @@ -23,6 +23,7 @@ protected function getRule(): TRule ), $this->shouldTreatPhpDocTypesAsCertain(), ), + new PossiblyImpureTipHelper(true), $this->shouldTreatPhpDocTypesAsCertain(), false, true, diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index 5ac746cd88..cde92ceee8 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -26,6 +26,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, ); } diff --git a/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php index 818bed70ae..3d8320a53a 100644 --- a/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php @@ -20,6 +20,7 @@ class NumberComparisonOperatorsConstantConditionRuleTest extends RuleTestCase protected function getRule(): Rule { return new NumberComparisonOperatorsConstantConditionRule( + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, true, ); diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 00fd8dc8a4..29f1b8d2de 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -24,6 +24,7 @@ protected function getRule(): Rule { return new StrictComparisonOfDifferentTypesRule( self::getContainer()->getByType(RicherScopeGetTypeHelper::class), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition, true, @@ -1056,4 +1057,84 @@ public function testBug11609(): void ]); } + public function testPossiblyImpureTip(): void + { + $impureTipFunction = 'If PossiblyImpureTip\maybeImpureFunction() is impure, add @phpstan-impure PHPDoc tag above its declaration.'; + $impureTipMethod = 'If PossiblyImpureTip\MethodCallTest::maybeImpureMethod() is impure, add @phpstan-impure PHPDoc tag above its declaration.'; + $impureTipStatic = 'If PossiblyImpureTip\StaticCallTest::maybeImpureStatic() is impure, add @phpstan-impure PHPDoc tag above its declaration.'; + $impureTipIntermediate = 'If PossiblyImpureTip\ObjectInvalidationTest::maybeImpureIntermediate() is impure, add @phpstan-impure PHPDoc tag above its declaration.'; + $this->analyse([__DIR__ . '/data/possibly-impure-tip.php'], [ + // Function calls: maybe-impure (tip expected) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 34, + $impureTipFunction, + ], + // Function calls: @phpstan-pure (no tip) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 40, + ], + // Function calls: @phpstan-impure - no error at all (value not remembered) + // Function calls: void - cannot appear in === comparison + + // Method calls: maybe-impure (tip expected) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 85, + $impureTipMethod, + ], + // Method calls: @phpstan-pure (no tip) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 94, + ], + // Method calls: @phpstan-impure - no error at all (value not remembered) + // Method calls: void - return type explains the error (no tip) + [ + 'Strict comparison using === between null and null will always evaluate to true.', + 114, + ], + + // Static method calls: maybe-impure (tip expected) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 156, + $impureTipStatic, + ], + // Static method calls: @phpstan-pure (no tip) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 165, + ], + // Static method calls: @phpstan-impure - no error at all (value not remembered) + // Static method calls: void - hasSideEffects()->yes() invalidates + + // Object invalidation: maybe-impure intermediate (tip expected) + // getValue() is @phpstan-pure, intermediate is maybe-impure + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 233, + $impureTipIntermediate, + ], + // Object invalidation: @phpstan-pure intermediate (no tip) + [ + 'Strict comparison using === between 1 and 2 will always evaluate to false.', + 244, + ], + // Object invalidation: @phpstan-impure intermediate - no error ($this invalidated) + // Object invalidation: void intermediate - no error ($this invalidated) + + // No tip when return type alone explains the error + [ + 'Strict comparison using === between string and null will always evaluate to false.', + 287, + ], + [ + 'Strict comparison using !== between string and null will always evaluate to true.', + 291, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php index 250f639068..300484a89b 100644 --- a/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/TernaryOperatorConstantConditionRuleTest.php @@ -25,6 +25,7 @@ protected function getRule(): Rule ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), $this->treatPhpDocTypesAsCertain, true, ); diff --git a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysFalseConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysFalseConditionRuleTest.php index c434368b47..e84fdb4d56 100644 --- a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysFalseConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysFalseConditionRuleTest.php @@ -23,6 +23,7 @@ protected function getRule(): Rule ), $this->shouldTreatPhpDocTypesAsCertain(), ), + new PossiblyImpureTipHelper(true), $this->shouldTreatPhpDocTypesAsCertain(), true, ); diff --git a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php index b0fe019bd4..f5e983dcbd 100644 --- a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php @@ -24,6 +24,7 @@ protected function getRule(): Rule ), $this->shouldTreatPhpDocTypesAsCertain(), ), + new PossiblyImpureTipHelper(true), $this->shouldTreatPhpDocTypesAsCertain(), true, ); diff --git a/tests/PHPStan/Rules/Comparison/data/possibly-impure-tip.php b/tests/PHPStan/Rules/Comparison/data/possibly-impure-tip.php new file mode 100644 index 0000000000..9a975bb807 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/possibly-impure-tip.php @@ -0,0 +1,295 @@ +no()) + if (pureFunction() === 1) { + if (pureFunction() === 2) { // always false, no tip because function is pure + } + } + + // impure function: no error (value not remembered because hasSideEffects()->yes()) + if (impureFunction() === 1) { + if (impureFunction() === 1) { // no error, impure invalidates + } + } + + // void function: hasSideEffects()->yes(), can't appear in === comparisons +} + +// --- Method calls --- + +class MethodCallTest +{ + + public function maybeImpureMethod(): int + { + return rand(1, 100); + } + + /** @phpstan-pure */ + public function pureMethod(): int + { + return 42; + } + + /** @phpstan-impure */ + public function impureMethod(): int + { + echo 'hello'; + return rand(1, 100); + } + + public function voidMethod(): void + { + echo 'hello'; + } + + public function testMaybeImpureMethod(): void + { + // maybe-impure method: tip should appear + if ($this->maybeImpureMethod() === 1) { + if ($this->maybeImpureMethod() === 2) { // always false, tip expected + } + } + } + + public function testPureMethod(): void + { + // pure method: error occurs but no impure tip (hasSideEffects()->no()) + if ($this->pureMethod() === 1) { + if ($this->pureMethod() === 2) { // always false, no tip because method is pure + } + } + } + + public function testImpureMethod(): void + { + // impure method: hasSideEffects()->yes() invalidates $this + // so no "always true/false" error occurs at all + if ($this->impureMethod() === 1) { + if ($this->impureMethod() === 1) { + // Not "always true" because impure invalidates + } + } + } + + public function testVoidMethod(): void + { + // void method: hasSideEffects()->yes() invalidates $this + // so no "always true/false" from strict comparison occurs + if ($this->voidMethod() === null) { + } + if ($this->maybeImpureMethod() === 1) { + // voidMethod() invalidated $this, so maybeImpureMethod() + // is evaluated fresh + } + } + +} + +// --- Static method calls --- + +class StaticCallTest +{ + + public static function maybeImpureStatic(): int + { + return rand(1, 100); + } + + /** @phpstan-pure */ + public static function pureStatic(): int + { + return 42; + } + + /** @phpstan-impure */ + public static function impureStatic(): int + { + echo 'hello'; + return rand(1, 100); + } + + public static function voidStatic(): void + { + echo 'hello'; + } + + public function testMaybeImpureStatic(): void + { + // maybe-impure static method: tip should appear + if (self::maybeImpureStatic() === 1) { + if (self::maybeImpureStatic() === 2) { // always false, tip expected + } + } + } + + public function testPureStatic(): void + { + // pure static method: error occurs but no impure tip (hasSideEffects()->no()) + if (self::pureStatic() === 1) { + if (self::pureStatic() === 2) { // always false, no tip because method is pure + } + } + } + + public function testImpureStatic(): void + { + // impure static method: hasSideEffects()->yes() invalidates $this + // so no "always true/false" error occurs at all + if (self::impureStatic() === 1) { + if (self::impureStatic() === 1) { + // Not "always true" because impure invalidates $this + } + } + } + + public function testVoidStatic(): void + { + // void static method: hasSideEffects()->yes() invalidates $this + // so any previously-tracked maybe-impure static call is cleared + self::voidStatic(); + if (self::maybeImpureStatic() === 1) { + // voidStatic() invalidated $this + } + } + +} + +// --- Object not invalidated by maybe-impure intermediate call --- + +class ObjectInvalidationTest +{ + + /** @phpstan-pure */ + public function getValue(): int + { + return 42; + } + + public function maybeImpureIntermediate(): int + { + return rand(1, 100); + } + + /** @phpstan-pure */ + public function pureIntermediate(): int + { + return 42; + } + + /** @phpstan-impure */ + public function impureIntermediate(): int + { + echo 'hello'; + return rand(1, 100); + } + + public function voidIntermediate(): void + { + echo 'hello'; + } + + public function testMaybeImpureIntermediate(): void + { + // getValue() narrowed to 1, maybeImpureIntermediate() doesn't invalidate $this + // tip should point to maybeImpureIntermediate() + if ($this->getValue() === 1) { + $this->maybeImpureIntermediate(); + if ($this->getValue() === 2) { // always false, tip for maybeImpureIntermediate() + } + } + } + + public function testPureIntermediate(): void + { + // getValue() narrowed to 1, pureIntermediate() doesn't invalidate $this + // no tip because pureIntermediate() is @phpstan-pure + if ($this->getValue() === 1) { + $this->pureIntermediate(); + if ($this->getValue() === 2) { // always false, no tip + } + } + } + + public function testImpureIntermediate(): void + { + // getValue() narrowed to 1, impureIntermediate() invalidates $this + // no error because $this is invalidated + if ($this->getValue() === 1) { + $this->impureIntermediate(); + if ($this->getValue() === 2) { // no error, $this invalidated + } + } + } + + public function testVoidIntermediate(): void + { + // getValue() narrowed to 1, voidIntermediate() invalidates $this + // no error because $this is invalidated + if ($this->getValue() === 1) { + $this->voidIntermediate(); + if ($this->getValue() === 2) { // no error, $this invalidated + } + } + } + +} + +// --- No tip when return type alone explains the error --- + +class NoTipWhenReturnTypeExplains +{ + + public function returnsString(): string + { + return 'foo'; + } + + public function test(): void + { + // returnsString() always returns string, so string === null + // is always false regardless of purity. No tip needed. + if ($this->returnsString() === null) { + } + + // string !== null is always true regardless of purity. + if ($this->returnsString() !== null) { + } + } + +} From dfe79f7699b4e83b8d445cd1ecd4a240a5e5478a Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Mon, 23 Feb 2026 08:25:56 +0000 Subject: [PATCH 2/4] Address review comments for possibly-impure tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Carry declared return type as expression type in assignExpression() instead of as a PossiblyImpureCallExpr constructor parameter - Remove unnecessary hasTips() check on new error builder in MatchExpressionRule - Remove hasTips condition in PossiblyImpureTipHelper and remove hasTips() method from RuleErrorBuilder Co-authored-by: Ondřej Mirtes --- src/Analyser/MutatingScope.php | 2 +- src/Analyser/NodeScopeResolver.php | 12 ++++++------ src/Node/Expr/PossiblyImpureCallExpr.php | 7 ------- src/Rules/Comparison/MatchExpressionRule.php | 6 ++---- src/Rules/Comparison/PossiblyImpureTipHelper.php | 3 --- src/Rules/RuleErrorBuilder.php | 5 ----- 6 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 02c8f40713..d3499fd45e 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -815,7 +815,7 @@ public function findPossiblyImpureCallDescriptions(Expr $expr): array // narrowing affected the type (the cached value was narrowed). assert($found instanceof Expr); $scopeType = $this->getType($found); - $declaredReturnType = $holderExpr->getDeclaredReturnType(); + $declaredReturnType = $holder->getType(); if ($declaredReturnType->isSuperTypeOf($scopeType)->yes() && $scopeType->isSuperTypeOf($declaredReturnType)->yes()) { continue; } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 7fbec1a825..6d56f6bcd7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2929,8 +2929,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto && $parametersAcceptor !== null ) { $scope = $scope->assignExpression( - new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName()), $parametersAcceptor->getReturnType()), - new MixedType(), + new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName())), + $parametersAcceptor->getReturnType(), new MixedType(), ); } @@ -3235,8 +3235,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass()); } elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) { $scope = $scope->assignExpression( - new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()), - new MixedType(), + new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())), + $parametersAcceptor->getReturnType(), new MixedType(), ); } @@ -3459,8 +3459,8 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) ) { $scope = $scope->assignExpression( - new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName()), $parametersAcceptor->getReturnType()), - new MixedType(), + new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())), + $parametersAcceptor->getReturnType(), new MixedType(), ); } diff --git a/src/Node/Expr/PossiblyImpureCallExpr.php b/src/Node/Expr/PossiblyImpureCallExpr.php index bf61d64e7b..8463238246 100644 --- a/src/Node/Expr/PossiblyImpureCallExpr.php +++ b/src/Node/Expr/PossiblyImpureCallExpr.php @@ -5,7 +5,6 @@ use Override; use PhpParser\Node\Expr; use PHPStan\Node\VirtualNode; -use PHPStan\Type\Type; final class PossiblyImpureCallExpr extends Expr implements VirtualNode { @@ -14,7 +13,6 @@ public function __construct( public Expr $callExpr, public Expr $impactedExpr, private string $callDescription, - private Type $declaredReturnType, ) { parent::__construct([]); @@ -25,11 +23,6 @@ public function getCallDescription(): string return $this->callDescription; } - public function getDeclaredReturnType(): Type - { - return $this->declaredReturnType; - } - #[Override] public function getType(): string { diff --git a/src/Rules/Comparison/MatchExpressionRule.php b/src/Rules/Comparison/MatchExpressionRule.php index c922a1ae98..665b7af1e3 100644 --- a/src/Rules/Comparison/MatchExpressionRule.php +++ b/src/Rules/Comparison/MatchExpressionRule.php @@ -117,10 +117,8 @@ public function processNode(Node $node, Scope $scope): array ); $errorBuilder = RuleErrorBuilder::message($message) ->line($armLine) - ->identifier('match.alwaysTrue'); - if (!$errorBuilder->hasTips()) { - $errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.'); - } + ->identifier('match.alwaysTrue') + ->tip('Remove remaining cases below this one and this error will disappear too.'); $this->possiblyImpureTipHelper->addTip($armConditionScope, $armConditionExpr, $errorBuilder); $errors[] = $errorBuilder->build(); } diff --git a/src/Rules/Comparison/PossiblyImpureTipHelper.php b/src/Rules/Comparison/PossiblyImpureTipHelper.php index 25fcaccb9c..9813a56a97 100644 --- a/src/Rules/Comparison/PossiblyImpureTipHelper.php +++ b/src/Rules/Comparison/PossiblyImpureTipHelper.php @@ -30,9 +30,6 @@ public function addTip( if (!$this->possiblyImpureTip) { return $ruleErrorBuilder; } - if ($ruleErrorBuilder->hasTips()) { - return $ruleErrorBuilder; - } if (!$scope instanceof MutatingScope) { return $ruleErrorBuilder; } diff --git a/src/Rules/RuleErrorBuilder.php b/src/Rules/RuleErrorBuilder.php index 760f086985..9323d64b9e 100644 --- a/src/Rules/RuleErrorBuilder.php +++ b/src/Rules/RuleErrorBuilder.php @@ -241,11 +241,6 @@ public function possiblyImpureTip(array $callDescriptions): self return $this; } - public function hasTips(): bool - { - return count($this->tips) > 0; - } - /** * Sets an error identifier. * From 9ad690016e74a890411c369220b4baf14a95b4d6 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Mon, 23 Feb 2026 08:48:42 +0000 Subject: [PATCH 3/4] Fix CS, PHPStan, and test failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add missing `use function assert;` import in MutatingScope.php - Add @template generic type annotations to PossiblyImpureTipHelper::addTip() - Update ElseIfConstantConditionRuleTest::testBug6947 expected tip to include possibly-impure tip alongside treatPhpDocTypesAsCertain tip - Fix native-types-81.php: closures with `: string` return type now correctly resolve to native type `string` instead of `non-empty-string` (the previous `non-empty-string` was due to a closure type cache key collision between regular and native-promoted scopes) Co-authored-by: Ondřej Mirtes --- src/Analyser/MutatingScope.php | 1 + src/Rules/Comparison/PossiblyImpureTipHelper.php | 6 ++++++ tests/PHPStan/Analyser/nsrt/native-types-81.php | 4 ++-- .../Rules/Comparison/ElseIfConstantConditionRuleTest.php | 2 +- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index d3499fd45e..3bfdcd425f 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -145,6 +145,7 @@ use Throwable; use ValueError; use function abs; +use function assert; use function array_filter; use function array_key_exists; use function array_key_first; diff --git a/src/Rules/Comparison/PossiblyImpureTipHelper.php b/src/Rules/Comparison/PossiblyImpureTipHelper.php index 9813a56a97..cf65c78384 100644 --- a/src/Rules/Comparison/PossiblyImpureTipHelper.php +++ b/src/Rules/Comparison/PossiblyImpureTipHelper.php @@ -7,6 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use function count; @@ -21,6 +22,11 @@ public function __construct( { } + /** + * @template T of RuleError + * @param RuleErrorBuilder $ruleErrorBuilder + * @return RuleErrorBuilder + */ public function addTip( Scope $scope, Expr $conditionExpr, diff --git a/tests/PHPStan/Analyser/nsrt/native-types-81.php b/tests/PHPStan/Analyser/nsrt/native-types-81.php index fb1b96e979..80f62536c5 100644 --- a/tests/PHPStan/Analyser/nsrt/native-types-81.php +++ b/tests/PHPStan/Analyser/nsrt/native-types-81.php @@ -25,12 +25,12 @@ public function doFoo(): void }; assertType('non-empty-string', $f()); - assertNativeType('non-empty-string', $f()); + assertNativeType('string', $f()); assertType('non-empty-string', (function (): string { return funcWithANativeReturnType(); })()); - assertNativeType('non-empty-string', (function (): string { + assertNativeType('string', (function (): string { return funcWithANativeReturnType(); })()); diff --git a/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php index cd927d3d58..54c3d99474 100644 --- a/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ElseIfConstantConditionRuleTest.php @@ -147,7 +147,7 @@ public function testBug6947(): void [ 'Elseif condition is always false.', 13, - 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.', + "• Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.\n• If Bug6947\HelloWorld::getValue() is impure, add @phpstan-impure PHPDoc tag above its declaration.", ], ]); } From 4f0994ea8f5b5d47c6b5256dbb881fde6af7d78b Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Mon, 23 Feb 2026 09:08:25 +0000 Subject: [PATCH 4/4] Address review: reorder conditions and fix closure cache key collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move $parametersAcceptor !== null check before hasSideEffects()->maybe() for function calls - Move $scope->isInClass() and getClassReflection()->is() checks before hasSideEffects()->maybe() for static method calls - Fix closure type cache key collision between PHPDoc-aware and native scopes by including nativeTypesPromoted flag in getClosureScopeCacheKey() - Revert native-types-81.php test expectations back to original values Co-authored-by: Ondřej Mirtes --- src/Analyser/MutatingScope.php | 4 ++++ src/Analyser/NodeScopeResolver.php | 6 +++--- tests/PHPStan/Analyser/nsrt/native-types-81.php | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 3bfdcd425f..10aea95d8b 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1050,6 +1050,10 @@ private function getClosureScopeCacheKey(): string $parts[] = sprintf(',%s', $parameter->getType()->describe(VerbosityLevel::cache())); } + if ($this->nativeTypesPromoted) { + $parts[] = '::native'; + } + return md5(implode("\n", $parts)); } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 6d56f6bcd7..0f158d7288 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2924,9 +2924,9 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto if ( $functionReflection !== null && $this->rememberPossiblyImpureFunctionValues + && $parametersAcceptor !== null && $functionReflection->hasSideEffects()->maybe() && !$functionReflection->isBuiltin() - && $parametersAcceptor !== null ) { $scope = $scope->assignExpression( new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, sprintf('%s()', $functionReflection->getName())), @@ -3452,11 +3452,11 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto } elseif ( $methodReflection !== null && $this->rememberPossiblyImpureFunctionValues - && $methodReflection->hasSideEffects()->maybe() - && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null && $scope->isInClass() && $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName()) + && $methodReflection->hasSideEffects()->maybe() + && !$methodReflection->getDeclaringClass()->isBuiltin() ) { $scope = $scope->assignExpression( new PossiblyImpureCallExpr($normalizedExpr, new Variable('this'), sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())), diff --git a/tests/PHPStan/Analyser/nsrt/native-types-81.php b/tests/PHPStan/Analyser/nsrt/native-types-81.php index 80f62536c5..fb1b96e979 100644 --- a/tests/PHPStan/Analyser/nsrt/native-types-81.php +++ b/tests/PHPStan/Analyser/nsrt/native-types-81.php @@ -25,12 +25,12 @@ public function doFoo(): void }; assertType('non-empty-string', $f()); - assertNativeType('string', $f()); + assertNativeType('non-empty-string', $f()); assertType('non-empty-string', (function (): string { return funcWithANativeReturnType(); })()); - assertNativeType('string', (function (): string { + assertNativeType('non-empty-string', (function (): string { return funcWithANativeReturnType(); })());