pointfreeco / swift-case-paths

🧰 Case paths extends the key path hierarchy to enum cases.
https://www.pointfree.co/collections/enums-and-structs/case-paths
MIT License
904 stars 105 forks source link

fix for Element compiler issue #185

Closed iwt-dahlborn closed 1 month ago

iwt-dahlborn commented 1 month ago

This would be the general idea with a very rough implementation. Basically just generating a typealias for every associated value that contains an Element type.

It could be refined to check if there is specifically a type named Element used since this will just catch anything containing element.

I think the macro definition can also better specify what names the typealias can have so it doesnt need to be arbitrary?

Like I said just a very rough idea.

edit: Sorry just saw that my formatter ran a bit wild there

mbrandonw commented 1 month ago

Hey @iwt-dahlborn, how do you feel about tweaking this PR slightly so that rather than introducing a bunch of type aliases for each payload we just stick with the one _$Element type alias, and then replace occurrences of Element in the payloads with _$Element:

-public var element: CasePaths.AnyCasePath<Action, _$elementElement> {
+public var element: CasePaths.AnyCasePath<Action, (_$Element, Int)> {
iwt-dahlborn commented 1 month ago

I updated the pull request with the rewriter based method you proposed and I think it definitely seems to be the most elegant solution.

While doing the changes I was wondering why the AllCasePath declarations default to a public access level instead of reusing the access level of the enum.

stephencelis commented 1 month ago

Thanks!

While doing the changes I was wondering why the AllCasePath declarations default to a public access level instead of reusing the access level of the enum.

We've found this to be the most convenient way to automate the access level, and Apple follows the convention in some of its macros (@Model, I believe). We used to match the access level but then we needed to update all our macros when package access was introduced, and we've also noticed issues around parent contexts (like private extension) that the macro is unaware of.