-
Notifications
You must be signed in to change notification settings - Fork 609
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve
andDo:
type safety and simplify usage.
The current `andDo` blocks have some major problems: a) They are difficult to use from ARC. When getting values out of NSInvocations, one has to be careful to use `__unsafe_unretained` on arguments, or risk over retaining/releasing and corrupting the heap. When returning values one has to be careful to make sure that they are handled correctly (often using `__autoreleasing`) or once again risk memory issues. b) They don't deal with refactors well. Since extracting argument inside of the blocks depends on extracting by index, it is easy to change a method signature and forget to deal with the index. In many cases it may "just pass" even though the arguments no longer line up correctly. In other cases it leads to crashes that are difficult to debug. c) The signature is often overly complicated for simple blocks. In many cases all we want is a block that just returns a value, or doesn't need to take arguments or return a value. This changes modifies andDo blocks so that the current `andDo:^void(NSInvocation *invocation) { ... }` block is now considered deprecated and gives you options to the type of block you want to pass in. The first is the simple block `^{ ... }` that takes no arguments and has an optional return value. The runtime code verifies that the return value (if supplied) is compatible with what the invocation expects. The second option is the full block which is `^(returnType)(id localSelf, Arg1Type arg1, ...)` where `returnType` and the argument list match the return type and arguments to the invocation. These values are checked at runtime to make sure that they match what the invocation expects. To aid in the transition from the deprecated block type to the new block types, the runtime will `NSLog` a warning about the deprecated block type, and will attempt to suggest a good block declaration to replace it with. They would look something like this: ``` Warning: Replace `^(NSInvocation *invocation) { ... }` with `^id(NSSet<ObjectType*> *localSelf, NSNumber *count, NSSet<ObjectType*> *set) { return ... }` ``` This changes code that previously would have looked like this: ``` OCMStub([mockURL setResourceValues:attributes error:[OCMArg anyObjectRef]]) .andDo(^(NSInvocation *invocation) { __unsafe_unretained NSError **errorOut = nil; [invocation getArgument:&errorOut atIndex:3]; *errorOut = error; BOOL returnValue = NO; [invocation setReturnValue:&returnValue]; }); ``` to ``` OCMStub([mockURL setResourceValues:attributes error:[OCMArg anyObjectRef]]) .andDo(^BOOL(NSURL *localSelf, NSDictionary<NSURLResourceKey, id> *keyedValues, NSError **error) { *error = [NSError errorWithDomain:@"Error" code:0 userInfo:nil]; return NO; }); ``` and adds a large amount of runtime checking to verify safety. I can break this up into some smaller CLs if we agree that this is a reasonable direction to head in. I have tested the changes here extensively on all of our code at Google.
- Loading branch information
Showing
19 changed files
with
1,008 additions
and
182 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.