Reference parameter not considered as variable mutation

[sstok]

Bug report

Code snippet that reproduces the problem

https://phpstan.org/r/d14466f0-f81e-4594-8ba9-8d87badb70f6

            $original = "I just wanna tell you how I'm feeling\nGotta make you understand";
            $encrypted = '';
            $decrypted = '';

            if (! @\openssl_public_encrypt($original, $encrypted, $pupKey, OPENSSL_PKCS1_OAEP_PADDING)) {
                throw new Exception('Unable to encrypt data, invalid key provided?');
            }

The openssl_public_encrypt() function sets $decrypted, but PHPStan reports the condition is always true
as it doesn't take-in the fact the the $decrypted variable is mutated as argument.

Removing $decrypted = ''; fixes the issue.

Expected output

No errors!

[ondrejmirtes]

Some implementation details: This is a bug in BooleanOrConstantConditionRule. This rule and similar rules like BooleanAndConstantConditionRule are buggy as they evaluate the right side of || as just $scope->filterByFalseyValue($node->left) but they miss that more can happen on the left side.

NodeScopeResolver does this correctly:

$leftResult = $this->processExprNode($expr->left, $scope, $nodeCallback, $context->enterDeep());
$rightResult = $this->processExprNode($expr->right, $leftResult->getFalseyScope(), $nodeCallback, $context);

The right solution is to introduce a virtual node that will carry the correct scopes to evaluate the expressions against and invoke BooleanOrConstantConditionRule for that node. There are already examples of such nodes under PHPStan\Node namespace, like LiteralArrayNode, that solve similar problems.

[phpstan-bot]

@sstok After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
-14: Result of || is always true.
+No errors

[sstok]

Thank you!

[github-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.