Skip to content

Commit e821ab7

Browse files
staabmstaabm
authored
Fix false positive of function.alreadyNarrowedType (function call variable assignment)
1 parent c42beff commit e821ab7

11 files changed

+338
-7
lines changed

‎src/Rules/Comparison/ImpossibleCheckTypeHelper.php‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,12 @@ public function findSpecifiedType(
6868
if ($node->isFirstClassCallable()) {
6969
return null;
7070
}
71-
$argsCount = count($node->getArgs());
71+
$args = $node->getArgs();
72+
$argsCount = count($args);
7273
if ($node->name instanceof Node\Name) {
7374
$functionName = strtolower((string) $node->name);
7475
if ($functionName === 'assert' && $argsCount >= 1) {
75-
$arg = $node->getArgs()[0]->value;
76+
$arg = $args[0]->value;
7677
$assertValue = ($this->treatPhpDocTypesAsCertain ? $scope->getType($arg) : $scope->getNativeType($arg))->toBoolean();
7778
$assertValueIsTrue = $assertValue->isTrue()->yes();
7879
if (! $assertValueIsTrue && ! $assertValue->isFalse()->yes()) {
@@ -96,7 +97,7 @@ public function findSpecifiedType(
9697
} elseif ($functionName === 'array_search') {
9798
return null;
9899
} elseif ($functionName === 'in_array' && $argsCount >= 2) {
99-
$haystackArg = $node->getArgs()[1]->value;
100+
$haystackArg = $args[1]->value;
100101
$haystackType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($haystackArg) : $scope->getNativeType($haystackArg));
101102
if ($haystackType instanceof MixedType) {
102103
return null;
@@ -106,12 +107,12 @@ public function findSpecifiedType(
106107
return null;
107108
}
108109

109-
$needleArg = $node->getArgs()[0]->value;
110+
$needleArg = $args[0]->value;
110111
$needleType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($needleArg) : $scope->getNativeType($needleArg));
111112

112113
$isStrictComparison = false;
113114
if ($argsCount >= 3) {
114-
$strictNodeType = $scope->getType($node->getArgs()[2]->value);
115+
$strictNodeType = $scope->getType($args[2]->value);
115116
$isStrictComparison = $strictNodeType->isTrue()->yes();
116117
}
117118

@@ -192,7 +193,7 @@ public function findSpecifiedType(
192193
}
193194
}
194195
} elseif ($functionName === 'method_exists' && $argsCount >= 2) {
195-
$objectArg = $node->getArgs()[0]->value;
196+
$objectArg = $args[0]->value;
196197
$objectType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($objectArg) : $scope->getNativeType($objectArg));
197198

198199
if ($objectType instanceof ConstantStringType
@@ -201,7 +202,7 @@ public function findSpecifiedType(
201202
return false;
202203
}
203204

204-
$methodArg = $node->getArgs()[1]->value;
205+
$methodArg = $args[1]->value;
205206
$methodType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($methodArg) : $scope->getNativeType($methodArg));
206207

207208
if ($methodType instanceof ConstantStringType) {
@@ -278,6 +279,27 @@ public function findSpecifiedType(
278279

279280
$results = [];
280281

282+
$assignedInCallVars = [];
283+
if ($node instanceof Expr\CallLike) {
284+
foreach ($node->getArgs() as $arg) {
285+
$expr = $arg->value;
286+
while ($expr instanceof Expr\Assign) {
287+
$expr = $expr->expr;
288+
}
289+
$assignedExpr = $expr;
290+
291+
$expr = $arg->value;
292+
while ($expr instanceof Expr\Assign) {
293+
$assignedInCallVars[] = new Expr\Assign(
294+
$expr->var,
295+
$assignedExpr,
296+
$expr->getAttributes(),
297+
);
298+
299+
$expr = $expr->expr;
300+
}
301+
}
302+
}
281303
foreach ($sureTypes as $sureType) {
282304
if (self::isSpecified($typeSpecifierScope, $node, $sureType[0])) {
283305
$results[] = TrinaryLogic::createMaybe();
@@ -293,6 +315,14 @@ public function findSpecifiedType(
293315
/** @var Type $resultType */
294316
$resultType = $sureType[1];
295317

318+
foreach ($assignedInCallVars as $assignedInCallVar) {
319+
if ($sureType[0] !== $assignedInCallVar->var) {
320+
continue;
321+
}
322+
323+
$argumentType = $scope->getType($assignedInCallVar->expr);
324+
}
325+
296326
$results[] = $resultType->isSuperTypeOf($argumentType)->result;
297327
}
298328

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,4 +1049,76 @@ public function testBug6788(): void
10491049
$this->analyse([__DIR__ . '/data/bug-6788.php'], []);
10501050
}
10511051

1052+
#[RequiresPhp('>= 8.0')]
1053+
public function testBug13268(): void
1054+
{
1055+
$this->treatPhpDocTypesAsCertain = true;
1056+
$this->analyse([__DIR__ . '/data/bug-13268.php'], []);
1057+
}
1058+
1059+
#[RequiresPhp('>= 8.1')]
1060+
public function testBug12087(): void
1061+
{
1062+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
1063+
1064+
$this->treatPhpDocTypesAsCertain = true;
1065+
$this->analyse([__DIR__ . '/data/bug-12087.php'], [
1066+
[
1067+
'Call to function is_null() with null will always evaluate to true.',
1068+
14,
1069+
$tipText,
1070+
],
1071+
]);
1072+
}
1073+
1074+
#[RequiresPhp('>= 8.1')]
1075+
public function testBug12087c(): void
1076+
{
1077+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
1078+
1079+
$this->treatPhpDocTypesAsCertain = true;
1080+
$this->analyse([__DIR__ . '/data/bug-12087c.php'], [
1081+
[
1082+
'Call to function is_null() with null will always evaluate to true.',
1083+
17,
1084+
$tipText,
1085+
],
1086+
[
1087+
'Call to function is_null() with 10 will always evaluate to false.',
1088+
23,
1089+
],
1090+
[
1091+
'Call to function is_null() with null will always evaluate to true.',
1092+
29,
1093+
],
1094+
]);
1095+
}
1096+
1097+
public function testBug9666(): void
1098+
{
1099+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
1100+
1101+
$this->treatPhpDocTypesAsCertain = true;
1102+
$this->analyse([__DIR__ . '/data/bug-9666.php'], [
1103+
[
1104+
'Call to function is_bool() with bool will always evaluate to true.',
1105+
20,
1106+
$tipText,
1107+
],
1108+
]);
1109+
}
1110+
1111+
#[RequiresPhp('>= 8.0')]
1112+
public function testBug9445(): void
1113+
{
1114+
$this->treatPhpDocTypesAsCertain = true;
1115+
$this->analyse([__DIR__ . '/data/bug-9445.php'], []);
1116+
}
1117+
1118+
public function testBug7773(): void
1119+
{
1120+
$this->treatPhpDocTypesAsCertain = true;
1121+
$this->analyse([__DIR__ . '/data/bug-7773.php'], []);
1122+
}
1123+
10521124
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,21 @@ public function testBug12473(): void
275275
]);
276276
}
277277

278+
#[RequiresPhp('>= 8.1')]
279+
public function testBug12087b(): void
280+
{
281+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
282+
283+
$this->treatPhpDocTypesAsCertain = true;
284+
$this->analyse([__DIR__ . '/data/bug-12087b.php'], [
285+
[
286+
'Call to method Bug12087b\MyAssert::is_null() with null will always evaluate to true.',
287+
37,
288+
$tipText,
289+
],
290+
]);
291+
}
292+
278293
public static function getAdditionalConfigFiles(): array
279294
{
280295
return [

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
77
use PHPUnit\Framework\Attributes\DataProvider;
8+
use PHPUnit\Framework\Attributes\RequiresPhp;
89

910
/**
1011
* @extends RuleTestCase<ImpossibleCheckTypeStaticMethodCallRule>
@@ -145,6 +146,21 @@ public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLast
145146
$this->analyse([__DIR__ . '/data/impossible-static-method-report-always-true-last-condition.php'], $expectedErrors);
146147
}
147148

149+
#[RequiresPhp('>= 8.1')]
150+
public function testBug12087b(): void
151+
{
152+
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
153+
154+
$this->treatPhpDocTypesAsCertain = true;
155+
$this->analyse([__DIR__ . '/data/bug-12087b.php'], [
156+
[
157+
'Call to static method Bug12087b\MyAssert::static_is_null() with null will always evaluate to true.',
158+
31,
159+
$tipText,
160+
],
161+
]);
162+
}
163+
148164
public static function getAdditionalConfigFiles(): array
149165
{
150166
return [

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12087;
4+
5+
enum Button: int
6+
{
7+
case On = 1;
8+
9+
case Off = 0;
10+
}
11+
12+
$value = 10;
13+
14+
is_null($value = Button::tryFrom($value));
15+

‎tests/PHPStan/Rules/Comparison/data/bug-12087b.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12087b;
4+
5+
enum Button: int
6+
{
7+
case On = 1;
8+
9+
case Off = 0;
10+
}
11+
12+
class MyAssert {
13+
/**
14+
* @return ($value is null ? true : false)
15+
*/
16+
static public function static_is_null(mixed $value): bool {
17+
return $value === null;
18+
}
19+
20+
/**
21+
* @return ($value is null ? true : false)
22+
*/
23+
public function is_null(mixed $value): bool {
24+
return $value === null;
25+
}
26+
}
27+
28+
function doFoo(): void {
29+
$value = 10;
30+
31+
MyAssert::static_is_null($value = Button::tryFrom($value));
32+
}
33+
34+
function doBar(MyAssert $assert): void {
35+
$value = 10;
36+
37+
$assert->is_null($value = Button::tryFrom($value));
38+
}

‎tests/PHPStan/Rules/Comparison/data/bug-12087c.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12087c;
4+
5+
enum Button: int
6+
{
7+
case On = 1;
8+
9+
case Off = 0;
10+
}
11+
12+
function doFoo()
13+
{
14+
$foo = 'abc';
15+
$value = 10;
16+
17+
is_null($value = $foo = Button::tryFrom($value));
18+
}
19+
20+
function doFoo2() {
21+
$value = 10;
22+
23+
is_null($value ??= Button::tryFrom($value));
24+
}
25+
26+
function doFoo3() {
27+
$value = null;
28+
29+
is_null($value ??= Button::tryFrom($value));
30+
}
31+

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug13268;
4+
5+
class HelloWorld
6+
{
7+
public function assignInCall(string $string): ?string
8+
{
9+
if (
10+
is_string(
11+
$string = realpath($string)
12+
)
13+
) {
14+
throw new \RuntimeException('Refusing to overwrite existing file: ' . $string);
15+
}
16+
return null;
17+
}
18+
19+
public function dumpToLog(mixed $dumpToLog): ?string
20+
{
21+
if (is_string($dumpToLog)) {
22+
if (file_exists($dumpToLog) || is_string($dumpToLog = realpath($dumpToLog))) {
23+
throw new \RuntimeException('Refusing to overwrite existing file: ' . $dumpToLog);
24+
}
25+
}
26+
return null;
27+
}
28+
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug7773;
4+
5+
class JSONEncodingException extends \Exception
6+
{
7+
}
8+
9+
class JSONDecodingException extends \Exception
10+
{
11+
}
12+
13+
class HelloWorld
14+
{
15+
/**
16+
* Encodes the data as JSON
17+
* @param array<mixed> $data json array
18+
* @return string json string
19+
* @throws JSONEncodingException
20+
*/
21+
public static function JSONEncode(array $data): string
22+
{
23+
if (!is_string($data = json_encode($data)))
24+
throw new JSONEncodingException();
25+
return $data;
26+
}
27+
28+
/**
29+
* Decodes the JSON data as an array
30+
* @param string $data json string
31+
* @return array<mixed> json array
32+
* @throws JSONDecodingException
33+
*/
34+
public static function JSONDecode(string $data): array
35+
{
36+
if (!is_array($data = json_decode($data, true)))
37+
throw new JSONDecodingException();
38+
return $data;
39+
}
40+
}

0 commit comments

Comments
 (0)