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 1/2] 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 8d6096963..601b870c0 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 93d17c3f7..9a2f36602 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php @@ -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( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php index 8dbcca2f9..d96d5d926 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php @@ -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( diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 4d48add4f..66cdeb669 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -3153,6 +3153,25 @@ class ConditionalTest extends TestCase '$existing' => 'null|stdClass', ], ], + 'nonStrictConditionWithoutExclusiveTruthyFalsyFuncCallNegated' => [ + 'code' => ' [], + 'ignored_issues' => ['InvalidReturnType'], + ], ]; } From e482c078a7a9d490808d179fad72332e98585c0a Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 20 Feb 2024 22:16:27 +0100 Subject: [PATCH 2/2] code style whitespace fixes including for unrelated tests --- tests/TypeReconciliation/ConditionalTest.php | 112 +++++++++---------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php index 66cdeb669..321832df7 100644 --- a/tests/TypeReconciliation/ConditionalTest.php +++ b/tests/TypeReconciliation/ConditionalTest.php @@ -41,28 +41,28 @@ class ConditionalTest extends TestCase 'nonStrictConditionTruthyFalsyNoOverlap' => [ 'code' => ' [ 'code' => ' [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyNegated' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyFuncCall' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'nonStrictConditionTruthyFalsyFuncCallNegated' => [ 'code' => ' 'RiskyTruthyFalsyComparison', ], 'redundantConditionForNonEmptyString' => [