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

Derive access level of generate CodingKeys from @Codable model #49

Open
andriyslyusar opened this issue Dec 20, 2023 · 5 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@andriyslyusar
Copy link

andriyslyusar commented Dec 20, 2023

Right now @Codable macro will generate CodingKeys with internal access level.

@Codable
@CodingKeys(.PascalCase)
public struct MessageReceiver {
    public let userId: UUID
    public let status: OData.Enum<Int>
}

extension MessageThread.Response.MessageReceiver {
    enum CodingKeys: String, CodingKey {
        case userId = "UserId"
        case status = "Status"
    }
}

It would great to generate CodingKeys with the same access level (public) as MessageReceiver. This would allow access to CodingKeys in multi package application. As of my particular use case I would like to build OData requests using CodingKeys instead of String from the domain layer of application.

Really appreciate effort in open sourcing the project, unfortunately internal macros plugin implementation is quite complicated to be able to make a quick PR with solution.

@soumyamahunt soumyamahunt added enhancement New feature or request good first issue Good for newcomers labels Dec 20, 2023
@soumyamahunt
Copy link
Contributor

While for your use case it sounds appropriate to make CodingKeys public, I don't think making this behaviour default will be appropriate.

CodingKeys are internal implementation and library authors should have control whether they want it to be exposed as public API.

This can be added as an additional option in Codable to choose the access level.

@andriyslyusar
Copy link
Author

Implementing additional option in Codable would work as well. In defense of automatically inheriting access modifier, in case you make object public, CodingKeys are quite minor implementation detail and can't be override due it is an enum.

@soumyamahunt
Copy link
Contributor

In defense of automatically inheriting access modifier, in case you make object public, CodingKeys are quite minor implementation detail and can't be override due it is an enum.

I agree it won't be overridable, my concern is once you make this implementation public any change made to CodingKeys will potentially be a breaking change for consumers of libraries. Hence I want the library authors to control this behaviour rather than MetaCodable deciding for them.

@andriyslyusar
Copy link
Author

andriyslyusar commented Dec 26, 2023

This can be added as an additional option in Codable to choose the access level.

Maybe better idea would be to add additional parameter to @CodingKeys to indicate access level, e.g. @CodingKeys(.PascalCase, accessLevel: .public). The default implementation would have .internal to avoid breaking backward comparability.

@soumyamahunt
Copy link
Contributor

Maybe better idea would be to add additional parameter to @CodingKeys to indicate access level, e.g. @CodingKeys(.PascalCase, accessLevel: .public). The default implementation would have .internal to avoid breaking backward comparability.

This wouldn't be a good idea considering I am adding enum support, and @CodingKeys can be applied per enum-case. The accessLevel option will be added to @Codable as it is applied once per type. And yes default option will preserve current behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants