fix(parser): detecting ill-formed value macros

ArkScript-lang · Oct 17, 2024 · b7e365c · b7e365c
1 parent a0cbf79
commit b7e365c
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -87,6 +87,7 @@
   - byte1 is 0 if the instruction takes a single argument on 16 bits, split on byte2 and byte3
   - if the instruction takes two arguments, they each have 12 bits ; the second one is on byte1 and upper half of byte2, the first on lower half of byte2 and then byte3
 - ast-to-json dump now supports macros
+- the parser can detect ill-formed macros (that are seen as function macros while being value macros)
 
 ### Removed
 - removed unused `NodeType::Closure`

diff --git a/src/arkreactor/Compiler/AST/Parser.cpp b/src/arkreactor/Compiler/AST/Parser.cpp
@@ -711,7 +711,19 @@ namespace Ark::internal
         if (value.has_value())
             leaf->push_back(value.value());
         else
-            errorWithNextToken(fmt::format("Expected a value while defining macro `{}'", symbol));
+        {
+            backtrack(position);
+
+            if (leaf->list().size() == 2)
+                errorWithNextToken(
+                    fmt::format(
+                        "Expected a body while defined macro `{0}'. If you were trying to define a macro based on a function call, try wrapping it inside a begin node: ($ {0} {{ {1} }})."
+                        "\nWithout the begin node, '{1}' is seen as an argument list.",
+                        symbol,
+                        leaf->list().back().repr()));
+            else
+                errorWithNextToken(fmt::format("Expected a value while defining macro `{}'", symbol));
+        }
 
         setNodePosAndFilename(leaf->list().back());
         comment.clear();

diff --git a/src/arkreactor/Compiler/Macros/Processor.cpp b/src/arkreactor/Compiler/Macros/Processor.cpp
@@ -267,10 +267,11 @@ namespace Ark::internal
                 return macro->constList()[1];
             return node;
         }
-        if (node.nodeType() == NodeType::List && node.constList().size() > 1 && node.list()[0].nodeType() == NodeType::Symbol)
+        if (node.nodeType() == NodeType::List && !node.constList().empty() && node.list()[0].nodeType() == NodeType::Symbol)
         {
             const std::string& name = node.list()[0].string();
             const std::size_t argcount = node.list().size() - 1;
+
             if (const Node* macro = findNearestMacro(name); macro != nullptr)
             {
                 applyMacro(node.list()[0], depth + 1);

diff --git a/src/arkreactor/Exceptions.cpp b/src/arkreactor/Exceptions.cpp
@@ -130,15 +130,19 @@ namespace Ark::Diagnostics
     }
 
     template <typename T>
-    void helper(std::ostream& os, const std::string& message, bool colorize, const std::string& filename, const std::string& code, const T& expr, const std::size_t line, std::size_t column, const std::size_t sym_size)
+    void helper(std::ostream& os, const std::string& message, const bool colorize, const std::string& filename, const std::string& code, const T& expr,
+                const std::size_t line, std::size_t column, const std::size_t sym_size)
     {
         if (filename != ARK_NO_NAME_FILE)
             fmt::print(os, "In file {}\n", filename);
         fmt::print(os, "At {} @ {}:{}\n", expr, line + 1, column);
 
         if (!code.empty())
             makeContext(os, code, line, column, sym_size, colorize);
-        fmt::print(os, "        {}", message);
+
+        const auto message_lines = Utils::splitString(message, '\n');
+        for (const auto& text : message_lines)
+            fmt::print(os, "        {}\n", text);
     }
 
     std::string makeContextWithNode(const std::string& message, const internal::Node& node)

diff --git a/tests/unittests/DiagnosticsSuite.cpp b/tests/unittests/DiagnosticsSuite.cpp
@@ -25,8 +25,9 @@ ut::suite<"Diagnostics"> diagnostics_suite = [] {
                 }
                 catch (const Ark::CodeError& e)
                 {
-                    const std::string diag = sanitize_error(e, /* remove_in_file_line= */ true);
-                    expect(that % (diag + "\n") == data.expected);
+                    std::string diag = sanitize_error(e, /* remove_in_file_line= */ true);
+                    rtrim(diag);
+                    expect(that % diag == data.expected);
                 }
             };
         });
@@ -52,7 +53,8 @@ ut::suite<"Diagnostics"> diagnostics_suite = [] {
                     std::string diag = e.what();
                     if (diag.find_first_of('\n') != std::string::npos)
                         diag.erase(diag.find_first_of('\n'), diag.size() - 1);
-                    expect(that % diag == data.expected or diag + "\n" == data.expected);
+                    ltrim(rtrim(diag));
+                    expect(that % diag == data.expected);
                 }
             };
         });

diff --git a/tests/unittests/FormatterSuite.cpp b/tests/unittests/FormatterSuite.cpp
@@ -21,7 +21,9 @@ ut::suite<"Formatter"> formatter_suite = [] {
                 }));
 
                 formatted_code = formatter.output();
-                expect(that % formatted_code == data.expected);
+                // data.expected is ltrim(rtrim(file content))
+                // we want to ensure that a blank line has been added
+                expect(that % formatted_code == (data.expected + "\n"));
             };
 
             should("not update an already correctly formatted code (" + data.stem + ")") = [&] {
@@ -30,8 +32,8 @@ ut::suite<"Formatter"> formatter_suite = [] {
                     mut(formatter).runWithString(formatted_code);
                 }));
 
-                std::string code = formatter.output();
+                const std::string code = formatter.output();
                 expect(that % code == formatted_code);
             };
         });
-};
+};
diff --git a/tests/unittests/ParserSuite.cpp b/tests/unittests/ParserSuite.cpp
@@ -60,7 +60,8 @@ ut::suite<"Parser"> parser_suite = [] {
                     }));
                 };
 
-                const std::string ast = astToString(parser);
+                std::string ast = astToString(parser);
+                ltrim(rtrim(ast));
 
                 should("output the same AST and imports (" + data.stem + ")") = [&] {
                     expect(that % ast == data.expected);
@@ -81,7 +82,8 @@ ut::suite<"Parser"> parser_suite = [] {
                 catch (const Ark::CodeError& e)
                 {
                     should("output the same error message (" + data.stem + ")") = [&] {
-                        const std::string tested = sanitize_error(e);
+                        std::string tested = sanitize_error(e);
+                        ltrim(rtrim(tested));
                         expect(that % tested == data.expected);
                     };
                 }

diff --git a/tests/unittests/TestsHelper.cpp b/tests/unittests/TestsHelper.cpp
@@ -14,6 +14,7 @@ void iter_test_files(const std::string& folder, std::function<void(TestData&&)>&
         std::string expected = Ark::Utils::readFile(expected_path.generic_string());
         // getting rid of the \r because of Windows
         expected.erase(std::remove(expected.begin(), expected.end(), '\r'), expected.end());
+        ltrim(rtrim(expected));
 
         auto data = TestData {
             .path = entry.path().generic_string(),

diff --git a/tests/unittests/TestsHelper.hpp b/tests/unittests/TestsHelper.hpp
@@ -8,6 +8,9 @@
 #include <algorithm>
 #include <functional>
 #include <filesystem>
+#include <cctype>
+#include <locale>
+#include <ranges>
 
 #ifndef ARK_TESTS_ROOT
 #    define ARK_TESTS_ROOT ""
@@ -24,6 +27,23 @@ void iter_test_files(const std::string& folder, std::function<void(TestData&&)>&
 
 std::string get_resource_path(const std::string& folder);
 
+inline std::string& ltrim(std::string& s)
+{
+    s.erase(s.begin(), std::ranges::find_if(s, [](const unsigned char ch) {
+                return !std::isspace(ch);
+            }));
+    return s;
+}
+
+inline std::string& rtrim(std::string& s)
+{
+    s.erase(std::ranges::find_if(s.rbegin(), s.rend(), [](const unsigned char ch) {
+                return !std::isspace(ch);
+            }).base(),
+            s.end());
+    return s;
+}
+
 std::string sanitize_error(const Ark::CodeError& e, bool remove_in_file_line = false);
 
 #endif  // ARK_TESTSHELPER_HPP
diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark b/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.ark
@@ -0,0 +1,2 @@
+($ b ($symcat c))
+(print b)
diff --git a/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected b/tests/unittests/resources/DiagnosticsSuite/compileTime/bad_macro_arg_list.expected
@@ -0,0 +1,7 @@
+At ( @ 1:6
+    1 | ($ b ($symcat c))
+      |      ^
+    2 | (print b)
+    3 |
+        Expected a body while defined macro `b'. If you were trying to define a macro based on a function call, try wrapping it inside a begin node: ($ b { ($symcat c) }).
+        Without the begin node, '($symcat c)' is seen as an argument list.