From de60c8b89a083d354539d3843da097c3232125e5 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan Date: Thu, 16 Apr 2020 14:01:45 -0700 Subject: [PATCH] Handle using nil as argument for andReturn in ObjC++ code. Fix for #403. This improves the special case handling of passing nil in return values to handle C++ cases that previously weren't working. Adds tests for C++11 and C++98 variants. To test fully tests need to be executed in both 32bit and 64 bit modes. --- Source/OCMock.xcodeproj/project.pbxproj | 14 +++++- Source/OCMock/OCMBoxedReturnValueProvider.m | 50 ++++++++++++++++--- .../OCMBoxedReturnValueProviderTests.m | 31 ++++++++---- Source/OCMockTests/OCMCPlusPlus11Tests.mm | 45 +++++++++++++++++ Source/OCMockTests/OCMCPlusPlus98Tests.mm | 45 +++++++++++++++++ Source/OCMockTests/OCMockObjectMacroTests.m | 8 +-- 6 files changed, 171 insertions(+), 22 deletions(-) create mode 100644 Source/OCMockTests/OCMCPlusPlus11Tests.mm create mode 100644 Source/OCMockTests/OCMCPlusPlus98Tests.mm diff --git a/Source/OCMock.xcodeproj/project.pbxproj b/Source/OCMock.xcodeproj/project.pbxproj index 2a9d519f..03f102d1 100644 --- a/Source/OCMock.xcodeproj/project.pbxproj +++ b/Source/OCMock.xcodeproj/project.pbxproj @@ -279,6 +279,10 @@ 817EB15C1BD765130047E85A /* OCMBlockArgCaller.h in Headers */ = {isa = PBXBuildFile; fileRef = 2FA2891034E7B73AA3511D17 /* OCMBlockArgCaller.h */; }; 817EB15D1BD765130047E85A /* OCMArgAction.h in Headers */ = {isa = PBXBuildFile; fileRef = 2FA2833B48908EAD36444671 /* OCMArgAction.h */; }; 817EB1661BD7674D0047E85A /* OCMFunctionsPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = 03F370CA1BAA1DE800CAD3E8 /* OCMFunctionsPrivate.h */; }; + 8B11D4B72448E2E900247BE2 /* OCMCPlusPlus98Tests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8B11D4B62448E2E900247BE2 /* OCMCPlusPlus98Tests.mm */; settings = {COMPILER_FLAGS = "-std=gnu++98"; }; }; + 8B11D4B82448E2F400247BE2 /* OCMCPlusPlus98Tests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8B11D4B62448E2E900247BE2 /* OCMCPlusPlus98Tests.mm */; settings = {COMPILER_FLAGS = "-std=gnu++98"; }; }; + 8B11D4BA2448E53600247BE2 /* OCMCPlusPlus11Tests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8B11D4B92448E53600247BE2 /* OCMCPlusPlus11Tests.mm */; settings = {COMPILER_FLAGS = "-std=gnu++11"; }; }; + 8B11D4BB2448E53600247BE2 /* OCMCPlusPlus11Tests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8B11D4B92448E53600247BE2 /* OCMCPlusPlus11Tests.mm */; settings = {COMPILER_FLAGS = "-std=gnu++11"; }; }; 8DE97C5522B43EE60098C63F /* OCMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B3159E146333BF0052CD09 /* OCMockObject.m */; }; 8DE97C5622B43EE60098C63F /* OCClassMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B3158C146333BF0052CD09 /* OCClassMockObject.m */; }; 8DE97C5722B43EE60098C63F /* OCPartialMockObject.m in Sources */ = {isa = PBXBuildFile; fileRef = 03B315AA146333BF0052CD09 /* OCPartialMockObject.m */; }; @@ -545,7 +549,7 @@ 2FA28006D043CBDBBAEF6E3F /* OCMMacroState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OCMMacroState.h; sourceTree = ""; }; 2FA280987F4EA8A4D79000D0 /* OCMMacroState.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMMacroState.m; sourceTree = ""; }; 2FA280EB5E8CDEEAE76861F7 /* OCMNonRetainingObjectReturnValueProvider.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMNonRetainingObjectReturnValueProvider.m; sourceTree = ""; }; - 2FA2813F93050582D83E1499 /* OCMockObjectRuntimeTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMockObjectRuntimeTests.m; sourceTree = ""; }; + 2FA2813F93050582D83E1499 /* OCMockObjectRuntimeTests.m */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.objcpp; fileEncoding = 4; path = OCMockObjectRuntimeTests.m; sourceTree = ""; }; 2FA2822E19948FC997965267 /* OCMockObjectTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMockObjectTests.m; sourceTree = ""; }; 2FA2833B48908EAD36444671 /* OCMArgAction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OCMArgAction.h; sourceTree = ""; }; 2FA283D58AA7569D8A5B0C57 /* OCMBlockArgCaller.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OCMBlockArgCaller.m; sourceTree = ""; }; @@ -569,6 +573,8 @@ 3CFBDD751BB3DB200050D9C5 /* TestClassWithCustomReferenceCounting.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TestClassWithCustomReferenceCounting.h; sourceTree = ""; }; 3CFBDD761BB3DB200050D9C5 /* TestClassWithCustomReferenceCounting.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = TestClassWithCustomReferenceCounting.m; sourceTree = ""; }; 817EB1621BD765130047E85A /* OCMock.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OCMock.framework; sourceTree = BUILT_PRODUCTS_DIR; }; + 8B11D4B62448E2E900247BE2 /* OCMCPlusPlus98Tests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = OCMCPlusPlus98Tests.mm; sourceTree = ""; }; + 8B11D4B92448E53600247BE2 /* OCMCPlusPlus11Tests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = OCMCPlusPlus11Tests.mm; sourceTree = ""; }; 8DE97CA022B43EE60098C63F /* OCMock.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OCMock.framework; sourceTree = BUILT_PRODUCTS_DIR; }; A02926811CA0725A00594AAF /* TestObjects.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = TestObjects.xcdatamodel; sourceTree = ""; }; D31108AD1828DB8700737925 /* OCMockLibTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = OCMockLibTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -747,6 +753,8 @@ 037ECD5318FAD84100AF0E4C /* OCMInvocationMatcherTests.m */, 031E50571BB4A56300E257C3 /* OCMBoxedReturnValueProviderTests.m */, 03B316211463350E0052CD09 /* OCMConstraintTests.m */, + 8B11D4B62448E2E900247BE2 /* OCMCPlusPlus98Tests.mm */, + 8B11D4B92448E53600247BE2 /* OCMCPlusPlus11Tests.mm */, 2FA28EDBF243639C57F88A1B /* OCMArgTests.m */, 036865631D3571A8005E6BEE /* OCMQuantifierTests.m */, 03B316291463350E0052CD09 /* OCObserverMockObjectTests.m */, @@ -1499,8 +1507,10 @@ 03565A4818F05721003AE91E /* OCMStubRecorderTests.m in Sources */, 03565A4518F05721003AE91E /* OCMockObjectForwardingTargetTests.m in Sources */, 2FA28FA53C57236B6DD64E82 /* OCMockObjectRuntimeTests.m in Sources */, + 8B11D4BA2448E53600247BE2 /* OCMCPlusPlus11Tests.mm in Sources */, 2FA2839F33289795284C32FB /* OCMockObjectTests.m in Sources */, 038599F723807B06002B3ABE /* OCMockObjectInternalTests.m in Sources */, + 8B11D4B72448E2E900247BE2 /* OCMCPlusPlus98Tests.mm in Sources */, 2FA28AB33F01A7D980F2C705 /* OCMockObjectDynamicPropertyMockingTests.m in Sources */, 031E50581BB4A56300E257C3 /* OCMBoxedReturnValueProviderTests.m in Sources */, ); @@ -1611,8 +1621,10 @@ 03C9CA1D18F05A75006DF94D /* OCMockObjectProtocolMocksTests.m in Sources */, 03E98D5118F310EE00522D42 /* OCMockObjectMacroTests.m in Sources */, A06930951CA1BFC900513023 /* TestObjects.xcdatamodeld in Sources */, + 8B11D4BB2448E53600247BE2 /* OCMCPlusPlus11Tests.mm in Sources */, 2FA28295E1F58F40A77D7448 /* OCMockObjectRuntimeTests.m in Sources */, 038599F823807B06002B3ABE /* OCMockObjectInternalTests.m in Sources */, + 8B11D4B82448E2F400247BE2 /* OCMCPlusPlus98Tests.mm in Sources */, 2FA28246CD449A01717B1CEC /* OCMockObjectTests.m in Sources */, 2FA28F12AAD384A8CB16094B /* OCMockObjectDynamicPropertyMockingTests.m in Sources */, ); diff --git a/Source/OCMock/OCMBoxedReturnValueProvider.m b/Source/OCMock/OCMBoxedReturnValueProvider.m index c29c8d2f..8be22419 100644 --- a/Source/OCMock/OCMBoxedReturnValueProvider.m +++ b/Source/OCMock/OCMBoxedReturnValueProvider.m @@ -18,6 +18,18 @@ #import "OCMFunctionsPrivate.h" #import "NSValue+OCMAdditions.h" +static BOOL IsZeroBuffer(const char* buffer, size_t length) +{ + for (size_t i = 0; i < length; ++i) + { + if (buffer[i] != 0) + { + return NO; + } + } + return YES; +} + @implementation OCMBoxedReturnValueProvider - (void)handleInvocation:(NSInvocation *)anInvocation @@ -26,10 +38,13 @@ - (void)handleInvocation:(NSInvocation *)anInvocation NSUInteger returnTypeSize = [[anInvocation methodSignature] methodReturnLength]; char valueBuffer[returnTypeSize]; NSValue *returnValueAsNSValue = (NSValue *)returnValue; + [returnValueAsNSValue getValue:valueBuffer]; - if([self isMethodReturnType:returnType compatibleWithValueType:[returnValueAsNSValue objCType]]) + if([self isMethodReturnType:returnType + compatibleWithValueType:[returnValueAsNSValue objCType] + value:valueBuffer + valueSize:returnTypeSize]) { - [returnValueAsNSValue getValue:valueBuffer]; [anInvocation setReturnValue:valueBuffer]; } else if([returnValueAsNSValue getBytes:valueBuffer objCType:returnType]) @@ -43,16 +58,37 @@ - (void)handleInvocation:(NSInvocation *)anInvocation } } - -- (BOOL)isMethodReturnType:(const char *)returnType compatibleWithValueType:(const char *)valueType +- (BOOL)isMethodReturnType:(const char *)returnType compatibleWithValueType:(const char *)valueType value:(const char*)value valueSize:(size_t)valueSize { /* Same types are obviously compatible */ if(strcmp(returnType, valueType) == 0) return YES; - /* Allow void* for methods that return id, mainly to be able to handle nil */ - if(strcmp(returnType, @encode(id)) == 0 && strcmp(valueType, @encode(void *)) == 0) - return YES; + // Special casing for nil. + if(strcmp(returnType, @encode(id)) == 0) + { + // Check to verify that the value is actually zero. + if(IsZeroBuffer(value, valueSize)) + { + // nil and Nil get potentially different encodings depending on the compilation + // settings of the file where the return value gets recorded. We check to verify + // against all the values we know of. + const char *validNilEncodings[] = + { + @encode(void *), // Standard Obj C + @encode(int), // 32 bit C++ (before nullptr) + @encode(long long), // 64 bit C++ (before nullptr) + @encode(char *), // C++ with nullptr + }; + for (size_t i = 0; i < sizeof(validNilEncodings) / sizeof(validNilEncodings[0]); ++i) + { + if (strcmp(valueType, validNilEncodings[i]) == 0) + { + return YES; + } + } + } + } return OCMEqualTypesAllowingOpaqueStructs(returnType, valueType); } diff --git a/Source/OCMockTests/OCMBoxedReturnValueProviderTests.m b/Source/OCMockTests/OCMBoxedReturnValueProviderTests.m index a5f0c2be..a1273730 100644 --- a/Source/OCMockTests/OCMBoxedReturnValueProviderTests.m +++ b/Source/OCMockTests/OCMBoxedReturnValueProviderTests.m @@ -18,15 +18,24 @@ #import "OCMBoxedReturnValueProvider.h" @interface OCMBoxedReturnValueProvider(Private) -- (BOOL)isMethodReturnType:(const char *)returnType compatibleWithValueType:(const char *)valueType; +- (BOOL)isMethodReturnType:(const char *)returnType compatibleWithValueType:(const char *)valueType value:(const char*)value valueSize:(size_t)valueSize; @end @interface OCMBoxedReturnValueProviderTests : XCTestCase - +{ + char value; + size_t valueSize; +} @end @implementation OCMBoxedReturnValueProviderTests +- (void)setUp { + [super setUp]; + value = 'A'; + valueSize = 1; +} + - (void)testCorrectEqualityForCppProperty { // see https://github.com/erikdoe/ocmock/issues/96 @@ -53,12 +62,12 @@ - (void)testCorrectEqualityForCppProperty "r^{GURL}"; OCMBoxedReturnValueProvider *boxed = [OCMBoxedReturnValueProvider new]; - XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2]); - XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type3]); - XCTAssertTrue([boxed isMethodReturnType:type2 compatibleWithValueType:type1]); - XCTAssertTrue([boxed isMethodReturnType:type2 compatibleWithValueType:type3]); - XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type1]); - XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type2]); + XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2 value:&value valueSize:valueSize]); + XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type3 value:&value valueSize:valueSize]); + XCTAssertTrue([boxed isMethodReturnType:type2 compatibleWithValueType:type1 value:&value valueSize:valueSize]); + XCTAssertTrue([boxed isMethodReturnType:type2 compatibleWithValueType:type3 value:&value valueSize:valueSize]); + XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type1 value:&value valueSize:valueSize]); + XCTAssertTrue([boxed isMethodReturnType:type3 compatibleWithValueType:type2 value:&value valueSize:valueSize]); } @@ -78,7 +87,7 @@ - (void)testCorrectEqualityForCppReturnTypesWithVtables "ar> >={__rep=(?={__long=QQ*}{__short=(?=Cc)[23c]}{__raw=[3Q]})}}}}"; OCMBoxedReturnValueProvider *boxed = [OCMBoxedReturnValueProvider new]; - XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2]); + XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2 value:&value valueSize:valueSize]); } @@ -89,7 +98,7 @@ - (void)testCorrectEqualityForStructureWithUnknownName const char *type2 = "{CLLocationCoordinate2D=dd}"; OCMBoxedReturnValueProvider *boxed = [OCMBoxedReturnValueProvider new]; - XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2]); + XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2 value:&value valueSize:valueSize]); } @@ -115,7 +124,7 @@ - (void)testCorrectEqualityForStructureWithoutName "pressed_pair >=^{GURL}}}}"; OCMBoxedReturnValueProvider *boxed = [OCMBoxedReturnValueProvider new]; - XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2]); + XCTAssertTrue([boxed isMethodReturnType:type1 compatibleWithValueType:type2 value:&value valueSize:valueSize]); } diff --git a/Source/OCMockTests/OCMCPlusPlus11Tests.mm b/Source/OCMockTests/OCMCPlusPlus11Tests.mm new file mode 100644 index 00000000..58c09349 --- /dev/null +++ b/Source/OCMockTests/OCMCPlusPlus11Tests.mm @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2020 Erik Doernenburg and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use these files except in compliance with the License. You may obtain + * a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +#import +#import + +#if !defined(__cplusplus) +#error This file must be compiled with C++ +#endif + +#if !__has_feature(cxx_nullptr) +#error This file must be compiled with a version of C++ that supports nullptr +#endif + +@interface OCMCPlusPlus11Tests : XCTestCase +@end + + +@implementation OCMCPlusPlus11Tests + +- (void)testSetsUpStubReturningNilForIdReturnType +{ + id mock = OCMPartialMock([NSArray arrayWithObject:@"Foo"]); + + OCMExpect([mock lastObject]).andReturn(nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); + + OCMExpect([mock lastObject]).andReturn(Nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); +} + +@end diff --git a/Source/OCMockTests/OCMCPlusPlus98Tests.mm b/Source/OCMockTests/OCMCPlusPlus98Tests.mm new file mode 100644 index 00000000..891149b0 --- /dev/null +++ b/Source/OCMockTests/OCMCPlusPlus98Tests.mm @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2020 Erik Doernenburg and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use these files except in compliance with the License. You may obtain + * a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +#import +#import + +#if !defined(__cplusplus) +#error This file must be compiled with C++ +#endif + +#if __has_feature(cxx_nullptr) +#error This file must be compiled with a version of C++ (98) that doesn't support nullptr +#endif + +@interface OCMCPlusPlus98Tests : XCTestCase +@end + + +@implementation OCMCPlusPlus98Tests + +- (void)testSetsUpStubReturningNilForIdReturnType +{ + id mock = OCMPartialMock([NSArray arrayWithObject:@"Foo"]); + + OCMExpect([mock lastObject]).andReturn(nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); + + OCMExpect([mock lastObject]).andReturn(Nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); +} + +@end diff --git a/Source/OCMockTests/OCMockObjectMacroTests.m b/Source/OCMockTests/OCMockObjectMacroTests.m index 312caea1..9cb10f94 100644 --- a/Source/OCMockTests/OCMockObjectMacroTests.m +++ b/Source/OCMockTests/OCMockObjectMacroTests.m @@ -177,11 +177,13 @@ - (void)testSetsUpStubsWithStructureReturnValues - (void)testSetsUpStubReturningNilForIdReturnType { - id mock = OCMClassMock([NSString class]); + id mock = OCMPartialMock([NSArray arrayWithObject:@"Foo"]); - OCMStub([mock lowercaseString]).andReturn(nil); + OCMExpect([mock lastObject]).andReturn(nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); - XCTAssertNil([mock lowercaseString], @"Should have returned stubbed value"); + OCMExpect([mock lastObject]).andReturn(Nil); + XCTAssertNil([mock lastObject], @"Should have returned stubbed value"); } - (void)testSetsUpExceptionThrowing