Use matcher's description in AllOf if matcher has no explanation.

PiperOrigin-RevId: 652634229
Change-Id: I5fd7eaef4db3dea3d4142e1fb5fc97e46f654358

3 files changed, 39 insertions, 23 deletions

diff --git a/googlemock/include/gmock/gmock-matchers.h b/googlemock/include/gmock/gmock-matchers.h
index 063ee6c..3daf617 100644
--- a/googlemock/include/gmock/gmock-matchers.h
+++ b/googlemock/include/gmock/gmock-matchers.h
@@ -1300,34 +1300,48 @@ class AllOfMatcherImpl : public MatcherInterface<const T&> {
 
   bool MatchAndExplain(const T& x,
                        MatchResultListener* listener) const override {
-    // If either matcher1_ or matcher2_ doesn't match x, we only need
-    // to explain why one of them fails.
+    // This method uses matcher's explanation when explaining the result.
+    // However, if matcher doesn't provide one, this method uses matcher's
+    // description.
     std::string all_match_result;
-
-    for (size_t i = 0; i < matchers_.size(); ++i) {
+    for (const Matcher<T>& matcher : matchers_) {
       StringMatchResultListener slistener;
-      if (matchers_[i].MatchAndExplain(x, &slistener)) {
-        if (all_match_result.empty()) {
-          all_match_result = slistener.str();
+      // Return explanation for first failed matcher.
+      if (!matcher.MatchAndExplain(x, &slistener)) {
+        const std::string explanation = slistener.str();
+        if (!explanation.empty()) {
+          *listener << explanation;
         } else {
-          std::string result = slistener.str();
-          if (!result.empty()) {
-            all_match_result += ", and ";
-            all_match_result += result;
-          }
+          *listener << "which doesn't match (" << Describe(matcher) << ")";
         }
-      } else {
-        *listener << slistener.str();
         return false;
       }
+      // Keep track of explanations in case all matchers succeed.
+      std::string explanation = slistener.str();
+      if (explanation.empty()) {
+        explanation = Describe(matcher);
+      }
+      if (all_match_result.empty()) {
+        all_match_result = explanation;
+      } else {
+        if (!explanation.empty()) {
+          all_match_result += ", and ";
+          all_match_result += explanation;
+        }
+      }
     }
 
-    // Otherwise we need to explain why *both* of them match.
     *listener << all_match_result;
     return true;
   }
 
  private:
+  // Returns matcher description as a string.
+  std::string Describe(const Matcher<T>& matcher) const {
+    StringMatchResultListener listener;
+    matcher.DescribeTo(listener.stream());
+    return listener.str();
+  }
   const std::vector<Matcher<T>> matchers_;
 };
 
diff --git a/googlemock/test/gmock-matchers-arithmetic_test.cc b/googlemock/test/gmock-matchers-arithmetic_test.cc
index f176962..7521c0d 100644
--- a/googlemock/test/gmock-matchers-arithmetic_test.cc
+++ b/googlemock/test/gmock-matchers-arithmetic_test.cc
@@ -559,10 +559,9 @@ TEST_P(AllOfTestP, ExplainsResult) {
   Matcher<int> m;
 
   // Successful match.  Both matchers need to explain.  The second
-  // matcher doesn't give an explanation, so only the first matcher's
-  // explanation is printed.
+  // matcher doesn't give an explanation, so the matcher description is used.
   m = AllOf(GreaterThan(10), Lt(30));
-  EXPECT_EQ("which is 15 more than 10", Explain(m, 25));
+  EXPECT_EQ("which is 15 more than 10, and is < 30", Explain(m, 25));
 
   // Successful match.  Both matchers need to explain.
   m = AllOf(GreaterThan(10), GreaterThan(20));
@@ -572,8 +571,9 @@ TEST_P(AllOfTestP, ExplainsResult) {
   // Successful match.  All matchers need to explain.  The second
   // matcher doesn't given an explanation.
   m = AllOf(GreaterThan(10), Lt(30), GreaterThan(20));
-  EXPECT_EQ("which is 15 more than 10, and which is 5 more than 20",
-            Explain(m, 25));
+  EXPECT_EQ(
+      "which is 15 more than 10, and is < 30, and which is 5 more than 20",
+      Explain(m, 25));
 
   // Successful match.  All matchers need to explain.
   m = AllOf(GreaterThan(10), GreaterThan(20), GreaterThan(30));
@@ -588,10 +588,10 @@ TEST_P(AllOfTestP, ExplainsResult) {
   EXPECT_EQ("which is 5 less than 10", Explain(m, 5));
 
   // Failed match.  The second matcher, which failed, needs to
-  // explain.  Since it doesn't given an explanation, nothing is
+  // explain.  Since it doesn't given an explanation, the matcher text is
   // printed.
   m = AllOf(GreaterThan(10), Lt(30));
-  EXPECT_EQ("", Explain(m, 40));
+  EXPECT_EQ("which doesn't match (is < 30)", Explain(m, 40));
 
   // Failed match.  The second matcher, which failed, needs to
   // explain.
diff --git a/googlemock/test/gmock-matchers-comparisons_test.cc b/googlemock/test/gmock-matchers-comparisons_test.cc
index 5b75b45..9881155 100644
--- a/googlemock/test/gmock-matchers-comparisons_test.cc
+++ b/googlemock/test/gmock-matchers-comparisons_test.cc
@@ -2334,9 +2334,11 @@ TEST(ExplainMatchResultTest, AllOf_True_True) {
   EXPECT_EQ("which is 0 modulo 2, and which is 0 modulo 3", Explain(m, 6));
 }
 
+// Tests that when AllOf() succeeds, but matchers have no explanation,
+// the matcher description is used.
 TEST(ExplainMatchResultTest, AllOf_True_True_2) {
   const Matcher<int> m = AllOf(Ge(2), Le(3));
-  EXPECT_EQ("", Explain(m, 2));
+  EXPECT_EQ("is >= 2, and is <= 3", Explain(m, 2));
 }
 
 INSTANTIATE_GTEST_MATCHER_TEST_P(ExplainmatcherResultTest);