danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-22 13:51:54 +01:00
Code Issues Projects Releases Wiki Activity

Merge pull request #10733 from kkmuffme/fix-false-positive-riskytruthyfalsycomparison

Browse Source
This commit is contained in:
Bruce Weirdan 2024-02-21 07:03:07 -04:00 committed by GitHub
commit c401490265
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 86 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/running_psalm/issues/RiskyTruthyFalsyComparison.md
View File

@ -1,6 +1,6 @@
# RiskyTruthyFalsyComparison
Emitted when comparing a value with multiple types that can both contain truthy and falsy values.
Emitted when comparing a value with multiple types, where at least one type can be only truthy or falsy and other types can contain both truthy and falsy values.
```php
<?php
@ -22,7 +22,7 @@ function foo($arg) {
## Why this is bad
The truthy/falsy type of one variable is often forgotten and not handled explicitly causing hard to track down errors.
The truthy/falsy type of a variable is often forgotten and not handled explicitly causing hard to track down errors.
## How to fix

4
src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
View File

@ -372,16 +372,18 @@ final class IfConditionalAnalyzer
&& !($stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical)
&& !($stmt instanceof PhpParser\Node\Expr\BooleanNot)) {
if (count($type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}
if (count($both_types->getAtomicTypes()) > 0) {
if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(

4
src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php
View File

@ -46,16 +46,18 @@ final class BooleanNotAnalyzer
$stmt_type = new TTrue($expr_type->from_docblock);
} else {
if (count($expr_type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $expr_type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}
if (count($both_types->getAtomicTypes()) > 0) {
if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(

4
src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php
View File

@ -65,16 +65,18 @@ final class EmptyAnalyzer
$stmt_type = new TTrue($expr_type->from_docblock);
} else {
if (count($expr_type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $expr_type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}
if (count($both_types->getAtomicTypes()) > 0) {
if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(

131
tests/TypeReconciliation/ConditionalTest.php
View File

@ -41,28 +41,28 @@ class ConditionalTest extends TestCase
'nonStrictConditionTruthyFalsyNoOverlap' => [
'code' => '<?php
/**
* @param non-empty-array|null $arg
* @return void
*/
function foo($arg) {
if ($arg) {
}
* @param non-empty-array|null $arg
* @return void
*/
function foo($arg) {
if ($arg) {
}
if (!$arg) {
}
if (!$arg) {
}
if (bar($arg)) {
}
if (bar($arg)) {
}
if (!bar($arg)) {
}
}
if (!bar($arg)) {
}
}
/**
* @param mixed $arg
* @return non-empty-array|null
*/
function bar($arg) {}',
/**
* @param mixed $arg
* @return non-empty-array|null
*/
function bar($arg) {}',
],
'typeResolutionFromDocblock' => [
'code' => '<?php
@ -3153,6 +3153,25 @@ class ConditionalTest extends TestCase
'$existing' => 'null|stdClass',
],
],
'nonStrictConditionWithoutExclusiveTruthyFalsyFuncCallNegated' => [
'code' => '<?php
/**
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (!bar($arg)) {
}
}
/**
* @param mixed $arg
* @return float|int
*/
function bar($arg) {}',
'assertions' => [],
'ignored_issues' => ['InvalidReturnType'],
],
];
}
@ -3539,61 +3558,61 @@ class ConditionalTest extends TestCase
'nonStrictConditionTruthyFalsy' => [
'code' => '<?php
/**
* @param array|null $arg
* @return void
*/
function foo($arg) {
if ($arg) {
}
}',
* @param array|null $arg
* @return void
*/
function foo($arg) {
if ($arg) {
}
}',
'error_message' => 'RiskyTruthyFalsyComparison',
],
'nonStrictConditionTruthyFalsyNegated' => [
'code' => '<?php
/**
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (!$arg) {
}
}',
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (!$arg) {
}
}',
'error_message' => 'RiskyTruthyFalsyComparison',
],
'nonStrictConditionTruthyFalsyFuncCall' => [
'code' => '<?php
/**
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (bar($arg)) {
}
}
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (bar($arg)) {
}
}
/**
* @param mixed $arg
* @return array|null
*/
function bar($arg) {}',
/**
* @param mixed $arg
* @return array|null
*/
function bar($arg) {}',
'error_message' => 'RiskyTruthyFalsyComparison',
],
'nonStrictConditionTruthyFalsyFuncCallNegated' => [
'code' => '<?php
/**
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (!bar($arg)) {
}
}
* @param array|null $arg
* @return void
*/
function foo($arg) {
if (!bar($arg)) {
}
}
/**
* @param mixed $arg
* @return array|null
*/
function bar($arg) {}',
/**
* @param mixed $arg
* @return array|null
*/
function bar($arg) {}',
'error_message' => 'RiskyTruthyFalsyComparison',
],
'redundantConditionForNonEmptyString' => [