Capture non-empty-string from concat more effectively

vimeo · Jan 7, 2020 · 1f777be · 1f777be
1 parent ce5917c
commit 1f777be
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 48 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -72,8 +72,6 @@ public static function analyze(
     ) {
         $codebase = $statements_analyzer->getCodebase();
 
-        $stmt_type = null;
-
         if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat && $nesting > 20) {
             // ignore deeply-nested string concatenation
         } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\BooleanAnd ||
@@ -569,25 +567,25 @@ function ($c) {
                 return false;
             }
 
-            $stmt_left_type = $statements_analyzer->node_data->getType($stmt->left);
-            $stmt_right_type = $statements_analyzer->node_data->getType($stmt->right);
+            $stmt_type = Type::getString();
 
-            if ($stmt_left_type
-                && $stmt_right_type
-                && ($stmt_left_type->getId() === 'non-empty-string'
-                    || $stmt_right_type->getId() === 'non-empty-string'
-                    || ($stmt_left_type->isSingleStringLiteral()
-                        && $stmt_left_type->getSingleStringLiteral()->value)
-                    || ($stmt_right_type->isSingleStringLiteral()
-                        && $stmt_right_type->getSingleStringLiteral()->value))
-            ) {
-                $stmt_type = new Type\Union([new Type\Atomic\TNonEmptyString()]);
-            } else {
-                $stmt_type = Type::getString();
+            self::analyzeConcatOp(
+                $statements_analyzer,
+                $stmt->left,
+                $stmt->right,
+                $context,
+                $result_type
+            );
+
+            if ($result_type) {
+                $stmt_type = $result_type;
             }
 
             $statements_analyzer->node_data->setType($stmt, $stmt_type);
 
+            $stmt_left_type = $statements_analyzer->node_data->getType($stmt->left);
+            $stmt_right_type = $statements_analyzer->node_data->getType($stmt->right);
+
             if ($codebase->taint) {
                 $sources = [];
                 $either_tainted = 0;
@@ -921,36 +919,6 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
 
                     $statements_analyzer->node_data->setType($stmt, $result_type);
                 }
-            } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat) {
-                self::analyzeConcatOp(
-                    $statements_analyzer,
-                    $stmt->left,
-                    $stmt->right,
-                    $context,
-                    $result_type
-                );
-
-                if ($result_type) {
-                    $stmt_type = $result_type;
-
-                    $statements_analyzer->node_data->setType($stmt, $stmt_type);
-                }
-
-                if ($codebase->taint && $stmt_type) {
-                    $sources = $stmt_left_type->sources ?: [];
-                    $either_tainted = $stmt_left_type->tainted;
-
-                    $sources = array_merge($sources, $stmt_right_type->sources ?: []);
-                    $either_tainted = $either_tainted | $stmt_right_type->tainted;
-
-                    if ($sources) {
-                        $stmt_type->sources = $sources;
-                    }
-
-                    if ($either_tainted) {
-                        $stmt_type->tainted = $either_tainted;
-                    }
-                }
             } elseif ($stmt instanceof PhpParser\Node\Expr\BinaryOp\BitwiseOr) {
                 self::analyzeNonDivArithmeticOp(
                     $statements_analyzer,
@@ -1929,6 +1897,18 @@ public static function analyzeConcatOp(
                 // Limit these to 10000 bytes to avoid extremely large union types from repeated concatenations, etc
                 $result_type = Type::getString($literal);
             }
+        } else {
+            if ($left_type
+                && $right_type
+                && ($left_type->getId() === 'non-empty-string'
+                    || $right_type->getId() === 'non-empty-string'
+                    || ($left_type->isSingleStringLiteral()
+                        && $left_type->getSingleStringLiteral()->value)
+                    || ($right_type->isSingleStringLiteral()
+                        && $right_type->getSingleStringLiteral()->value))
+            ) {
+                $result_type = new Type\Union([new Type\Atomic\TNonEmptyString()]);
+            }
         }
     }
 }
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php
@@ -980,7 +980,6 @@ function (PhpParser\Node\Arg $arg) {
         }
 
         if ($codebase->store_node_types
-            && $method_id
             && !$context->collect_initializations
             && !$context->collect_mutations
         ) {

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php
@@ -1061,7 +1061,7 @@ function (Assertion $assertion) use ($generic_params) : Assertion {
                 }
 
                 if ($return_type_candidate) {
-                    if ($codebase->taint && $method_id) {
+                    if ($codebase->taint) {
                         if ($method_storage && $method_storage->pure) {
                             $code_location = new CodeLocation($statements_analyzer->getSource(), $stmt);
 

diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
@@ -2516,6 +2516,22 @@ function exampleWithOr(Convertor $convertor, string $value): SomeObject {
                         return $value; // $value is SomeObject here and cannot be a string
                     }'
             ],
+            'nonEmptyStringFromConcat' => [
+                '<?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("a" . $name);
+                        }
+                    }',
+            ],
         ];
     }