Prevent string being used in non-empty-string location

vimeo · Jan 7, 2020 · cc9e0fa · cc9e0fa
1 parent 07aaa3f
commit cc9e0fa
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 36 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -561,10 +561,6 @@ function ($c) {
                 $context->vars_possibly_in_scope
             );
         } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat) {
-            $stmt_type = Type::getString();
-
-            $statements_analyzer->node_data->setType($stmt, $stmt_type);
-
             if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->left, $context) === false) {
                 return false;
             }
@@ -573,16 +569,31 @@ function ($c) {
                 return false;
             }
 
+            $stmt_left_type = $statements_analyzer->node_data->getType($stmt->left);
+            $stmt_right_type = $statements_analyzer->node_data->getType($stmt->right);
+
+            if ($stmt_left_type
+                && $stmt_right_type
+                && $stmt_left_type->getId() === 'non-empty-string'
+                && $stmt_right_type->getId() === 'non-empty-string'
+            ) {
+                $stmt_type = new Type\Union([new Type\Atomic\TNonEmptyString()]);
+            } else {
+                $stmt_type = Type::getString();
+            }
+
+            $statements_analyzer->node_data->setType($stmt, $stmt_type);
+
             if ($codebase->taint) {
                 $sources = [];
                 $either_tainted = 0;
 
-                if ($stmt_left_type = $statements_analyzer->node_data->getType($stmt->left)) {
+                if ($stmt_left_type) {
                     $sources = $stmt_left_type->sources ?: [];
                     $either_tainted = $stmt_left_type->tainted;
                 }
 
-                if ($stmt_right_type = $statements_analyzer->node_data->getType($stmt->right)) {
+                if ($stmt_right_type) {
                     $sources = array_merge($sources, $stmt_right_type->sources ?: []);
                     $either_tainted = $either_tainted | $stmt_right_type->tainted;
                 }

diff --git a/src/Psalm/Internal/Analyzer/TypeAnalyzer.php b/src/Psalm/Internal/Analyzer/TypeAnalyzer.php
@@ -767,6 +767,16 @@ public static function isAtomicContainedBy(
             }
         }
 
+        if ($container_type_part instanceof TNonEmptyString
+            && get_class($input_type_part) === TString::class
+        ) {
+            if ($atomic_comparison_result) {
+                $atomic_comparison_result->type_coerced = true;
+            }
+
+            return false;
+        }
+
         if ($input_type_part->shallowEquals($container_type_part)
             || ($input_type_part instanceof Type\Atomic\TCallableObjectLikeArray
                 && $container_type_part instanceof TArray)
@@ -1154,16 +1164,6 @@ public static function isAtomicContainedBy(
             return true;
         }
 
-        if ($container_type_part instanceof TNonEmptyString
-            && get_class($input_type_part) === TString::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 === ''

diff --git a/src/Psalm/Internal/Type/TemplateResult.php b/src/Psalm/Internal/Type/TemplateResult.php
@@ -7,18 +7,18 @@
 class TemplateResult
 {
     /**
-     * @var array<string, array<non-empty-string, array{0: Union}>>
+     * @var array<string, array<string, array{0: Union}>>
      */
     public $template_types;
 
     /**
-     * @var array<string, array<non-empty-string, array{0: Union, 1?: int}>>
+     * @var array<string, array<string, array{0: Union, 1?: int}>>
      */
     public $generic_params;
 
     /**
-     * @param  array<string, array<non-empty-string, array{0: Union}>> $template_types
-     * @param  array<string, array<non-empty-string, array{0: Union, 1?: int}>> $generic_params
+     * @param  array<string, array<string, array{0: Union}>> $template_types
+     * @param  array<string, array<string, array{0: Union, 1?: int}>> $generic_params
      */
     public function __construct(array $template_types, array $generic_params)
     {

diff --git a/src/Psalm/Type/Atomic/TTemplateIndexedAccess.php b/src/Psalm/Type/Atomic/TTemplateIndexedAccess.php
@@ -14,13 +14,10 @@ class TTemplateIndexedAccess extends \Psalm\Type\Atomic
     public $offset_param_name;
 
     /**
-     * @var non-empty-string
+     * @var string
      */
     public $defining_class;
 
-    /**
-     * @param non-empty-string $defining_class
-     */
     public function __construct(
         string $array_param_name,
         string $offset_param_name,

diff --git a/src/Psalm/Type/Atomic/TTemplateKeyOf.php b/src/Psalm/Type/Atomic/TTemplateKeyOf.php
@@ -9,13 +9,10 @@ class TTemplateKeyOf extends TArrayKey
     public $param_name;
 
     /**
-     * @var non-empty-string
+     * @var string
      */
     public $defining_class;
 
-    /**
-     * @param non-empty-string $defining_class
-     */
     public function __construct(
         string $param_name,
         string $defining_class

diff --git a/src/Psalm/Type/Atomic/TTemplateParam.php b/src/Psalm/Type/Atomic/TTemplateParam.php
@@ -24,12 +24,12 @@ class TTemplateParam extends \Psalm\Type\Atomic
     public $as;
 
     /**
-     * @var non-empty-string
+     * @var string
      */
     public $defining_class;
 
     /**
-     * @param non-empty-string $defining_class
+     * @param string $defining_class
      */
     public function __construct(string $param_name, Union $extends, string $defining_class)
     {

diff --git a/src/Psalm/Type/Atomic/TTemplateParamClass.php b/src/Psalm/Type/Atomic/TTemplateParamClass.php
@@ -19,14 +19,10 @@ class TTemplateParamClass extends TClassString
     public $as_type;
 
     /**
-     * @var non-empty-string
+     * @var string
      */
     public $defining_class;
 
-    /**
-     * @param non-empty-string $defining_class
-     * @param string $param_name
-     */
     public function __construct(
         string $param_name,
         string $as,

diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php
@@ -1091,7 +1091,7 @@ public function substitute(Union $old_type, Union $new_type = null)
     }
 
     /**
-     * @param  array<string, array<non-empty-string, array{Type\Union, 1?:int}>>  $template_types
+     * @param  array<string, array<string, array{Type\Union, 1?:int}>>  $template_types
      *
      * @return void
      */

diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
@@ -2872,6 +2872,23 @@ function test(App $app) : void {
                     }',
                 'error_message' => 'TypeDoesNotContainType',
             ],
+            'nonEmptyString' => [
+                '<?php
+                    /**
+                     * @psalm-param non-empty-string $name
+                     */
+                    function sayHello(string $name) : void {
+                        echo "Hello " . $name;
+                    }
+
+                    function takeInput() : void {
+                        if (isset($_GET["name"]) && is_string($_GET["name"])) {
+                            $name = trim($_GET["name"]);
+                            sayHello($name);
+                        }
+                    }',
+                'error_message' => 'ArgumentTypeCoercion',
+            ],
         ];
     }
 }