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

PiperOrigin-RevId: 652798234
Change-Id: I8e92248a2d9faf2a5719fe220145ea563acc14ff

3 files changed, 23 insertions, 39 deletions

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