From 9d8080096c999cd29cf951e35164a6ae9fd8f3c3 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 20 Feb 2024 22:15:11 +0100 Subject: [PATCH] Fix RiskyTruthyFalsyComparison reporting irrelevant errors when there is no explicit truthy/falsy type --- .../issues/RiskyTruthyFalsyComparison.md | 4 ++-- .../Block/IfConditionalAnalyzer.php | 4 +++- .../Expression/BooleanNotAnalyzer.php | 4 +++- .../Statements/Expression/EmptyAnalyzer.php | 4 +++- tests/TypeReconciliation/ConditionalTest.php | 19 +++++++++++++++++++ 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md b/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md index 8d60969633e..601b870c038 100644 --- a/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md +++ b/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md @@ -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 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( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php index 93d17c3f7f5..9a2f36602c8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php @@ -46,16 +46,18 @@ public static function analyze( $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( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php index 8dbcca2f9bb..d96d5d9267a 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php @@ -65,16 +65,18 @@ public static function analyze( $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( diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 4d48add4f1d..66cdeb669b1 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -3153,6 +3153,25 @@ function foo( $a, $b ) { '$existing' => 'null|stdClass', ], ], + 'nonStrictConditionWithoutExclusiveTruthyFalsyFuncCallNegated' => [ + 'code' => ' [], + 'ignored_issues' => ['InvalidReturnType'], + ], ]; }