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

Add support for existential type (any) to AutoMockable.stencil #1169

Conversation

paul1893
Copy link
Contributor

Context

With Swift 5.6, any keyword have been introduced.
This allows us to define existential types in Swift by prefixing a type with the any keyword. An existential means “any type, but conforming to protocol X.”
As reported in #1149 Sourcery does not handle it in the current AutoMockable.stencil

In this PR

I've updated the AutoMockable.stencil to support this new keyword.

@art-divin art-divin self-requested a review July 3, 2023 18:30
@art-divin
Copy link
Collaborator

Hello @paul1893 ,

thank you a lot for your contribution!

I would like to ask you to add unit tests to cover failing and successful test, to see if your fix covers all of the new syntax features at once 👍

If needed, I can assist with that, just need some examples what needs to be verified, or a set of data you have applied the template against.

Thank you 🙏

@paul1893
Copy link
Contributor Author

paul1893 commented Jul 4, 2023

Hello 👋
Yes sure.
Here's a class that should cover most of use cases (at least the ones we have in my company. Project ~1M loc).

protocol StubProtocol {}
// sourcery: AutoMockable
protocol AnyProtocol {
    var a: any StubProtocol { get }
    var b: (any StubProtocol)? { get }
    var c: (any StubProtocol)! { get }
    var d: (((any StubProtocol)?) -> Void) { get }
    var e: [(any StubProtocol)?] { get }
    func f(_ x: (any StubProtocol)?, y: (any StubProtocol)!, z: any StubProtocol)
}

If you think about others, don't hesitate.

Have you some leads or advice to give me before I start to implement tests ? When looking at the history of AutoMockable.stencil I intended to be inspired by this.

@paul1893
Copy link
Contributor Author

paul1893 commented Jul 17, 2023

Hello 👋

I've spent some time tonight to enrich the sample. Here it is:

protocol StubProtocol {}
protocol StubWithAnyNameProtocol {}

// sourcery: AutoMockable
protocol AnyProtocol {
    var a: any StubProtocol { get }
    var b: (any StubProtocol)? { get }
    var c: (any StubProtocol)! { get }
    var d: (((any StubProtocol)?) -> Void) { get }
    var e: [(any StubProtocol)?] { get }
    func f(_ x: (any StubProtocol)?, y: (any StubProtocol)!, z: any StubProtocol)
    var g: any StubProtocol { get }
    var h: (any StubProtocol)? { get }
    var i: (any StubProtocol)! { get }
    func j(x: (any StubProtocol)?, y: (any StubProtocol)!, z: any StubProtocol) async -> String
    func k(x: ((any StubProtocol)?) -> Void, y: (any StubProtocol) -> Void)
    func l(x: (((any StubProtocol)?) -> Void), y: ((any StubProtocol) -> Void))
    var anyConfusingPropertyName: any StubProtocol { get }
    func m(anyConfusingArgumentName: any StubProtocol)
    func n(x: @escaping ((any StubProtocol)?) -> Void)
    var o: any StubWithAnyNameProtocol { get }
    func p(_ x: (any StubWithAnyNameProtocol)?)
}

The stencil have basically 4 new macros: existentialVariableTypeName, existentialClosureVariableTypeName, existentialParameterTypeName, methodName that helps me cover the any keyword by introducing parenthesis when required by the Swift language.
I hope I've covered all the cases. This is what I've listed so far: variable, optional variables, closure variable, array of optionals, array of closures, func parameters, func optional parameters, async func, closure parameters, optional closure parameters with and without external parameter name.

I wrote a test to verify everything ok. But I'm still a little bit lost by the project hierarchy, as previously said I tried to add my test into Templates folder where async tests where added almost one year ago but could not find any Podfile that seems to be required to make the project works. After some time with no success I decided to re-create it locally in order to move forward. I was then able to launch test target into Templates.xcworkspace but before that some files seem to need to be touched (./Sourcery/Templates/Tests/Context/AutoMockable.swift) even on master in order to make the project compile.
For example

protocol StaticMethodProtocol:AutoMockable {
    static func staticFunction(String) -> String
}

needs to be changed

protocol StaticMethodProtocol:AutoMockable {
    static func staticFunction(_: String) -> String
}

So all of this let me think I'm on the wrong track for the test part. Using a side sample app I know my produced output looks like this with the sample above 👆:

class AnyProtocolMock: AnyProtocol {


    var a: any StubProtocol {
        get { return underlyingA }
        set(value) { underlyingA = value }
    }
    var underlyingA: (any StubProtocol)!
    var b: (any StubProtocol)?
    var c: (any StubProtocol)!
    var d: (((any StubProtocol)?) -> Void) {
        get { return underlyingD }
        set(value) { underlyingD = value }
    }
    var underlyingD: ((((any StubProtocol)?) -> Void))!
    var e: [(any StubProtocol)?] = []
    var g: any StubProtocol {
        get { return underlyingG }
        set(value) { underlyingG = value }
    }
    var underlyingG: (any StubProtocol)!
    var h: (any StubProtocol)?
    var i: (any StubProtocol)!
    var anyConfusingPropertyName: any StubProtocol {
        get { return underlyingAnyConfusingPropertyName }
        set(value) { underlyingAnyConfusingPropertyName = value }
    }
    var underlyingAnyConfusingPropertyName: (any StubProtocol)!
    var o: any StubWithAnyNameProtocol {
        get { return underlyingO }
        set(value) { underlyingO = value }
    }
    var underlyingO: (any StubWithAnyNameProtocol)!


    //MARK: - f

    var fyzCallsCount = 0
    var fyzCalled: Bool {
        return fyzCallsCount > 0
    }
    var fyzReceivedArguments: (x: (any StubProtocol)?, y: (any StubProtocol)?, z: any StubProtocol)?
    var fyzReceivedInvocations: [(x: (any StubProtocol)?, y: (any StubProtocol)?, z: any StubProtocol)] = []
    var fyzClosure: (((any StubProtocol)?, (any StubProtocol)?, any StubProtocol) -> Void)?

    func f(_ x: (any StubProtocol)?, y: (any StubProtocol)!, z: any StubProtocol) {
        fyzCallsCount += 1
        fyzReceivedArguments = (x: x, y: y, z: z)
        fyzReceivedInvocations.append((x: x, y: y, z: z))
        fyzClosure?(x, y, z)
    }

    //MARK: - j

    var jxyzCallsCount = 0
    var jxyzCalled: Bool {
        return jxyzCallsCount > 0
    }
    var jxyzReceivedArguments: (x: (any StubProtocol)?, y: (any StubProtocol)?, z: any StubProtocol)?
    var jxyzReceivedInvocations: [(x: (any StubProtocol)?, y: (any StubProtocol)?, z: any StubProtocol)] = []
    var jxyzReturnValue: String!
    var jxyzClosure: (((any StubProtocol)?, (any StubProtocol)?, any StubProtocol) async -> String)?

    func j(x: (any StubProtocol)?, y: (any StubProtocol)!, z: any StubProtocol) async -> String {
        jxyzCallsCount += 1
        jxyzReceivedArguments = (x: x, y: y, z: z)
        jxyzReceivedInvocations.append((x: x, y: y, z: z))
        if let jxyzClosure = jxyzClosure {
            return await jxyzClosure(x, y, z)
        } else {
            return jxyzReturnValue
        }
    }

    //MARK: - k

    var kxyCallsCount = 0
    var kxyCalled: Bool {
        return kxyCallsCount > 0
    }
    var kxyClosure: ((((any StubProtocol)?) -> Void, (any StubProtocol) -> Void) -> Void)?

    func k(x: ((any StubProtocol)?) -> Void, y: (any StubProtocol) -> Void) {
        kxyCallsCount += 1
        kxyClosure?(x, y)
    }

    //MARK: - l

    var lxyCallsCount = 0
    var lxyCalled: Bool {
        return lxyCallsCount > 0
    }
    var lxyClosure: ((((any StubProtocol)?) -> Void, (any StubProtocol) -> Void) -> Void)?

    func l(x: ((any StubProtocol)?) -> Void, y: (any StubProtocol) -> Void) {
        lxyCallsCount += 1
        lxyClosure?(x, y)
    }

    //MARK: - m

    var mAnyConfusingArgumentNameCallsCount = 0
    var mAnyConfusingArgumentNameCalled: Bool {
        return mAnyConfusingArgumentNameCallsCount > 0
    }
    var mAnyConfusingArgumentNameReceivedAnyConfusingArgumentName: (any StubProtocol)?
    var mAnyConfusingArgumentNameReceivedInvocations: [(any StubProtocol)] = []
    var mAnyConfusingArgumentNameClosure: ((any StubProtocol) -> Void)?

    func m(anyConfusingArgumentName: any StubProtocol) {
        mAnyConfusingArgumentNameCallsCount += 1
        mAnyConfusingArgumentNameReceivedAnyConfusingArgumentName = anyConfusingArgumentName
        mAnyConfusingArgumentNameReceivedInvocations.append(anyConfusingArgumentName)
        mAnyConfusingArgumentNameClosure?(anyConfusingArgumentName)
    }

    //MARK: - n

    var nxCallsCount = 0
    var nxCalled: Bool {
        return nxCallsCount > 0
    }
    var nxReceivedX: ((((any StubProtocol)?) -> Void))?
    var nxReceivedInvocations: [((((any StubProtocol)?) -> Void))] = []
    var nxClosure: ((@escaping ((any StubProtocol)?) -> Void) -> Void)?

    func n(x: @escaping ((any StubProtocol)?) -> Void) {
        nxCallsCount += 1
        nxReceivedX = x
        nxReceivedInvocations.append(x)
        nxClosure?(x)
    }

    //MARK: - p

    var pCallsCount = 0
    var pCalled: Bool {
        return pCallsCount > 0
    }
    var pReceivedX: (any StubWithAnyNameProtocol)?
    var pReceivedInvocations: [(any StubWithAnyNameProtocol)?] = []
    var pClosure: (((any StubWithAnyNameProtocol)?) -> Void)?

    func p(_ x: (any StubWithAnyNameProtocol)?) {
        pCallsCount += 1
        pReceivedX = x
        pReceivedInvocations.append(x)
        pClosure?(x)
    }

}

which seems correct.
If in the coming days you have some advice to help me with the project hierarchy and where I should put my tests would be superhelpful for me.
Have a nice evening 🙂

@art-divin
Copy link
Collaborator

🙌🏻 Hello @paul1893 ,

I have checked out your branch and checked all the tests we have, and seems like the tests you have put in place are the right way to verify the new behaviour. @krzysztofzablocki could you please confirm? I'd like this to be merged sooner than later, if possible, since it is a missing functionality,

thank you 🙏🏻

Copy link
Owner

@krzysztofzablocki krzysztofzablocki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me!

@krzysztofzablocki krzysztofzablocki merged commit 6db4d41 into krzysztofzablocki:master Jul 27, 2023
@art-divin
Copy link
Collaborator

Thanks @krzysztofzablocki 👍🏻

Thanks @paul1893 as well 👍🏻

🕵🏻 Seems like we might have introduced a failing test with this PR (but I am unsure). I'll take a look 👀

@art-divin
Copy link
Collaborator

👋🏻 Hey @paul1893 ,

could you please extrapolate why this change was made:

-public protocol AccessLevelProtocol
+protocol AccessLevelProtocol

Because that is one of the reasons UTs are failing after this merge. But that's not the only issue - file AutoMockable.expected contains different contents than those of Context/AutoMockable.swift file produces. There's different sorting in the generated file (under /Generated/AutoMockable.generated.swift).

There's also additional redundancy I could find:

-public var underlyingName: String!
+public var underlyingName: (String)!

Thank you 🙏🏻

@paul1893
Copy link
Contributor Author

Hello 👋

  1. ) No particular explanation for this:
-public protocol AccessLevelProtocol
+protocol AccessLevelProtocol

as I tried to explain Templates.xcworkspace was not compiling on my local machine but mostly due to misconfiguration on my side I guess. So this line could totally be reverted if needed as well as

-protocol StaticMethodProtocol:AutoMockable {
-    static func staticFunction(String) -> String
-}
+protocol StaticMethodProtocol: AutoMockable {
+    static func staticFunction(_: String) -> String
+}

These changes were introduced only because the compiler complains on my side.

2.)

file AutoMockable.expected contains different contents than those of Context/AutoMockable.swift file produces. There's different sorting in the generated file (under /Generated/AutoMockable.generated.swift)

I noticed that too during my deep dive but seems to be the case before the #1169 merge right ?
We could also note that AccessLevelProtocolMock seems missing from AutoMockable.generated.swift even if it is specified in AutoMockable.expected

There's also additional redundancy I could find:
-public var underlyingName: String!
+public var underlyingName: (String)!

Where did you find those ? I couldn't find public var underlyingName: (String)! in AutoMockable.generated.swift or AutoMockable.expected

Thanks

art-divin added a commit to art-divin/Sourcery that referenced this pull request Jul 29, 2023
art-divin added a commit that referenced this pull request Jul 29, 2023
* fixed failing TemplatesTests after #1169

* added missing padding
@art-divin
Copy link
Collaborator

Hey @paul1893 ,

I found them in master branch, under /Templates/Tests/*.

My MR - #1187 fixed the issue, I have just merged it. Now tests are green alright.

Thank you for collaboration 🤝 (back to bug fixing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants