Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Add nullability annotations to all APIs. #88

Merged
merged 3 commits into from
Mar 11, 2016
Merged

Add nullability annotations to all APIs. #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f97e352
Add nullability annotations to NSObject+FBKVOController.
nlutsenko Mar 4, 2016
6371fb5
Add nullability annotations to FBKVOController.
nlutsenko Mar 4, 2016
78f1810
Fix nullability warnings in tests that have validation for nonnull th…
nlutsenko Mar 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions FBKVOController/FBKVOController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

/**
@abstract Block called on key-value change notification.
@param observer The observer of the change.
@param object The object changed.
@param change The change dictionary.
*/
typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *change);

typedef void (^FBKVONotificationBlock)(id _Nullable observer, id object, NSDictionary *change);

/**
@abstract FBKVOController makes Key-Value Observing simpler and safer.
Expand All @@ -29,7 +30,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param observer The object notified on key-value change.
@return The initialized KVO controller instance.
*/
+ (instancetype)controllerWithObserver:(id)observer;
+ (instancetype)controllerWithObserver:(nullable id)observer;

/**
@abstract The designated initializer.
Expand All @@ -38,18 +39,18 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@return The initialized KVO controller instance.
@discussion Use retainObserved = NO when a strong reference between controller and observee would create a retain loop. When not retaining observees, special care must be taken to remove observation info prior to observee dealloc.
*/
- (instancetype)initWithObserver:(id)observer retainObserved:(BOOL)retainObserved;
- (instancetype)initWithObserver:(nullable id)observer retainObserved:(BOOL)retainObserved;

/**
@abstract Convenience initializer.
@param observer The object notified on key-value change. The specified observer must support weak references.
@return The initialized KVO controller instance.
@discussion By default, KVO controller retains objects observed.
*/
- (instancetype)initWithObserver:(id)observer;
- (instancetype)initWithObserver:(nullable id)observer;

/// The observer notified on key-value change. Specified on initialization.
@property (atomic, weak, readonly) id observer;
@property (nullable, atomic, weak, readonly) id observer;

/**
@abstract Registers observer for key-value change notification.
Expand All @@ -59,7 +60,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param block The block to execute on notification.
@discussion On key-value change, the specified block is called. In order to avoid retain loops, the block must avoid referencing the KVO controller or an owner thereof. Observing an already observed object key path or nil results in no operation.
*/
- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block;
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is 'object' nullable? Are we explicitly allowing no-ops here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, inside implementation there is logic for bailing out early if the object is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change it, but I think since we already are handling the null usecase, this is more permissive model. I don't feel strong about adding a constraint here, but we can defiently do it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wonder wether this is a useful for the API to expose or not. Again, the eternal debate of "should nullable/nonnull imply runtime semantics" or not...

My thinking is that observe is something that only logically makes sense on a nonnull value, but considering the existing API is out there and implicitly deals with nil, maybe it makes sense to not force null checks on clients. Not sold either way.


/**
@abstract Registers observer for key-value change notification.
Expand All @@ -69,7 +70,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param action The observer selector called on key-value change.
@discussion On key-value change, the observer's action selector is called. The selector provided should take the form of -propertyDidChange, -propertyDidChange: or -propertyDidChange:object:, where optional parameters delivered will be KVO change dictionary and object observed. Observing nil or observing an already observed object's key path results in no operation.
*/
- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options action:(SEL)action;
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options action:(SEL)action;

/**
@abstract Registers observer for key-value change notification.
Expand All @@ -79,7 +80,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param context The context specified.
@discussion On key-value change, the observer's -observeValueForKeyPath:ofObject:change:context: method is called. Observing an already observed object key path or nil results in no operation.
*/
- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context;
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(nullable void *)context;


/**
Expand All @@ -90,7 +91,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param block The block to execute on notification.
@discussion On key-value change, the specified block is called. Inorder to avoid retain loops, the block must avoid referencing the KVO controller or an owner thereof. Observing an already observed object key path or nil results in no operation.
*/
- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block;
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block;

/**
@abstract Registers observer for key-value change notification.
Expand All @@ -100,7 +101,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param action The observer selector called on key-value change.
@discussion On key-value change, the observer's action selector is called. The selector provided should take the form of -propertyDidChange, -propertyDidChange: or -propertyDidChange:object:, where optional parameters delivered will be KVO change dictionary and object observed. Observing nil or observing an already observed object's key path results in no operation.
*/
- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options action:(SEL)action;
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options action:(SEL)action;

/**
@abstract Registers observer for key-value change notification.
Expand All @@ -110,7 +111,7 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param context The context specified.
@discussion On key-value change, the observer's -observeValueForKeyPath:ofObject:change:context: method is called. Observing an already observed object key path or nil results in no operation.
*/
- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options context:(void *)context;
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options context:(nullable void *)context;


/**
Expand All @@ -119,14 +120,14 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
@param keyPath The key path to observe.
@discussion If not observing object key path, or unobserving nil, this method results in no operation.
*/
- (void)unobserve:(id)object keyPath:(NSString *)keyPath;
- (void)unobserve:(nullable id)object keyPath:(NSString *)keyPath;

/**
@abstract Unobserve all object key paths.
@param object The object to unobserve.
@discussion If not observing object, or unobserving nil, this method results in no operation.
*/
- (void)unobserve:(id)object;
- (void)unobserve:(nullable id)object;

/**
@abstract Unobserve all objects.
Expand All @@ -135,3 +136,5 @@ typedef void (^FBKVONotificationBlock)(id observer, id object, NSDictionary *cha
- (void)unobserveAll;

@end

NS_ASSUME_NONNULL_END
50 changes: 31 additions & 19 deletions FBKVOController/FBKVOController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#error This file must be compiled with ARC. Convert your project to ARC or specify the -fobjc-arc flag.
#endif

NS_ASSUME_NONNULL_BEGIN

#pragma mark Utilities -

static NSString *describe_option(NSKeyValueObservingOptions option)
Expand Down Expand Up @@ -98,7 +100,12 @@ @implementation _FBKVOInfo
FBKVONotificationBlock _block;
}

- (instancetype)initWithController:(FBKVOController *)controller keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block action:(SEL)action context:(void *)context
- (instancetype)initWithController:(FBKVOController *)controller
keyPath:(NSString *)keyPath
options:(NSKeyValueObservingOptions)options
block:(nullable FBKVONotificationBlock)block
action:(nullable SEL)action
context:(nullable void *)context
{
self = [super init];
if (nil != self) {
Expand Down Expand Up @@ -184,13 +191,13 @@ @interface _FBKVOSharedController : NSObject
+ (instancetype)sharedController;

/** observe an object, info pair */
- (void)observe:(id)object info:(_FBKVOInfo *)info;
- (void)observe:(id)object info:(nullable _FBKVOInfo *)info;

/** unobserve an object, info pair */
- (void)unobserve:(id)object info:(_FBKVOInfo *)info;
- (void)unobserve:(id)object info:(nullable _FBKVOInfo *)info;

/** unobserve an object with a set of infos */
- (void)unobserve:(id)object infos:(NSSet *)infos;
- (void)unobserve:(id)object infos:(nullable NSSet *)infos;

@end

Expand Down Expand Up @@ -255,7 +262,7 @@ - (NSString *)debugDescription
return s;
}

- (void)observe:(id)object info:(_FBKVOInfo *)info
- (void)observe:(id)object info:(nullable _FBKVOInfo *)info
{
if (nil == info) {
return;
Expand All @@ -270,7 +277,7 @@ - (void)observe:(id)object info:(_FBKVOInfo *)info
[object addObserver:self forKeyPath:info->_keyPath options:info->_options context:(void *)info];
}

- (void)unobserve:(id)object info:(_FBKVOInfo *)info
- (void)unobserve:(id)object info:(nullable _FBKVOInfo *)info
{
if (nil == info) {
return;
Expand All @@ -285,7 +292,7 @@ - (void)unobserve:(id)object info:(_FBKVOInfo *)info
[object removeObserver:self forKeyPath:info->_keyPath context:(void *)info];
}

- (void)unobserve:(id)object infos:(NSSet *)infos
- (void)unobserve:(id)object infos:(nullable NSSet *)infos
{
if (0 == infos.count) {
return;
Expand All @@ -304,7 +311,10 @@ - (void)unobserve:(id)object infos:(NSSet *)infos
}
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
- (void)observeValueForKeyPath:(nullable NSString *)keyPath
ofObject:(nullable id)object
change:(nullable NSDictionary *)change
context:(nullable void *)context
{
NSAssert(context, @"missing context keyPath:%@ object:%@ change:%@", keyPath, object, change);

Expand Down Expand Up @@ -355,12 +365,12 @@ @implementation FBKVOController

#pragma mark Lifecycle -

+ (instancetype)controllerWithObserver:(id)observer
+ (instancetype)controllerWithObserver:(nullable id)observer
{
return [[self alloc] initWithObserver:observer];
}

- (instancetype)initWithObserver:(id)observer retainObserved:(BOOL)retainObserved
- (instancetype)initWithObserver:(nullable id)observer retainObserved:(BOOL)retainObserved
{
self = [super init];
if (nil != self) {
Expand All @@ -372,7 +382,7 @@ - (instancetype)initWithObserver:(id)observer retainObserved:(BOOL)retainObserve
return self;
}

- (instancetype)initWithObserver:(id)observer
- (instancetype)initWithObserver:(nullable id)observer
{
return [self initWithObserver:observer retainObserved:YES];
}
Expand Down Expand Up @@ -514,7 +524,7 @@ - (void)_unobserveAll

#pragma mark API -

- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block
{
NSAssert(0 != keyPath.length && NULL != block, @"missing required parameters observe:%@ keyPath:%@ block:%p", object, keyPath, block);
if (nil == object || 0 == keyPath.length || NULL == block) {
Expand All @@ -529,7 +539,7 @@ - (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObserv
}


- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options block:(FBKVONotificationBlock)block
{
NSAssert(0 != keyPaths.count && NULL != block, @"missing required parameters observe:%@ keyPath:%@ block:%p", object, keyPaths, block);
if (nil == object || 0 == keyPaths.count || NULL == block) {
Expand All @@ -542,7 +552,7 @@ - (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObser
}
}

- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options action:(SEL)action
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options action:(SEL)action
{
NSAssert(0 != keyPath.length && NULL != action, @"missing required parameters observe:%@ keyPath:%@ action:%@", object, keyPath, NSStringFromSelector(action));
NSAssert([_observer respondsToSelector:action], @"%@ does not respond to %@", _observer, NSStringFromSelector(action));
Expand All @@ -557,7 +567,7 @@ - (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObserv
[self _observe:object info:info];
}

- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options action:(SEL)action
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options action:(SEL)action
{
NSAssert(0 != keyPaths.count && NULL != action, @"missing required parameters observe:%@ keyPath:%@ action:%@", object, keyPaths, NSStringFromSelector(action));
NSAssert([_observer respondsToSelector:action], @"%@ does not respond to %@", _observer, NSStringFromSelector(action));
Expand All @@ -571,7 +581,7 @@ - (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObser
}
}

- (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context
- (void)observe:(nullable id)object keyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(nullable void *)context
{
NSAssert(0 != keyPath.length, @"missing required parameters observe:%@ keyPath:%@", object, keyPath);
if (nil == object || 0 == keyPath.length) {
Expand All @@ -585,7 +595,7 @@ - (void)observe:(id)object keyPath:(NSString *)keyPath options:(NSKeyValueObserv
[self _observe:object info:info];
}

- (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options context:(void *)context
- (void)observe:(nullable id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObservingOptions)options context:(nullable void *)context
{
NSAssert(0 != keyPaths.count, @"missing required parameters observe:%@ keyPath:%@", object, keyPaths);
if (nil == object || 0 == keyPaths.count) {
Expand All @@ -598,7 +608,7 @@ - (void)observe:(id)object keyPaths:(NSArray *)keyPaths options:(NSKeyValueObser
}
}

- (void)unobserve:(id)object keyPath:(NSString *)keyPath
- (void)unobserve:(nullable id)object keyPath:(NSString *)keyPath
{
// create representative info
_FBKVOInfo *info = [[_FBKVOInfo alloc] initWithController:self keyPath:keyPath];
Expand All @@ -607,7 +617,7 @@ - (void)unobserve:(id)object keyPath:(NSString *)keyPath
[self _unobserve:object info:info];
}

- (void)unobserve:(id)object
- (void)unobserve:(nullable id)object
{
if (nil == object) {
return;
Expand All @@ -622,3 +632,5 @@ - (void)unobserveAll
}

@end

NS_ASSUME_NONNULL_END
5 changes: 5 additions & 0 deletions FBKVOController/NSObject+FBKVOController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
*/

#import <Foundation/Foundation.h>

#import "FBKVOController.h"

NS_ASSUME_NONNULL_BEGIN

@interface NSObject (FBKVOController)

/**
Expand All @@ -21,3 +24,5 @@
@property (nonatomic, strong) FBKVOController *KVOControllerNonRetaining;

@end

NS_ASSUME_NONNULL_END
4 changes: 4 additions & 0 deletions FBKVOController/NSObject+FBKVOController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#pragma mark NSObject Category -

NS_ASSUME_NONNULL_BEGIN

static void *NSObjectKVOControllerKey = &NSObjectKVOControllerKey;
static void *NSObjectKVOControllerNonRetainingKey = &NSObjectKVOControllerNonRetainingKey;

Expand Down Expand Up @@ -60,3 +62,5 @@ - (void)setKVOControllerNonRetaining:(FBKVOController *)KVOControllerNonRetainin

@end


NS_ASSUME_NONNULL_END
10 changes: 5 additions & 5 deletions FBKVOControllerTests/FBKVOControllerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ - (void)testObserveKeyPathsOptionsBlockWhenKeyPathsIsNilRaises
{
FBKVOController *controller = [FBKVOController controllerWithObserver:nil];
FBKVONotificationBlock arbitraryBlock = ^(id observer, id object, NSDictionary *change) { /* noop */ };
XCTAssertThrows([controller observe:nil keyPaths:nil options:0 block:arbitraryBlock]);
XCTAssertThrows([controller observe:nil keyPaths:(id _Nonnull)nil options:0 block:arbitraryBlock]);
}

- (void)testObserveKeyPathsOptionsBlockWhenKeyPathsIsEmptyRaises
Expand All @@ -170,7 +170,7 @@ - (void)testObserveKeyPathsOptionsBlockWhenBlockIsNilRaises
{
FBKVOController *controller = [FBKVOController controllerWithObserver:nil];
NSArray *arbitraryKeyPaths = @[@"ante", @"bellum"];
XCTAssertThrows([controller observe:nil keyPaths:arbitraryKeyPaths options:0 block:nil]);
XCTAssertThrows([controller observe:nil keyPaths:arbitraryKeyPaths options:0 block:(id _Nonnull)nil]);
}

- (void)testObserveKeyPathsOptionsBlockWhenObjectIsNilDoesNothing
Expand Down Expand Up @@ -207,7 +207,7 @@ - (void)testObserveKeyPathsOptionsActionWhenKeyPathsIsNilRaises
{
FBKVOController *controller = [FBKVOController controllerWithObserver:nil];
SEL arbitrarySelector = @selector(cookies);
XCTAssertThrows([controller observe:nil keyPaths:nil options:0 action:arbitrarySelector]);
XCTAssertThrows([controller observe:nil keyPaths:(id _Nonnull)nil options:0 action:arbitrarySelector]);
}

- (void)testObserveKeyPathsOptionsActionWhenKeyPathsIsEmptyRaises
Expand All @@ -222,7 +222,7 @@ - (void)testObserveKeyPathsOptionsActionWhenActionIsNullRaises
{
FBKVOController *controller = [FBKVOController controllerWithObserver:nil];
NSArray *arbitraryKeyPaths = @[@"carpe", @"diem"];
XCTAssertThrows([controller observe:nil keyPaths:arbitraryKeyPaths options:0 action:NULL]);
XCTAssertThrows([controller observe:nil keyPaths:arbitraryKeyPaths options:0 action:(SEL _Nonnull)NULL]);
}

- (void)testObserveKeyPathsOptionsActionWhenObjectIsNilRaises
Expand Down Expand Up @@ -263,7 +263,7 @@ - (void)testObserveKeyPathsOptionsActionObservesEachOfTheKeyPaths
- (void)testObserveKeyPathsOptionsContextWhenKeyPathsIsNilRaises
{
FBKVOController *controller = [FBKVOController controllerWithObserver:nil];
XCTAssertThrows([controller observe:nil keyPaths:nil options:0 context:NULL]);
XCTAssertThrows([controller observe:nil keyPaths:(id _Nonnull)nil options:0 context:NULL]);
}

- (void)testObserveKeyPathsOptionsContextWhenKeyPathsIsEmptyRaises
Expand Down