Fix #10501 - report error for non-strict comparison on truthy+falsy u…

…nion
vimeo · Jan 12, 2024 · 38e80ce · 38e80ce
1 parent 5d1d5a2
commit 38e80ce
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 0 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
@@ -366,6 +366,41 @@ public static function handleParadoxicalCondition(
                         $statements_analyzer->getSuppressedIssues(),
                     );
                 }
+            } elseif (!($stmt instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical)
+                && !($stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical)
+                && !($stmt instanceof PhpParser\Node\Expr\BooleanNot)) {
+                $has_both = false;
+                $both_types = $type->getBuilder();
+                if (count($type->getAtomicTypes()) > 1) {
+                    foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
+                        if ($atomic_type->isTruthy()) {
+                            $both_types->removeType($key);
+                            continue;
+                        }
+
+                        if ($atomic_type->isFalsy()) {
+                            $both_types->removeType($key);
+                            continue;
+                        }
+
+                        $has_both = true;
+                    }
+                }
+
+                if ($has_both) {
+                    $both_types->freeze();
+                    IssueBuffer::maybeAdd(
+                        new TypeDoesNotContainType(
+                            'Operand of type ' . $type->getId() . ' contains ' .
+                            'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
+                            $both_types->getId() . ', which can be falsy and truthy. ' .
+                            'This can cause possibly unexpected behavior. Use strict comparison instead.',
+                            new CodeLocation($statements_analyzer, $stmt),
+                            $type->getId() . ' truthy-falsy',
+                        ),
+                        $statements_analyzer->getSuppressedIssues(),
+                    );
+                }
             }
         }
     }

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php
@@ -3,15 +3,20 @@
 namespace Psalm\Internal\Analyzer\Statements\Expression;
 
 use PhpParser;
+use Psalm\CodeLocation;
 use Psalm\Context;
 use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
+use Psalm\Issue\TypeDoesNotContainType;
+use Psalm\IssueBuffer;
 use Psalm\Type;
 use Psalm\Type\Atomic\TBool;
 use Psalm\Type\Atomic\TFalse;
 use Psalm\Type\Atomic\TTrue;
 use Psalm\Type\Union;
 
+use function count;
+
 /**
  * @internal
  */
@@ -40,6 +45,39 @@ public static function analyze(
             } elseif ($expr_type->isAlwaysFalsy()) {
                 $stmt_type = new TTrue($expr_type->from_docblock);
             } else {
+                $has_both = false;
+                $both_types = $expr_type->getBuilder();
+                if (count($expr_type->getAtomicTypes()) > 1) {
+                    foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
+                        if ($atomic_type->isTruthy()) {
+                            $both_types->removeType($key);
+                            continue;
+                        }
+
+                        if ($atomic_type->isFalsy()) {
+                            $both_types->removeType($key);
+                            continue;
+                        }
+
+                        $has_both = true;
+                    }
+                }
+
+                if ($has_both) {
+                    $both_types->freeze();
+                    IssueBuffer::maybeAdd(
+                        new TypeDoesNotContainType(
+                            'Operand of type ' . $expr_type->getId() . ' contains ' .
+                            'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
+                            $both_types->getId() . ', which can be falsy and truthy. ' .
+                            'This can cause possibly unexpected behavior. Use strict comparison instead.',
+                            new CodeLocation($statements_analyzer, $stmt),
+                            $expr_type->getId() . ' truthy-falsy',
+                        ),
+                        $statements_analyzer->getSuppressedIssues(),
+                    );
+                }
+
                 $stmt_type = new TBool();
             }
 

diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
@@ -38,6 +38,32 @@ function foo($a): void {
                         if ($b === $a) { }
                     }',
             ],
+            'nonStrictConditionTruthyFalsyNoOverlap' => [
+                'code' => '<?php
+                    /**
+					 * @param non-empty-array|null $arg
+					 * @return void
+					 */
+					function foo($arg) {
+						if ($arg) {
+						}
+
+						if (!$arg) {
+						}
+
+						if (bar($arg)) {
+						}
+
+						if (!bar($arg)) {
+						}
+					}
+
+					/**
+					 * @param mixed $arg
+					 * @return non-empty-array|null
+					 */
+					function bar($arg) {}',
+            ],
             'typeResolutionFromDocblock' => [
                 'code' => '<?php
                     class A { }
@@ -3492,6 +3518,66 @@ public function fluent(): self
                     ',
                 'error_message' => 'TypeDoesNotContainType',
             ],
+            'nonStrictConditionTruthyFalsy' => [
+                'code' => '<?php
+                    /**
+					 * @param array|null $arg
+					 * @return void
+					 */
+					function foo($arg) {
+						if ($arg) {
+						}
+					}',
+                'error_message' => 'TypeDoesNotContainType',
+            ],
+            'nonStrictConditionTruthyFalsyNegated' => [
+                'code' => '<?php
+                    /**
+					 * @param array|null $arg
+					 * @return void
+					 */
+					function foo($arg) {
+						if (!$arg) {
+						}
+					}',
+                'error_message' => 'TypeDoesNotContainType',
+            ],
+            'nonStrictConditionTruthyFalsyFuncCall' => [
+                'code' => '<?php
+                    /**
+					 * @param array|null $arg
+					 * @return void
+					 */
+					function foo($arg) {
+						if (bar($arg)) {
+						}
+					}
+
+					/**
+					 * @param mixed $arg
+					 * @return non-empty-array|null
+					 */
+					function bar($arg) {}',
+                'error_message' => 'TypeDoesNotContainType',
+            ],
+            'nonStrictConditionTruthyFalsyFuncCallNegated' => [
+                'code' => '<?php
+                    /**
+					 * @param array|null $arg
+					 * @return void
+					 */
+					function foo($arg) {
+						if (!bar($arg)) {
+						}
+					}
+
+					/**
+					 * @param mixed $arg
+					 * @return non-empty-array|null
+					 */
+					function bar($arg) {}',
+                'error_message' => 'TypeDoesNotContainType',
+            ],
             'redundantConditionForNonEmptyString' => [
                 'code' => '<?php