Add non-falsy-string to allow more accurate checks

vimeo · Feb 3, 2021 · a0420fb · orklah · Feb 3, 2021 · muglug
1 parent 03665b9
commit a0420fb
Show file tree

Hide file tree

Showing 10 changed files with 136 additions and 13 deletions.
diff --git a/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php b/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php
@@ -27,6 +27,7 @@
 use Psalm\Type\Atomic\TLiteralString;
 use Psalm\Type\Atomic\TNamedObject;
 use Psalm\Type\Atomic\TNonEmptyString;
+use Psalm\Type\Atomic\TNonFalsyString;
 use Psalm\Type\Atomic\TNumeric;
 use Psalm\Type\Atomic\TNumericString;
 use Psalm\Type\Atomic\TPositiveInt;
@@ -258,15 +259,39 @@ public static function isContainedBy(
             return true;
         }
 
+        if ($container_type_part instanceof TNonEmptyString
+            && $input_type_part instanceof TNonFalsyString
+        ) {
+            return true;
+        }
+
+        if ($container_type_part instanceof TNonFalsyString
+            && get_class($input_type_part) === TNonEmptyString::class
+        ) {
+            if ($atomic_comparison_result) {
+                $atomic_comparison_result->type_coerced = true;
+            }
+
+            return false;
+        }
+
         if ($container_type_part instanceof TNonEmptyString
             && $input_type_part instanceof TLiteralString
             && $input_type_part->value === ''
         ) {
             return false;
         }
 
+        if ($container_type_part instanceof TNonFalsyString
+            && $input_type_part instanceof TLiteralString
+            && $input_type_part->value === '0'
+        ) {
+            return false;
+        }
+
         if ((get_class($container_type_part) === TString::class
                 || get_class($container_type_part) === TNonEmptyString::class
+                || get_class($container_type_part) === TNonFalsyString::class
                 || get_class($container_type_part) === TSingleLetter::class)
             && $input_type_part instanceof TLiteralString
         ) {
@@ -321,7 +346,8 @@ public static function isContainedBy(
 
         if ((get_class($input_type_part) === TString::class
                 || get_class($input_type_part) === TSingleLetter::class
-                || get_class($input_type_part) === TNonEmptyString::class)
+                || get_class($input_type_part) === TNonEmptyString::class
+                || get_class($input_type_part) === TNonFalsyString::class)
             && $container_type_part instanceof TLiteralString
         ) {
             if ($atomic_comparison_result) {
@@ -365,7 +391,8 @@ public static function isContainedBy(
 
         if ($container_type_part instanceof TTraitString
             && (get_class($input_type_part) === TString::class
-                || get_class($input_type_part) === TNonEmptyString::class)
+                || get_class($input_type_part) === TNonEmptyString::class
+                || get_class($input_type_part) === TNonFalsyString::class)
         ) {
             if ($atomic_comparison_result) {
                 $atomic_comparison_result->type_coerced = true;
@@ -378,15 +405,17 @@ public static function isContainedBy(
             || $input_type_part instanceof TLiteralClassString)
             && (get_class($container_type_part) === TString::class
                 || get_class($container_type_part) === TSingleLetter::class
-                || get_class($container_type_part) === TNonEmptyString::class)
+                || get_class($container_type_part) === TNonEmptyString::class
+                || get_class($container_type_part) === TNonFalsyString::class)
         ) {
             return true;
         }
 
         if ($input_type_part instanceof TCallableString
             && (get_class($container_type_part) === TString::class
                 || get_class($container_type_part) === TSingleLetter::class
-                || get_class($container_type_part) === TNonEmptyString::class)
+                || get_class($container_type_part) === TNonEmptyString::class
+                || get_class($container_type_part) === TNonFalsyString::class)
         ) {
             return true;
         }

diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
@@ -1971,6 +1971,7 @@ private static function reconcileCallable(
                 $callable_types[] = $type;
             } elseif (get_class($type) === TString::class
                 || get_class($type) === Type\Atomic\TNonEmptyString::class
+                || get_class($type) === Type\Atomic\TNonFalsyString::class
             ) {
                 $callable_types[] = new Type\Atomic\TCallableString();
                 $did_remove_type = true;
@@ -2203,6 +2204,8 @@ private static function reconcileFalsyOrEmpty(
                     if (!$existing_var_atomic_types['string'] instanceof Type\Atomic\TNonEmptyString) {
                         $existing_var_type->addType(new Type\Atomic\TLiteralString(''));
                         $existing_var_type->addType(new Type\Atomic\TLiteralString('0'));
+                    } elseif (!$existing_var_atomic_types['string'] instanceof Type\Atomic\TNonFalsyString) {
+                        $existing_var_type->addType(new Type\Atomic\TLiteralString('0'));
                     }
                 }
             }

diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
@@ -582,7 +582,7 @@ private static function reconcileFalsyOrEmpty(
                 if ($existing_var_atomic_types['string'] instanceof Type\Atomic\TLowercaseString) {
                     $existing_var_type->addType(new Type\Atomic\TNonEmptyLowercaseString);
                 } else {
-                    $existing_var_type->addType(new Type\Atomic\TNonEmptyString);
+                    $existing_var_type->addType(new Type\Atomic\TNonFalsyString);
                 }
             }
 
@@ -707,7 +707,7 @@ private static function reconcileFalsyOrEmpty(
         }
 
         if (isset($existing_var_atomic_types['string'])) {
-            if (!$existing_var_atomic_types['string'] instanceof Type\Atomic\TNonEmptyString
+            if (!$existing_var_atomic_types['string'] instanceof Type\Atomic\TNonFalsyString
                 && !$existing_var_atomic_types['string'] instanceof Type\Atomic\TClassString
                 && !$existing_var_atomic_types['string'] instanceof Type\Atomic\TDependentGetClass
             ) {
@@ -718,7 +718,7 @@ private static function reconcileFalsyOrEmpty(
                 if ($existing_var_atomic_types['string'] instanceof Type\Atomic\TLowercaseString) {
                     $existing_var_type->addType(new Type\Atomic\TNonEmptyLowercaseString);
                 } else {
-                    $existing_var_type->addType(new Type\Atomic\TNonEmptyString);
+                    $existing_var_type->addType(new Type\Atomic\TNonFalsyString);
                 }
             } elseif ($existing_var_type->isSingle() && !$is_equality) {
                 if ($code_location && $key) {

diff --git a/src/Psalm/Internal/Type/TypeCombiner.php b/src/Psalm/Internal/Type/TypeCombiner.php
@@ -36,6 +36,7 @@
 use Psalm\Type\Atomic\TNonEmptyLowercaseString;
 use Psalm\Type\Atomic\TNonEmptyMixed;
 use Psalm\Type\Atomic\TNonEmptyString;
+use Psalm\Type\Atomic\TNonFalsyString;
 use Psalm\Type\Atomic\TNull;
 use Psalm\Type\Atomic\TObject;
 use Psalm\Type\Atomic\TPositiveInt;
@@ -932,10 +933,15 @@ private static function scrapeTypeProperties(
                         ) {
                             // do nothing
                         } elseif (isset($combination->value_types['string'])
-                            && $combination->value_types['string'] instanceof Type\Atomic\TNonEmptyString
+                            && $combination->value_types['string'] instanceof Type\Atomic\TNonFalsyString
                             && $type->value
                         ) {
                             // do nothing
+                        } elseif (isset($combination->value_types['string'])
+                            && $combination->value_types['string'] instanceof Type\Atomic\TNonEmptyString
+                            && $type->value !== ''
+                        ) {
+                            // do nothing
                         } else {
                             $combination->value_types['string'] = new TString();
                         }
@@ -1036,10 +1042,20 @@ private static function scrapeTypeProperties(
                         unset($combination->value_types['string']);
                     } elseif (get_class($combination->value_types['string']) !== get_class($type)) {
                         if (get_class($type) === TNonEmptyString::class
+                            && get_class($combination->value_types['string']) === TNonFalsyString::class
+                        ) {
+                            $combination->value_types['string'] = $type;
+                        } elseif (get_class($type) === TNonFalsyString::class
+                            && get_class($combination->value_types['string']) === TNonEmptyString::class
+                        ) {
+                            // do nothing
+                        } elseif ((get_class($type) === TNonEmptyString::class
+                                || get_class($type) === TNonFalsyString::class)
                             && get_class($combination->value_types['string']) === TNonEmptyLowercaseString::class
                         ) {
                             $combination->value_types['string'] = $type;
-                        } elseif (get_class($combination->value_types['string']) === TNonEmptyString::class
+                        } elseif ((get_class($combination->value_types['string']) === TNonEmptyString::class
+                                || get_class($combination->value_types['string']) === TNonFalsyString::class)
                             && get_class($type) === TNonEmptyLowercaseString::class
                         ) {
                             //no-change

diff --git a/src/Psalm/Internal/Type/TypeTokenizer.php b/src/Psalm/Internal/Type/TypeTokenizer.php
@@ -34,6 +34,7 @@ class TypeTokenizer
         'array' => true,
         'non-empty-array' => true,
         'non-empty-string' => true,
+        'non-falsy-string' => true,
         'iterable' => true,
         'null' => true,
         'mixed' => true,

diff --git a/src/Psalm/Type/Atomic.php b/src/Psalm/Type/Atomic.php
@@ -177,6 +177,9 @@ public static function create(
             case 'non-empty-string':
                 return new Type\Atomic\TNonEmptyString();
 
+            case 'non-falsy-string':
+                return new Type\Atomic\TNonFalsyString();
+
             case 'lowercase-string':
                 return new Type\Atomic\TLowercaseString();
 

diff --git a/src/Psalm/Type/Atomic/TNonEmptyLowercaseString.php b/src/Psalm/Type/Atomic/TNonEmptyLowercaseString.php
@@ -4,7 +4,7 @@
 /**
  * Denotes a non-empty-string where every character is lowercased. (which can also result from a `strtolower` call).
  */
-class TNonEmptyLowercaseString extends TNonEmptyString
+class TNonEmptyLowercaseString extends TNonFalsyString
 {
     public function getKey(bool $include_extra = true): string
     {

diff --git a/src/Psalm/Type/Atomic/TNonFalsyString.php b/src/Psalm/Type/Atomic/TNonFalsyString.php
@@ -0,0 +1,13 @@
+<?php
+namespace Psalm\Type\Atomic;
+
+/**
+ * Denotes a string, that is also non-empty
+ */
+class TNonFalsyString extends TNonEmptyString
+{
+    public function getId(bool $nested = false): string
+    {
+        return 'non-falsy-string';
+    }
+}
diff --git a/tests/TypeCombinationTest.php b/tests/TypeCombinationTest.php
@@ -637,27 +637,41 @@ public function providerTestValidTypeCombination(): array
                     'positive-int',
                 ],
             ],
-            'combinNonEmptyArrayAndKeyedArray' => [
+            'combineNonEmptyArrayAndKeyedArray' => [
                 'array<int, int>',
                 [
                     'non-empty-array<int, int>',
                     'array{0?:int}',
                 ]
             ],
-            'combinNonEmptyStringAndLiteral' => [
+            'combineNonEmptyStringAndLiteral' => [
                 'non-empty-string',
                 [
                     'non-empty-string',
                     '"foo"',
                 ]
             ],
-            'combinLiteralAndNonEmptyString' => [
+            'combineLiteralAndNonEmptyString' => [
                 'non-empty-string',
                 [
                     '"foo"',
                     'non-empty-string'
                 ]
             ],
+            'combineNonFalsyNonEmptyString' => [
+                'non-empty-string',
+                [
+                    'non-falsy-string',
+                    'non-empty-string'
+                ]
+            ],
+            'combineNonEmptyNonFalsyString' => [
+                'non-empty-string',
+                [
+                    'non-empty-string',
+                    'non-falsy-string'
+                ]
+            ],
         ];
     }
 

diff --git a/tests/TypeReconciliation/ValueTest.php b/tests/TypeReconciliation/ValueTest.php
@@ -854,6 +854,38 @@ class A {
                 [],
                 '7.4'
             ],
+            'zeroIsNonEmptyString' => [
+                '<?php
+                    /**
+                     * @param non-empty-string $s
+                     */
+                    function foo(string $s) : void {}
+
+                    foo("0");',
+            ],
+            'notLiteralEmptyCanBeNotEmptyString' => [
+                '<?php
+                    /**
+                     * @param non-empty-string $s
+                     */
+                    function foo(string $s) : void {}
+
+                    function takesString(string $s) : void {
+                        if ($s !== "") {
+                            foo($s);
+                        }
+                    }',
+            ],
+            'nonEmptyStringCanBeStringZero' => [
+                '<?php
+                    /**
+                     * @param non-empty-string $s
+                     */
+                    function foo(string $s) : void {
+                        if ($s === "0") {}
+                        if (empty($s)) {}
+                    }',
+            ],
         ];
     }
 
@@ -1053,6 +1085,18 @@ function bar(string $s) : void {
                     }',
                 'error_message' => 'ArgumentTypeCoercion'
             ],
+            'stringCoercedToNonEmptyString' => [
+                '<?php
+                    /**
+                     * @param non-empty-string $name
+                     */
+                    function sayHello(string $name) : void {}
+
+                    function takeInput(string $name) : void {
+                        sayHello($name);
+                    }',
+                'error_message' => 'ArgumentTypeCoercion',
+            ],
         ];
     }
 }