Activity

1. ondrejmirtes

   added

   feature-request

   on Feb 22, 2026
2. ondrejmirtes commented on Feb 22, 2026

   

   ondrejmirtes

   on Feb 22, 2026

   MemberAuthor

   More actions

   Claude's plan:

   Plan: Add @phpstan-impure tips to comparison rules

   Context

   When rememberPossiblyImpureFunctionValues is true (default), PHPStan remembers return values of possibly-impure functions/methods in conditions. This leads to "always true/false" errors that confuse users. Two scenarios:

   Scenario 1 - TypeSpecifier remembers a possibly-impure function's return value:

   if (calculateSomething() > 0) {
       if (calculateSomething() > 0) { /* always true */ }
   }

   Scenario 2 - NodeScopeResolver doesn't invalidate object after maybe-impure method call:

   if ($value->isLocked()) { throw ...; }
   $value->changeIndicatorValueDate($id, $date); // maybe-impure, doesn't invalidate $value
   if ($value->isLocked()) { } // always false

   Users need a tip suggesting @phpstan-impure on the relevant function/method.

   Implementation

   1. New virtual expr node: PossiblyImpureCallExpr

   New file: src/Node/Expr/PossiblyImpureCallExpr.php

   final class PossiblyImpureCallExpr extends Expr implements VirtualNode
   {
       public function __construct(
           public Expr $callExpr,       // The FuncCall/MethodCall/StaticCall
           public Expr $impactedExpr,   // What was affected: the object ($value) for method calls,
                                        // or the call expr itself for function calls
           private string $callDescription // "calculateSomething()" or "Foo::bar()"
       )
   
       public function getSubNodeNames(): array { return ['callExpr', 'impactedExpr']; }
   }

   impactedExpr is always non-null:

   • Scenario 1 (function calls): same as callExpr (the function call itself is what's affected)
   • Scenario 2 (method calls): the object variable ($value)

   Exposing sub-nodes in getSubNodeNames() ensures shouldInvalidateExpression() (via NodeFinder) can find the object variable inside, so the tracking entry is automatically invalidated when the object is invalidated by a definitely-impure call.

   2. Printer method

   File: src/Node/Printer/Printer.php

   protected function pPHPStan_Node_PossiblyImpureCallExpr(PossiblyImpureCallExpr $expr): string
   {
       return sprintf('__phpstanPossiblyImpure(%s, %s)', $this->p($expr->callExpr), $this->p($expr->impactedExpr));
   }

   3. Tracking in NodeScopeResolver

   File: src/Analyser/NodeScopeResolver.php

   Add #[AutowiredParameter] private readonly bool $rememberPossiblyImpureFunctionValues to constructor.

   3a. Method calls (~line 3216): After the hasSideEffects()->yes() invalidation branch, add:

   } elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe()) {
       $desc = sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName());
       $scope = $scope->assignExpression(
           new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, $desc),
           new MixedType(), new MixedType(),
       );
   }

   3b. Static calls (~line 3423): Similar pattern, with impactedExpr = new Variable('this') when the call is on the current class.

   3c. Function calls (~line 2905): After function processing:

   if ($functionReflection !== null && $this->rememberPossiblyImpureFunctionValues
       && $functionReflection->hasSideEffects()->maybe()) {
       $desc = sprintf('%s()', $functionReflection->getName());
       $scope = $scope->assignExpression(
           new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr, $desc),
           new MixedType(), new MixedType(),
       );
   }

   Note: for function calls, impactedExpr = callExpr = the FuncCall expression.

   4. Query method on MutatingScope

   File: src/Analyser/MutatingScope.php

   /** @return list<string> */
   public function findPossiblyImpureCallDescriptions(Expr $expr): array

   Logic: Iterate $this->expressionTypes, find PossiblyImpureCallExpr entries. For each:

   • Scenario 1: Check if tracked callExpr's key matches any sub-node of the error expression (via NodeFinder)
   • Scenario 2: Check if tracked impactedExpr's key matches any sub-node of the error expression

   Return unique descriptions.

   5. Tip on RuleErrorBuilder

   File: src/Rules/RuleErrorBuilder.php

   Add a convenience method:

   public function possiblyImpureTip(array $callDescriptions): self

   For each description, calls addTip("If {description} has side effects, add @phpstan-impure PHPDoc tag above its declaration.").

   6. Configuration

   File: conf/config.neon - Add tips.possiblyImpure: true
   File: conf/parametersSchema.neon - Add possiblyImpure: bool() to tips schema

   7. Common tip helper service

   New file: src/Rules/Comparison/PossiblyImpureTipHelper.php

   Create a shared service that all comparison rules can use:

   #[AutowiredService]
   final class PossiblyImpureTipHelper
   {
       public function __construct(
           #[AutowiredParameter(ref: '%tips.possiblyImpure%')]
           private bool $possiblyImpureTip,
       ) {}
   
       public function addTip(
           Scope $scope,
           Expr $conditionExpr,
           RuleErrorBuilder $ruleErrorBuilder,
       ): RuleErrorBuilder
       {
           if (!$this->possiblyImpureTip) {
               return $ruleErrorBuilder;
           }
           if (!$scope instanceof MutatingScope) {
               return $ruleErrorBuilder;
           }
           $descriptions = $scope->findPossiblyImpureCallDescriptions($conditionExpr);
           if (count($descriptions) === 0) {
               return $ruleErrorBuilder;
           }
           return $ruleErrorBuilder->possiblyImpureTip($descriptions);
       }
   }

   8. Update all comparison rules

   Files: All ~16 rules in src/Rules/Comparison/ (excluding UsageOfVoidMatchExpressionRule)

   Inject PossiblyImpureTipHelper into each rule. In each rule's $addTip closure, call the helper first (highest priority). If the helper adds a tip, return immediately. Otherwise, fall through to existing logic (treatPhpDocTypesAsCertain, reasons, etc.).

   The expression to pass varies by rule:

   • IfConstantConditionRule, ElseIfConstantConditionRule, WhileLoop*, DoWhileLoop*, TernaryOperator*: $node->cond
   • StrictComparisonOfDifferentTypesRule, NumberComparison*, ConstantLooseComparison*: the $node (BinaryOp)
   • BooleanAndConstantConditionRule, BooleanOrConstantConditionRule, LogicalXor*: left/right/overall expression
   • BooleanNotConstantConditionRule: $node->expr
   • ImpossibleCheckType*Rule: the $node (FuncCall/MethodCall/StaticCall)
   • MatchExpressionRule: the arm condition

   Pattern in each $addTip closure:

   $addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder {
       // NEW: Possibly-impure tip has highest priority
       $ruleErrorBuilder = $this->possiblyImpureTipHelper->addTip($scope, $conditionExpr, $ruleErrorBuilder);
       if ($ruleErrorBuilder->hasTips()) {
           return $ruleErrorBuilder;
       }
   
       // Existing logic: reasons, treatPhpDocTypesAsCertain, etc.
       // ...
   };

   Note: May need to add a hasTips() method on RuleErrorBuilder (or just have the helper return a boolean indicating whether it added tips).

   Verification

   1. make tests - run full test suite
   2. Create test data files for both scenarios and verify tips appear in expected errors
   3. Verify tips do NOT appear when:
      • Function is already @phpstan-pure (hasSideEffects -> no)
      • Function is already @phpstan-impure (hasSideEffects -> yes, error doesn't occur)
      • rememberPossiblyImpureFunctionValues: false (error doesn't occur)
   4. Verify the possibly-impure tip takes priority over treatPhpDocTypesAsCertain tip
   5. Verify tracking entries are properly invalidated when a definitely-impure method call happens later

   Key files to modify

   • src/Node/Expr/PossiblyImpureCallExpr.php (new)
   • src/Rules/Comparison/PossiblyImpureTipHelper.php (new)
   • src/Node/Printer/Printer.php
   • src/Analyser/NodeScopeResolver.php
   • src/Analyser/MutatingScope.php
   • src/Rules/RuleErrorBuilder.php
   • conf/config.neon, conf/parametersSchema.neon
   • All 16 rules in src/Rules/Comparison/
   • Test files in tests/PHPStan/Rules/Comparison/data/
3. phpstan-bot commented on Mar 4, 2026

   

   phpstan-bot

   on Mar 4, 2026

   Contributor

   More actions

   Closed via merging phpstan/phpstan-src#5031
4. phpstan-bot

   closed this as completedon Mar 4, 2026
5. github-actions commented on Apr 5, 2026

   

   github-actionsBot

   on Apr 5, 2026 – with GitHub Actions

   More actions

   This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
6. github-actions

   locked as resolved and limited conversation to collaborators on Apr 5, 2026