swift-spyable icon indicating copy to clipboard operation
swift-spyable copied to clipboard

Support generic functions

Open knellr opened this issue 10 months ago • 4 comments

Is your feature request related to a problem? Please describe.

Currently the generated spy doesn't handle generic functions:

Screenshot 2023-11-01 at 12 53 36

Describe the solution you'd like

In this instance I would expect something like the following, though obviously conformances would change the types used.

class MyProtocolSpy: MyProtocol {
    var genericFuncThingCallsCount = 0
    var genericFuncThingCalled: Bool {
        return genericFuncThingCallsCount > 0
    }
    var genericFuncThingReceivedThing: Any?
    var genericFuncThingReceivedInvocations: [Any] = []
    var genericFuncThingClosure: ((Any) -> Void)?
    func genericFunc<Thing>(thing: Thing) {
        genericFuncThingCallsCount += 1
        genericFuncThingReceivedThing = (thing)
        genericFuncThingReceivedInvocations.append((thing))
        genericFuncThingClosure?(thing)
    }
}

Describe alternatives you've considered N/A

Additional context N/A

knellr avatar Nov 01 '23 14:11 knellr

I'm extremely bad at Swift Syntax, so I'm not sure I'm ready to take a stab at the solution myself, but I was thinking about how I'd solve this. Might not actually be helpful – just a thought.

I think the wrapped type of, e.g., genericFuncThingReceivedThing could be built up, starting from any Any†, then &-ing protocol conformance requirements from the generic type. That would eliminate the need for branching in the macro implementation (which, for me at least, is often a source of problems), leaving you with just iteration.

That would give you a somewhat unnatural but completely semantic var genericFuncThingReceivedThing: Optional<any Any & Equatable & CustomStringConvertible> in some cases, and just plain var genericFuncThingReceivedThing: Optional<any Any> in the minimal case.

† I think this is identical to just plain Any, but I don't think I've read that anywhere official.

arennow avatar Nov 05 '23 18:11 arennow

@arennow I think you're right about a solution having to revolve around the use of Any for storage. I'm not sure that we need to be concerned with protocol conformances though. It seems like we could do something like this, where we just swap out most generics with Any:

@Spyable
protocol MyProtocol {
  func useGenerics<A, B, C>(one: A, two: B, three: C, four: String) -> (A, (B, C), String)
}

// Which generates
class MyProtocolSpy: MyProtocol {
    var useGenericsOneTwoThreeFourCallsCount = 0
    var useGenericsOneTwoThreeFourCalled: Bool {
        return useGenericsOneTwoThreeFourCallsCount > 0
    }
    var useGenericsOneTwoThreeFourReceivedArguments: (one: Any, two: Any, three: Any, four: String)?
    var useGenericsOneTwoThreeFourReceivedInvocations: [(one: Any, two: Any, three: Any, four: String)] = []
    var useGenericsOneTwoThreeFourReturnValue: (Any, (Any, Any), String)!
    var useGenericsOneTwoThreeFourClosure: ((Any, Any, Any, String) -> (Any, (Any, Any), String))?
      func useGenerics<A, B, C>(one: A, two: B, three: C, four: String) -> (A, (B, C), String) {
        useGenericsOneTwoThreeFourCallsCount += 1
        useGenericsOneTwoThreeFourReceivedArguments = (one, two, three, four)
        useGenericsOneTwoThreeFourReceivedInvocations.append((one, two, three, four))
        if useGenericsOneTwoThreeFourClosure != nil {
            return useGenericsOneTwoThreeFourClosure!(one, two, three, four) as! (A, (B, C), String)
        } else {
            return useGenericsOneTwoThreeFourReturnValue as! (A, (B, C), String)
        }
    }
}

This compiles and would be usable, but it would require that users of the spy do type casting if they want to use these Any types in a meaningful way beyond checking their existence. I'm not sure we can avoid that though.

One way to improve usability could be to introduce nice fatalError messaging instead of just force casts - something like:

guard let returnValue = useGenericsOneTwoThreeFourReturnValue as? (A, (B, C), String) else {
	fatalError("\(useGenericsOneTwoThreeFourReturnValue) was of type \(type(of: useGenericsOneTwoThreeFourReturnValue)). Expected: (A, (B, C), String)")
}
return returnValue

I'm currently exploring a PR that'll go with this approach.

dafurman avatar Nov 25 '23 07:11 dafurman

@dafurman In the case of unconstrained generics, like in your example, the only static type we could use is Any, but in other (more common, I'd say) cases, you can use the generic's constraint to form the existential type, which is tremendously useful.

Consider the following situation:

protocol User {
	var name: String { get }
	var friendCount: Int { get set }
}

@Spyable
protocol MyFakeThing {
	func process<U: User>(user: U)
}

You'd want to be able to do something like

var spy: MyFakeThingSpy = // whatever
// Do something with it
let sumOfFriends = spy.processUserReceivedInvocations.map(\.friendCount).reduce(0, +)

But right now you can't because the generated code makes reference to U[^some_types_too] without ever defining it.

Currently generated invalid spy code
class MyFakeThingSpy: MyFakeThing {
	var processUserCallsCount = 0
	var processUserCalled: Bool {
		self.processUserCallsCount > 0
	}

	var processUserReceivedUser: U?
	var processUserReceivedInvocations: [U] = []
	var processUserClosure: ((U) -> Void)?
	func process<U: User>(user: U) {
		self.processUserCallsCount += 1
		self.processUserReceivedUser = (user)
		self.processUserReceivedInvocations.append((user))
		self.processUserClosure?(user)
	}
}

If we instead replaced[^or_a_typealias] U with any User[^composite_existential_types], we'd end up with exactly what we want, without requiring users of the spy type to do any casting in order to access members defined in the protocol (though they might still want to cast in order to get access to the underlying concrete types).

Hypothetical existential-using spy code
class MyFakeThingSpy: MyFakeThing {
	var processUserCallsCount = 0
	var processUserCalled: Bool {
		self.processUserCallsCount > 0
	}

	var processUserReceivedUser: (any User)?
	var processUserReceivedInvocations: [any User] = []
	var processUserClosure: ((any User) -> Void)?
	func process<U: User>(user: U) { // This could also be `any User`, but it doesn't have to be
		self.processUserCallsCount += 1
		self.processUserReceivedUser = (user)
		self.processUserReceivedInvocations.append((user))
		self.processUserClosure?(user)
	}
}

[^some_types_too]: A similar thing happens if we write the generic as some User. Ideally we would support both, but an MVP that only supported "traditional" generics (<U: User>-style) would be fine IMO

[^composite_existential_types]: We would of course want to make composite existential types if the generic were constrained to multiple protocols. That's what I was getting at with my suggestion of any Any, combined with composing "real" protocols. We would do just as well to type processUserReceivedUser as (any Any & User)?, and that would probably be simpler to implement than only emitting a composite type (ones with & in them) when there are multiple protocol conformance constraints on the generic type.

[^or_a_typealias]: We could also go the perhaps simpler route of emitting typealias U = any User. It might make the code less "natural" looking, but it should work.

arennow avatar Nov 25 '23 12:11 arennow

@arennow Thanks for the clarification! I see what you mean now. Yeah, being able to use generic constraints could help eliminate some situations where devs need to cast from Any. I definitely see the benefit of that, so I'll see if I can work your suggestions into my PR (though possibly as an iteration upon my "just swap generics with Any" approach depending on how large this change gets)

One thing about

  1. We could also go the perhaps simpler route of emitting typealias U = any User. It might make the code less "natural" looking, but it should work.

I like the idea, but I think we'd run into trouble if generic identifier names are reused, like the commonly used "T" for generics:

protocol MyProtocol {
  func process<T: User>(user: T)
  func process<T: Customer>(customer: T)
}

In this example, we'd want to typealias T = any User but also typealias T = any Customer Of course, devs could work around this by renaming the generic identifier, but it could add a point of complication that'd need to be documented.

dafurman avatar Nov 25 '23 17:11 dafurman