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

Generated AutoMockable compilation issue due to variadic parameters #1265

Closed
rokridi opened this issue Jan 24, 2024 · 11 comments · Fixed by #1305
Closed

Generated AutoMockable compilation issue due to variadic parameters #1265

rokridi opened this issue Jan 24, 2024 · 11 comments · Fixed by #1305
Assignees
Labels
Milestone

Comments

@rokridi
Copy link
Contributor

rokridi commented Jan 24, 2024

Hello 👋

The current AutoMockable.stencil (version 2.1.7) does not properly handle methods with variadic parameters, which causes a compilation failure.

Example:

Given the following protocol:

// sourcery: AutoMockable
protocol ExampleVararg {
  func toto(args: any StubWithSomeNameProtocol...)
} 

The expected mock should like this:

var totoArgsAnyStubWithSomeNameProtocolVoidCallsCount = 0
var totoArgsAnyStubWithSomeNameProtocolVoidCalled: Bool {
  return totoArgsAnyStubWithSomeNameProtocolVoidCallsCount > 0
}
var totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs: ([(any StubWithSomeNameProtocol)])?
var totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations: [[(any StubWithSomeNameProtocol)]] = []
var totoArgsAnyStubWithSomeNameProtocolVoidClosure: (([(any StubWithSomeNameProtocol)]) -> Void)?

func toto(args: any StubWithSomeNameProtocol...) {
  totoArgsAnyStubWithSomeNameProtocolVoidCallsCount += 1
  totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs = args
  totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations.append(args)
  totoArgsAnyStubWithSomeNameProtocolVoidClosure?(args)
}

However it looks like this:

  var totoArgsAnyStubWithSomeNameProtocolVoidCallsCount = 0
  var totoArgsAnyStubWithSomeNameProtocolVoidCalled: Bool {
    return totoArgsAnyStubWithSomeNameProtocolVoidCallsCount > 0
  }
  var totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs: (any StubWithSomeNameProtocol...)?
  var totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations: [(any StubWithSomeNameProtocol...)] = []
  var totoArgsAnyStubWithSomeNameProtocolVoidClosure: ((any StubWithSomeNameProtocol...) -> Void)?

  func toto(args: any StubWithSomeNameProtocol...) {
    totoArgsAnyStubWithSomeNameProtocolVoidCallsCount += 1
    totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs = args
    totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations.append(args)
    totoArgsAnyStubWithSomeNameProtocolVoidClosure?(args)
  }

Note that for each method variadic parameter the corresponding received argument should be of type [ArgumentType] and not ArgumentType...

@art-divin art-divin added the bug label Jan 24, 2024
@rokridi
Copy link
Contributor Author

rokridi commented Jan 24, 2024

Hello @art-divin

@pocketal and I are working on a fix for this issue.
We wonder if it is possible to add isVariadic information to closure parameter. We need that information to be able to handle variadic closure parameters.

@art-divin
Copy link
Collaborator

Hello @rokridi , @pocketal , this is very nice of you to invest your time into improving Sourcery 🤝

We wonder if it is possible to add isVariadic information to closure parameter

Let's see!

@art-divin
Copy link
Collaborator

art-divin commented Jan 24, 2024

a1343ff introduces isVariadic member to ClosureParameter. I am adding UTs for it; in the meanwhile, you could take the leverage using that implementation already 👍🏻

Edit: since Swift itself does not support variadic arguments for closures, it is unreasonable to add such implementation detail.

@art-divin
Copy link
Collaborator

art-divin commented Jan 24, 2024

@rokridi I would like to mention that acceptable implementation would be:

var totoArgsAnyStubWithSomeNameProtocolVoidCallsCount = 0
var totoArgsAnyStubWithSomeNameProtocolVoidCalled: Bool {
  return totoArgsAnyStubWithSomeNameProtocolVoidCallsCount > 0
}
var totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs: ([(any StubWithSomeNameProtocol)])?
var totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations: [[(any StubWithSomeNameProtocol)]] = []
var totoArgsAnyStubWithSomeNameProtocolVoidClosure: (([(any StubWithSomeNameProtocol)]) -> Void)?

func toto(args: any StubWithSomeNameProtocol...) {
  totoArgsAnyStubWithSomeNameProtocolVoidCallsCount += 1
  totoArgsAnyStubWithSomeNameProtocolVoidReceivedArgs = args
  totoArgsAnyStubWithSomeNameProtocolVoidReceivedInvocations.append(args)
  totoArgsAnyStubWithSomeNameProtocolVoidClosure?(args)
}

i.e. keeping any and parentheses around the type, as opposed to the expected result in the OP.

@art-divin
Copy link
Collaborator

I am not 100% sure you actually need ClosureParameter.isVariadic, rather, try simply to modify AutoMockable.stencil template like here:

{% macro existentialClosureVariableTypeName typeName isVariadic -%}
    {%- if typeName|contains:"any " and typeName|contains:"!" -%}
        {{ typeName | replace:"any","(any" | replace:"!",")?" }}
    {%- elif typeName|contains:"any " and typeName.isClosure and typeName|contains:"?" -%}
        {{ typeName | replace:"any","(any" | replace:"?",")?" }}
    {%- elif typeName|contains:"any " and typeName|contains:"?" -%}
        {{ typeName | replace:"any","(any" | replace:"?",")?" }}
    {%- elif typeName|contains:"some " and typeName|contains:"!" -%}
        {{ typeName | replace:"some","(any" | replace:"!",")?" }}
    {%- elif typeName|contains:"some " and typeName.isClosure and typeName|contains:"?" -%}
        {{ typeName | replace:"some","(any" | replace:"?",")?" }}
    {%- elif typeName|contains:"some " and typeName|contains:"?" -%}
        {{ typeName | replace:"some","(any" | replace:"?",")?" }}
    {%- elif isVariadic -%}
-        {{ typeName }}...
+        [{{ typeName }}]
    {%- else -%}
        {{ typeName|replace:"some ","any " }}
    {%- endif -%}
{%- endmacro %}

@rokridi
Copy link
Contributor Author

rokridi commented Jan 24, 2024

@art-divin thanks for replying. I updated the issue's description.
The actual stencil has issues. It does not handle closures with existential parameters properly. The macros need to be re-written.
Here is an example of a function having a parameter which is a closure having a variadic parameter:

func foo(closure: (String...) -> Void)

But this case is tricky to handle.

@art-divin
Copy link
Collaborator

@rokridi thank you, I see. So the branch I've referred above should support ClosureParameter.isVariadic. Please check

@rokridi
Copy link
Contributor Author

rokridi commented Jan 25, 2024

Here is what the Stencil could look like (AutoMockable.txt), however we did not succeed at handling functions having a parameter which is a closure having a variadic parameter.

Example:

func foo(closure: (String...) -> Void)

The corresponding mock:

    var func13ParamStringVoidVoidClosure: (((String) -> Void) -> Void)?
    func func13(param: (String) -> Void) {
        func13ParamStringVoidVoidCallsCount += 1
        func13ParamStringVoidVoidClosure?(param)
    }

Expected mock:

    var func13ParamStringVoidVoidClosure: (((String...) -> Void) -> Void)?
    func func13(param: (String...) -> Void) {
        func13ParamStringVoidVoidCallsCount += 1
        func13ParamStringVoidVoidClosure?(param)
    }

@art-divin
Copy link
Collaborator

@rokridi

var func13ParamStringVoidVoidClosure: (((String...) -> Void) -> Void)?

This is not compilable swift - checked yesterday also, to my amusement, variadic is only supported in function signature, does not work with variables.

So, it'd work as:

    var func13ParamStringVoidVoidClosure: ((([(String)]) -> Void) -> Void)?

@art-divin
Copy link
Collaborator

I can pick it up from here, feel free to take a look at other, easy to reproduce bugs that are to be fixed for the next release:
https://github.com/krzysztofzablocki/Sourcery/milestone/6

@rokridi
Copy link
Contributor Author

rokridi commented Jan 25, 2024

This PR should fix the issue

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