fix(parser): report unclosed multiline comments (#924)

* fix(parser): report unclosed multiline comments

* chore: make multi line comment variable names clearer

---------

Co-authored-by: Jan Laupetin <[email protected]>

src/Parser/Parsing/Impl/CommentRemovingStreamProxy.cpp

@@ -1,9 +1,12 @@
 #include "CommentRemovingStreamProxy.h"
 
+#include "Parsing/ParsingException.h"
+
 CommentRemovingStreamProxy::CommentRemovingStreamProxy(IParserLineStream* stream)
     : m_stream(stream),
       m_inside_multi_line_comment(false),
-      m_next_line_is_comment(false)
+      m_next_line_is_comment(false),
+      m_multi_line_comment_pos_in_start_line(0u)
 {
 }
 
@@ -11,13 +14,20 @@ ParserLine CommentRemovingStreamProxy::NextLine()
 {
     auto line = m_stream->NextLine();
 
+    if (line.IsEof() && m_inside_multi_line_comment)
+    {
+        throw ParsingException(
+            TokenPos(*m_multi_line_comment_start_line.m_filename, m_multi_line_comment_start_line.m_line_number, m_multi_line_comment_pos_in_start_line + 1u),
+            "Unclosed multi-line comment");
+    }
+
     if (m_next_line_is_comment)
     {
         m_next_line_is_comment = !line.m_line.empty() && line.m_line[line.m_line.size() - 1] == '\\';
         return ParserLine(line.m_filename, line.m_line_number, std::string());
     }
 
-    auto multiLineCommentStart = 0u;
+    auto multiLineCommentStartInCurrentLine = 0u;
     auto inString = false;
     auto isEscaped = false;
     for (auto i = 0u; i < line.m_line.size(); i++)
@@ -28,9 +38,9 @@ ParserLine CommentRemovingStreamProxy::NextLine()
         {
             if (c == '*' && i + 1 < line.m_line.size() && line.m_line[i + 1] == '/')
             {
-                line.m_line.erase(multiLineCommentStart, i + 2 - multiLineCommentStart);
+                line.m_line.erase(multiLineCommentStartInCurrentLine, i + 2 - multiLineCommentStartInCurrentLine);
-                i = multiLineCommentStart - 1;
+                i = multiLineCommentStartInCurrentLine - 1;
-                multiLineCommentStart = 0;
+                multiLineCommentStartInCurrentLine = 0;
                 m_inside_multi_line_comment = false;
             }
         }
@@ -55,8 +65,10 @@ ParserLine CommentRemovingStreamProxy::NextLine()
 
                 if (c1 == '*')
                 {
-                    multiLineCommentStart = i;
+                    multiLineCommentStartInCurrentLine = i;
                     m_inside_multi_line_comment = true;
+                    m_multi_line_comment_start_line = line;
+                    m_multi_line_comment_pos_in_start_line = i;
                 }
                 else if (c1 == '/')
                 {
@@ -69,7 +81,7 @@ ParserLine CommentRemovingStreamProxy::NextLine()
     }
 
     if (m_inside_multi_line_comment)
-        line.m_line.erase(multiLineCommentStart);
+        line.m_line.erase(multiLineCommentStartInCurrentLine);
 
     return line;
 }

src/Parser/Parsing/Impl/CommentRemovingStreamProxy.h

@@ -7,6 +7,8 @@ class CommentRemovingStreamProxy final : public IParserLineStream
     IParserLineStream* const m_stream;
     bool m_inside_multi_line_comment;
     bool m_next_line_is_comment;
+    ParserLine m_multi_line_comment_start_line;
+    size_t m_multi_line_comment_pos_in_start_line;
 
 public:
     explicit CommentRemovingStreamProxy(IParserLineStream* stream);

test/ParserTests/Parsing/Impl/CommentRemovingStreamProxyTests.cpp

@@ -1,8 +1,13 @@
 #include "Parsing/Impl/CommentRemovingStreamProxy.h"
 #include "Parsing/Mock/MockParserLineStream.h"
+#include "Parsing/ParsingException.h"
 
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/generators/catch_generators.hpp>
+#include <catch2/matchers/catch_matchers_exception.hpp>
+#include <catch2/matchers/catch_matchers_string.hpp>
+
+using namespace Catch::Matchers;
 
 namespace test::parsing::impl::comment_removing_stream_proxy
 {
@@ -192,6 +197,19 @@ namespace test::parsing::impl::comment_removing_stream_proxy
         REQUIRE(proxy.Eof());
     }
 
+    TEST_CASE("CommentRemovingStreamProxy: Unclosed multiline comment at end of file throws an error", "[parsing][parsingstream]")
+    {
+        const std::vector<std::string> lines{"before /* comment", "still comment"};
+
+        MockParserLineStream mockStream(lines);
+        CommentRemovingStreamProxy proxy(&mockStream);
+
+        REQUIRE(proxy.NextLine().m_line == "before ");
+        REQUIRE(proxy.NextLine().m_line.empty());
+        REQUIRE_THROWS_MATCHES(
+            proxy.NextLine(), ParsingException, MessageMatches(ContainsSubstring("L1:8") && ContainsSubstring("Unclosed multi-line comment")));
+    }
+
     TEST_CASE("CommentRemovingStreamProxy: Can have multiple comment blocks in one line", "[parsing][parsingstream]")
     {
         const std::vector<std::string> lines{