Skip to content

Fix false positive of function.alreadyNarrowedType (function call variable assignment) #4237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions src/Rules/Comparison/ImpossibleCheckTypeHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@ public function findSpecifiedType(
if ($node->isFirstClassCallable()) {
return null;
}
$argsCount = count($node->getArgs());
$args = $node->getArgs();
$argsCount = count($args);
if ($node->name instanceof Node\Name) {
$functionName = strtolower((string) $node->name);
if ($functionName === 'assert' && $argsCount >= 1) {
$arg = $node->getArgs()[0]->value;
$arg = $args[0]->value;
$assertValue = ($this->treatPhpDocTypesAsCertain ? $scope->getType($arg) : $scope->getNativeType($arg))->toBoolean();
$assertValueIsTrue = $assertValue->isTrue()->yes();
if (! $assertValueIsTrue && ! $assertValue->isFalse()->yes()) {
Expand All @@ -96,7 +97,7 @@ public function findSpecifiedType(
} elseif ($functionName === 'array_search') {
return null;
} elseif ($functionName === 'in_array' && $argsCount >= 2) {
$haystackArg = $node->getArgs()[1]->value;
$haystackArg = $args[1]->value;
$haystackType = ($this->treatPhpDocTypesAsCertain ? $scope->getType($haystackArg) : $scope->getNativeType($haystackArg));
if ($haystackType instanceof MixedType) {
return null;
Expand All @@ -106,12 +107,12 @@ public function findSpecifiedType(
return null;
}

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

$isStrictComparison = false;
if ($argsCount >= 3) {
$strictNodeType = $scope->getType($node->getArgs()[2]->value);
$strictNodeType = $scope->getType($args[2]->value);
$isStrictComparison = $strictNodeType->isTrue()->yes();
}

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

if ($objectType instanceof ConstantStringType
Expand All @@ -201,7 +202,7 @@ public function findSpecifiedType(
return false;
}

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

if ($methodType instanceof ConstantStringType) {
Expand Down Expand Up @@ -278,6 +279,16 @@ public function findSpecifiedType(

$results = [];

$assignedInCallVars = [];
if ($node instanceof Expr\CallLike) {
foreach ($node->getArgs() as $arg) {
if (!$arg->value instanceof Expr\Assign) {
continue;
}

$assignedInCallVars[] = $arg->value;
}
}
foreach ($sureTypes as $sureType) {
if (self::isSpecified($typeSpecifierScope, $node, $sureType[0])) {
$results[] = TrinaryLogic::createMaybe();
Expand All @@ -293,6 +304,14 @@ public function findSpecifiedType(
/** @var Type $resultType */
$resultType = $sureType[1];

foreach ($assignedInCallVars as $assignedInCallVar) {
if ($sureType[0] !== $assignedInCallVar->var) {
continue;
}

$argumentType = $scope->getType($assignedInCallVar->expr);
}

$results[] = $resultType->isSuperTypeOf($argumentType)->result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1032,4 +1032,53 @@ public function testBug13291(): void
$this->analyse([__DIR__ . '/data/bug-13291.php'], []);
}

#[RequiresPhp('>= 8.0')]
public function testBug13268(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-13268.php'], []);
}

#[RequiresPhp('>= 8.1')]
public function testBug12087(): void
{
$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%</>.';

$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-12087.php'], [
[
'Call to function is_null() with null will always evaluate to true.',
14,
$tipText,
],
]);
}

public function testBug9666(): void
{
$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%</>.';

$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-9666.php'], [
[
'Call to function is_bool() with bool will always evaluate to true.',
20,
$tipText,
],
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug9445(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-9445.php'], []);
}

public function testBug7773(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-7773.php'], []);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,21 @@ public function testBug12473(): void
]);
}

#[RequiresPhp('>= 8.1')]
public function testBug12087b(): void
{
$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%</>.';

$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-12087b.php'], [
[
'Call to method Bug12087b\MyAssert::is_null() with null will always evaluate to true.',
37,
$tipText,
],
]);
}

public static function getAdditionalConfigFiles(): array
{
return [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\RequiresPhp;

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

#[RequiresPhp('>= 8.1')]
public function testBug12087b(): void
{
$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%</>.';

$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-12087b.php'], [
[
'Call to static method Bug12087b\MyAssert::static_is_null() with null will always evaluate to true.',
31,
$tipText,
],
]);
}

public static function getAdditionalConfigFiles(): array
{
return [
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-12087.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php // lint >= 8.1

namespace Bug12087;

enum Button: int
{
case On = 1;

case Off = 0;
}

$value = 10;

is_null($value = Button::tryFrom($value));

38 changes: 38 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-12087b.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php // lint >= 8.1

namespace Bug12087b;

enum Button: int
{
case On = 1;

case Off = 0;
}

class MyAssert {
/**
* @return ($value is null ? true : false)
*/
static public function static_is_null(mixed $value): bool {
return $value === null;
}

/**
* @return ($value is null ? true : false)
*/
public function is_null(mixed $value): bool {
return $value === null;
}
}

function doFoo(): void {
$value = 10;

MyAssert::static_is_null($value = Button::tryFrom($value));
}

function doBar(MyAssert $assert): void {
$value = 10;

$assert->is_null($value = Button::tryFrom($value));
}
28 changes: 28 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-13268.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php declare(strict_types = 1);

namespace Bug13268;

class HelloWorld
{
public function assignInCall(string $string): ?string
{
if (
is_string(
$string = realpath($string)
)
) {
throw new \RuntimeException('Refusing to overwrite existing file: ' . $string);
}
return null;
}

public function dumpToLog(mixed $dumpToLog): ?string
{
if (is_string($dumpToLog)) {
if (file_exists($dumpToLog) || is_string($dumpToLog = realpath($dumpToLog))) {
throw new \RuntimeException('Refusing to overwrite existing file: ' . $dumpToLog);
}
}
return null;
}
}
40 changes: 40 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-7773.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php declare(strict_types=1);

namespace Bug7773;

class JSONEncodingException extends \Exception
{
}

class JSONDecodingException extends \Exception
{
}

class HelloWorld
{
/**
* Encodes the data as JSON
* @param array<mixed> $data json array
* @return string json string
* @throws JSONEncodingException
*/
public static function JSONEncode(array $data): string
{
if (!is_string($data = json_encode($data)))
throw new JSONEncodingException();
return $data;
}

/**
* Decodes the JSON data as an array
* @param string $data json string
* @return array<mixed> json array
* @throws JSONDecodingException
*/
public static function JSONDecode(string $data): array
{
if (!is_array($data = json_decode($data, true)))
throw new JSONDecodingException();
return $data;
}
}
22 changes: 22 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-9445.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php // lint >= 8.0

declare(strict_types=1);

namespace Bug9445;

class Foo
{
public int $id;
public null|self $parent;

public function contains(self $foo): bool
{
do {
if ($this->id === $foo->id) {
return true;
}
} while (!is_null($foo = $foo->parent));

return false;
}
}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-9666.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php declare(strict_types=1);

namespace Bug9666;

class A
{
/**
* @return array<int,bool>
*/
function b()
{
return [];
}
}

function doFoo() {
$a = new A();
$b = $a->b();
$c = null;
if ($b && is_bool($c = reset($b))) {
//
}
}

Loading