Improve misleading error message when an output is defined multiple times

6 files changed, 18 insertions, 11 deletions

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index aa52989..c4b2980 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -336,8 +336,8 @@ bool ManifestParser::ParseEdge(string* err) {
       return lexer_.Error("empty path", err);
     uint64_t slash_bits;
     CanonicalizePath(&path, &slash_bits);
-    if (!state_->AddOut(edge, path, slash_bits)) {
-      lexer_.Error("multiple rules generate " + path, err);
+    if (!state_->AddOut(edge, path, slash_bits, err)) {
+      lexer_.Error(std::string(*err), err);
       return false;
     }
   }
diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc
index 8a7b135..c5a1fe8 100644
--- a/src/manifest_parser_test.cc
+++ b/src/manifest_parser_test.cc
@@ -978,7 +978,7 @@ TEST_F(ParserTest, ImplicitOutputDupeError) {
   ManifestParser parser(&state, &fs_);
   string err;
   EXPECT_FALSE(parser.ParseTest(kInput, &err));
-  EXPECT_EQ("input:4: multiple rules generate foo\n", err);
+  EXPECT_EQ("input:4: foo is defined as an output multiple times\n", err);
 }
 
 TEST_F(ParserTest, ImplicitOutputDupesError) {
@@ -989,7 +989,7 @@ TEST_F(ParserTest, ImplicitOutputDupesError) {
   ManifestParser parser(&state, &fs_);
   string err;
   EXPECT_FALSE(parser.ParseTest(kInput, &err));
-  EXPECT_EQ("input:4: multiple rules generate foo\n", err);
+  EXPECT_EQ("input:4: foo is defined as an output multiple times\n", err);
 }
 
 TEST_F(ParserTest, NoExplicitOutput) {
diff --git a/src/missing_deps_test.cc b/src/missing_deps_test.cc
index 12ae8ed..dae377b 100644
--- a/src/missing_deps_test.cc
+++ b/src/missing_deps_test.cc
@@ -64,9 +64,9 @@ struct MissingDependencyScannerTest : public testing::Test {
     compile_rule_.AddBinding("deps", deps_type);
     generator_rule_.AddBinding("deps", deps_type);
     Edge* header_edge = state_.AddEdge(&generator_rule_);
-    state_.AddOut(header_edge, "generated_header", 0);
+    state_.AddOut(header_edge, "generated_header", 0, nullptr);
     Edge* compile_edge = state_.AddEdge(&compile_rule_);
-    state_.AddOut(compile_edge, "compiled_object", 0);
+    state_.AddOut(compile_edge, "compiled_object", 0, nullptr);
   }
 
   void CreateGraphDependencyBetween(const char* from, const char* to) {
@@ -130,7 +130,7 @@ TEST_F(MissingDependencyScannerTest, MissingDepFixedIndirect) {
   CreateInitialState();
   // Adding an indirect dependency also fixes the issue
   Edge* intermediate_edge = state_.AddEdge(&generator_rule_);
-  state_.AddOut(intermediate_edge, "intermediate", 0);
+  state_.AddOut(intermediate_edge, "intermediate", 0, nullptr);
   CreateGraphDependencyBetween("compiled_object", "intermediate");
   CreateGraphDependencyBetween("intermediate", "generated_header");
   RecordDepsLogDep("compiled_object", "generated_header");
diff --git a/src/state.cc b/src/state.cc
index 0a68f21..d4b9a71 100644
--- a/src/state.cc
+++ b/src/state.cc
@@ -133,10 +133,17 @@ void State::AddIn(Edge* edge, StringPiece path, uint64_t slash_bits) {
   node->AddOutEdge(edge);
 }
 
-bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits) {
+bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits,
+                   std::string* err) {
   Node* node = GetNode(path, slash_bits);
-  if (node->in_edge())
+  if (Edge* other = node->in_edge()) {
+    if (other == edge) {
+      *err = path.AsString() + " is defined as an output multiple times";
+    } else {
+      *err = "multiple rules generate " + path.AsString();
+    }
     return false;
+  }
   edge->outputs_.push_back(node);
   node->set_in_edge(edge);
   node->set_generated_by_dep_loader(false);
diff --git a/src/state.h b/src/state.h
index 886b78f..29bed56 100644
--- a/src/state.h
+++ b/src/state.h
@@ -109,7 +109,7 @@ struct State {
   /// ensures that the generated_by_dep_loader() flag for all these nodes
   /// is set to false, to indicate that they come from the input manifest.
   void AddIn(Edge* edge, StringPiece path, uint64_t slash_bits);
-  bool AddOut(Edge* edge, StringPiece path, uint64_t slash_bits);
+  bool AddOut(Edge* edge, StringPiece path, uint64_t slash_bits, std::string* err);
   void AddValidation(Edge* edge, StringPiece path, uint64_t slash_bits);
   bool AddDefault(StringPiece path, std::string* error);
 
diff --git a/src/state_test.cc b/src/state_test.cc
index 96469f9..e0e3060 100644
--- a/src/state_test.cc
+++ b/src/state_test.cc
@@ -36,7 +36,7 @@ TEST(State, Basic) {
   Edge* edge = state.AddEdge(rule);
   state.AddIn(edge, "in1", 0);
   state.AddIn(edge, "in2", 0);
-  state.AddOut(edge, "out", 0);
+  state.AddOut(edge, "out", 0, nullptr);
 
   EXPECT_EQ("cat in1 in2 > out", edge->EvaluateCommand());