refactor rule creation

diff --git a/manifest_parser.h b/manifest_parser.h
index 7d11e3a..175aa6a 100644 (file)
--- a/manifest_parser.h
+++ b/manifest_parser.h
@@ -319,7 +319,13 @@ bool ManifestParser::ParseRule(string* err) {
   if (!parser_.Newline(err))
     return false;
 
-  string command;
+  if (state_->LookupRule(name) != NULL) {
+    *err = "duplicate rule '" + name + "'";
+    return false;
+  }
+
+  Rule* rule = new Rule(name);  // XXX scoped_ptr
+
   if (parser_.PeekToken() == Token::INDENT) {
     parser_.ConsumeToken();
 
@@ -328,16 +334,17 @@ bool ManifestParser::ParseRule(string* err) {
       if (!ParseLet(&key, &val, err))
         return false;
 
-      if (key == "command")
-        command = val;
+      if (key == "command") {
+        rule->ParseCommand(val);
+      }
     }
     parser_.ConsumeToken();
   }
 
-  if (command.empty())
+  if (rule->command_.unparsed().empty())
     return parser_.Error("expected 'command =' line", err);
 
-  state_->AddRule(name, command);
+  state_->AddRule(rule);
   return true;
 }
 

diff --git a/ninja.h b/ninja.h
index e49a84a..1611d9f 100644 (file)
--- a/ninja.h
+++ b/ninja.h
@@ -121,8 +121,9 @@ struct Node {
 };
 
 struct Rule {
-  Rule(const string& name, const string& command) :
-    name_(name) {
+  Rule(const string& name) : name_(name) { }
+
+  void ParseCommand(const string& command) {
     assert(command_.Parse(command));  // XXX
   }
   string name_;
@@ -268,7 +269,7 @@ struct State : public EvalString::Env {
   // EvalString::Env impl
   virtual string Evaluate(const string& var);
 
-  Rule* AddRule(const string& name, const string& command);
+  void AddRule(Rule* rule);
   Rule* LookupRule(const string& rule_name);
   Edge* AddEdge(Rule* rule);
   Node* GetNode(const string& path);
@@ -294,10 +295,9 @@ Rule* State::LookupRule(const string& rule_name) {
   return i->second;
 }
 
-Rule* State::AddRule(const string& name, const string& command) {
-  Rule* rule = new Rule(name, command);
-  rules_[name] = rule;
-  return rule;
+void State::AddRule(Rule* rule) {
+  assert(LookupRule(rule->name_) == NULL);
+  rules_[rule->name_] = rule;
 }
 
 Edge* State::AddEdge(Rule* rule) {

diff --git a/ninja_test.cc b/ninja_test.cc
index 32afaa0..0270f96 100644 (file)
--- a/ninja_test.cc
+++ b/ninja_test.cc
@@ -75,8 +75,6 @@ TEST(Parser, Comment) {
 }
 
 TEST(Parser, Errors) {
-  State state;
-
   {
     ManifestParser parser(NULL);
     string err;
@@ -99,6 +97,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("x = 3\ny 2", &err));
@@ -106,6 +105,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("build x: y z\n", &err));
@@ -113,6 +113,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("build x:: y z\n", &err));
@@ -120,6 +121,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("rule cat\n  command = cat ok\n"
@@ -129,6 +131,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("rule cat\n"
@@ -138,6 +141,7 @@ TEST(Parser, Errors) {
   }
 
   {
+    State state;
     ManifestParser parser(&state);
     string err;
     EXPECT_FALSE(parser.Parse("rule %foo\n",
@@ -161,7 +165,9 @@ TEST(Parser, BuildDir) {
 
 TEST(State, Basic) {
   State state;
-  Rule* rule = state.AddRule("cat", "cat @in > $out");
+  Rule* rule = new Rule("cat");
+  rule->ParseCommand("cat @in > $out");
+  state.AddRule(rule);
   Edge* edge = state.AddEdge(rule);
   state.AddInOut(edge, Edge::IN, "in1");
   state.AddInOut(edge, Edge::IN, "in2");