From: g0dil Date: Tue, 23 Sep 2008 17:58:09 +0000 (+0000) Subject: Console: Parser error handling X-Git-Url: http://g0dil.de/git?a=commitdiff_plain;h=a9b1698a021469bf9d65da01b732712d978f5ac9;p=senf.git Console: Parser error handling git-svn-id: https://svn.berlios.de/svnroot/repos/senf/trunk@916 270642c3-0616-0410-b53a-bc976706d245 --- diff --git a/Console/ConfigFile.cc b/Console/ConfigFile.cc index d57a0a8..45e0eee 100644 --- a/Console/ConfigFile.cc +++ b/Console/ConfigFile.cc @@ -37,10 +37,9 @@ prefix_ void senf::console::detail::ConfigFileSource::v_parse(RestrictedExecutor & executor) { - if (! parser_.parseFile(filename_, boost::bind( boost::ref(executor), - boost::ref(std::cerr), - _1 )) ) - throw SyntaxErrorException(); + parser_.parseFile(filename_, boost::bind( boost::ref(executor), + boost::ref(std::cerr), + _1 )); } /////////////////////////////////////////////////////////////////////////// diff --git a/Console/Parse.cc b/Console/Parse.cc index 57a5532..83ff10c 100644 --- a/Console/Parse.cc +++ b/Console/Parse.cc @@ -30,7 +30,9 @@ #include #include #include +#include #include "../Utils/Exception.hh" +#include "../Utils/senfassert.hh" //#include "Parse.mpp" #define prefix_ @@ -241,6 +243,22 @@ struct senf::console::CommandParser::Impl #endif +namespace { + + template + void throwParserError(Error const & err) + { + static char const * msg [] = { "end of statement expected", + "'{' or arguments expected", + "path expected", + "')' expected", + "'\"' expected" }; + boost::spirit::file_position pos (err.where.get_position()); + throw senf::console::CommandParser::ParserErrorException(msg[err.descriptor]) + << "\nat " << pos.file << ":" << pos.line << ":" << pos.column; + } +} + prefix_ senf::console::CommandParser::CommandParser() : impl_ (new Impl()) {} @@ -252,52 +270,83 @@ prefix_ senf::console::CommandParser::~CommandParser() // we would need to expose the Impl member to the public, which we don't want to do. template -prefix_ Iterator senf::console::CommandParser::parseLoop(Iterator b, Iterator e, Callback cb) +prefix_ Iterator senf::console::CommandParser::parseLoop(Iterator npb, Iterator npe, + std::string const & source, Callback cb) { + typedef boost::spirit::position_iterator PositionIterator; + PositionIterator b (npb, npe, source); + PositionIterator e (npe, npe, source); ParseCommandInfo info; detail::ParseDispatcher::BindInfo bind (impl().dispatcher, info); - boost::spirit::parse_info result; + boost::spirit::parse_info result; for(;;) { result = boost::spirit::parse( b, e, * impl().grammar.use_parser()); b = result.stop; if (b == e) - return e; + return e.base(); info.clear(); - result = boost::spirit::parse(b, e, - impl().grammar.use_parser(), - impl().grammar.use_parser()); - if (! result.hit) - return b; + try { + result = boost::spirit::parse(b, e, + impl().grammar.use_parser(), + impl().grammar.use_parser()); + } + catch (boost::spirit::parser_error & ex) { + if (impl().grammar.incremental && ex.where == e) + return b.base(); + else + throwParserError(ex); + } + // Otherwise the error handling in the parser is broken + SENF_ASSERT( result.hit ); if (! info.empty()) - cb(info); + try { + cb(info); + } + catch (senf::ExceptionMixin & ex) { + boost::spirit::file_position pos (result.stop.get_position()); + ex << "\nat " << pos.file << ":" << pos.line << ":" << pos.column; + throw; + } b = result.stop; } } -prefix_ bool senf::console::CommandParser::parse(std::string const & command, Callback cb) +prefix_ void senf::console::CommandParser::parse(std::string const & command, Callback cb) { - return parseLoop(command.begin(), command.end(), cb) == command.end(); + parseLoop(command.begin(), command.end(), "", cb); } -prefix_ bool senf::console::CommandParser::parseFile(std::string const & filename, Callback cb) +prefix_ void senf::console::CommandParser::parseFile(std::string const & filename, Callback cb) { boost::spirit::file_iterator<> i (filename); if (!i) throw SystemException(ENOENT SENF_EXC_DEBUGINFO); boost::spirit::file_iterator<> const i_end (i.make_end()); - - return parseLoop(i, i_end, cb) == i_end; + parseLoop(i, i_end, filename, cb); } -prefix_ bool senf::console::CommandParser::parseArguments(std::string const & arguments, +prefix_ void senf::console::CommandParser::parseArguments(std::string const & arguments, ParseCommandInfo & info) { + typedef boost::spirit::position_iterator PositionIterator; + PositionIterator b (arguments.begin(), arguments.end(), std::string("")); + PositionIterator e (arguments.end(), arguments.end(), std::string("")); detail::ParseDispatcher::BindInfo bind (impl().dispatcher, info); - return boost::spirit::parse( arguments.begin(), arguments.end(), - impl().grammar.use_parser(), - impl().grammar.use_parser() - ).full; + boost::spirit::parse_info result; + try { + result = boost::spirit::parse( b, e, + impl().grammar.use_parser(), + impl().grammar.use_parser() ); + } + catch (boost::spirit::parser_error & ex) { + throwParserError(ex); + } + if (! result.full) { + boost::spirit::file_position pos (result.stop.get_position()); + throw ParserErrorException("argument expected") + << "\nat " << pos.file << ":" << pos.line << ":" << pos.column; + } } struct senf::console::CommandParser::SetIncremental @@ -318,16 +367,7 @@ senf::console::CommandParser::parseIncremental(std::string const & commands, Cal { SetIncremental si (*this); return std::distance( commands.begin(), - parseLoop(commands.begin(), commands.end(), cb) ); -} - -/////////////////////////////////////////////////////////////////////////// -// senf::console::SyntaxErrorException - -prefix_ char const * senf::console::SyntaxErrorException::what() - const throw() -{ - return message().empty() ? "syntax error" : message().c_str(); + parseLoop(commands.begin(), commands.end(), "", cb) ); } ///////////////////////////////cc.e//////////////////////////////////////// diff --git a/Console/Parse.cci b/Console/Parse.cci index 322fc44..b5a71be 100644 --- a/Console/Parse.cci +++ b/Console/Parse.cci @@ -229,23 +229,6 @@ prefix_ void senf::console::ParseCommandInfo::ArgumentIterator::increment() } /////////////////////////////////////////////////////////////////////////// -// senf::console::SyntaxErrorException - -prefix_ senf::console::SyntaxErrorException::SyntaxErrorException(std::string const & msg) - : message_(msg) -{} - -prefix_ senf::console::SyntaxErrorException::~SyntaxErrorException() - throw() -{} - -prefix_ std::string const & senf::console::SyntaxErrorException::message() - const -{ - return message_; -} - -/////////////////////////////////////////////////////////////////////////// prefix_ senf::console::CheckedArgumentIteratorWrapper:: CheckedArgumentIteratorWrapper(ParseCommandInfo::ArgumentsRange const & range, diff --git a/Console/Parse.hh b/Console/Parse.hh index 5653566..288166e 100644 --- a/Console/Parse.hh +++ b/Console/Parse.hh @@ -197,6 +197,7 @@ #include #include #include "../Utils/safe_bool.hh" +#include "../Utils/Exception.hh" //#include "Parse.mpp" ///////////////////////////////hh.p//////////////////////////////////////// @@ -323,7 +324,7 @@ namespace console { Every command parsed is returned in a ParseCommandInfo instance. This information is purely taken from the parser, no semantic information is attached at this point, the config/console - node tree is not involved in any why. ParseCommandInfo consist of + node tree is not involved in any way. ParseCommandInfo consist of \li the type of command: built-in or normal command represented by a possibly relative path into the command tree. @@ -438,17 +439,9 @@ namespace console { All errors while parsing the arguments of a command must be signaled by throwing an instance of SyntaxErrorException. This is important, so command overloading works. */ - struct SyntaxErrorException : public std::exception - { - explicit SyntaxErrorException(std::string const & msg = ""); - virtual ~SyntaxErrorException() throw(); - - virtual char const * what() const throw(); - std::string const & message() const; - - private: - std::string message_; - }; + struct SyntaxErrorException : public senf::Exception + { explicit SyntaxErrorException(std::string const & msg = "syntax error") + : senf::Exception(msg) {} }; /** \brief Wrapper checking argument iterator access for validity @@ -543,7 +536,7 @@ namespace console { using IteratorFacade::operator++; ParseCommandInfo::ArgumentIterator operator++(int); - + private: reference dereference() const; void increment(); @@ -596,13 +589,13 @@ namespace console { ///@} /////////////////////////////////////////////////////////////////////////// - bool parse(std::string const & command, Callback cb); ///< Parse string - bool parseFile(std::string const & filename, Callback cb); ///< Parse file + void parse(std::string const & command, Callback cb); ///< Parse string + void parseFile(std::string const & filename, Callback cb); ///< Parse file /**< \throws SystemException if the file cannot be read. */ - bool parseArguments(std::string const & arguments, ParseCommandInfo & info); - ///< Parse \a argumtns + void parseArguments(std::string const & arguments, ParseCommandInfo & info); + ///< Parse \a arguments /**< parseArguments() parses the string \a arguments which contains arbitrary command arguments (without the name of the command). The argument tokens are written into @@ -619,12 +612,16 @@ namespace console { to be terminated explicitly. This means, that the last ';' is \e not optional in this case. */ + /** \brief Exception thrown when the parser detects an error */ + struct ParserErrorException : public SyntaxErrorException + { explicit ParserErrorException(std::string const & msg) : SyntaxErrorException(msg) {} }; + private: struct Impl; struct SetIncremental; template - Iterator parseLoop(Iterator b, Iterator e, Callback cb); + Iterator parseLoop(Iterator b, Iterator e, std::string const & source, Callback cb); Impl & impl(); diff --git a/Console/Parse.ih b/Console/Parse.ih index e2859ff..70791d4 100644 --- a/Console/Parse.ih +++ b/Console/Parse.ih @@ -78,6 +78,17 @@ namespace detail { ParseDispatcher & dispatcher; /////////////////////////////////////////////////////////////////////////// + // Errors + + enum Errors { + EndOfStatementExpected, + GroupOrArgumentsExpected, + PathExpected, + ClosingParenExpected, + QuoteExpected + }; + + /////////////////////////////////////////////////////////////////////////// CommandGrammar(ParseDispatcher & d, Context & c) : context(c), incremental(false), dispatcher(d) {} @@ -98,7 +109,7 @@ namespace detail { definition(CommandGrammar const & self) : // Characters with a special meaning within the parser - special_p ("/(){};"), + special_p ("/(){};\""), // Additional characters which are returned as punctuation tokens // (only allowed within '()'). @@ -131,6 +142,12 @@ namespace detail { actor< variable< Token > > token_ (self.context.token); actor< variable< ParseDispatcher > > d_ (self.dispatcher); + assertion end_of_statement_expected (EndOfStatementExpected); + assertion group_or_arguments_expected (GroupOrArgumentsExpected); + assertion path_expected (PathExpected); + assertion closing_paren_expected (ClosingParenExpected); + assertion quote_expected (QuoteExpected); + /////////////////////////////////////////////////////////////////// // Spirit grammar // @@ -170,15 +187,16 @@ namespace detail { // More info is in the Boost.Spirit documentation command - = builtin >> statement_end - | path >> ( group_start | statement ) + = builtin >> end_of_statement_expected(statement_end) | group_close | ch_p(';') // Ignore empty commands + | path_expected(path) + >> group_or_arguments_expected( group_start | statement ) ; builtin = keyword_p("cd") - >> path + >> path_expected(path) >> eps_p [ bind(&PD::builtin_cd)(d_, path_) ] | keyword_p("ls") >> ! path @@ -200,7 +218,8 @@ namespace detail { statement = eps_p [ bind(&PD::beginCommand)(d_, path_) ] >> arguments - >> statement_end [ bind(&PD::endCommand)(d_) ] + >> end_of_statement_expected(statement_end) + [ bind(&PD::endCommand)(d_) ] ; arguments @@ -227,14 +246,17 @@ namespace detail { - '"' ) [ str_ += ch_ ] ) - >> ch_p('"') [ token_ = construct_(Token::BasicString, + >> quote_expected(ch_p('"')) + [ token_ = construct_(Token::BasicString, str_) ] ] ; hexstring // Returns value in context.token = eps_p [ clear(str_) ] - >> confix_p( "x\"", * hexbyte, '"' ) + >> "x\"" + >> * ( hexbyte - ch_p('"') ) + >> quote_expected(ch_p('"')) [ token_ = construct_(Token::HexString, str_) ] ; @@ -262,7 +284,8 @@ namespace detail { "(") ] [ bind(&PD::pushToken)(d_, token_) ] >> * token - >> ch_p(')') [ token_ = construct_( + >> closing_paren_expected(ch_p(')')) + [ token_ = construct_( Token::ArgumentGroupClose, ")") ] [ bind(&PD::pushToken)(d_, token_) ] diff --git a/Console/Parse.test.cc b/Console/Parse.test.cc index c650745..3981972 100644 --- a/Console/Parse.test.cc +++ b/Console/Parse.test.cc @@ -197,7 +197,7 @@ BOOST_AUTO_UNIT_TEST(commandParser) " 0304\";" "ls /foo/bar; "; - BOOST_CHECK( parser.parse(text, &setInfo) ); + BOOST_CHECK_NO_THROW( parser.parse(text, &setInfo) ); BOOST_CHECK_EQUAL( commands.size(), 2u ); { @@ -271,14 +271,14 @@ BOOST_AUTO_UNIT_TEST(checkedArgumentIterator) { senf::console::CommandParser parser; - BOOST_CHECK( parser.parse("foo a", &setInfo) ); + BOOST_CHECK_NO_THROW( parser.parse("foo a", &setInfo) ); BOOST_CHECK_THROW( parseArgs(commands.back().arguments()), senf::console::SyntaxErrorException ); - BOOST_CHECK( parser.parse("foo a b", &setInfo) ); + BOOST_CHECK_NO_THROW( parser.parse("foo a b", &setInfo) ); BOOST_CHECK_NO_THROW( parseArgs(commands.back().arguments()) ); - BOOST_CHECK( parser.parse("foo a b c", &setInfo) ); + BOOST_CHECK_NO_THROW( parser.parse("foo a b c", &setInfo) ); BOOST_CHECK_THROW( parseArgs(commands.back().arguments()), senf::console::SyntaxErrorException ); @@ -311,6 +311,35 @@ BOOST_AUTO_UNIT_TEST(parseIncremental) commands.clear(); } +namespace { + std::string parseErrorMessage(std::string const & msg) + { + std::string::size_type i (msg.find("-- \n")); + return i == std::string::npos ? msg : msg.substr(i+4); + } +} + +BOOST_AUTO_UNIT_TEST(parseExceptions) +{ + senf::console::CommandParser parser; + std::string msg; + +# define CheckParseEx(c, e) \ + commands.clear(); \ + msg.clear(); \ + try { parser.parse(c, &setInfo); } \ + catch (std::exception & ex) { msg = parseErrorMessage(ex.what()); } \ + BOOST_CHECK_EQUAL( msg, e ) + + CheckParseEx( "/foo/bar;\n ()", "path expected\nat :2:3" ); + CheckParseEx( "cd /foo/bar foo/bar", "end of statement expected\nat :1:13" ); + CheckParseEx( "/foo/bar foo /", "end of statement expected\nat :1:14" ); + CheckParseEx( "cd \"foo\"", "path expected\nat :1:4" ); + CheckParseEx( "/foo/bar \"string", "'\"' expected\nat :1:17" ); + CheckParseEx( "/foo/bar x\"hi\"", "'\"' expected\nat :1:12" ); + CheckParseEx( "/foo/bar (", "')' expected\nat :1:11" ); +} + ///////////////////////////////cc.e//////////////////////////////////////// #undef prefix_ diff --git a/Console/Server.cc b/Console/Server.cc index 63b922b..e69bca1 100644 --- a/Console/Server.cc +++ b/Console/Server.cc @@ -271,7 +271,6 @@ prefix_ std::string::size_type senf::console::Client::handleInput(std::string da else lastCommand_ = data; - bool state (true); std::string::size_type n (data.size()); try { @@ -280,11 +279,9 @@ prefix_ std::string::size_type senf::console::Client::handleInput(std::string da boost::ref(stream()), _1 )); else - state = parser_.parse(data, boost::bind( boost::ref(executor_), - boost::ref(stream()), - _1 )); - if (! state ) - stream() << "syntax error" << std::endl; + parser_.parse(data, boost::bind( boost::ref(executor_), + boost::ref(stream()), + _1 )); } catch (Executor::ExitException &) { // This generates an EOF condition on the Handle. This EOF condition is expected