From 79d08cd5c1f0f9dc222563a044f5f2b05eb1f82b Mon Sep 17 00:00:00 2001
From: Stephane Del Pino <stephane.delpino44@gmail.com>
Date: Fri, 26 Jul 2019 12:40:50 +0200
Subject: [PATCH] Continue clean-up. Add ASTNodeJumpPlacementChecker and its
 tests.

---
 src/language/ASTNodeJumpPlacementChecker.cpp |  32 ++++
 src/language/ASTNodeJumpPlacementChecker.hpp |  15 ++
 src/language/CMakeLists.txt                  |   1 +
 src/language/PugsParser.cpp                  |  35 +----
 tests/CMakeLists.txt                         |   1 +
 tests/test_ASTNodeJumpPlacementChecker.cpp   | 147 +++++++++++++++++++
 6 files changed, 199 insertions(+), 32 deletions(-)
 create mode 100644 src/language/ASTNodeJumpPlacementChecker.cpp
 create mode 100644 src/language/ASTNodeJumpPlacementChecker.hpp
 create mode 100644 tests/test_ASTNodeJumpPlacementChecker.cpp

diff --git a/src/language/ASTNodeJumpPlacementChecker.cpp b/src/language/ASTNodeJumpPlacementChecker.cpp
new file mode 100644
index 000000000..ef1610ec7
--- /dev/null
+++ b/src/language/ASTNodeJumpPlacementChecker.cpp
@@ -0,0 +1,32 @@
+#include <ASTNodeJumpPlacementChecker.hpp>
+
+#include <PEGGrammar.hpp>
+
+#include <vector>
+
+void
+ASTNodeJumpPlacementChecker::_checkJumpPlacement(ASTNode& n, bool is_inside_loop)
+{
+  if (n.is<language::for_statement>() or n.is<language::do_while_statement>() or n.is<language::while_statement>()) {
+    for (auto& child : n.children) {
+      this->_checkJumpPlacement(*child, true);
+    }
+  } else if (n.is<language::break_kw>() or n.is<language::continue_kw>()) {
+    if (not is_inside_loop) {
+      std::ostringstream error_message;
+      error_message << "unexpected '" << rang::fgB::red << n.string() << rang::fg::reset
+                    << "' outside of loop or switch statement";
+      throw parse_error(error_message.str(), std::vector{n.begin()});
+    }
+  } else {
+    for (auto& child : n.children) {
+      this->_checkJumpPlacement(*child, is_inside_loop);
+    }
+  }
+}
+
+ASTNodeJumpPlacementChecker::ASTNodeJumpPlacementChecker(ASTNode& n)
+{
+  Assert(n.is_root());
+  this->_checkJumpPlacement(n, false);
+}
diff --git a/src/language/ASTNodeJumpPlacementChecker.hpp b/src/language/ASTNodeJumpPlacementChecker.hpp
new file mode 100644
index 000000000..23da8732a
--- /dev/null
+++ b/src/language/ASTNodeJumpPlacementChecker.hpp
@@ -0,0 +1,15 @@
+#ifndef AST_NODE_JUMP_PLACEMENT_CHECKER_HPP
+#define AST_NODE_JUMP_PLACEMENT_CHECKER_HPP
+
+#include <ASTNode.hpp>
+
+class ASTNodeJumpPlacementChecker
+{
+ private:
+  void _checkJumpPlacement(ASTNode& node, bool is_inside_loop);
+
+ public:
+  ASTNodeJumpPlacementChecker(ASTNode& root_node);
+};
+
+#endif   // AST_NODE_JUMP_PLACEMENT_CHECKER_HPP
diff --git a/src/language/CMakeLists.txt b/src/language/CMakeLists.txt
index b6a386df0..ae4c49563 100644
--- a/src/language/CMakeLists.txt
+++ b/src/language/CMakeLists.txt
@@ -14,6 +14,7 @@ add_library(
   ASTNodeDataTypeChecker.cpp
   ASTNodeExpressionBuilder.cpp
   ASTNodeIncDecExpressionBuilder.cpp
+  ASTNodeJumpPlacementChecker.cpp
   ASTNodeUnaryOperatorExpressionBuilder.cpp
   ASTNodeValueBuilder.cpp
   ASTPrinter.cpp
diff --git a/src/language/PugsParser.cpp b/src/language/PugsParser.cpp
index b9b0a4677..518250a29 100644
--- a/src/language/PugsParser.cpp
+++ b/src/language/PugsParser.cpp
@@ -22,6 +22,8 @@
 #include <ASTNodeDataTypeBuilder.hpp>
 #include <ASTNodeDataTypeChecker.hpp>
 
+#include <ASTNodeJumpPlacementChecker.hpp>
+
 #include <ASTNodeExpressionBuilder.hpp>
 
 #include <ASTSymbolInitializationChecker.hpp>
@@ -34,37 +36,6 @@
 
 namespace language
 {
-namespace internal
-{
-void
-check_break_or_continue_placement(const ASTNode& n, bool is_inside_loop)
-{
-  if (n.is<language::for_statement>() or n.is<language::do_while_statement>() or n.is<language::while_statement>()) {
-    for (auto& child : n.children) {
-      check_break_or_continue_placement(*child, true);
-    }
-  } else if (n.is<language::break_kw>() or n.is<language::continue_kw>()) {
-    if (not is_inside_loop) {
-      std::ostringstream error_message;
-      error_message << "unexpected '" << rang::fgB::red << n.string() << rang::fg::reset
-                    << "' outside of loop or switch statement";
-      throw parse_error(error_message.str(), std::vector{n.begin()});
-    }
-  } else {
-    for (auto& child : n.children) {
-      check_break_or_continue_placement(*child, is_inside_loop);
-    }
-  }
-}
-}   // namespace internal
-
-void
-check_break_or_continue_placement(const ASTNode& n)
-{
-  Assert(n.is_root());
-  internal::check_break_or_continue_placement(n, false);
-}
-
 namespace internal
 {
 void
@@ -126,7 +97,7 @@ parser(const std::string& filename)
 
     ASTNodeValueBuilder{*root_node};
 
-    language::check_break_or_continue_placement(*root_node);
+    ASTNodeJumpPlacementChecker{*root_node};
 
     // optimizations
     language::simplify_declarations(*root_node);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index c3c89e619..246771d48 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -10,6 +10,7 @@ add_executable (unit_tests
   test_ASTNodeDataType.cpp
   test_ASTNodeDataTypeBuilder.cpp
   test_ASTNodeDataTypeChecker.cpp
+  test_ASTNodeJumpPlacementChecker.cpp
   test_ASTNodeValueBuilder.cpp
   test_ASTPrinter.cpp
   test_ASTSymbolTableBuilder.cpp
diff --git a/tests/test_ASTNodeJumpPlacementChecker.cpp b/tests/test_ASTNodeJumpPlacementChecker.cpp
new file mode 100644
index 000000000..7a7679199
--- /dev/null
+++ b/tests/test_ASTNodeJumpPlacementChecker.cpp
@@ -0,0 +1,147 @@
+#include <catch2/catch.hpp>
+
+#include <ASTBuilder.hpp>
+
+#include <ASTNodeDataTypeBuilder.hpp>
+#include <ASTNodeJumpPlacementChecker.hpp>
+
+TEST_CASE("ASTNodeJumpPlacementChecker", "[language]")
+{
+  SECTION("break")
+  {
+    SECTION("in for loop")
+    {
+      std::string_view data = R"(
+for(;;) {
+  break;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("in while loop")
+    {
+      std::string_view data = R"(
+while(true) {
+  break;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("in do while loop")
+    {
+      std::string_view data = R"(
+do {
+  break;
+} while(true);
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("misplaced")
+    {
+      std::string_view data = R"(
+{
+  break;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      ast->children[0]->m_data_type = ASTNodeDataType::undefined_t;
+
+      REQUIRE_THROWS_AS(ASTNodeJumpPlacementChecker{*ast}, parse_error);
+    }
+  }
+
+  SECTION("continue")
+  {
+    SECTION("in for loop")
+    {
+      std::string_view data = R"(
+for(;;) {
+  continue;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("in while loop")
+    {
+      std::string_view data = R"(
+while(true) {
+  continue;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("in do while loop")
+    {
+      std::string_view data = R"(
+do {
+  continue;
+} while(true);
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      REQUIRE_NOTHROW(ASTNodeJumpPlacementChecker{*ast});
+    }
+
+    SECTION("misplaced")
+    {
+      std::string_view data = R"(
+{
+  continue;
+}
+)";
+
+      string_input input{data, "test.pgs"};
+      auto ast = ASTBuilder::build(input);
+
+      ASTNodeDataTypeBuilder{*ast};
+
+      ast->children[0]->m_data_type = ASTNodeDataType::undefined_t;
+
+      REQUIRE_THROWS_AS(ASTNodeJumpPlacementChecker{*ast}, parse_error);
+    }
+  }
+}
-- 
GitLab