Skip to content

Commit 38f6006

Browse files
committed
Fix phpstan/phpstan#14473: false positive logicalOr.leftAlwaysTrue on repeated builtin function call
When a builtin function with possible side effects (like mysqli_connect()) was assigned with the `or die()` pattern, the conditional expression mechanism stored a narrowed type for the function call expression in the scope. On a subsequent assignment of the same function call, this stale type was retrieved instead of the fresh return type, causing a false positive "left side of or is always true" error. The fix skips creating conditional expressions for FuncCall expressions that refer to builtin functions with possible side effects, since those function calls can return different values each time and the narrowed type should not persist across assignments.
1 parent 58b873f commit 38f6006

3 files changed

Lines changed: 59 additions & 2 deletions

File tree

src/Analyser/ExprHandler/AssignHandler.php

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
use PHPStan\Node\PropertyAssignNode;
4646
use PHPStan\Node\VariableAssignNode;
4747
use PHPStan\Php\PhpVersion;
48+
use PHPStan\Reflection\ReflectionProvider;
4849
use PHPStan\ShouldNotHappenException;
4950
use PHPStan\TrinaryLogic;
5051
use PHPStan\Type\Accessory\AccessoryArrayListType;
@@ -84,6 +85,7 @@ final class AssignHandler implements ExprHandler
8485
public function __construct(
8586
private TypeSpecifier $typeSpecifier,
8687
private PhpVersion $phpVersion,
88+
private ReflectionProvider $reflectionProvider,
8789
)
8890
{
8991
}
@@ -861,10 +863,13 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco
861863
if ($expr->name === $variableName) {
862864
continue;
863865
}
866+
} elseif ($expr instanceof FuncCall) {
867+
if ($this->isBuiltinFunctionCallWithPossibleSideEffects($expr, $scope)) {
868+
continue;
869+
}
864870
} elseif (
865871
!$expr instanceof PropertyFetch
866872
&& !$expr instanceof ArrayDimFetch
867-
&& !$expr instanceof FuncCall
868873
) {
869874
continue;
870875
}
@@ -900,10 +905,13 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $
900905
if ($expr->name === $variableName) {
901906
continue;
902907
}
908+
} elseif ($expr instanceof FuncCall) {
909+
if ($this->isBuiltinFunctionCallWithPossibleSideEffects($expr, $scope)) {
910+
continue;
911+
}
903912
} elseif (
904913
!$expr instanceof PropertyFetch
905914
&& !$expr instanceof ArrayDimFetch
906-
&& !$expr instanceof FuncCall
907915
) {
908916
continue;
909917
}
@@ -924,6 +932,21 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $
924932
return $conditionalExpressions;
925933
}
926934

935+
private function isBuiltinFunctionCallWithPossibleSideEffects(Expr $expr, Scope $scope): bool
936+
{
937+
if (!$expr instanceof FuncCall || !$expr->name instanceof Name) {
938+
return false;
939+
}
940+
941+
if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) {
942+
return false;
943+
}
944+
945+
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
946+
947+
return $functionReflection->isBuiltin() && !$functionReflection->hasSideEffects()->no();
948+
}
949+
927950
/**
928951
* @param list<ArrayDimFetch> $dimFetchStack
929952
*/

tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,4 +384,11 @@ public function testBug10305(): void
384384
]);
385385
}
386386

387+
public function testBug14473(): void
388+
{
389+
$this->treatPhpDocTypesAsCertain = true;
390+
391+
$this->analyse([__DIR__ . '/data/bug-14473.php'], []);
392+
}
393+
387394
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14473;
4+
5+
$link = mysqli_connect('host', 'user', 'pass', 'database') or die('Could not connect: ' . mysqli_connect_error());
6+
7+
// (assume long-running code that can cause the connection to time out)
8+
9+
$link = mysqli_connect('host', 'user', 'pass', 'database') or die('Could not reconnect: ' . mysqli_connect_error());
10+
11+
// okay, so let's make certain that the connection is closed
12+
mysqli_close($link);
13+
14+
$link = mysqli_connect('host', 'user', 'pass', 'database') or die('Could not reconnect: ' . mysqli_connect_error());
15+
16+
// close it and destroy the variable
17+
mysqli_close($link);
18+
unset($link);
19+
20+
$link = mysqli_connect('host', 'user', 'pass', 'database') or die('Could not reconnect: ' . mysqli_connect_error());
21+
22+
// close, destroy ...
23+
mysqli_close($link);
24+
unset($link);
25+
26+
// ... and assign to a different variable
27+
$newLink = mysqli_connect('host', 'user', 'pass', 'database') or die('Could not reconnect: ' . mysqli_connect_error());

0 commit comments

Comments
 (0)