From 17e7fe70c1bfef3d6eef302b7f4f56e44a5620a6 Mon Sep 17 00:00:00 2001
From: Matthew Brown <github@muglug.com>
Date: Mon, 19 Aug 2019 22:45:24 -0400
Subject: [PATCH] Fix #2035 more comprehensively

---
 .../Expression/BinaryOpAnalyzer.php           | 61 +++++++++++++++++++
 .../BuildInfoCollector.php                    |  6 +-
 tests/Config/ConfigTest.php                   |  4 +-
 tests/TypeReconciliationTest.php              | 20 ++++++
 4 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
index b7eb6c743..e9c5e08bc 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php
@@ -434,6 +434,52 @@ class BinaryOpAnalyzer
                 $t_if_context->vars_in_scope = $t_if_vars_in_scope_reconciled;
             }
 
+            if (!self::hasArrayDimFetch($stmt->left)) {
+                // check first if the variable was good
+
+                IssueBuffer::startRecording();
+
+                if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->left, clone $context) === false) {
+                    return false;
+                }
+
+                IssueBuffer::clearRecordingLevel();
+                IssueBuffer::stopRecording();
+
+                $naive_type = $stmt->left->inferredType ?? null;
+
+                if ($naive_type
+                    && !$naive_type->isMixed()
+                    && !$naive_type->isNullable()
+                ) {
+                    $var_id = ExpressionAnalyzer::getVarId($stmt->left, $context->self);
+
+                    if (!$var_id || !\in_array($var_id, $changed_var_ids, true)) {
+                        if ($naive_type->from_docblock) {
+                            if (IssueBuffer::accepts(
+                                new \Psalm\Issue\DocblockTypeContradiction(
+                                    $naive_type->getId() . ' does not contain null',
+                                    new CodeLocation($statements_analyzer, $stmt->left)
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
+                        } else {
+                            if (IssueBuffer::accepts(
+                                new \Psalm\Issue\TypeDoesNotContainType(
+                                    $naive_type->getId() . ' is always defined and non-null',
+                                    new CodeLocation($statements_analyzer, $stmt->left)
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
+                        }
+                    }
+                }
+            }
+
             $t_if_context->inside_isset = true;
 
             if (ExpressionAnalyzer::analyze($statements_analyzer, $stmt->left, $t_if_context) === false) {
@@ -663,6 +709,21 @@ class BinaryOpAnalyzer
         return null;
     }
 
+    private static function hasArrayDimFetch(PhpParser\Node\Expr $expr) : bool
+    {
+        if ($expr instanceof PhpParser\Node\Expr\ArrayDimFetch) {
+            return true;
+        }
+
+        if ($expr instanceof PhpParser\Node\Expr\PropertyFetch
+            || $expr instanceof PhpParser\Node\Expr\MethodCall
+        ) {
+            return self::hasArrayDimFetch($expr->var);
+        }
+
+        return false;
+    }
+
     /**
      * @param  StatementsSource|null $statements_source
      * @param  PhpParser\Node\Expr   $left
diff --git a/src/Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php b/src/Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php
index 4844cdd15..fb2a88053 100644
--- a/src/Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php
+++ b/src/Psalm/Internal/ExecutionEnvironment/BuildInfoCollector.php
@@ -79,7 +79,7 @@ class BuildInfoCollector
                 $this->readEnv['CI_REPO_NAME'] = $slug_parts[1];
             }
 
-            $pr_slug = (string) $this->env['TRAVIS_PULL_REQUEST_SLUG'] ?? '';
+            $pr_slug = (string) ($this->env['TRAVIS_PULL_REQUEST_SLUG'] ?? '');
 
             if ($pr_slug) {
                 $slug_parts = explode('/', $pr_slug);
@@ -150,7 +150,7 @@ class BuildInfoCollector
             $this->readEnv['APPVEYOR_REPO_BRANCH'] = $this->env['APPVEYOR_REPO_BRANCH'];
             $this->readEnv['CI_NAME'] = $this->env['CI_NAME'];
 
-            $repo_slug = (string) $this->env['APPVEYOR_REPO_NAME'] ?? '';
+            $repo_slug = (string) ($this->env['APPVEYOR_REPO_NAME'] ?? '');
 
             if ($repo_slug) {
                 $slug_parts = explode('/', $repo_slug);
@@ -215,7 +215,7 @@ class BuildInfoCollector
             // backup
             $this->readEnv['CI_NAME'] = 'Scrutinizer';
 
-            $repo_slug = (string) $this->env['SCRUTINIZER_PROJECT'] ?? '';
+            $repo_slug = (string) ($this->env['SCRUTINIZER_PROJECT'] ?? '');
 
             if ($repo_slug) {
                 $slug_parts = explode('/', $repo_slug);
diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php
index 664ba26eb..7d7cfc807 100644
--- a/tests/Config/ConfigTest.php
+++ b/tests/Config/ConfigTest.php
@@ -1237,12 +1237,12 @@ class ConfigTest extends \Psalm\Tests\TestCase
                     $glob3->func();
                 }
                 namespace {
-                    ord($glob1 ?? "str");
+                    ord($glob1 ?: "str");
                     ord($_GET["str"] ?? "str");
 
                     function example4(): void {
                         global $glob1;
-                        ord($glob1 ?? "str");
+                        ord($glob1 ?: "str");
                         ord($_GET["str"] ?? "str");
                     }
                 }'
diff --git a/tests/TypeReconciliationTest.php b/tests/TypeReconciliationTest.php
index 998ab6dd0..d03bf714e 100644
--- a/tests/TypeReconciliationTest.php
+++ b/tests/TypeReconciliationTest.php
@@ -1468,6 +1468,19 @@ class TypeReconciliationTest extends TestCase
                         return null;
                     }'
             ],
+            'nullCoalesceTypedArrayValue' => [
+                '<?php
+                    /** @param string[] $arr */
+                    function foo(array $arr) : string {
+                        return $arr["b"] ?? "bar";
+                    }',
+            ],
+            'nullCoalesceTypedValue' => [
+                '<?php
+                    function foo(?string $s) : string {
+                        return $s ?? "bar";
+                    }',
+            ],
         ];
     }
 
@@ -1752,6 +1765,13 @@ class TypeReconciliationTest extends TestCase
                     if (false !== firstChar("sdf")) {}',
                 'error_message' => 'RedundantCondition',
             ],
+            'nullCoalesceImpossible' => [
+                '<?php
+                    function foo(?string $s) : string {
+                        return ((string) $s) ?? "bar";
+                    }',
+                'error_message' => 'TypeDoesNotContainType'
+            ],
         ];
     }
 }