Write additional parser tests, update some of the existing tests to work with throw_on_error

include/ss/common.hpp

@@ -19,6 +19,12 @@ inline void assert_string_error_defined() {
                   "'string_error' needs to be enabled to use 'error_msg'");
 }
 
+template <bool ThrowOnError>
+inline void assert_throw_on_error_not_defined() {
+    static_assert(!ThrowOnError, "cannot handle errors manually if "
+                                 "'throw_on_error' is enabled");
+}
+
 #if __unix__
 inline ssize_t get_line(char** lineptr, size_t* n, FILE* stream) {
     return getline(lineptr, n, stream);

include/ss/extract.hpp

@@ -167,6 +167,7 @@ inline bool sub_overflow(long long& result, long long operand) {
     return __builtin_ssubll_overflow(result, operand, &result);
 }
 
+// Note: sub_overflow function should be unreachable for unsigned values
 template <>
 inline bool sub_overflow(unsigned int& result, unsigned int operand) {
     return __builtin_usub_overflow(result, operand, &result);
@@ -184,8 +185,8 @@ inline bool sub_overflow(unsigned long long& result,
 }
 
 template <typename T, typename F>
-bool shift_and_add_overflow(T& value, T digit, F add_last_digit_owerflow) {
-    if (mul_overflow<T>(value, 10) || add_last_digit_owerflow(value, digit)) {
+bool shift_and_add_overflow(T& value, T digit, F add_last_digit_overflow) {
+    if (mul_overflow<T>(value, 10) || add_last_digit_overflow(value, digit)) {
         return true;
     }
     return false;
@@ -223,17 +224,17 @@ std::enable_if_t<std::is_integral_v<T>, std::optional<T>> to_num(
 
 #if (defined(__clang__) || defined(__GNUC__) || defined(__GUNG__)) &&          \
     !defined(MINGW32_CLANG)
-    auto add_last_digit_owerflow =
+    auto add_last_digit_overflow =
         (is_negative) ? sub_overflow<T> : add_overflow<T>;
 #else
-    auto add_last_digit_owerflow = is_negative;
+    auto add_last_digit_overflow = is_negative;
 #endif
 
     T value = 0;
     for (auto i = begin; i != end; ++i) {
         if (auto digit = from_char(*i);
             !digit || shift_and_add_overflow<T>(value, digit.value(),
-                                                add_last_digit_owerflow)) {
+                                                add_last_digit_overflow)) {
             return std::nullopt;
         }
     }

include/ss/parser.hpp

@@ -287,7 +287,7 @@ public:
 
         template <typename Fun>
         auto on_error(Fun&& fun) {
-            // TODO disable these if throw_on_error
+            assert_throw_on_error_not_defined<throw_on_error>();
             if (!parser_.valid()) {
                 if constexpr (std::is_invocable_v<Fun>) {
                     fun();
@@ -355,6 +355,7 @@ public:
     template <typename... Ts, typename Fun = none>
     composite<std::optional<no_void_validator_tup_t<Ts...>>> try_next(
         Fun&& fun = none{}) {
+        assert_throw_on_error_not_defined<throw_on_error>();
         using Ret = no_void_validator_tup_t<Ts...>;
         return try_invoke_and_make_composite<
             std::optional<Ret>>(get_next<Ts...>(), std::forward<Fun>(fun));
@@ -364,6 +365,7 @@ public:
     // tuple
     template <typename T, typename... Ts, typename Fun = none>
     composite<std::optional<T>> try_object(Fun&& fun = none{}) {
+        assert_throw_on_error_not_defined<throw_on_error>();
         return try_invoke_and_make_composite<
             std::optional<T>>(get_object<T, Ts...>(), std::forward<Fun>(fun));
     }
@@ -742,7 +744,6 @@ private:
             return size;
         }
 
-        // TODO check why multiline fields result in additional allocations
         void realloc_concat(char*& first, size_t& first_size,
                             const char* const second, size_t second_size) {
             // TODO make buffer_size an argument

test/meson.build

@@ -14,13 +14,14 @@ tests = [
 ]
 
 foreach name : tests
+  test_name = 'test_' + name
 
   exe = executable(
-    name,
-    'test_' + name + '.cpp',
+    test_name,
+    test_name + '.cpp',
     dependencies: [doctest_dep, ssp_dep]
     )
 
-  test('test_' + name, exe, timeout: 60)
+  test(test_name, exe, timeout: 60)
 endforeach
 

test/test_parser.cpp

@@ -9,6 +9,9 @@
 #include <unordered_map>
 #include <unordered_set>
 
+// TODO remove
+#include <iostream>
+
 namespace {
 [[maybe_unused]] void replace_all(std::string& s, const std::string& from,
                                   const std::string& to) {
@@ -86,13 +89,35 @@ static void make_and_write(const std::string& file_name,
 }
 } /* namespace */
 
-TEST_CASE("parser test various cases") {
+TEST_CASE("test file not found") {
+    unique_file_name f{"test_parser"};
+
+    {
+        ss::parser p{f.name, ","};
+        CHECK_FALSE(p.valid());
+    }
+
+    {
+        ss::parser<ss::string_error> p{f.name, ","};
+        CHECK_FALSE(p.valid());
+    }
+
+    try {
+        ss::parser<ss::throw_on_error> p{f.name, ","};
+        FAIL("Expected exception...");
+    } catch (const std::exception& e) {
+        CHECK_FALSE(std::string{e.what()}.empty());
+    }
+}
+
+template <typename... Ts>
+void test_various_cases() {
     unique_file_name f{"test_parser"};
     std::vector<X> data = {{1, 2, "x"}, {3, 4, "y"},  {5, 6, "z"},
                            {7, 8, "u"}, {9, 10, "v"}, {11, 12, "w"}};
     make_and_write(f.name, data);
     {
-        ss::parser<ss::string_error> p{f.name, ","};
+        ss::parser<Ts...> p{f.name, ","};
         ss::parser p0{std::move(p)};
         p = std::move(p0);
         std::vector<X> i;
@@ -101,7 +126,7 @@ TEST_CASE("parser test various cases") {
         std::vector<X> i2;
 
         while (!p.eof()) {
-            auto a = p.get_next<int, double, std::string>();
+            auto a = p.template get_next<int, double, std::string>();
             i.emplace_back(ss::to_object<X>(a));
         }
 
@@ -319,6 +344,12 @@ TEST_CASE("parser test various cases") {
     }
 }
 
+TEST_CASE("parser test various cases") {
+    test_various_cases();
+    test_various_cases<ss::string_error>();
+    test_various_cases<ss::throw_on_error>();
+}
+
 using test_tuple = std::tuple<double, char, double>;
 struct test_struct {
     int i;
@@ -332,8 +363,8 @@ struct test_struct {
 static inline void expect_test_struct(const test_struct&) {
 }
 
-// various scenarios
-TEST_CASE("parser test composite conversion") {
+template <typename... Ts>
+void test_composite_conversion() {
     unique_file_name f{"test_parser"};
     {
         std::ofstream out{f.name};
@@ -344,9 +375,10 @@ TEST_CASE("parser test composite conversion") {
         }
     }
 
-    ss::parser<ss::string_error> p{f.name, ","};
+    ss::parser<Ts...> p{f.name, ","};
     auto fail = [] { FAIL(""); };
     auto expect_error = [](auto error) { CHECK(!error.empty()); };
+    auto ignore_error = [] {};
 
     REQUIRE(p.valid());
     REQUIRE_FALSE(p.eof());
@@ -355,12 +387,12 @@ TEST_CASE("parser test composite conversion") {
         constexpr static auto expectedData = std::tuple{10, 'a', 11.1};
 
         auto [d1, d2, d3, d4] =
-            p.try_next<int, int, double>(fail)
-                .or_else<test_struct>(fail)
-                .or_else<int, char, double>(
+            p.template try_next<int, int, double>(fail)
+                .template or_else<test_struct>(fail)
+                .template or_else<int, char, double>(
                     [&](auto&& data) { CHECK_EQ(data, expectedData); })
                 .on_error(fail)
-                .or_else<test_tuple>(fail)
+                .template or_else<test_tuple>(fail)
                 .values();
 
         REQUIRE(p.valid());
@@ -376,15 +408,16 @@ TEST_CASE("parser test composite conversion") {
         constexpr static auto expectedData = std::tuple{10, 20, 11.1};
 
         auto [d1, d2, d3, d4] =
-            p.try_next<int, int, double>([&](auto& i1, auto i2, double d) {
+            p.template try_next<int, int, double>(
+                 [&](auto& i1, auto i2, double d) {
                      CHECK_EQ(std::tie(i1, i2, d), expectedData);
                  })
                 .on_error(fail)
-                .or_object<test_struct, int, double, char>(fail)
+                .template or_object<test_struct, int, double, char>(fail)
                 .on_error(fail)
-                .or_else<test_tuple>(fail)
+                .template or_else<test_tuple>(fail)
                 .on_error(fail)
-                .or_else<int, char, double>(fail)
+                .template or_else<int, char, double>(fail)
                 .values();
 
         REQUIRE(p.valid());
@@ -399,12 +432,12 @@ TEST_CASE("parser test composite conversion") {
         REQUIRE(!p.eof());
 
         auto [d1, d2, d3, d4, d5] =
-            p.try_object<test_struct, int, double, char>(fail)
+            p.template try_object<test_struct, int, double, char>(fail)
                 .on_error(expect_error)
-                .or_else<int, char, char>(fail)
-                .or_else<test_struct>(fail)
-                .or_else<test_tuple>(fail)
-                .or_else<int, char, double>(fail)
+                .template or_else<int, char, char>(fail)
+                .template or_else<test_struct>(fail)
+                .template or_else<test_tuple>(fail)
+                .template or_else<int, char, double>(fail)
                 .values();
 
         REQUIRE_FALSE(p.valid());
@@ -419,10 +452,10 @@ TEST_CASE("parser test composite conversion") {
         REQUIRE(!p.eof());
 
         auto [d1, d2] =
-            p.try_next<int, double>([](auto& i, auto& d) {
+            p.template try_next<int, double>([](auto& i, auto& d) {
                  REQUIRE_EQ(std::tie(i, d), std::tuple{10, 11.1});
              })
-                .or_else<int, double>([](auto&, auto&) { FAIL(""); })
+                .template or_else<int, double>([](auto&, auto&) { FAIL(""); })
                 .values();
 
         REQUIRE(p.valid());
@@ -433,8 +466,9 @@ TEST_CASE("parser test composite conversion") {
     {
         REQUIRE(!p.eof());
 
-        auto [d1, d2] = p.try_next<int, double>([](auto&, auto&) { FAIL(""); })
-                            .or_else<test_struct>(expect_test_struct)
+        auto [d1, d2] =
+            p.template try_next<int, double>([](auto&, auto&) { FAIL(""); })
+                .template or_else<test_struct>(expect_test_struct)
                 .values();
 
         REQUIRE(p.valid());
@@ -447,11 +481,12 @@ TEST_CASE("parser test composite conversion") {
         REQUIRE(!p.eof());
 
         auto [d1, d2, d3, d4, d5] =
-            p.try_next<int, int, double>(fail)
-                .or_object<test_struct, int, double, char>()
-                .or_else<test_struct>(expect_test_struct)
-                .or_else<test_tuple>(fail)
-                .or_else<std::tuple<int, double>>(fail)
+            p.template try_next<int, int, double>(fail)
+                .template or_object<test_struct, int, double, char>()
+                .template or_else<test_struct>(expect_test_struct)
+                .template or_else<test_tuple>(fail)
+                .template or_else<std::tuple<int, double>>(fail)
+                .on_error(ignore_error)
                 .on_error(expect_error)
                 .values();
 
@@ -466,10 +501,14 @@ TEST_CASE("parser test composite conversion") {
     {
         REQUIRE(!p.eof());
 
-        auto [d1, d2] = p.try_next<int, std::optional<int>>()
+        auto [d1, d2] =
+            p.template try_next<int, std::optional<int>>()
+                .on_error(ignore_error)
                 .on_error(fail)
-                            .or_else<std::tuple<int, std::string>>(fail)
+                .template or_else<std::tuple<int, std::string>>(fail)
+                .on_error(ignore_error)
                 .on_error(fail)
+                .on_error(ignore_error)
                 .values();
 
         REQUIRE(p.valid());
@@ -481,9 +520,10 @@ TEST_CASE("parser test composite conversion") {
     {
         REQUIRE_FALSE(p.eof());
 
-        auto [d1, d2] = p.try_next<int, std::variant<int, std::string>>()
+        auto [d1, d2] =
+            p.template try_next<int, std::variant<int, std::string>>()
                 .on_error(fail)
-                            .or_else<std::tuple<int, std::string>>(fail)
+                .template or_else<std::tuple<int, std::string>>(fail)
                 .on_error(fail)
                 .values();
 
@@ -496,8 +536,8 @@ TEST_CASE("parser test composite conversion") {
     {
         REQUIRE(!p.eof());
 
-        auto [d1, d2] = p.try_object<test_struct, int, double, char>()
-                            .or_else<int>(fail)
+        auto [d1, d2] = p.template try_object<test_struct, int, double, char>()
+                            .template or_else<int>(fail)
                             .values();
         REQUIRE(p.valid());
         REQUIRE(d1);
@@ -509,10 +549,10 @@ TEST_CASE("parser test composite conversion") {
         REQUIRE_FALSE(p.eof());
 
         auto [d1, d2, d3, d4] =
-            p.try_next<int, int>([] { return false; })
-                .or_else<int, double>([](auto&) { return false; })
-                .or_else<int, int>()
-                .or_else<int, int>(fail)
+            p.template try_next<int, int>([] { return false; })
+                .template or_else<int, double>([](auto&) { return false; })
+                .template or_else<int, int>()
+                .template or_else<int, int>(fail)
                 .values();
 
         REQUIRE(p.valid());
@@ -527,10 +567,11 @@ TEST_CASE("parser test composite conversion") {
         REQUIRE(!p.eof());
 
         auto [d1, d2, d3, d4] =
-            p.try_object<test_struct, int, double, char>([] { return false; })
-                .or_else<int, double>([](auto&) { return false; })
-                .or_object<test_struct, int, double, char>()
-                .or_else<int, int>(fail)
+            p.template try_object<test_struct, int, double, char>(
+                 [] { return false; })
+                .template or_else<int, double>([](auto&) { return false; })
+                .template or_object<test_struct, int, double, char>()
+                .template or_else<int, int>(fail)
                 .values();
 
         REQUIRE(p.valid());
@@ -544,6 +585,11 @@ TEST_CASE("parser test composite conversion") {
     CHECK(p.eof());
 }
 
+// various scenarios
+TEST_CASE("parser test composite conversion") {
+    test_composite_conversion<ss::string_error>();
+}
+
 struct my_string {
     char* data{nullptr};
 
@@ -585,19 +631,26 @@ struct xyz {
     }
 };
 
-TEST_CASE("parser test the moving of parsed composite values") {
+template <typename... Ts>
+void test_moving_of_parsed_composite_values() {
     // to compile is enough
     return;
-    ss::parser p{"", ""};
-    p.try_next<my_string, my_string, my_string>()
-        .or_else<my_string, my_string, my_string, my_string>([](auto&&) {})
-        .or_else<my_string>([](auto&) {})
-        .or_else<xyz>([](auto&&) {})
-        .or_object<xyz, my_string, my_string, my_string>([](auto&&) {})
-        .or_else<std::tuple<my_string, my_string, my_string>>(
+    ss::parser<Ts...> p{"", ""};
+    p.template try_next<my_string, my_string, my_string>()
+        .template or_else<my_string, my_string, my_string, my_string>(
+            [](auto&&) {})
+        .template or_else<my_string>([](auto&) {})
+        .template or_else<xyz>([](auto&&) {})
+        .template or_object<xyz, my_string, my_string, my_string>([](auto&&) {})
+        .template or_else<std::tuple<my_string, my_string, my_string>>(
             [](auto&, auto&, auto&) {});
 }
 
+TEST_CASE("parser test the moving of parsed composite values") {
+    test_moving_of_parsed_composite_values();
+    test_moving_of_parsed_composite_values<ss::string_error>();
+}
+
 TEST_CASE("parser test error mode") {
     unique_file_name f{"test_parser"};
     {
@@ -614,6 +667,25 @@ TEST_CASE("parser test error mode") {
     CHECK_FALSE(p.error_msg().empty());
 }
 
+TEST_CASE("parser throw on error mode") {
+    unique_file_name f{"test_parser"};
+    {
+        std::ofstream out{f.name};
+        out << "junk" << std::endl;
+        out << "junk" << std::endl;
+    }
+
+    ss::parser<ss::throw_on_error> p(f.name, ",");
+
+    REQUIRE_FALSE(p.eof());
+    try {
+        p.get_next<int>();
+        FAIL("Expected exception...");
+    } catch (const std::exception& e) {
+        CHECK_FALSE(std::string{e.what()}.empty());
+    }
+}
+
 static inline std::string no_quote(const std::string& s) {
     if (!s.empty() && s[0] == '"') {
         return {std::next(begin(s)), std::prev(end(s))};
@@ -793,6 +865,114 @@ TEST_CASE("parser test multiline restricted") {
     CHECK_EQ(i, data);
 }
 
+template <typename... Ts>
+void expect_error_on_command(ss::parser<Ts...>& p,
+                             const std::function<void()> command) {
+    if (ss::setup<Ts...>::throw_on_error) {
+        try {
+            command();
+        } catch (const std::exception& e) {
+            CHECK_FALSE(std::string{e.what()}.empty());
+        }
+    } else {
+        command();
+        CHECK(!p.valid());
+        if constexpr (ss::setup<Ts...>::string_error) {
+            CHECK_FALSE(p.error_msg().empty());
+        }
+    }
+}
+
+template <typename... Ts>
+void test_unterminated_line_impl(const std::vector<std::string>& lines,
+                                 size_t bad_line) {
+    unique_file_name f{"test_parser"};
+    std::ofstream out{f.name};
+    for (const auto& line : lines) {
+        out << line << std::endl;
+    }
+    out.close();
+
+    ss::parser<Ts...> p{f.name};
+    size_t line = 0;
+    while (!p.eof()) {
+        auto command = [&] { p.template get_next<int, double, std::string>(); };
+
+        if (line == bad_line) {
+            expect_error_on_command(p, command);
+            break;
+        } else {
+            CHECK(p.valid());
+            ++line;
+        }
+    }
+}
+
+template <typename... Ts>
+void test_unterminated_line(const std::vector<std::string>& lines,
+                            size_t bad_line) {
+    test_unterminated_line_impl<Ts...>(lines, bad_line);
+    test_unterminated_line_impl<Ts..., ss::string_error>(lines, bad_line);
+    test_unterminated_line_impl<Ts..., ss::throw_on_error>(lines, bad_line);
+}
+
+// TODO add more cases
+TEST_CASE("parser test csv on multiple with errors") {
+    using multiline = ss::multiline_restricted<3>;
+    using escape = ss::escape<'\\'>;
+    using quote = ss::quote<'"'>;
+
+    // unterminated escape
+    {
+        const std::vector<std::string> lines{"1,2,just\\"};
+        test_unterminated_line<multiline, escape>(lines, 0);
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+
+    // unterminated quote
+    {
+        const std::vector<std::string> lines{"1,2,\"just"};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+        test_unterminated_line<multiline, quote>(lines, 0);
+    }
+
+    // unterminated quote and escape
+    {
+        const std::vector<std::string> lines{"1,2,\"just\\"};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+
+    {
+        const std::vector<std::string> lines{"1,2,\"just\\\n\\"};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+
+    {
+        const std::vector<std::string> lines{"1,2,\"just\n\\"};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+
+    // multiline limmit reached escape
+    {
+        const std::vector<std::string> lines{"1,2,\\\n\\\n\\\n\\\njust"};
+        test_unterminated_line<multiline, escape>(lines, 0);
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+
+    // multiline limmit reached quote
+    {
+        const std::vector<std::string> lines{"1,2,\"\n\n\n\n\njust\""};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+        test_unterminated_line<multiline, quote>(lines, 0);
+    }
+
+    // multiline limmit reached quote and escape
+    {
+        const std::vector<std::string> lines{"1,2,\"\\\n\n\\\n\\\n\\\njust"};
+        test_unterminated_line<multiline, escape, quote>(lines, 0);
+    }
+}
+
 template <typename T, typename Tuple>
 struct has_type;
 
@@ -978,6 +1158,64 @@ TEST_CASE("parser test various cases with header") {
     test_fields<double, int, str>(o, d, {Dbl, Int, Str});
 }
 
+template <typename... Ts>
+void test_invalid_fields_impl(const std::vector<std::string>& lines,
+                              const std::vector<std::string>& fields) {
+    unique_file_name f{"test_parser"};
+    std::ofstream out{f.name};
+    for (const auto& line : lines) {
+        out << line << std::endl;
+    }
+    out.close();
+
+    /* TODO test
+    {
+        // No fields specified
+        ss::parser<Ts...> p{f.name, ","};
+        p.use_fields();
+        CHECK(!p.valid());
+    }
+    */
+
+    {
+        // Unknown field
+        ss::parser<Ts...> p{f.name, ","};
+        auto command = [&] { p.use_fields("Unknown"); };
+        expect_error_on_command(p, command);
+    }
+
+    {
+        // Field used multiple times
+        ss::parser<Ts...> p{f.name, ","};
+        auto command = [&] { p.use_fields(fields[0], fields[0]); };
+        expect_error_on_command(p, command);
+    }
+
+    {
+        // Mapping out of range
+        ss::parser<Ts...> p{f.name, ","};
+        auto command = [&] {
+            p.use_fields(fields[0]);
+            p.template get_next<std::string, std::string>();
+        };
+        expect_error_on_command(p, command);
+    }
+}
+
+template <typename... Ts>
+void test_invalid_fields(const std::vector<std::string>& lines,
+                         const std::vector<std::string>& fields) {
+    test_invalid_fields_impl(lines, fields);
+    test_invalid_fields_impl<ss::string_error>(lines, fields);
+    test_invalid_fields_impl<ss::throw_on_error>(lines, fields);
+}
+
+// TODO add more test cases
+TEST_CASE("parser test invalid header fields usage") {
+    test_invalid_fields({"Int,String,Double", "1,hi,2.34"},
+                        {"Int", "String", "Double"});
+}
+
 static inline void test_ignore_empty(const std::vector<X>& data) {
     unique_file_name f{"test_parser"};
     make_and_write(f.name, data);