Improves error messages for undefined return value (by Sverre Sundsdal); improves gmock_doctor.
diff --git a/include/gmock/gmock-actions.h b/include/gmock/gmock-actions.h
index a432758..95e075d 100644
--- a/include/gmock/gmock-actions.h
+++ b/include/gmock/gmock-actions.h
@@ -69,6 +69,8 @@
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 @@
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 @@
template <typename T>
class BuiltInDefaultValue<T*> {
public:
+ static bool Exists() { return true; }
static T* Get() { return NULL; }
};
@@ -100,6 +104,7 @@
template <> \
class BuiltInDefaultValue<type> { \
public: \
+ static bool Exists() { return true; } \
static type Get() { return value; } \
}
@@ -191,6 +196,12 @@
// 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 @@
// 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 @@
template <>
class DefaultValue<void> {
public:
+ static bool Exists() { return true; }
static void Get() {}
};
diff --git a/include/gmock/gmock-spec-builders.h b/include/gmock/gmock-spec-builders.h
index 84e0b51..0364b57 100644
--- a/include/gmock/gmock-spec-builders.h
+++ b/include/gmock/gmock-spec-builders.h
@@ -1061,15 +1061,21 @@
return NULL;
}
- // Performs the default action of this mock function on the given
- // arguments and returns the result. This method doesn't depend on
- // the mutable state of this object, and thus can be called
- // concurrently without locking.
+ // Performs the default action of this mock function on the given arguments
+ // and returns the result. Asserts with a helpful call descrption if there is
+ // no valid return value. This method doesn't depend on the mutable state of
+ // 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)
- : DefaultValue<Result>::Get();
+ if (spec != NULL) {
+ 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 @@
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 @@
args, &exp, &action, &is_excessive, &ss, &why);
ss << " Function call: " << mocker->Name();
UniversalPrinter<ArgumentTuple>::Print(args, &ss);
- Result result =
- action.IsDoDefault() ? mocker->PerformDefaultAction(args)
+ Result result = action.IsDoDefault() ?
+ mocker->PerformDefaultAction(args, ss.str())
: action.Perform(args);
ss << "\n Returns: ";
UniversalPrinter<Result>::Print(result, &ss);
@@ -1480,7 +1486,7 @@
const CallReaction reaction =
Mock::GetReactionOnUninterestingCalls(mocker->MockObject());
- mocker->PerformDefaultAction(args);
+ mocker->PerformDefaultAction(args, ss.str());
ReportUninterestingCall(reaction, ss.str());
return;
}
@@ -1499,7 +1505,7 @@
UniversalPrinter<ArgumentTuple>::Print(args, &ss);
ss << "\n" << why.str();
if (action.IsDoDefault()) {
- mocker->PerformDefaultAction(args);
+ mocker->PerformDefaultAction(args, ss.str());
} else {
action.Perform(args);
}
diff --git a/scripts/gmock_doctor.py b/scripts/gmock_doctor.py
index ce8ec49..ca7935c 100755
--- a/scripts/gmock_doctor.py
+++ b/scripts/gmock_doctor.py
@@ -55,6 +55,7 @@
'Ge',
'Gt',
'HasSubstr',
+ 'IsInitializedProto',
'Le',
'Lt',
'MatcherCast',
@@ -63,6 +64,7 @@
'Not',
'NotNull',
'Pointee',
+ 'PointeeIsInitializedProto',
'Property',
'Ref',
'StartsWith',
@@ -307,12 +309,29 @@
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,
diff --git a/test/gmock-actions_test.cc b/test/gmock-actions_test.cc
index 1000e30..6fb47bc 100644
--- a/test/gmock-actions_test.cc
+++ b/test/gmock-actions_test.cc
@@ -82,6 +82,13 @@
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 @@
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 @@
#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 @@
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 @@
// 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 @@
// 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 @@
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 @@
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());
}
diff --git a/test/gmock-spec-builders_test.cc b/test/gmock-spec-builders_test.cc
index 3e27aa8..3e944ea 100644
--- a/test/gmock-spec-builders_test.cc
+++ b/test/gmock-spec-builders_test.cc
@@ -987,6 +987,18 @@
#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) {