Skip to content

Commit

Permalink
Make sure we handle subclasses properly when we use instancetype. (#2…
Browse files Browse the repository at this point in the history
…5169)

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

Fixes #23384
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 9, 2023
1 parent 14cb6a9 commit 3625177
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 3625177

Please sign in to comment.