Improves error messages for undefined return value (by Sverre Sundsdal); improves gmock_doctor.

include/gmock/gmock-actions.h

@@ -69,6 +69,8 @@ class ActionAdaptor;
 template <typename T>
 class BuiltInDefaultValue {
  public:
+  // This function returns true iff type T has a built-in default value.
+  static bool Exists() { return false; }
   static T Get() {
     Assert(false, __FILE__, __LINE__,
            "Default action undefined for the function return type.");
@@ -83,6 +85,7 @@ class BuiltInDefaultValue {
 template <typename T>
 class BuiltInDefaultValue<const T> {
  public:
+  static bool Exists() { return BuiltInDefaultValue<T>::Exists(); }
   static T Get() { return BuiltInDefaultValue<T>::Get(); }
 };
 
@@ -91,6 +94,7 @@ class BuiltInDefaultValue<const T> {
 template <typename T>
 class BuiltInDefaultValue<T*> {
  public:
+  static bool Exists() { return true; }
   static T* Get() { return NULL; }
 };
 
@@ -100,6 +104,7 @@ class BuiltInDefaultValue<T*> {
   template <> \
   class BuiltInDefaultValue<type> { \
    public: \
+    static bool Exists() { return true; } \
     static type Get() { return value; } \
   }
 
@@ -191,6 +196,12 @@ class DefaultValue {
   // Returns true iff the user has set the default value for type T.
   static bool IsSet() { return value_ != NULL; }
 
+  // Returns true if T has a default return value set by the user or there
+  // exists a built-in default value.
+  static bool Exists() {
+    return IsSet() || internal::BuiltInDefaultValue<T>::Exists();
+  }
+
   // Returns the default value for type T if the user has set one;
   // otherwise returns the built-in default value if there is one;
   // otherwise aborts the process.
@@ -220,6 +231,12 @@ class DefaultValue<T&> {
   // Returns true iff the user has set the default value for type T&.
   static bool IsSet() { return address_ != NULL; }
 
+  // Returns true if T has a default return value set by the user or there
+  // exists a built-in default value.
+  static bool Exists() {
+    return IsSet() || internal::BuiltInDefaultValue<T&>::Exists();
+  }
+
   // Returns the default value for type T& if the user has set one;
   // otherwise returns the built-in default value if there is one;
   // otherwise aborts the process.
@@ -236,6 +253,7 @@ class DefaultValue<T&> {
 template <>
 class DefaultValue<void> {
  public:
+  static bool Exists() { return true; }
   static void Get() {}
 };
 

include/gmock/gmock-spec-builders.h

@@ -1061,15 +1061,21 @@ class FunctionMockerBase : public UntypedFunctionMockerBase {
     return NULL;
   }
 
-  // Performs the default action of this mock function on the given
+  // Performs the default action of this mock function on the given arguments
-  // arguments and returns the result. This method doesn't depend on
+  // and returns the result. Asserts with a helpful call descrption if there is
-  // the mutable state of this object, and thus can be called
+  // no valid return value. This method doesn't depend on the mutable state of
-  // concurrently without locking.
+  // this object, and thus can be called concurrently without locking.
   // L = *
-  Result PerformDefaultAction(const ArgumentTuple& args) const {
+  Result PerformDefaultAction(const ArgumentTuple& args,
+                              const string& call_description) const {
     const DefaultActionSpec<F>* const spec = FindDefaultActionSpec(args);
-    return (spec != NULL) ? spec->GetAction().Perform(args)
+    if (spec != NULL) {
-        : DefaultValue<Result>::Get();
+      return spec->GetAction().Perform(args);
+    }
+    Assert(DefaultValue<Result>::Exists(), "", -1,
+           call_description + "\n    The mock function has no default action "
+           "set, and its return type has no default value set.");
+    return DefaultValue<Result>::Get();
   }
 
   // Registers this function mocker and the mock object owning it;
@@ -1407,7 +1413,7 @@ class InvokeWithHelper {
           Mock::GetReactionOnUninterestingCalls(mocker->MockObject());
 
       // Calculates the function result.
-      Result result = mocker->PerformDefaultAction(args);
+      Result result = mocker->PerformDefaultAction(args, ss.str());
 
       // Prints the function result.
       ss << "\n          Returns: ";
@@ -1429,8 +1435,8 @@ class InvokeWithHelper {
         args, &exp, &action, &is_excessive, &ss, &why);
     ss << "    Function call: " << mocker->Name();
     UniversalPrinter<ArgumentTuple>::Print(args, &ss);
-    Result result =
+    Result result = action.IsDoDefault() ?
-        action.IsDoDefault() ? mocker->PerformDefaultAction(args)
+        mocker->PerformDefaultAction(args, ss.str())
         : action.Perform(args);
     ss << "\n          Returns: ";
     UniversalPrinter<Result>::Print(result, &ss);
@@ -1480,7 +1486,7 @@ class InvokeWithHelper<void, F> {
       const CallReaction reaction =
           Mock::GetReactionOnUninterestingCalls(mocker->MockObject());
 
-      mocker->PerformDefaultAction(args);
+      mocker->PerformDefaultAction(args, ss.str());
       ReportUninterestingCall(reaction, ss.str());
       return;
     }
@@ -1499,7 +1505,7 @@ class InvokeWithHelper<void, F> {
     UniversalPrinter<ArgumentTuple>::Print(args, &ss);
     ss << "\n" << why.str();
     if (action.IsDoDefault()) {
-      mocker->PerformDefaultAction(args);
+      mocker->PerformDefaultAction(args, ss.str());
     } else {
       action.Perform(args);
     }

scripts/gmock_doctor.py

@@ -55,6 +55,7 @@ _COMMON_GMOCK_SYMBOLS = [
     'Ge',
     'Gt',
     'HasSubstr',
+    'IsInitializedProto',
     'Le',
     'Lt',
     'MatcherCast',
@@ -63,6 +64,7 @@ _COMMON_GMOCK_SYMBOLS = [
     'Not',
     'NotNull',
     'Pointee',
+    'PointeeIsInitializedProto',
     'Property',
     'Ref',
     'StartsWith',
@@ -307,12 +309,29 @@ Did you forget to write
       yield ('NUS', 'Need to Use Symbol', diagnosis % m.groupdict())
 
 
+def _NeedToUseReturnNullDiagnoser(msg):
+  """Diagnoses the NRNULL disease, given the error messages by gcc."""
+
+  regex = (r'(?P<file>.*):(?P<line>\d+):\s+instantiated from here\n'
+           r'.*gmock-actions\.h.*error: invalid conversion from '
+           r'\'long int\' to \'(?P<type>.+\*)')
+
+  diagnosis = """%(file)s:%(line)s:
+You are probably calling Return(NULL) and the compiler isn't sure how to turn
+NULL into a %(type)s*. Use ReturnNull() instead.
+Note: the line number may be off; please fix all instances of Return(NULL)."""
+  return _GenericDiagnoser('NRNULL', 'Need to use ReturnNull',
+                           regex, diagnosis, msg)
+
+
+
 _DIAGNOSERS = [
     _IncompleteByReferenceArgumentDiagnoser,
     _MockObjectPointerDiagnoser,
     _NeedToReturnNothingDiagnoser,
     _NeedToReturnReferenceDiagnoser,
     _NeedToReturnSomethingDiagnoser,
+    _NeedToUseReturnNullDiagnoser,
     _NeedToUseSymbolDiagnoser,
     _OverloadedFunctionActionDiagnoser,
     _OverloadedFunctionMatcherDiagnoser,

test/gmock-actions_test.cc

@@ -82,6 +82,13 @@ TEST(BuiltInDefaultValueTest, IsNullForPointerTypes) {
   EXPECT_TRUE(BuiltInDefaultValue<void*>::Get() == NULL);
 }
 
+// Tests that BuiltInDefaultValue<T*>::Exists() return true.
+TEST(BuiltInDefaultValueTest, ExistsForPointerTypes) {
+  EXPECT_TRUE(BuiltInDefaultValue<int*>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<const char*>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<void*>::Exists());
+}
+
 // Tests that BuiltInDefaultValue<T>::Get() returns 0 when T is a
 // built-in numeric type.
 TEST(BuiltInDefaultValueTest, IsZeroForNumericTypes) {
@@ -108,11 +115,42 @@ TEST(BuiltInDefaultValueTest, IsZeroForNumericTypes) {
   EXPECT_EQ(0, BuiltInDefaultValue<double>::Get());
 }
 
+// Tests that BuiltInDefaultValue<T>::Exists() returns true when T is a
+// built-in numeric type.
+TEST(BuiltInDefaultValueTest, ExistsForNumericTypes) {
+  EXPECT_TRUE(BuiltInDefaultValue<unsigned char>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<signed char>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<char>::Exists());
+#ifndef GTEST_OS_WINDOWS
+  EXPECT_TRUE(BuiltInDefaultValue<unsigned wchar_t>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<signed wchar_t>::Exists());
+#endif  // GTEST_OS_WINDOWS
+  EXPECT_TRUE(BuiltInDefaultValue<wchar_t>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<unsigned short>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<signed short>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<short>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<unsigned int>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<signed int>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<int>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<unsigned long>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<signed long>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<long>::Exists());  // NOLINT
+  EXPECT_TRUE(BuiltInDefaultValue<UInt64>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<Int64>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<float>::Exists());
+  EXPECT_TRUE(BuiltInDefaultValue<double>::Exists());
+}
+
 // Tests that BuiltInDefaultValue<bool>::Get() returns false.
 TEST(BuiltInDefaultValueTest, IsFalseForBool) {
   EXPECT_FALSE(BuiltInDefaultValue<bool>::Get());
 }
 
+// Tests that BuiltInDefaultValue<bool>::Exists() returns true.
+TEST(BuiltInDefaultValueTest, BoolExists) {
+  EXPECT_TRUE(BuiltInDefaultValue<bool>::Exists());
+}
+
 // Tests that BuiltInDefaultValue<T>::Get() returns "" when T is a
 // string type.
 TEST(BuiltInDefaultValueTest, IsEmptyStringForString) {
@@ -125,6 +163,18 @@ TEST(BuiltInDefaultValueTest, IsEmptyStringForString) {
 #endif  // GTEST_HAS_STD_STRING
 }
 
+// Tests that BuiltInDefaultValue<T>::Exists() returns true when T is a
+// string type.
+TEST(BuiltInDefaultValueTest, ExistsForString) {
+#if GTEST_HAS_GLOBAL_STRING
+  EXPECT_TRUE(BuiltInDefaultValue< ::string>::Exists());
+#endif  // GTEST_HAS_GLOBAL_STRING
+
+#if GTEST_HAS_STD_STRING
+  EXPECT_TRUE(BuiltInDefaultValue< ::std::string>::Exists());
+#endif  // GTEST_HAS_STD_STRING
+}
+
 // Tests that BuiltInDefaultValue<const T>::Get() returns the same
 // value as BuiltInDefaultValue<T>::Get() does.
 TEST(BuiltInDefaultValueTest, WorksForConstTypes) {
@@ -142,6 +192,10 @@ struct UserType {
   int value;
 };
 
+TEST(BuiltInDefaultValueTest, UserTypeHasNoDefault) {
+  EXPECT_FALSE(BuiltInDefaultValue<UserType>::Exists());
+}
+
 #ifdef GTEST_HAS_DEATH_TEST
 
 // Tests that BuiltInDefaultValue<T&>::Get() aborts the program.
@@ -170,17 +224,26 @@ TEST(DefaultValueTest, IsInitiallyUnset) {
 
 // Tests that DefaultValue<T> can be set and then unset.
 TEST(DefaultValueTest, CanBeSetAndUnset) {
+  EXPECT_TRUE(DefaultValue<int>::Exists());
+  EXPECT_FALSE(DefaultValue<const UserType>::Exists());
+
   DefaultValue<int>::Set(1);
   DefaultValue<const UserType>::Set(UserType());
 
   EXPECT_EQ(1, DefaultValue<int>::Get());
   EXPECT_EQ(0, DefaultValue<const UserType>::Get().value);
 
+  EXPECT_TRUE(DefaultValue<int>::Exists());
+  EXPECT_TRUE(DefaultValue<const UserType>::Exists());
+
   DefaultValue<int>::Clear();
   DefaultValue<const UserType>::Clear();
 
   EXPECT_FALSE(DefaultValue<int>::IsSet());
   EXPECT_FALSE(DefaultValue<const UserType>::IsSet());
+
+  EXPECT_TRUE(DefaultValue<int>::Exists());
+  EXPECT_FALSE(DefaultValue<const UserType>::Exists());
 }
 
 // Tests that DefaultValue<T>::Get() returns the
@@ -188,7 +251,9 @@ TEST(DefaultValueTest, CanBeSetAndUnset) {
 // false.
 TEST(DefaultValueDeathTest, GetReturnsBuiltInDefaultValueWhenUnset) {
   EXPECT_FALSE(DefaultValue<int>::IsSet());
+  EXPECT_TRUE(DefaultValue<int>::Exists());
   EXPECT_FALSE(DefaultValue<UserType>::IsSet());
+  EXPECT_FALSE(DefaultValue<UserType>::Exists());
 
   EXPECT_EQ(0, DefaultValue<int>::Get());
 
@@ -212,6 +277,12 @@ TEST(DefaultValueOfReferenceTest, IsInitiallyUnset) {
   EXPECT_FALSE(DefaultValue<UserType&>::IsSet());
 }
 
+// Tests that DefaultValue<T&>::Exists is false initiallly.
+TEST(DefaultValueOfReferenceTest, IsInitiallyNotExisting) {
+  EXPECT_FALSE(DefaultValue<int&>::Exists());
+  EXPECT_FALSE(DefaultValue<UserType&>::Exists());
+}
+
 // Tests that DefaultValue<T&> can be set and then unset.
 TEST(DefaultValueOfReferenceTest, CanBeSetAndUnset) {
   int n = 1;
@@ -219,12 +290,18 @@ TEST(DefaultValueOfReferenceTest, CanBeSetAndUnset) {
   UserType u;
   DefaultValue<UserType&>::Set(u);
 
+  EXPECT_TRUE(DefaultValue<const int&>::Exists());
+  EXPECT_TRUE(DefaultValue<UserType&>::Exists());
+
   EXPECT_EQ(&n, &(DefaultValue<const int&>::Get()));
   EXPECT_EQ(&u, &(DefaultValue<UserType&>::Get()));
 
   DefaultValue<const int&>::Clear();
   DefaultValue<UserType&>::Clear();
 
+  EXPECT_FALSE(DefaultValue<const int&>::Exists());
+  EXPECT_FALSE(DefaultValue<UserType&>::Exists());
+
   EXPECT_FALSE(DefaultValue<const int&>::IsSet());
   EXPECT_FALSE(DefaultValue<UserType&>::IsSet());
 }

test/gmock-spec-builders_test.cc

@@ -987,6 +987,18 @@ TEST(UnexpectedCallTest, UnsatisifiedPrerequisites) {
 
 #endif  // GMOCK_HAS_REGEX
 
+#ifdef GTEST_HAS_DEATH_TEST
+
+TEST(UndefinedReturnValueTest, ReturnValueIsMandatory) {
+  MockA a;
+  // TODO(wan@google.com): We should really verify the output message,
+  // but we cannot yet due to that EXPECT_DEATH only captures stderr
+  // while Google Mock logs to stdout.
+  EXPECT_DEATH(a.ReturnResult(1), "");
+}
+
+#endif  // GTEST_HAS_DEATH_TEST
+
 // Tests that an excessive call (one whose arguments match the
 // matchers but is called too many times) performs the default action.
 TEST(ExcessiveCallTest, DoesDefaultAction) {