Improve error handling
The errors now bubble up all the way to the command error handler. This
means, that there is less redundant code, since we only print the
error message in one location. Also, we can now differentiate between more
error types, because more error handlers are defined.
Change-Id: I1407eafbd99d79a76dd6a181a23448d58110d6cd
diff --git a/src/ast_handlers.hpp b/src/ast_handlers.hpp
index 4b703aa..0d3159b 100644
--- a/src/ast_handlers.hpp
+++ b/src/ast_handlers.hpp
@@ -21,14 +21,23 @@
if (parserContext.m_tmpListKeys.find(ast.first) != parserContext.m_tmpListKeys.end()) {
_pass(context) = false;
parserContext.m_errorMsg = "Key \"" + ast.first + "\" was entered more than once.";
- } else if (schema.listHasKey(parserContext.m_curPath, parserContext.m_tmpListName, ast.first)) {
- parserContext.m_tmpListKeys.insert(ast.first);
- } else {
+ } else if (!schema.listHasKey(parserContext.m_curPath, parserContext.m_tmpListName, ast.first)) {
_pass(context) = false;
parserContext.m_errorMsg = parserContext.m_tmpListName + " is not indexed by \"" + ast.first + "\".";
+ } else {
+ parserContext.m_tmpListKeys.insert(ast.first);
}
}
+
+ template <typename Iterator, typename Exception, typename Context>
+ x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const&, Context const& context)
+ {
+ auto& parserContext = x3::get<parser_context_tag>(context);
+ parserContext.m_errorMsg = "Error parsing key values here:";
+ return x3::error_handler_result::rethrow;
+ }
};
+
struct identifier_class;
struct listPrefix_class {
@@ -73,6 +82,16 @@
_pass(context) = false;
}
}
+
+ template <typename Iterator, typename Exception, typename Context>
+ x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const&, Context const& context)
+ {
+ auto& parserContext = x3::get<parser_context_tag>(context);
+ if (parserContext.m_errorMsg.empty())
+ parserContext.m_errorMsg = "Expecting ']' here:";
+ return x3::error_handler_result::rethrow;
+
+ }
};
struct listElement_class {
template <typename T, typename Iterator, typename Context>
@@ -83,18 +102,15 @@
}
template <typename Iterator, typename Exception, typename Context>
- x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const& ex, Context const& context)
+ x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const&, Context const& context)
{
auto& parserContext = x3::get<parser_context_tag>(context);
- auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- if (parserContext.m_errorHandled) // someone already handled our error
+ if (parserContext.m_errorMsg.empty()) {
return x3::error_handler_result::fail;
+ } else {
+ return x3::error_handler_result::rethrow;
+ }
- parserContext.m_errorHandled = true;
-
- std::string message = parserContext.m_errorMsg;
- error_handler(ex.where(), message);
- return x3::error_handler_result::fail;
}
};
@@ -149,17 +165,15 @@
}
template <typename Iterator, typename Exception, typename Context>
- x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const& x, Context const& context)
+ x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const&, Context const& context)
{
auto& parserContext = x3::get<parser_context_tag>(context);
- auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- std::string message = "invalid path.";
- if (parserContext.m_errorHandled) // someone already handled our error
+ if (parserContext.m_errorMsg.empty()) {
+ parserContext.m_errorMsg = "Expected path.";
return x3::error_handler_result::fail;
-
- parserContext.m_errorHandled = true;
- error_handler(x.where(), message);
- return x3::error_handler_result::fail;
+ } else {
+ return x3::error_handler_result::rethrow;
+ }
}
};
@@ -176,14 +190,9 @@
x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const& x, Context const& context)
{
auto& parserContext = x3::get<parser_context_tag>(context);
- auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- std::string message = "This isn't a list or a container or nothing.";
- if (parserContext.m_errorHandled) // someone already handled our error
- return x3::error_handler_result::fail;
-
- parserContext.m_errorHandled = true;
- error_handler(x.where(), message);
- return x3::error_handler_result::fail;
+ if (parserContext.m_errorMsg.empty())
+ parserContext.m_errorMsg = "Expected " + x.which() + " here:";
+ return x3::error_handler_result::rethrow;
}
};
@@ -199,27 +208,22 @@
parserContext.m_curPath.m_nodes.end() - 1)};
if (!schema.isPresenceContainer(location, cont.m_name)) {
+ parserContext.m_errorMsg = "This container is not a presence container.";
_pass(context) = false;
- return;
}
} catch (boost::bad_get&) {
+ parserContext.m_errorMsg = "This is not a container.";
_pass(context) = false;
- return;
}
}
template <typename Iterator, typename Exception, typename Context>
- x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const& x, Context const& context)
+ x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const&, Context const& context)
{
auto& parserContext = x3::get<parser_context_tag>(context);
- auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- std::string message = "This isn't a path to a presence container.";
- if (parserContext.m_errorHandled) // someone already handled our error
- return x3::error_handler_result::fail;
-
- parserContext.m_errorHandled = true;
- error_handler(x.where(), message);
- return x3::error_handler_result::fail;
+ if (parserContext.m_errorMsg.empty())
+ parserContext.m_errorMsg = "Couldn't parse create/delete command.";
+ return x3::error_handler_result::rethrow;
}
};
@@ -233,11 +237,12 @@
template <typename T, typename Iterator, typename Context>
void on_success(Iterator const&, Iterator const&, T& ast, Context const& context)
{
+ auto& parserContext = x3::get<parser_context_tag>(context);
try {
auto leaf = boost::get<leaf_>(ast.m_path.m_nodes.back());
} catch (boost::bad_get&) {
+ parserContext.m_errorMsg = "This is not a leaf.";
_pass(context) = false;
- return;
}
}
@@ -245,14 +250,9 @@
x3::error_handler_result on_error(Iterator&, Iterator const&, Exception const& x, Context const& context)
{
auto& parserContext = x3::get<parser_context_tag>(context);
- auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- std::string message = "This isn't a path to leaf.";
- if (parserContext.m_errorHandled) // someone already handled our error
- return x3::error_handler_result::fail;
-
- parserContext.m_errorHandled = true;
- error_handler(x.where(), message);
- return x3::error_handler_result::fail;
+ if (parserContext.m_errorMsg.empty())
+ parserContext.m_errorMsg = "Expected " + x.which() + " here:";
+ return x3::error_handler_result::rethrow;
}
};
@@ -262,12 +262,10 @@
{
auto& parserContext = x3::get<parser_context_tag>(context);
auto& error_handler = x3::get<x3::error_handler_tag>(context).get();
- std::string message = "Couldn't parse command.";
- if (parserContext.m_errorHandled) // someone already handled our error
- return x3::error_handler_result::fail;
-
- parserContext.m_errorHandled = true;
- error_handler(x.where(), message);
+ if (parserContext.m_errorMsg.empty()) {
+ parserContext.m_errorMsg = "Unknown command.";
+ }
+ error_handler(x.where(), parserContext.m_errorMsg);
return x3::error_handler_result::fail;
}
};
diff --git a/src/grammars.hpp b/src/grammars.hpp
index 937fd3c..ab410c8 100644
--- a/src/grammars.hpp
+++ b/src/grammars.hpp
@@ -28,9 +28,13 @@
x3::rule<delete_class, delete_> const delete_rule = "delete_rule";
x3::rule<command_class, command_> const command = "command";
+#if __clang__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverloaded-shift-op-parentheses"
+#endif
auto const keyValue_def =
- lexeme[+alnum >> '=' >> +alnum];
+ lexeme[+alnum > '=' > +alnum];
auto const identifier_def =
lexeme[
@@ -58,7 +62,7 @@
// leaf cannot be in the middle of a path, however, I need the grammar's attribute to be a vector of variants
auto const path_def =
- (container | listElement | nodeup | leaf) % '/';
+ (x3::expect[container | listElement | nodeup | leaf]) % '/';
auto const data_string_def =
lexeme[+char_];
@@ -67,19 +71,23 @@
x3::omit[x3::no_skip[space]];
auto const cd_def =
- lit("cd") > space_separator > path >> x3::eoi;
+ lit("cd") >> space_separator > path;
auto const create_def =
- lit("create") > space_separator > path >> x3::eoi;
+ lit("create") >> space_separator > path;
auto const delete_rule_def =
- lit("delete") > space_separator > path >> x3::eoi;
+ lit("delete") >> space_separator > path;
auto const set_def =
- lit("set") > space_separator > path > space_separator > data_string >> x3::eoi;
+ lit("set") >> space_separator > path > data_string;
auto const command_def =
- cd | create | delete_rule | set;
+ x3::expect[cd | create | delete_rule | set] >> x3::eoi;
+
+#if __clang__
+#pragma GCC diagnostic pop
+#endif
BOOST_SPIRIT_DEFINE(keyValue)
BOOST_SPIRIT_DEFINE(identifier)