Skip to content

Commit 6e85ec9

Browse files
staabmstaabm
authored
Fix "ob_get_*() === false will always evaluate to false" false positive (#6097)
1 parent 6aaa8d1 commit 6e85ec9

4 files changed

Lines changed: 51 additions & 29 deletions

File tree

‎src/Type/Php/OutputBufferingDynamicReturnTypeExtension.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@
88
use PHPStan\DependencyInjection\AutowiredService;
99
use PHPStan\Reflection\FunctionReflection;
1010
use PHPStan\Reflection\ParametersAcceptorSelector;
11-
use PHPStan\Type\Constant\ConstantBooleanType;
1211
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
1312
use PHPStan\Type\IntegerRangeType;
1413
use PHPStan\Type\Type;
15-
use PHPStan\Type\TypeCombinator;
14+
use PHPStan\Type\TypeUtils;
1615
use function in_array;
1716

1817
/**
@@ -48,7 +47,10 @@ public function getTypeFromFunctionCall(
4847

4948
$outputBufferLevelType = $scope->getType(new FuncCall(new Name('ob_get_level'), []));
5049
if (IntegerRangeType::createAllGreaterThanOrEqualTo(1)->isSuperTypeOf($outputBufferLevelType)->yes()) {
51-
return TypeCombinator::remove($defaultReturnType, new ConstantBooleanType(false));
50+
// checking error state return values of ob_* functions is essentially useless
51+
// as this usually means that your system is out of memory and your process is going to die anyway.
52+
// that's why error states oftentimes are not checked.
53+
return TypeUtils::toBenevolentUnion($defaultReturnType);
5254
}
5355

5456
return $defaultReturnType;

‎tests/PHPStan/Analyser/nsrt/output-buffering.php‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ function activeBuffer(): void
1717
{
1818
ob_start();
1919
assertType('int<1, max>', ob_get_level());
20-
assertType('string', ob_get_contents());
21-
assertType('int', ob_get_length());
20+
assertType('(string|false)', ob_get_contents());
21+
assertType('(int|false)', ob_get_length());
2222
}
2323

2424
function obCleanAndFlushKeepBuffer(): void
@@ -27,17 +27,17 @@ function obCleanAndFlushKeepBuffer(): void
2727
assertType('int<1, max>', ob_get_level());
2828
ob_clean();
2929
assertType('int<1, max>', ob_get_level());
30-
assertType('string', ob_get_contents());
30+
assertType('(string|false)', ob_get_contents());
3131
ob_flush();
3232
assertType('int<1, max>', ob_get_level());
33-
assertType('string', ob_get_contents());
33+
assertType('(string|false)', ob_get_contents());
3434
}
3535

3636
function getCleanClosesBuffer(): void
3737
{
3838
ob_start();
3939
assertType('int<1, max>', ob_get_level());
40-
assertType('string', ob_get_clean());
40+
assertType('(string|false)', ob_get_clean());
4141
assertType('int<0, max>', ob_get_level());
4242
assertType('string|false', ob_get_contents());
4343
}
@@ -46,7 +46,7 @@ function getFlushClosesBuffer(): void
4646
{
4747
ob_start();
4848
assertType('int<1, max>', ob_get_level());
49-
assertType('string', ob_get_flush());
49+
assertType('(string|false)', ob_get_flush());
5050
assertType('int<0, max>', ob_get_level());
5151
assertType('string|false', ob_get_contents());
5252
}
@@ -55,7 +55,7 @@ function endCleanClosesBuffer(): void
5555
{
5656
ob_start();
5757
assertType('int<1, max>', ob_get_level());
58-
assertType('string', ob_get_contents());
58+
assertType('(string|false)', ob_get_contents());
5959
ob_end_clean();
6060
assertType('int<0, max>', ob_get_level());
6161
assertType('string|false', ob_get_contents());
@@ -65,7 +65,7 @@ function endFlushClosesBuffer(): void
6565
{
6666
ob_start();
6767
assertType('int<1, max>', ob_get_level());
68-
assertType('string', ob_get_contents());
68+
assertType('(string|false)', ob_get_contents());
6969
ob_end_flush();
7070
assertType('int<0, max>', ob_get_level());
7171
assertType('string|false', ob_get_contents());
@@ -77,10 +77,10 @@ function nested(): void
7777
assertType('int<1, max>', ob_get_level());
7878
ob_start();
7979
assertType('int<2, max>', ob_get_level());
80-
assertType('string', ob_get_contents());
80+
assertType('(string|false)', ob_get_contents());
8181
ob_end_clean();
8282
assertType('int<1, max>', ob_get_level());
83-
assertType('string', ob_get_contents());
83+
assertType('(string|false)', ob_get_contents());
8484
ob_end_clean();
8585
assertType('int<0, max>', ob_get_level());
8686
assertType('string|false', ob_get_contents());
@@ -99,15 +99,15 @@ function fullyQualified(): void
9999
{
100100
\ob_start();
101101
assertType('int<1, max>', ob_get_level());
102-
assertType('string', \ob_get_contents());
103-
assertType('string', ob_get_contents());
102+
assertType('(string|false)', \ob_get_contents());
103+
assertType('(string|false)', ob_get_contents());
104104
}
105105

106106
function levelNarrowedToConstInt(): void
107107
{
108108
if (ob_get_level() === 2) {
109109
assertType('2', ob_get_level());
110-
assertType('string', ob_get_clean());
110+
assertType('(string|false)', ob_get_clean());
111111
// closing call decrements the const-int level, keeping it exact
112112
assertType('1', ob_get_level());
113113
}
@@ -123,18 +123,18 @@ function levelNarrowedToIntRange(): void
123123
{
124124
if (ob_get_level() >= 1) {
125125
assertType('int<1, max>', ob_get_level());
126-
assertType('string', ob_get_clean());
126+
assertType('(string|false)', ob_get_clean());
127127
}
128128
}
129129

130130
function levelNarrowedToUnionInt(): void
131131
{
132132
if (ob_get_level() === 1 || ob_get_level() === 3) {
133133
assertType('1|3', ob_get_level());
134-
assertType('string', ob_get_contents());
134+
assertType('(string|false)', ob_get_contents());
135135
ob_start();
136136
assertType('2|4', ob_get_level());
137-
assertType('string', ob_get_contents());
137+
assertType('(string|false)', ob_get_contents());
138138
}
139139
}
140140

@@ -150,7 +150,7 @@ function levelNarrowedToBoundedIntRange(): void
150150
{
151151
if (ob_get_level() >= 2 && ob_get_level() <= 5) {
152152
assertType('int<2, 5>', ob_get_level());
153-
assertType('string', ob_get_clean());
153+
assertType('(string|false)', ob_get_clean());
154154
// closing call shifts the whole range down, preserving the upper bound
155155
assertType('int<1, 4>', ob_get_level());
156156
}
@@ -180,7 +180,7 @@ function pureCallableKeepsLevel(callable $cb): void
180180
ob_start();
181181
$cb();
182182
assertType('int<1, max>', ob_get_level());
183-
assertType('string', ob_get_clean());
183+
assertType('(string|false)', ob_get_clean());
184184
}
185185

186186
function impureFunctionForgetsLevel(): void
@@ -206,7 +206,7 @@ function pureFunctionKeepsLevel(): void
206206
ob_start();
207207
$x=pureFunction();
208208
assertType('int<1, max>', ob_get_level());
209-
assertType('string', ob_get_clean());
209+
assertType('(string|false)', ob_get_clean());
210210
}
211211

212212
class Service
@@ -245,7 +245,7 @@ function pureMethodKeepsLevel(Service $service): void
245245
ob_start();
246246
$x=$service->pureMethod();
247247
assertType('int<1, max>', ob_get_level());
248-
assertType('string', ob_get_clean());
248+
assertType('(string|false)', ob_get_clean());
249249
}
250250

251251
function impureStaticMethodForgetsLevel(): void
@@ -269,23 +269,23 @@ function arrayMapPureCallbackKeepsLevel(array $a): void
269269
ob_start();
270270
array_map('strtoupper', $a);
271271
assertType('int<1, max>', ob_get_level());
272-
assertType('string', ob_get_clean());
272+
assertType('(string|false)', ob_get_clean());
273273
}
274274

275275
function laterInvokedCallableKeepsLevel(callable $cb): void
276276
{
277277
ob_start();
278278
register_shutdown_function($cb);
279279
assertType('int<1, max>', ob_get_level());
280-
assertType('string', ob_get_clean());
280+
assertType('(string|false)', ob_get_clean());
281281
}
282282

283283
function builtinKeepsLevel(): void
284284
{
285285
ob_start();
286286
printf('hello');
287287
assertType('int<1, max>', ob_get_level());
288-
assertType('string', ob_get_clean());
288+
assertType('(string|false)', ob_get_clean());
289289
}
290290

291291
class WithImpureConstructor
@@ -326,15 +326,15 @@ function pureConstructorKeepsLevel(): void
326326
ob_start();
327327
new WithPureConstructor();
328328
assertType('int<1, max>', ob_get_level());
329-
assertType('string', ob_get_clean());
329+
assertType('(string|false)', ob_get_clean());
330330
}
331331

332332
function noConstructorKeepsLevel(): void
333333
{
334334
ob_start();
335335
new WithoutConstructor();
336336
assertType('int<1, max>', ob_get_level());
337-
assertType('string', ob_get_clean());
337+
assertType('(string|false)', ob_get_clean());
338338
}
339339

340340
/** @param class-string $className */
@@ -351,7 +351,7 @@ function builtinConstructorKeepsLevel(): void
351351
ob_start();
352352
new \ArrayObject();
353353
assertType('int<1, max>', ob_get_level());
354-
assertType('string', ob_get_clean());
354+
assertType('(string|false)', ob_get_clean());
355355
}
356356

357357
function withRequire(): void

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,11 @@ public function testBug14878(): void
12611261
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-14878.php'], []);
12621262
}
12631263

1264+
public function testBug14985(): void
1265+
{
1266+
$this->analyse([__DIR__ . '/data/bug-14985.php'], []);
1267+
}
1268+
12641269
public function testBug14847(): void
12651270
{
12661271
$this->analyse([__DIR__ . '/data/bug-14847.php'], [

‎tests/PHPStan/Rules/Comparison/data/bug-14985.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug14985;
6+
7+
ob_start();
8+
$a = ob_get_clean();
9+
10+
// There is no check whether ob_start() was successful, so the if condition cannot be guaranteed to be always false, as PHPStan claims.
11+
if ($a === false) {
12+
echo "false";
13+
}
14+
15+

0 commit comments

Comments
 (0)