Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support generic functions #52

Open
knellr opened this issue Nov 1, 2023 · 4 comments
Open

Support generic functions #52

knellr opened this issue Nov 1, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@knellr
Copy link

knellr commented Nov 1, 2023

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

@arennow
Copy link
Contributor

arennow commented Nov 5, 2023

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.

@Matejkob Matejkob added the enhancement New feature or request label Nov 9, 2023
@dafurman
Copy link
Contributor

@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.

@arennow
Copy link
Contributor

arennow commented Nov 25, 2023

@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 U1 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 replaced2 U with any User3, 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)
	}
}

Footnotes

  1. 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

  2. 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.

  3. 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.

@dafurman
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants