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

Unstable AutoMockable output when protocols have associated type #1359

Closed
magauran opened this issue Aug 14, 2024 · 6 comments · Fixed by #1377
Closed

Unstable AutoMockable output when protocols have associated type #1359

magauran opened this issue Aug 14, 2024 · 6 comments · Fixed by #1377
Assignees

Comments

@magauran
Copy link

Hey 👋, thank you for this fantastic tool! Sourcery has significantly improved our workflow.

Issue

I've encountered an issue that first appeared in Sourcery version 2.2.5. The problem occurs during mock generation with AutoMockable for protocols that contain associatedtype. The generated mocks are inconsistent — sometimes the mock is generated correctly, and other times it isn’t generated at all.

Below, you can found a small code example that illustrates this behavior.

protocol ProtocolWithAssociatedType {
    associatedtype Value: AnotherProtocol

    var value: Value { get }
}

// sourcery:AutoMockable
protocol AnotherProtocol {
    func foo() -> String
}

Sometimes, the mock is generated correctly:

class AnotherProtocolMock: AnotherProtocol {
    //MARK: - foo

    var fooStringCallsCount = 0
    var fooStringCalled: Bool {
        return fooStringCallsCount > 0
    }
    var fooStringReturnValue: String!
    var fooStringClosure: (() -> String)?

    func foo() -> String {
        fooStringCallsCount += 1
        if let fooStringClosure = fooStringClosure {
            return fooStringClosure()
        } else {
            return fooStringReturnValue
        }
    }
}

but in about 50% of runs (without any changes to the code) mock is not generated.

Investigation

This issue appeared after commit 7153768, specifically due to changes in ParserResultsComposed.swift

/// Map associated types
associatedTypes.forEach {
    typeMap[$0.key] = $0.value.type
}

These lines insert associated types to typeMap in a random order, as the order in Dictionary is not guaranteed.
And it seems likely that this code conflicts with unifyTypes function, which removes types with duplicate globalName from the typeMap.

If the original type is listed before the associated type in the typeMap, the generation works correctly:

[
    "AnotherProtocol": AnotherProtocol.self, // type with sourcery annotation
    "ProtocolWithAssociatedType": ProtocolWithAssociatedType.self,
    "Value": AnotherProtocol.self // associated type
]

However, if the associated type is listed before the original type, the original type gets removed from the typeMap, and the mock is not generated.

[
    "ProtocolWithAssociatedType": ProtocolWithAssociatedType.self,
    "Value": AnotherProtocol.self, // associated type
    "AnotherProtocol": AnotherProtocol.self // type with sourcery annotation
]
@magauran
Copy link
Author

@art-divin do you have any ideas on how this could be fixed?

@rokridi
Copy link
Contributor

rokridi commented Aug 21, 2024

Hi @magauran 👋

I have recently encountered this kind of issue when a protocol has a single non constrained associated type.

Here is an example that fails to compile:

protocol AProtocol {}

// sourcery: AutoMockable
protocol ProtocolWithAssociatedType {
    associatedtype Foo
    
    func foo(param: Foo)
}

The generated mock looks like this:

class ProtocolWithAssociatedTypeMock<>: ProtocolWithAssociatedTypes { <= The generic parameters <> list is empty. This causes compilation failure
    typealias Foo = Any




    //MARK: - foo

    var fooParamFooVoidCallsCount = 0
    var fooParamFooVoidCalled: Bool {
        return fooParamFooVoidCallsCount > 0
    }
    var fooParamFooVoidReceivedParam: (Foo)?
    var fooParamFooVoidReceivedInvocations: [(Foo)] = []
    var fooParamFooVoidClosure: ((Foo) -> Void)?

    func foo(param: Foo) {
        fooParamFooVoidCallsCount += 1
        fooParamFooVoidReceivedParam = param
        fooParamFooVoidReceivedInvocations.append(param)
        fooParamFooVoidClosure?(param)
    }


}

Potential fix:

  • Deleted this line from the Stencil
  • Rewrote extractProtocolCompositionFromAssociatedTypes macro:
{% macro extractProtocolCompositionFromAssociatedTypes type -%}
    {%- if type.associatedTypes|sortedValuesByKeys|count > 0 -%}
    <
    {%- for associatedType in type.associatedTypes|sortedValuesByKeys -%}
        {%- if associatedType.type.kind != nil -%}
            {{ associatedType.name }}: {{ associatedType.typeName }},
        {%- else -%}
            {{ associatedType.name }},
        {%- endif -%}
    {%- endfor -%}
    >
    {%- endif -%}
{%- endmacro %}

@alexdmotoc
Copy link
Contributor

@rokridi I think the issue is the same as #1357

The problem is not with the stencil, but with Sourcery itself. Sourcery fails to read the annotation.

Whenever an AutoMockable protocol is used as generic constraint to another protocol, in 50% of the cases, Sourcery fails to read the annotation hence it does not generate mocks for it.

@krzysztofzablocki
Copy link
Owner

What @magauran found seems sensible to me, composer is the pretty complicated and makes a bunch of assumptions and simplification and it's all written by hand (no apple support).

The earliest I could look at it is after next week since I'm at NSSpain, if anyone has ideas how to fix this bit or even improve composer that would be amazing contribution 🙇

@krzysztofzablocki krzysztofzablocki self-assigned this Sep 13, 2024
@fabianmuecke
Copy link
Contributor

fabianmuecke commented Oct 29, 2024

I've investigated this as far as I could. Bisecting shows that it was introduced by this PR. In Line 58 of ParserResultsComposed.swift the associated type is added using the key, not the globalName of the type, where everywhere else the globalName seems to be used(?). This leads to the same type being present multiple times in typeMap, once properly including all properties etc., once pretty much empty, because it was just a constraint of an associated type. It's up to the internal sorting of typeMap then, which one will be included in types in the end (so up to chance, basically), as they're filtered by globalName in lines 177ff.

I've tried to fix this simple stupid, replacing line 58 with typeMap[$0.value.type?.globalName ?? $0.key] = $0.value.type. This consistently gives me the correct results, running our templates for our codebase, but unfortunately makes these tests fail for me:

  • ParserComposer__uniqueTypesAndFunctions__given_associated_type__extracts_associated_type_properly_when_constrained_to_a_typealias()
  • SwiftTemplate__rethrows_template_parsing_errors()

Maybe given these insights, @krzysztofzablocki or @art-divin could pick it up from here? 🙏 This is currently blocking us from updating to the latest version.

Edit: Added a PR with an attempted fix here.

@alexdmotoc
Copy link
Contributor

@krzysztofzablocki when are you going to release this? We need it sir! 🙏

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

Successfully merging a pull request may close this issue.

5 participants