Feat/better type error messages

CHANGELOG.md

@@ -1,6 +1,9 @@
 # Change Log
 
 ## [Unreleased version] - 20XX-XX-XX
+### Breaking changes
+- Function arguments are now immutable by default and an argument attribute `mut` must be added: `(fun (a b c) (set b 5))` -> `(fun (a (mut b) c) (set b 5))`
+
 ### Added
 - new builtin `disassemble` to print the bytecode of a function
 - new builtin `io:readFileLines` to read lines from a file as a list of strings

include/Ark/Error/Exceptions.hpp

@@ -22,6 +22,8 @@
 
 namespace Ark
 {
+    class VM;
+
     namespace internal
     {
         class Node;
@@ -34,7 +36,7 @@ namespace Ark
             std::runtime_error(message)
         {}
 
-        [[nodiscard]] virtual std::string details(bool colorize [[maybe_unused]]) const
+        [[nodiscard]] virtual std::string details(bool colorize [[maybe_unused]], VM& vm [[maybe_unused]]) const
         {
             return what();
         }
@@ -67,9 +69,9 @@ namespace Ark
     class ARK_API NestedError final : public Error
     {
     public:
-        NestedError(const Error& e, const std::string& details) :
+        NestedError(const Error& e, const std::string& details, VM& vm) :
             Error("NestedError"),
-            m_details(e.details(/* colorize= */ false))
+            m_details(e.details(/* colorize= */ false, vm))
         {
             if (!m_details.empty() && m_details.back() != '\n')
                 m_details += '\n';

include/Ark/TypeChecker.hpp

@@ -19,6 +19,11 @@
 #include <Ark/Error/Exceptions.hpp>
 #include <Ark/VM/Value.hpp>
 
+namespace Ark
+{
+    class VM;
+}
+
 namespace Ark::types
 {
     namespace details
@@ -92,13 +97,15 @@ namespace Ark::types
      * @param funcname ArkScript name of the function
      * @param contracts types contracts the function can follow
      * @param args provided argument list
+     * @param vm reference to the VM used for pretty printing closures
      * @param os output stream, default to cout
      * @param colorize enable output colorizing
      */
     ARK_API void generateError(
         const std::string_view& funcname,
         const std::vector<Contract>& contracts,
         const std::vector<Value>& args,
+        VM& vm,
         std::ostream& os = std::cout,
         bool colorize = true);
 
@@ -112,10 +119,10 @@ namespace Ark::types
             m_passed_args(args)
         {}
 
-        [[nodiscard]] std::string details(const bool colorize) const override
+        [[nodiscard]] std::string details(const bool colorize, VM& vm) const override
         {
             std::stringstream stream;
-            generateError(m_funcname, m_contracts, m_passed_args, stream, colorize);
+            generateError(m_funcname, m_contracts, m_passed_args, vm, stream, colorize);
             return stream.str();
         }
 

src/arkreactor/TypeChecker.cpp

@@ -7,6 +7,8 @@
 #include <fmt/color.h>
 #include <fmt/ostream.h>
 
+#include <Ark/VM/VM.hpp>
+
 namespace Ark::types
 {
     std::string typeListToString(const std::vector<ValueType>& types)
@@ -20,13 +22,18 @@ namespace Ark::types
         {
             if (i > 0)
                 acc += ", ";
+            if (i + 1 == end && types.size() > 1)
+                acc += "or ";
             acc += std::to_string(types[i]);
         }
         return acc;
     }
 
-    void displayContract(const Contract& contract, const std::vector<Value>& args, std::ostream& os, const bool colorize)
+    void displayContract(const std::string_view& funcname, const Contract& contract, const std::vector<Value>& args, VM& vm, std::ostream& os, const bool colorize)
     {
+        const std::string checkmark = "✓";
+        const std::string crossmark = "×";
+
         auto displayArg = [colorize, &os](const Typedef& td, const bool correct) {
             const std::string arg_str = typeListToString(td.types);
 
@@ -43,35 +50,51 @@ namespace Ark::types
                 store.push_back(td.name);
             store.push_back(arg_str);
 
-            fmt::vprint(os, "  -> {}{} ({})", store);
+            fmt::vprint(os, "  → {}`{}' (expected {})", store);
         };
 
+        fmt::print(os, "Signature\n  ↳ ({}", funcname);
+        for (const Typedef& td : contract.arguments)
+            fmt::print(os, " {}", td.name);
+        fmt::print(os, ")\nArguments\n");
+
         for (std::size_t i = 0, end = contract.arguments.size(); i < end; ++i)
         {
             const Typedef& td = contract.arguments[i];
 
             if (td.variadic && i < args.size())
             {
                 // variadic argument in contract and enough provided arguments
-                std::size_t bad_type = 0;
+                std::size_t bad_type_count = 0;
+                std::string formatted_varargs;
                 for (std::size_t j = i, args_end = args.size(); j < args_end; ++j)
                 {
+                    bool type_ok = true;
                     if (td.types[0] != ValueType::Any && std::ranges::find(td.types, args[j].valueType()) == td.types.end())
-                        bad_type++;
+                    {
+                        bad_type_count++;
+                        type_ok = false;
+                    }
+                    formatted_varargs += fmt::format(
+                        "\n    {} ({}) {}",
+                        args[j].toString(vm),
+                        std::to_string(args[j].valueType()),
+                        type_ok ? checkmark : crossmark);
                 }
 
-                if (bad_type)
+                if (bad_type_count)
                 {
                     displayArg(td, /* correct= */ false);
 
                     fmt::dynamic_format_arg_store<fmt::format_context> store;
                     if (colorize)
-                        store.push_back(fmt::styled(bad_type, fmt::fg(fmt::color::red)));
+                        store.push_back(fmt::styled(bad_type_count, fmt::fg(fmt::color::red)));
                     else
-                        store.push_back(bad_type);
-                    store.push_back(bad_type > 1 ? "s" : "");
+                        store.push_back(bad_type_count);
+                    store.push_back(bad_type_count > 1 ? "s" : "");
+                    store.push_back(formatted_varargs);
 
-                    fmt::vprint(os, " {} argument{} do not match", store);
+                    fmt::vprint(os, ": {} argument{} do not match:{}", store);
                 }
                 else
                     displayArg(td, /* correct= */ true);
@@ -85,11 +108,12 @@ namespace Ark::types
                     const auto type = std::to_string(args[i].valueType());
 
                     fmt::dynamic_format_arg_store<fmt::format_context> store;
+                    store.push_back(args[i].toString(vm));
                     if (colorize)
                         store.push_back(fmt::styled(type, fmt::fg(fmt::color::red)));
                     else
                         store.push_back(type);
-                    fmt::vprint(os, " was of type {}", store);
+                    fmt::vprint(os, ", got {} ({})", store);
                 }
                 // non-provided argument
                 else if (i >= args.size())
@@ -101,13 +125,28 @@ namespace Ark::types
                         fmt::print(os, " was not provided");
                 }
                 else
+                {
                     displayArg(td, /* correct= */ true);
+                    fmt::print(os, " {}", checkmark);
+                }
+            }
+            fmt::print(os, "\n");
+        }
+
+        if (contract.arguments.size() < args.size())
+        {
+            fmt::print(os, "  → unexpected additional args: ");
+            for (std::size_t i = contract.arguments.size(), end = args.size(); i < end; ++i)
+            {
+                fmt::print(os, "{} ({})", args[i].toString(vm), std::to_string(args[i].valueType()));
+                if (i + 1 != end)
+                    fmt::print(os, ", ");
             }
             fmt::print(os, "\n");
         }
     }
 
-    void generateError(const std::string_view& funcname, const std::vector<Contract>& contracts, const std::vector<Value>& args, std::ostream& os, bool colorize)
+    void generateError(const std::string_view& funcname, const std::vector<Contract>& contracts, const std::vector<Value>& args, VM& vm, std::ostream& os, bool colorize)
     {
         {
             fmt::dynamic_format_arg_store<fmt::format_context> store;
@@ -183,13 +222,16 @@ namespace Ark::types
                 fmt::print(os, " {} got {}", preposition, sanitizedArgs.size());
         }
 
-        fmt::print(os, "\n");
+        fmt::print(os, "\nCall\n  ↳ ({}", funcname);
+        for (const Value& arg : args)
+            fmt::print(os, " {}", arg.toString(vm));
+        fmt::print(os, ")\n");
 
-        displayContract(contracts[0], sanitizedArgs, os, colorize);
+        displayContract(funcname, contracts[0], sanitizedArgs, vm, os, colorize);
         for (std::size_t i = 1, end = contracts.size(); i < end; ++i)
         {
-            fmt::print(os, "Alternative {}:\n", i + 1);
-            displayContract(contracts[i], sanitizedArgs, os, colorize);
+            fmt::print(os, "\nAlternative {}:\n", i + 1);
+            displayContract(funcname, contracts[i], sanitizedArgs, vm, os, colorize);
         }
     }
 }

src/arkreactor/VM/VM.cpp

@@ -1997,10 +1997,10 @@ namespace Ark
                 backtrace(context, stream, /* colorize= */ false);
                 // It's important we have an Ark::Error here, as the constructor for NestedError
                 // does more than just aggregate error messages, hence the code duplication.
-                throw NestedError(e, stream.str());
+                throw NestedError(e, stream.str(), *this);
             }
             else
-                showBacktraceWithException(Error(e.details(/* colorize= */ true)), context);
+                showBacktraceWithException(Error(e.details(/* colorize= */ true, *this)), context);
         }
         catch (const std::exception& e)
         {

tests/unittests/Main.cpp

@@ -2,15 +2,20 @@
 #include <fmt/format.h>
 #include <fmt/chrono.h>
 
+#include <TestsHelper.hpp>
+
 #include <string>
 #include <chrono>
 
 int main(const int argc, char** argv)
 {
     using namespace boost;
 
+    if (argc == 2 && std::string(argv[1]) == "update")
+        shouldWriteNewDiffsTofile(true);
+
     std::string filter = "*";
-    if (argc >= 2)
+    if (argc >= 2 && std::string(argv[1]) != "update")
         filter = argv[1];
 
     bool failed = false;

tests/unittests/Suites/DiagnosticsSuite.cpp

@@ -28,6 +28,8 @@ ut::suite<"Diagnostics"> diagnostics_suite = [] {
                     std::string diag = sanitizeCodeError(e, /* remove_in_file_line= */ true);
                     Ark::Utils::rtrim(diag);
                     expectOrDiff(data.expected, diag);
+                    if (shouldWriteNewDiffsTofile() && data.expected != diag)
+                        updateExpectedFile(data, diag);
                 }
             };
         });
@@ -52,6 +54,8 @@ ut::suite<"Diagnostics"> diagnostics_suite = [] {
                 {
                     std::string diag = sanitizeRuntimeError(e);
                     expectOrDiff(data.expected, diag);
+                    if (shouldWriteNewDiffsTofile() && data.expected != diag)
+                        updateExpectedFile(data, diag);
                 }
             };
         });
@@ -76,6 +80,8 @@ ut::suite<"Diagnostics"> diagnostics_suite = [] {
                 {
                     std::string diag = sanitizeRuntimeError(e);
                     expectOrDiff(data.expected, diag);
+                    if (shouldWriteNewDiffsTofile() && data.expected != diag)
+                        updateExpectedFile(data, diag);
                 }
             };
         });

tests/unittests/Suites/TypeCheckerSuite.cpp

@@ -5,6 +5,8 @@
 #include <sstream>
 #include <fmt/core.h>
 
+#include <Ark/VM/State.hpp>
+#include <Ark/VM/VM.hpp>
 #include <Ark/VM/Value.hpp>
 #include <Ark/TypeChecker.hpp>
 #include <Ark/Utils/Utils.hpp>
@@ -207,17 +209,22 @@ ut::suite<"TypeChecker"> type_checker_suite = [] {
             }
 
             should("generate error message " + data.stem) = [inputs, contracts, data] {
+                Ark::State dummy_state;
+                Ark::VM dummy_VM(dummy_state);
                 std::stringstream stream;
                 Ark::types::generateError(
                     inputs.front().func,
                     contracts,
                     inputs.front().given_args,
+                    dummy_VM,
                     stream,
                     /* colorize= */ false);
 
                 auto result = stream.str();
                 Ark::Utils::rtrim(Ark::Utils::ltrim(result));
                 expectOrDiff(data.expected, result);
+                if (shouldWriteNewDiffsTofile() && data.expected != result)
+                    updateExpectedFile(data, result);
             };
         },
         { .skip_folders = false });

tests/unittests/TestsHelper.cpp

@@ -6,6 +6,28 @@
 #include <sstream>
 #include <boost/ut.hpp>
 
+bool shouldWriteNewDiffsTofile(const std::optional<bool> should)
+{
+    static bool update = false;
+    if (should.has_value())
+        update = should.value();
+
+    return update;
+}
+
+void updateExpectedFile(const TestData& data, const std::string& actual)
+{
+    std::filesystem::path expected_path = data.path;
+    expected_path.replace_extension("expected");
+
+    std::ofstream f(expected_path.generic_string());
+    if (f.is_open())
+    {
+        f << actual;
+        f.close();
+    }
+}
+
 void iterTestFiles(const std::string& folder, std::function<void(TestData&&)>&& test, IterTestFilesParam&& params)
 {
     boost::ut::test(folder) = [&] {

tests/unittests/TestsHelper.hpp

@@ -21,6 +21,13 @@
 const auto lib_path = std::filesystem::path(ARK_TESTS_ROOT "/lib/");
 const auto unittests_path = std::filesystem::path(ARK_TESTS_ROOT "/tests/unittests/");
 
+/**
+ * @brief Check if the tests should update their dataset
+ * @param should used by Main.cpp to toggle on/off the updating mechanism
+ * @return true if the tests should write the actual data to the 'expected' file
+ */
+bool shouldWriteNewDiffsTofile(std::optional<bool> should = std::nullopt);
+
 struct TestData
 {
     std::string path;      ///< The file we are testing, eg tests/unittests/resources/ASTSuite/testname.ark
@@ -37,6 +44,8 @@ struct IterTestFilesParam
     bool ignore_expected = false;
 };
 
+void updateExpectedFile(const TestData& data, const std::string& actual);
+
 /**
  * @brief Iterate over the files inside a folder, looking for "name.ark" & "name.expected" files to create a TestData structure
  * @param folder folder to list files in