Brightify / Cuckoo

Boilerplate-free mocking framework for Swift!
MIT License
1.67k stars 174 forks source link

Protocols extending Sendable lead to concurrency warnings #450

Open JavaAdam opened 1 year ago

JavaAdam commented 1 year ago

When a protocol extends Sendable the generated mock is not complaint with the concurrency policy. And the following warnings occurs. I cannot guess if this is a bigger issue, because of the internal data structure?

Non-final class 'MockMyProtocol' cannot conform to 'Sendable'; use '@unchecked Sendable'

Stored property 'cuckoo_manager' of 'Sendable'-conforming class 'MockMyProtocol' has non-sendable type 'MockManager'

Stored property '__defaultImplStub' of 'Sendable'-conforming class 'MockMyProtocol' is mutable

dalemyers commented 2 months ago

These are going to be errors in Swift 6. Is there any plans for correcting these?

MatyasKriz commented 2 months ago

These are going to be errors in Swift 6. Is there any plans for correcting these?

Since Cuckoo isn't funded, there's no roadmap of any kind and PRs are more than welcome! 🙂

Is the workaround of marking every single mock @unchecked Sendable sufficient?

chaseklingelzen commented 1 month ago

Hey @MatyasKriz that workaround would be sufficient from my perspective! I saw this has been open for 3 weeks. Did you have plans to open a pr with that change?

MatyasKriz commented 1 month ago

I didn't want to hastily implement something where I'm not sure what the tests should look like. If @unchecked Sendable is fine, I'll see what I can do. There are a number of these protocol behaviors, so for the sake of code complexity I think we'll have to abstract the search for them a bit so that bugs are less likely in this part of the generator.

chaseklingelzen commented 1 month ago

@MatyasKriz understand your concern there. Out of curiosity, what template file(s) would make this update be made in? We may have to temporarily fork the repository to unblock us and looking for any guidance you might have here! Thanks in advance.

MatyasKriz commented 1 month ago

The main template file is MockTemplate.swift, from what I've seen you might be able to get away with adding @unchecked Sendable to every single mock without the additional logic if you just want to get unblocked.

chaseklingelzen commented 1 month ago

We will give that a shot! It sounds like it's unlikely you would commit this to the repo itself any time soon, correct?

MatyasKriz commented 1 month ago

@chaseklingelzen try using branch fix/swift6-sendable, it just adds the @unchecked Sendable to every mock because the current inheritance implementation is really unoptimized and I'd rather refactor it completely for which I don't have time right now.

chaseklingelzen commented 1 month ago

Were you able to see it generated @unchecked Sendable for stubs on your end @MatyasKriz? It could be something to do w/ our set up. I've verified that I am in fact using the right branch (see verification image below).

Actual:

Screenshot 2024-10-08 at 2 02 06 PM

Expected:

Screenshot 2024-10-08 at 2 02 45 PM

The protocol being stubbed:

public protocol AppAttestService: Sendable {
    var isSupported: Bool { get async }
    func generateKey() async throws -> String
    func attestKey(_ keyId: String, clientDataHash: Data) async throws -> Data
    func generateAssertion(_ keyId: String, clientDataHash: Data) async throws -> Data
}

Verification:

Screenshot 2024-10-08 at 2 04 41 PM
MatyasKriz commented 1 month ago

If you're using the run script, simply pass --clean argument. If not, then I'm actually not sure, but when I run tests, the generated mocks all have @unchecked Sendable, so that points to your Cuckoonator being outdated.

chaseklingelzen commented 1 month ago

Awesome, that is more than likely the issue! Thanks again for the quick responses. It's much appreciated @MatyasKriz.