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 Sendable compliant protocols and classes #254

Merged

Conversation

nhiroyasu
Copy link
Contributor

@nhiroyasu nhiroyasu commented Apr 13, 2024

Summary

I have implemented the enhancements proposed in issue #252.

For mock classes that are required to conform to the Sendable protocol, I have made them inherit from @unchecked Sendable. This change should help reduce the number of unnecessary compiler warnings.

Examples

Source

protocol SendableProtocol: Sendable {}
class UncheckedSendableClass: @unchecked Sendable {}
final class SendableClass: Sendable {}
Output

class SendableProtocolMock: SendableProtocol, @unchecked Sendable {}
class UncheckedSendableClassMock: UncheckedSendableClass, @unchecked Sendable {}
// final class SendableClass is skipped, because final class can't make a mock class.

Previews

Before After

Copy link
Collaborator

@sidepelican sidepelican left a comment

Choose a reason for hiding this comment

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

Thank you very much. This is probably a feature many of us have been waiting for.

It seems to be mostly fine.

for inheritedType in inheritedTypes {
inheritedTypesStr += ", " + inheritedType
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this can be more beautify like this:

var inheritedTypes = inheritedTypes
inheritedTypes.insert("\(moduleDot)\(identifier)", at: 0)
\(acl)\(finalStr)class \(name): \(inheritedTypes.joined(", ")) {

models.append(contentsOf: parentModels)
processedModels.append(contentsOf: parentProcessedModels)
attributes.append(contentsOf: parentAttributes)
inheritedTypes = inheritedTypes.union(parentInheritedTypes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use formUnion to avoid CoW overhead

Copy link
Contributor Author

@nhiroyasu nhiroyasu Apr 14, 2024

Choose a reason for hiding this comment

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

Thank you for the review!
I was not aware of the issue with CoW overhead. I will fix this along with the other review points.

@@ -0,0 +1,20 @@
import Foundation
Copy link
Collaborator

Choose a reason for hiding this comment

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

unused import.

import MockoloFramework

let sendableProtocol = """
import Foundation
Copy link
Collaborator

Choose a reason for hiding this comment

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

unused import.

@nhiroyasu
Copy link
Contributor Author

@sidepelican
I have made the corrections to the areas you specified.

Copy link
Collaborator

@sidepelican sidepelican 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! Thank you!

@sidepelican sidepelican merged commit b57ef4a into uber:master Apr 15, 2024
4 checks passed
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 this pull request may close these issues.

None yet

2 participants