From 344d37f0b113ba1a473c3807739bd2e53e786dc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Valts=20Liepi=C5=86=C5=A1?= <cipharius@pm.me>
Date: Thu, 4 Nov 2021 06:23:20 +0200
Subject: [PATCH] Fixes incorrect MismatchedCount error message when returning
 (#103)

---
 Analysis/src/Error.cpp   | 21 ++++++++++-----------
 Analysis/src/Unifier.cpp |  6 +++---
 tests/TypeInfer.test.cpp | 21 ++++++++++++++++++++-
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/Analysis/src/Error.cpp b/Analysis/src/Error.cpp
index 680bcf3..a622f8a 100644
--- a/Analysis/src/Error.cpp
+++ b/Analysis/src/Error.cpp
@@ -112,21 +112,20 @@ struct ErrorConverter
 
     std::string operator()(const Luau::CountMismatch& e) const
     {
+        const std::string expectedS = e.expected == 1 ? "" : "s";
+        const std::string actualS = e.actual == 1 ? "" : "s";
+        const std::string actualVerb = e.actual == 1 ? "is" : "are";
+
         switch (e.context)
         {
         case CountMismatch::Return:
-        {
-            const std::string expectedS = e.expected == 1 ? "" : "s";
-            const std::string actualS = e.actual == 1 ? "is" : "are";
-            return "Expected to return " + std::to_string(e.expected) + " value" + expectedS + ", but " + std::to_string(e.actual) + " " + actualS +
-                   " returned here";
-        }
+            return "Expected to return " + std::to_string(e.expected) + " value" + expectedS + ", but " +
+                   std::to_string(e.actual) + " " + actualVerb + " returned here";
         case CountMismatch::Result:
-            if (e.expected > e.actual)
-                return "Function returns " + std::to_string(e.expected) + " values but there are only " + std::to_string(e.expected) +
-                       " values to unpack them into.";
-            else
-                return "Function only returns " + std::to_string(e.expected) + " values. " + std::to_string(e.actual) + " are required here";
+            // It is alright if right hand side produces more values than the
+            // left hand side accepts. In this context consider only the opposite case.
+            return "Function only returns " + std::to_string(e.expected) + " value" + expectedS + ". " +
+                   std::to_string(e.actual) + " are required here";
         case CountMismatch::Arg:
             return "Argument count mismatch. Function " + wrongNumberOfArgsString(e.expected, e.actual);
         }
diff --git a/Analysis/src/Unifier.cpp b/Analysis/src/Unifier.cpp
index 89c3f80..07aee38 100644
--- a/Analysis/src/Unifier.cpp
+++ b/Analysis/src/Unifier.cpp
@@ -626,11 +626,11 @@ void Unifier::tryUnify_(TypePackId superTp, TypePackId subTp, bool isFunctionCal
                 }
 
                 // This is a bit weird because we don't actually know expected vs actual.  We just know
-                // subtype vs supertype.  If we are checking a return value, we swap these to produce
-                // the expected error message.
+                // subtype vs supertype.  If we are checking the values returned by a function, we swap
+                // these to produce the expected error message.
                 size_t expectedSize = size(superTp);
                 size_t actualSize = size(subTp);
-                if (ctx == CountMismatch::Result || ctx == CountMismatch::Return)
+                if (ctx == CountMismatch::Result)
                     std::swap(expectedSize, actualSize);
                 errors.push_back(TypeError{location, CountMismatch{expectedSize, actualSize, ctx}});
 
diff --git a/tests/TypeInfer.test.cpp b/tests/TypeInfer.test.cpp
index 37333b1..d8dc0fb 100644
--- a/tests/TypeInfer.test.cpp
+++ b/tests/TypeInfer.test.cpp
@@ -3251,7 +3251,24 @@ TEST_CASE_FIXTURE(Fixture, "too_many_return_values")
 
     CountMismatch* acm = get<CountMismatch>(result.errors[0]);
     REQUIRE(acm);
-    CHECK(acm->context == CountMismatch::Result);
+    CHECK_EQ(acm->context, CountMismatch::Result);
+    CHECK_EQ(acm->expected, 1);
+    CHECK_EQ(acm->actual, 2);
+}
+
+TEST_CASE_FIXTURE(Fixture, "ignored_return_values")
+{
+    CheckResult result = check(R"(
+        --!strict
+
+        function f()
+            return 55, ""
+        end
+
+        local a = f()
+    )");
+
+    LUAU_REQUIRE_ERROR_COUNT(0, result);
 }
 
 TEST_CASE_FIXTURE(Fixture, "function_does_not_return_enough_values")
@@ -3269,6 +3286,8 @@ TEST_CASE_FIXTURE(Fixture, "function_does_not_return_enough_values")
     CountMismatch* acm = get<CountMismatch>(result.errors[0]);
     REQUIRE(acm);
     CHECK_EQ(acm->context, CountMismatch::Return);
+    CHECK_EQ(acm->expected, 2);
+    CHECK_EQ(acm->actual, 1);
 }
 
 TEST_CASE_FIXTURE(Fixture, "typecheck_unary_minus")