Console: Parser error handling
g0dil [Tue, 23 Sep 2008 17:58:09 +0000 (17:58 +0000)]
git-svn-id: https://svn.berlios.de/svnroot/repos/senf/trunk@916 270642c3-0616-0410-b53a-bc976706d245

Console/ConfigFile.cc
Console/Parse.cc
Console/Parse.cci
Console/Parse.hh
Console/Parse.ih
Console/Parse.test.cc
Console/Server.cc

index d57a0a8..45e0eee 100644 (file)
 
 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 ));
 }
 
 ///////////////////////////////////////////////////////////////////////////
index 57a5532..83ff10c 100644 (file)
@@ -30,7 +30,9 @@
 #include <cerrno>
 #include <boost/iterator/transform_iterator.hpp>
 #include <boost/spirit/iterator/file_iterator.hpp>
+#include <boost/spirit/iterator/position_iterator.hpp>
 #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 <class Error>
+    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 <class Iterator>
-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<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<Iterator> result;
+    boost::spirit::parse_info<PositionIterator> result;
 
     for(;;) {
         result = boost::spirit::parse(
             b, e, * impl().grammar.use_parser<Impl::Grammar::SkipParser>());
         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::CommandParser>(),
-                                      impl().grammar.use_parser<Impl::Grammar::SkipParser>());
-        if (! result.hit) 
-            return b;
+        try {
+            result = boost::spirit::parse(b, e,
+                                          impl().grammar.use_parser<Impl::Grammar::CommandParser>(),
+                                          impl().grammar.use_parser<Impl::Grammar::SkipParser>());
+        }
+        catch (boost::spirit::parser_error<Impl::Grammar::Errors, PositionIterator> & 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(), "<unknown>", 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<std::string::const_iterator> PositionIterator;
+    PositionIterator b (arguments.begin(), arguments.end(), std::string("<unknown>"));
+    PositionIterator e (arguments.end(), arguments.end(), std::string("<unknown>"));
     detail::ParseDispatcher::BindInfo bind (impl().dispatcher, info);
-    return boost::spirit::parse( arguments.begin(), arguments.end(), 
-                                 impl().grammar.use_parser<Impl::Grammar::ArgumentsParser>(),
-                                 impl().grammar.use_parser<Impl::Grammar::SkipParser>()
-        ).full;
+    boost::spirit::parse_info<PositionIterator> result;
+    try {
+        result = boost::spirit::parse( b, e, 
+                                       impl().grammar.use_parser<Impl::Grammar::ArgumentsParser>(),
+                                       impl().grammar.use_parser<Impl::Grammar::SkipParser>() );
+    }
+    catch (boost::spirit::parser_error<Impl::Grammar::Errors, PositionIterator> & 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(), "<unknown>", cb) );
 }
 
 ///////////////////////////////cc.e////////////////////////////////////////
index 322fc44..b5a71be 100644 (file)
@@ -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,
index 5653566..288166e 100644 (file)
 #include <boost/iterator/iterator_facade.hpp>
 #include <boost/function.hpp>
 #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 <class Iterator>
-        Iterator parseLoop(Iterator b, Iterator e, Callback cb);
+        Iterator parseLoop(Iterator b, Iterator e, std::string const & source, Callback cb);
 
         Impl & impl();
 
index e2859ff..70791d4 100644 (file)
@@ -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<Errors> end_of_statement_expected   (EndOfStatementExpected);
+                assertion<Errors> group_or_arguments_expected (GroupOrArgumentsExpected);
+                assertion<Errors> path_expected               (PathExpected);
+                assertion<Errors> closing_paren_expected      (ClosingParenExpected);
+                assertion<Errors> 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>(Token::BasicString, 
+                          >> quote_expected(ch_p('"'))
+                                                  [ token_ = construct_<Token>(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>(Token::HexString,
                                                                                str_) ]
                     ;
@@ -262,7 +284,8 @@ namespace detail {
                                                         "(") ]
                                                   [ bind(&PD::pushToken)(d_, token_) ]
                       >> * token
-                      >> ch_p(')')                [ token_ = construct_<Token>(
+                      >> closing_paren_expected(ch_p(')'))
+                                                  [ token_ = construct_<Token>(
                                                         Token::ArgumentGroupClose,
                                                         ")") ]
                                                   [ bind(&PD::pushToken)(d_, token_) ]
index c650745..3981972 100644 (file)
@@ -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 <unknown>:2:3" );
+    CheckParseEx( "cd /foo/bar foo/bar", "end of statement expected\nat <unknown>:1:13" );
+    CheckParseEx( "/foo/bar foo /", "end of statement expected\nat <unknown>:1:14" );
+    CheckParseEx( "cd \"foo\"", "path expected\nat <unknown>:1:4" );
+    CheckParseEx( "/foo/bar \"string", "'\"' expected\nat <unknown>:1:17" );
+    CheckParseEx( "/foo/bar x\"hi\"", "'\"' expected\nat <unknown>:1:12" );
+    CheckParseEx( "/foo/bar (", "')' expected\nat <unknown>:1:11" );
+}
+
 ///////////////////////////////cc.e////////////////////////////////////////
 #undef prefix_
 
index 63b922b..e69bca1 100644 (file)
@@ -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<void>( boost::ref(executor_),
-                                                           boost::ref(stream()),
-                                                           _1 ));
-        if (! state )
-            stream() << "syntax error" << std::endl;
+            parser_.parse(data, boost::bind<void>( boost::ref(executor_),
+                                                   boost::ref(stream()),
+                                                   _1 ));
     }
     catch (Executor::ExitException &) {
         // This generates an EOF condition on the Handle. This EOF condition is expected