erikdoe / ocmock

Mock objects for Objective-C
http://ocmock.org
Apache License 2.0
2.16k stars 607 forks source link

OCPartialMockObject static analyzer warning #543

Open LowAmmo opened 1 week ago

LowAmmo commented 1 week ago

In Xcode if running "Analyze" on a target that includes OCMock, getting a warning about OCPartialMockObject#setupForwarderForSelector. On the last line, the call to class_addMethod about the possibility of passing a null parameter as the 3rd parameter. https://github.com/erikdoe/ocmock/blob/7f6e9e9b6c9ffa19a9040860da9f7a1f202b9f4d/Source/OCMock/OCPartialMockObject.m#L217

 - (void)setupForwarderForSelector:(SEL)sel
 {
     SEL aliasSelector = OCMAliasForOriginalSelector(sel);
     if(class_getInstanceMethod(object_getClass(realObject), aliasSelector) != NULL)
         return;

     Method originalMethod = class_getInstanceMethod(mockedClass, sel);
     /* Might be NULL if the selector is forwarded to another class */
     IMP originalIMP = (originalMethod != NULL) ? method_getImplementation(originalMethod) : NULL;
     const char *types = (originalMethod != NULL) ? method_getTypeEncoding(originalMethod) : NULL;
     // TODO: check the fallback implementation is actually sufficient
     if(types == NULL)
         types = ([[mockedClass instanceMethodSignatureForSelector:sel] fullObjCTypes]);

     Class subclass = object_getClass([self realObject]);
     IMP forwarderIMP = [mockedClass instanceMethodForwarderForSelector:sel];
     class_replaceMethod(subclass, sel, forwarderIMP, types);
     class_addMethod(subclass, aliasSelector, originalIMP, types);  // <-- Possible for originalIMP to be nil
 }

I'm not a pro at everything OCMock does, but I'm guessing we probably at least don't want to call class_addMethod if originalIMP is NULL.

Unsure if it would make sense to not bother calling class_replaceMethod also...?

This isn't currently causing us any runtime issues - I just like to have 0 warnings in our code if we can.

-Thanks!

LowAmmo commented 1 day ago

Created PR #544 to address this