Update baseline

nicelocal · Jan 17, 2024 · 11e2163 · 11e2163
2 parents 950293c + 680c8cd
commit 11e2163
Show file tree

Hide file tree

Showing 30 changed files with 2,107 additions and 65 deletions.
diff --git a/config.xsd b/config.xsd
@@ -429,6 +429,7 @@
             <xs:element name="ReferenceReusedFromConfusingScope" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ReservedWord" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="RiskyCast" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="RiskyTruthyFalsyComparison" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="StringIncrement" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedCallable" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedCookie" type="IssueHandlerType" minOccurs="0" />

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -246,6 +246,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
 - [RedundantConditionGivenDocblockType](issues/RedundantConditionGivenDocblockType.md)
 - [RedundantFunctionCallGivenDocblockType](issues/RedundantFunctionCallGivenDocblockType.md)
 - [ReferenceConstraintViolation](issues/ReferenceConstraintViolation.md)
+- [RiskyTruthyFalsyComparison](issues/RiskyTruthyFalsyComparison.md)
 - [UndefinedTrace](issues/UndefinedTrace.md)
 - [UnresolvableInclude](issues/UnresolvableInclude.md)
 - [UnsafeInstantiation](issues/UnsafeInstantiation.md)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -230,6 +230,7 @@
  - [ReferenceReusedFromConfusingScope](issues/ReferenceReusedFromConfusingScope.md)
  - [ReservedWord](issues/ReservedWord.md)
  - [RiskyCast](issues/RiskyCast.md)
+ - [RiskyTruthyFalsyComparison](issues/RiskyTruthyFalsyComparison.md)
  - [StringIncrement](issues/StringIncrement.md)
  - [TaintedCallable](issues/TaintedCallable.md)
  - [TaintedCookie](issues/TaintedCookie.md)

diff --git a/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md b/docs/running_psalm/issues/RiskyTruthyFalsyComparison.md
@@ -0,0 +1,29 @@
+# RiskyTruthyFalsyComparison
+
+Emitted when comparing a value with multiple types that can both contain truthy and falsy values.
+
+```php
+<?php
+
+/**
+ * @param array|null $arg
+ * @return void
+ */
+function foo($arg) {
+    if ($arg) {
+        // this is risky, bc the empty array and null case are handled together
+    }
+
+    if (!$arg) {
+        // this is risky, bc the empty array and null case are handled together  
+    }
+}
+```
+
+## Why this is bad
+
+The truthy/falsy type of one variable is often forgotten and not handled explicitly causing hard to track down errors.
+
+## How to fix
+
+Explicitly validate the variable with strict comparison.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
@@ -17,8 +17,10 @@
 use Psalm\Issue\DocblockTypeContradiction;
 use Psalm\Issue\RedundantCondition;
 use Psalm\Issue\RedundantConditionGivenDocblockType;
+use Psalm\Issue\RiskyTruthyFalsyComparison;
 use Psalm\Issue\TypeDoesNotContainType;
 use Psalm\IssueBuffer;
+use Psalm\Type\Atomic\TBool;
 use Psalm\Type\Reconciler;
 
 use function array_diff_key;
@@ -368,6 +370,34 @@ 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)) {
+                if (count($type->getAtomicTypes()) > 1) {
+                    $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);
+                        }
+                    }
+
+                    if (count($both_types->getAtomicTypes()) > 0) {
+                        $both_types = $both_types->freeze();
+                        IssueBuffer::maybeAdd(
+                            new RiskyTruthyFalsyComparison(
+                                '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(),
+                            ),
+                            $statements_analyzer->getSuppressedIssues(),
+                        );
+                    }
+                }
             }
         }
     }

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BooleanNotAnalyzer.php
@@ -5,15 +5,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\RiskyTruthyFalsyComparison;
+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
  */
@@ -42,6 +47,32 @@ public static function analyze(
             } elseif ($expr_type->isAlwaysFalsy()) {
                 $stmt_type = new TTrue($expr_type->from_docblock);
             } else {
+                if (count($expr_type->getAtomicTypes()) > 1) {
+                    $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);
+                        }
+                    }
+
+                    if (count($both_types->getAtomicTypes()) > 0) {
+                        $both_types = $both_types->freeze();
+                        IssueBuffer::maybeAdd(
+                            new RiskyTruthyFalsyComparison(
+                                '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(),
+                            ),
+                            $statements_analyzer->getSuppressedIssues(),
+                        );
+                    }
+                }
+
                 $stmt_type = new TBool();
             }
 

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EmptyAnalyzer.php
@@ -10,8 +10,15 @@
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Issue\ForbiddenCode;
 use Psalm\Issue\InvalidArgument;
+use Psalm\Issue\RiskyTruthyFalsyComparison;
 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
@@ -37,21 +44,64 @@ public static function analyze(
             );
         }
 
-        if (($stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr))
-            && $stmt_expr_type->hasBool()
-            && $stmt_expr_type->isSingle()
-            && !$stmt_expr_type->from_docblock
-        ) {
-            IssueBuffer::maybeAdd(
-                new InvalidArgument(
-                    'Calling empty on a boolean value is almost certainly unintended',
-                    new CodeLocation($statements_analyzer->getSource(), $stmt->expr),
-                    'empty',
-                ),
-                $statements_analyzer->getSuppressedIssues(),
-            );
+        $expr_type = $statements_analyzer->node_data->getType($stmt->expr);
+
+        if ($expr_type) {
+            if ($expr_type->hasBool()
+                && $expr_type->isSingle()
+                && !$expr_type->from_docblock
+            ) {
+                IssueBuffer::maybeAdd(
+                    new InvalidArgument(
+                        'Calling empty on a boolean value is almost certainly unintended',
+                        new CodeLocation($statements_analyzer->getSource(), $stmt->expr),
+                        'empty',
+                    ),
+                    $statements_analyzer->getSuppressedIssues(),
+                );
+            }
+
+            if ($expr_type->isAlwaysTruthy() && $expr_type->possibly_undefined === false) {
+                $stmt_type = new TFalse($expr_type->from_docblock);
+            } elseif ($expr_type->isAlwaysFalsy()) {
+                $stmt_type = new TTrue($expr_type->from_docblock);
+            } else {
+                if (count($expr_type->getAtomicTypes()) > 1) {
+                    $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);
+                        }
+                    }
+
+                    if (count($both_types->getAtomicTypes()) > 0) {
+                        $both_types = $both_types->freeze();
+                        IssueBuffer::maybeAdd(
+                            new RiskyTruthyFalsyComparison(
+                                '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(),
+                            ),
+                            $statements_analyzer->getSuppressedIssues(),
+                        );
+                    }
+                }
+
+                $stmt_type = new TBool();
+            }
+
+            $stmt_type = new Union([$stmt_type], [
+                'parent_nodes' => $expr_type->parent_nodes,
+            ]);
+        } else {
+            $stmt_type = Type::getBool();
         }
 
-        $statements_analyzer->node_data->setType($stmt, Type::getBool());
+        $statements_analyzer->node_data->setType($stmt, $stmt_type);
     }
 }
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php
@@ -314,14 +314,18 @@ public static function analyze(
                 && !$context->inside_unset
                 && ($stmt_var_type && !$stmt_var_type->hasMixed())
             ) {
-                IssueBuffer::maybeAdd(
+                if (IssueBuffer::accepts(
                     new PossiblyUndefinedArrayOffset(
                         'Possibly undefined array key ' . $keyed_array_var_id
                             . ' on ' . $stmt_var_type->getId(),
                         new CodeLocation($statements_analyzer->getSource(), $stmt),
                     ),
                     $statements_analyzer->getSuppressedIssues(),
-                );
+                )) {
+                    $stmt_type = $stmt_type->getBuilder()->addType(new TNull())->freeze();
+                }
+            } elseif ($stmt_type->possibly_undefined) {
+                $stmt_type = $stmt_type->getBuilder()->addType(new TNull())->freeze();
             }
 
             $stmt_type = $stmt_type->setPossiblyUndefined(false);

diff --git a/src/Psalm/Issue/RiskyTruthyFalsyComparison.php b/src/Psalm/Issue/RiskyTruthyFalsyComparison.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Psalm\Issue;
+
+use Psalm\CodeLocation;
+
+final class RiskyTruthyFalsyComparison extends CodeIssue
+{
+    public const ERROR_LEVEL = 2;
+    public const SHORTCODE = 356;
+
+    public function __construct(string $message, CodeLocation $code_location, ?string $dupe_key)
+    {
+        parent::__construct($message, $code_location);
+        $this->dupe_key = $dupe_key;
+    }
+}
diff --git a/src/Psalm/PluginFileExtensionsSocket.php b/src/Psalm/PluginFileExtensionsSocket.php
@@ -55,8 +55,8 @@ public function addFileTypeScanner(string $fileExtension, string $className): vo
                 1_622_727_271,
             );
         }
-        if (!empty($this->config->getFiletypeScanners()[$fileExtension])
-            || !empty($this->additionalFileTypeScanners[$fileExtension])
+        if (isset($this->config->getFiletypeScanners()[$fileExtension])
+            || isset($this->additionalFileTypeScanners[$fileExtension])
         ) {
             throw new LogicException(
                 sprintf('Cannot redeclare scanner for file-type %s', $fileExtension),
@@ -91,8 +91,8 @@ public function addFileTypeAnalyzer(string $fileExtension, string $className): v
                 1_622_727_281,
             );
         }
-        if (!empty($this->config->getFiletypeAnalyzers()[$fileExtension])
-            || !empty($this->additionalFileTypeAnalyzers[$fileExtension])
+        if (isset($this->config->getFiletypeAnalyzers()[$fileExtension])
+            || isset($this->additionalFileTypeAnalyzers[$fileExtension])
         ) {
             throw new LogicException(
                 sprintf('Cannot redeclare analyzer for file-type %s', $fileExtension),

diff --git a/tests/ArrayAccessTest.php b/tests/ArrayAccessTest.php
@@ -656,6 +656,19 @@ function f(array $p): void
                     '$x3===' => "array{b: 'value'}",
                 ],
             ],
+            'possiblyUndefinedArrayOffsetKeyedArray' => [
+                'code' => '<?php
+                    $d = [];
+                    if (!rand(0,1)) {
+                        $d[0] = "a";
+                    }
+
+                    $x = $d[0];',
+                'assertions' => [
+                    '$x===' => '\'a\'',
+                ],
+                'ignored_issues' => ['PossiblyUndefinedArrayOffset'],
+            ],
             'domNodeListAccessible' => [
                 'code' => '<?php
                     $doc = new DOMDocument();
@@ -680,7 +693,7 @@ function example(array $x, $y) : void {
                 'assertions' => [],
                 'ignored_issues' => ['MixedArgument', 'MixedArrayOffset', 'MissingParamType'],
             ],
-            'suppressPossiblyUndefinedStringArrayOffet' => [
+            'suppressPossiblyUndefinedStringArrayOffset' => [
                 'code' => '<?php
                     /** @var array{a?:string} */
                     $entry = ["a"];
@@ -1338,15 +1351,15 @@ public function __toString() {
                     echo $a[new Foo];',
                 'error_message' => 'InvalidArrayOffset',
             ],
-            'possiblyUndefinedIntArrayOffet' => [
+            'possiblyUndefinedIntArrayOffset' => [
                 'code' => '<?php
                     /** @var array{0?:string} */
                     $entry = ["a"];
 
                     [$elt] = $entry;',
                 'error_message' => 'PossiblyUndefinedArrayOffset',
             ],
-            'possiblyUndefinedStringArrayOffet' => [
+            'possiblyUndefinedStringArrayOffset' => [
                 'code' => '<?php
                     /** @var array{a?:string} */
                     $entry = ["a"];
@@ -1531,6 +1544,19 @@ function takesArrayOfFloats(array $arr): void {
                     avg(["a" => 0.5, "b" => 1.5, "c" => new Exception()]);',
                 'error_message' => 'InvalidArgument',
             ],
+            'possiblyUndefinedArrayOffsetKeyedArray' => [
+                'code' => '<?php
+                    $d = [];
+                    if (!rand(0,1)) {
+                        $d[0] = "a";
+                    }
+
+                    $x = $d[0];
+
+                    //  should not report TypeDoesNotContainNull
+                    if ($x === null) {}',
+                'error_message' => 'PossiblyUndefinedArrayOffset',
+            ],
         ];
     }
 }
diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php
@@ -1939,7 +1939,7 @@ function badpattern() {
             'strposAllowDictionary' => [
                 'code' => '<?php
                     function sayHello(string $format): void {
-                        if (strpos("abcdefghijklmno", $format)) {}
+                        if (strpos("abcdefghijklmno", $format) !== false) {}
                     }',
             ],
             'curlInitIsResourceAllowedIn7x' => [
@@ -2140,7 +2140,7 @@ function foo(A $a1, A $a2 = null): void
             'strposFirstParamAllowClassString' => [
                 'code' => '<?php
                     function sayHello(string $needle): void {
-                        if (strpos(DateTime::class, $needle)) {}
+                        if (strpos(DateTime::class, $needle) !== false) {}
                     }',
             ],
             'mb_strtolowerProducesStringWithSecondArgument' => [