refactor parser, check in some failing tests

3 files changed, 51 insertions, 9 deletions

diff --git a/src/parsers.cc b/src/parsers.cc
index bbcc56b..78d6164 100644
--- a/src/parsers.cc
+++ b/src/parsers.cc
@@ -41,19 +41,21 @@ string Token::AsString() const {
   return "";
 }
 
+bool SourceLocation::Error(const string& message, string* err) {
+  char buf[1024];
+  snprintf(buf, sizeof(buf), "line %d, col %d: %s", line_, column_,
+           message.c_str());
+  err->assign(buf);
+  return false;
+}
+
 void Tokenizer::Start(const char* start, const char* end) {
   cur_line_ = cur_ = start;
   end_ = end;
 }
 
 bool Tokenizer::Error(const string& message, string* err) {
-  char buf[1024];
-  snprintf(buf, sizeof(buf), "line %d, col %d: %s",
-          line_number_,
-          (int)(token_.pos_ - cur_line_) + 1,
-          message.c_str());
-  err->assign(buf);
-  return false;
+  return Location().Error(message, err);
 }
 
 bool Tokenizer::ErrorExpected(const string& expected, string* err) {
diff --git a/src/parsers.h b/src/parsers.h
index 9dc61d9..55c0eb7 100644
--- a/src/parsers.h
+++ b/src/parsers.h
@@ -48,16 +48,30 @@ struct Token {
   const char* end_;
 };
 
+/// Represents a user-understandable position within a source file.
+struct SourceLocation {
+  SourceLocation(int line, int col) : line_(line), column_(col) {}
+
+  /// Construct an error message based on the position and message,
+  /// write it into \a err, then return false.
+  bool Error(const string& message, string* err);
+
+  /// 1-based line and column numbers.
+  int line_;
+  int column_;
+};
+
 /// Processes an input stream into Tokens.
 struct Tokenizer {
   Tokenizer(bool whitespace_significant)
       : whitespace_significant_(whitespace_significant),
-        token_(Token::NONE), line_number_(1),
+        token_(Token::NONE), line_number_(0),
         last_indent_(0), cur_indent_(-1) {}
 
   void Start(const char* start, const char* end);
+  /// Report an error with a location pointing at the current token.
   bool Error(const string& message, string* err);
-  // Call Error() with "expected foo, got bar".
+  /// Call Error() with "expected foo, got bar".
   bool ErrorExpected(const string& expected, string* err);
 
   const Token& token() const { return token_; }
@@ -73,6 +87,10 @@ struct Tokenizer {
   Token::Type PeekToken();
   void ConsumeToken();
 
+  SourceLocation Location() {
+    return SourceLocation(line_number_ + 1, token_.pos_ - cur_line_ + 1);
+  }
+
   bool whitespace_significant_;
 
   const char* cur_;
diff --git a/src/parsers_test.cc b/src/parsers_test.cc
index f21cccf..6e3d7dd 100644
--- a/src/parsers_test.cc
+++ b/src/parsers_test.cc
@@ -274,6 +274,27 @@ TEST_F(ParserTest, Errors) {
     State state;
     ManifestParser parser(&state, NULL);
     string err;
+    EXPECT_FALSE(parser.Parse("rule cat\n  command = ${fafsd\n  foo = bar\n",
+                              &err));
+    // XXX EXPECT_EQ("line 2, col 20: expected closing curly after ${", err);
+    EXPECT_EQ("line 3, col 0: expected closing curly after ${", err);
+  }
+
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
+    EXPECT_FALSE(parser.Parse("rule cat\n  command = cat\nbuild $: cat foo\n",
+                              &err));
+    // XXX EXPECT_EQ("line 3, col 7: expected variable after $", err);
+    EXPECT_EQ("line 4, col 1: expected variable after $", err);
+  }
+
+  {
+    State state;
+    ManifestParser parser(&state, NULL);
+    string err;
     EXPECT_FALSE(parser.Parse("rule %foo\n",
                               &err));
     EXPECT_EQ("line 1, col 6: expected rule name, got unknown '%'", err);
@@ -296,6 +317,7 @@ TEST_F(ParserTest, Errors) {
     string err;
     EXPECT_FALSE(parser.Parse("rule cc\n  command = foo\n  othervar = bar\n",
                               &err));
+    // XXX EXPECT_EQ("line 3, col 3: unexpected variable 'othervar'", err);
     EXPECT_EQ("line 4, col 0: unexpected variable 'othervar'", err);
   }