From 417e7b1a878722f7dea7570d0c47454734e2127a Mon Sep 17 00:00:00 2001
From: Evan Martin <martine@danga.com>
Date: Sun, 31 Oct 2010 18:04:06 -0700
Subject: [PATCH] report variable parse errors

---
 eval_env.h      |  2 +-
 ninja.h         |  4 ++--
 ninja_jumble.cc |  4 ++--
 ninja_test.cc   | 20 ++++++++++++++++----
 parsers.cc      | 12 ++++++++----
 5 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/eval_env.h b/eval_env.h
index 62f6c04..915de59 100644
--- a/eval_env.h
+++ b/eval_env.h
@@ -3,7 +3,7 @@ struct EvalString {
   struct Env {
     virtual string Evaluate(const string& var) = 0;
   };
-  bool Parse(const string& input);
+  bool Parse(const string& input, string* err);
   string Evaluate(Env* env);
 
   const string& unparsed() const { return unparsed_; }
diff --git a/ninja.h b/ninja.h
index 94e9d19..2b8c37c 100644
--- a/ninja.h
+++ b/ninja.h
@@ -81,8 +81,8 @@ struct Node {
 struct Rule {
   Rule(const string& name) : name_(name) { }
 
-  void ParseCommand(const string& command) {
-    assert(command_.Parse(command));  // XXX
+  bool ParseCommand(const string& command, string* err) {
+    return command_.Parse(command, err);
   }
   string name_;
   EvalString command_;
diff --git a/ninja_jumble.cc b/ninja_jumble.cc
index cbba387..3d6da2d 100644
--- a/ninja_jumble.cc
+++ b/ninja_jumble.cc
@@ -470,7 +470,7 @@ bool Builder::Build(Shell* shell, string* err) {
   return true;
 }
 
-bool EvalString::Parse(const string& input) {
+bool EvalString::Parse(const string& input, string* err) {
   unparsed_ = input;
 
   string::size_type start, end;
@@ -490,7 +490,7 @@ bool EvalString::Parse(const string& input) {
         break;
     }
     if (end == start + 1) {
-      // XXX report bad parse here
+      *err = "expected variable after $";
       return false;
     }
     parsed_.push_back(make_pair(input.substr(start, end - start), SPECIAL));
diff --git a/ninja_test.cc b/ninja_test.cc
index e376935..d122677 100644
--- a/ninja_test.cc
+++ b/ninja_test.cc
@@ -14,7 +14,9 @@ static void AssertParse(State* state, const char* input) {
 TEST(State, Basic) {
   State state;
   Rule* rule = new Rule("cat");
-  rule->ParseCommand("cat @in > $out");
+  string err;
+  EXPECT_TRUE(rule->ParseCommand("cat @in > $out", &err));
+  ASSERT_EQ("", err);
   state.AddRule(rule);
   Edge* edge = state.AddEdge(rule);
   state.AddInOut(edge, Edge::IN, "in1");
@@ -41,18 +43,28 @@ struct TestEnv : public EvalString::Env {
 };
 TEST(EvalString, PlainText) {
   EvalString str;
-  str.Parse("plain text");
-  ASSERT_EQ("plain text", str.Evaluate(NULL));
+  string err;
+  EXPECT_TRUE(str.Parse("plain text", &err));
+  EXPECT_EQ("", err);
+  EXPECT_EQ("plain text", str.Evaluate(NULL));
 }
 TEST(EvalString, OneVariable) {
   EvalString str;
-  ASSERT_TRUE(str.Parse("hi $var"));
+  string err;
+  EXPECT_TRUE(str.Parse("hi $var", &err));
+  EXPECT_EQ("", err);
   EXPECT_EQ("hi $var", str.unparsed());
   TestEnv env;
   EXPECT_EQ("hi ", str.Evaluate(&env));
   env.vars["$var"] = "there";
   EXPECT_EQ("hi there", str.Evaluate(&env));
 }
+TEST(EvalString, Error) {
+  EvalString str;
+  string err;
+  EXPECT_FALSE(str.Parse("bad $", &err));
+  EXPECT_EQ("expected variable after $", err);
+}
 
 struct StateTestWithBuiltinRules : public testing::Test {
   StateTestWithBuiltinRules() {
diff --git a/parsers.cc b/parsers.cc
index d9b236c..8628d83 100644
--- a/parsers.cc
+++ b/parsers.cc
@@ -284,10 +284,13 @@ bool ManifestParser::ParseRule(string* err) {
       if (!ParseLet(&key, &val, err))
         return false;
 
+      string parse_err;
       if (key == "command") {
-        rule->ParseCommand(val);
+        if (!rule->ParseCommand(val, &parse_err))
+          return tokenizer_.Error(parse_err, err);
       } else if (key == "depfile") {
-        rule->depfile_.Parse(val);
+        if (!rule->depfile_.Parse(val, &parse_err))
+          return tokenizer_.Error(parse_err, err);
       }
     }
     tokenizer_.ConsumeToken();
@@ -377,8 +380,9 @@ bool ManifestParser::ParseSubNinja(string* err) {
     return false;
 
   ManifestParser subparser(state_, file_reader_);
-  if (!subparser.Parse(contents, err))
-    return false;
+  string sub_err;
+  if (!subparser.Parse(contents, &sub_err))
+    return tokenizer_.Error("in '" + path + "': " + sub_err, err);
 
   return true;
 }
-- 
2.7.4