danielsaidi / MockingKit

MockingKit is a Swift SDK that lets you easily mock protocols and classes in `Swift`.
MIT License
87 stars 7 forks source link

Add key path support for methods on `Mockable` #18

Closed Boerworz closed 1 year ago

Boerworz commented 1 year ago

This pull request fixes a small annoyance that I've come across when using MockingKit: having to duplicate the name of the mock whenever you invoke a method on it that accepts a ref. For example, consider the following snippet:

someMockableInstance.hasCalled(someMockableInstance.someMockedMethodRef)

Since the ref is in 99% of cases a property of the same mockable instance that we invoke the MockingKit method on, having to specify the mockable instance twice is redundant information. Instead, it would be useful if we could specify the ref using a key path:

someMockableInstance.hasCalled(\.someMockedMethodRef)

So that's what this pull request implements! :-) It adds key path support to the following Mockable methods:

I opted not to add key path support to the call(…) methods since those are typically called from inside the implementation that conforms to Mockable, and so they can reference the ref property directly without duplicating anything.

I should note that when making use of this support in the BookBeat code base, I did run into a handful of instances where using the key path variant of the method would crash the Swift compiler… So keeping the non-key path variants around is probably a good idea!

danielsaidi commented 1 year ago

Hi @Boerworz

Really happy to see this addition, thank you for sharing!

I'll merge this and prepare a new version 👍

Boerworz commented 1 year ago

Hooray! Thanks for building MockingKit! 🙏

danielsaidi commented 1 year ago

1.3 is now out, with the new keypath capabilities 👍

danielsaidi commented 1 year ago

@Boerworz

Hey, I just wanted to say that I love the keypath approach 🎉

I'm actually thinking that we should perhaps remove the non-keypath variants altogether, since they open up to bugs.

What do you think?

Boerworz commented 1 year ago

@Boerworz

Hey, I just wanted to say that I love the keypath approach 🎉

I'm actually thinking that we should perhaps remove the non-keypath variants altogether, since they open up to bugs.

What do you think?

@danielsaidi Great to hear! We're finding it very nice as well :-) Removing the non-keypath variants would be cool, BUT like I wrote in the description, I did run into some issues with the Swift compiler crashing when I tried to replace all our calls with the keypath-based ones. I haven't investigated exactly what causes the crashes—I just reverted the changes made to the problematic files—but I think it might have something to do with generics. Soooo, maybe keep the non-keypath variants around for a while?

danielsaidi commented 1 year ago

@Boerworz Sounds good, thanks for clarifying! 👍