Skip to content

Commit

Permalink
Make sure we handle subclasses properly when we use instancetype. (pr…
Browse files Browse the repository at this point in the history
…oject-chip#25169)

We should not be claiming to return instancetype if we actually always return an
instance of our class.

Fixes project-chip#23384
  • Loading branch information
bzbarsky-apple authored and David Lechner committed Mar 22, 2023
1 parent db177f2 commit fffcdb8
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 33 deletions.
26 changes: 14 additions & 12 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
* fabric, but attempts to actually use the MTRBaseDevice will fail
* (asynchronously) in that case.
*/
+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID
controller:(MTRDeviceController *)controller API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+ (MTRBaseDevice *)deviceWithNodeID:(NSNumber *)nodeID
controller:(MTRDeviceController *)controller
API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));

/**
* The transport used by the current session with this device, or
Expand Down Expand Up @@ -377,7 +378,7 @@ API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
@property (nonatomic, readonly, copy) NSNumber * endpoint;
@property (nonatomic, readonly, copy) NSNumber * cluster;

+ (instancetype)clusterPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;
+ (MTRClusterPath *)clusterPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
Expand All @@ -390,9 +391,10 @@ API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
@interface MTRAttributePath : MTRClusterPath
@property (nonatomic, readonly, copy) NSNumber * attribute;

+ (instancetype)attributePathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
attributeID:(NSNumber *)attributeID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+ (MTRAttributePath *)attributePathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
attributeID:(NSNumber *)attributeID
API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
@end

/**
Expand All @@ -403,9 +405,9 @@ API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
@interface MTREventPath : MTRClusterPath
@property (nonatomic, readonly, copy) NSNumber * event;

+ (instancetype)eventPathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
eventID:(NSNumber *)eventID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+ (MTREventPath *)eventPathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
eventID:(NSNumber *)eventID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
@end

/**
Expand All @@ -415,9 +417,9 @@ API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
@interface MTRCommandPath : MTRClusterPath
@property (nonatomic, readonly, copy) NSNumber * command;

+ (instancetype)commandPathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+ (MTRCommandPath *)commandPathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
@end

@interface MTRAttributeReport : NSObject
Expand Down
14 changes: 7 additions & 7 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
return self;
}

+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
+ (MTRBaseDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
{
// Indirect through the controller to give it a chance to create an
// MTRBaseDeviceOverXPC instead of just an MTRBaseDevice.
Expand Down Expand Up @@ -1870,7 +1870,7 @@ - (NSString *)description
(uint32_t) _cluster.unsignedLongValue];
}

+ (instancetype)clusterPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID
+ (MTRClusterPath *)clusterPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID
{
ConcreteClusterPath path(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
static_cast<chip::ClusterId>([clusterID unsignedLongValue]));
Expand Down Expand Up @@ -1919,9 +1919,9 @@ - (NSString *)description
(uint32_t) _attribute.unsignedLongValue];
}

+ (instancetype)attributePathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
attributeID:(NSNumber *)attributeID
+ (MTRAttributePath *)attributePathWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
attributeID:(NSNumber *)attributeID
{
ConcreteDataAttributePath path(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
static_cast<chip::ClusterId>([clusterID unsignedLongValue]),
Expand Down Expand Up @@ -1979,7 +1979,7 @@ - (NSString *)description
(uint32_t) self.cluster.unsignedLongValue, (uint32_t) _event.unsignedLongValue];
}

+ (instancetype)eventPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID eventID:(NSNumber *)eventID
+ (MTREventPath *)eventPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID eventID:(NSNumber *)eventID
{
ConcreteEventPath path(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
static_cast<chip::ClusterId>([clusterID unsignedLongValue]), static_cast<chip::EventId>([eventID unsignedLongValue]));
Expand Down Expand Up @@ -2009,7 +2009,7 @@ - (instancetype)initWithPath:(const ConcreteCommandPath &)path
return self;
}

+ (instancetype)commandPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID commandID:(NSNumber *)commandID
+ (MTRCommandPath *)commandPathWithEndpointID:(NSNumber *)endpointID clusterID:(NSNumber *)clusterID commandID:(NSNumber *)commandID
{
ConcreteCommandPath path(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
static_cast<chip::ClusterId>([clusterID unsignedLongValue]), static_cast<chip::CommandId>([commandID unsignedLongValue]));
Expand Down
8 changes: 4 additions & 4 deletions src/darwin/Framework/CHIP/MTRDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ typedef NS_ENUM(NSUInteger, MTRDeviceState) {
* retrieved when performing actions using a combination of MTRBaseDevice
* and MTRAsyncCallbackQueue.
*/
+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID
controller:(MTRDeviceController *)controller API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+ (MTRDevice *)deviceWithNodeID:(NSNumber *)nodeID
controller:(MTRDeviceController *)controller API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));

/**
* The current state of the device.
Expand Down Expand Up @@ -238,8 +238,8 @@ extern NSString * const MTREventTimestampDateKey MTR_NEWLY_AVAILABLE;
/**
* Deprecated MTRDevice APIs.
*/
+ (instancetype)deviceWithNodeID:(uint64_t)nodeID
deviceController:(MTRDeviceController *)deviceController
+ (MTRDevice *)deviceWithNodeID:(uint64_t)nodeID
deviceController:(MTRDeviceController *)deviceController
API_DEPRECATED(
"Please use deviceWithNodeID:controller:", ios(16.1, 16.4), macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4));

Expand Down
8 changes: 4 additions & 4 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ - (instancetype)initWithFirst:(id)first second:(id)second
}
+ (instancetype)pairWithFirst:(id)first second:(id)second
{
return [[MTRPair alloc] initWithFirst:first second:second];
return [[self alloc] initWithFirst:first second:second];
}
@end

Expand All @@ -91,7 +91,7 @@ - (instancetype)initWithObject:(id)object
}
+ (instancetype)weakReferenceWithObject:(id)object
{
return [[MTRWeakReference alloc] initWithObject:object];
return [[self alloc] initWithObject:object];
}
- (id)strongObject
{
Expand Down Expand Up @@ -221,7 +221,7 @@ - (NSString *)description
return [NSString stringWithFormat:@"<MTRDevice: %p>[fabric: %u, nodeID: %@]", self, _fabricIndex, _nodeID];
}

+ (instancetype)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
+ (MTRDevice *)deviceWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
{
return [controller deviceForNodeID:nodeID];
}
Expand Down Expand Up @@ -982,7 +982,7 @@ - (MTRBaseDevice *)newBaseDevice

@implementation MTRDevice (Deprecated)

+ (instancetype)deviceWithNodeID:(uint64_t)nodeID deviceController:(MTRDeviceController *)deviceController
+ (MTRDevice *)deviceWithNodeID:(uint64_t)nodeID deviceController:(MTRDeviceController *)deviceController
{
return [self deviceWithNodeID:@(nodeID) controller:deviceController];
}
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
* Return the single MTRDeviceControllerFactory we support existing. It starts off
* in a "not started" state.
*/
+ (instancetype)sharedInstance;
+ (MTRDeviceControllerFactory *)sharedInstance;

/**
* If true, the factory is in a state where it can create controllers:
Expand Down Expand Up @@ -167,7 +167,7 @@ API_DEPRECATED(
API_DEPRECATED("Please use MTRDeviceControllerFactory", ios(16.1, 16.4), macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4))
@interface MTRControllerFactory : NSObject
@property (readonly, nonatomic) BOOL isRunning;
+ (instancetype)sharedInstance;
+ (MTRControllerFactory *)sharedInstance;
- (BOOL)startup:(MTRControllerFactoryParams *)startupParams;
- (void)shutdown;
- (MTRDeviceController * _Nullable)startControllerOnExistingFabric:(MTRDeviceControllerStartupParams *)startupParams;
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ + (void)initialize
MTRFrameworkInit();
}

+ (instancetype)sharedInstance
+ (MTRDeviceControllerFactory *)sharedInstance
{
static MTRDeviceControllerFactory * factory = nil;
static dispatch_once_t onceToken;
Expand Down Expand Up @@ -815,7 +815,7 @@ - (BOOL)isRunning
return [[MTRDeviceControllerFactory sharedInstance] isRunning];
}

+ (instancetype)sharedInstance
+ (MTRControllerFactory *)sharedInstance
{
// We could try to delegate to MTRDeviceControllerFactory's sharedInstance
// here, but then we would have to add the backwards-compar selectors to
Expand Down
3 changes: 2 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerXPCConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ typedef void (^MTRGetProxyHandleHandler)(dispatch_queue_t queue, MTRDeviceContro
/**
* This method is just for test purpsoe.
*/
+ (instancetype)connectionWithWorkQueue:(dispatch_queue_t)workQueue connectBlock:(MTRXPCConnectBlock)connectBlock;
+ (MTRDeviceControllerXPCConnection *)connectionWithWorkQueue:(dispatch_queue_t)workQueue
connectBlock:(MTRXPCConnectBlock)connectBlock;

- (void)getProxyHandleWithCompletion:(MTRGetProxyHandleHandler)completion;
- (void)registerReportHandlerWithController:(id<NSCopying>)controller
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ - (instancetype)initWithWorkQueue:(dispatch_queue_t)workQueue connectBlock:(NSXP
}

// This class method is for unit testing
+ (instancetype)connectionWithWorkQueue:(dispatch_queue_t)workQueue connectBlock:(NSXPCConnection * (^)(void) )connectBlock
+ (MTRDeviceControllerXPCConnection *)connectionWithWorkQueue:(dispatch_queue_t)workQueue
connectBlock:(NSXPCConnection * (^)(void) )connectBlock
{
return [[MTRDeviceControllerXPCConnection alloc] initWithWorkQueue:workQueue connectBlock:connectBlock];
}
Expand Down

0 comments on commit fffcdb8

Please sign in to comment.