Skip to content

Add an accessor macro example #2565

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

Merged
merged 6 commits into from
Mar 26, 2024

Conversation

rockname
Copy link
Contributor

@rockname rockname commented Mar 24, 2024

Issue: #2169

This PR introduces a new accessor macro example called EnvironmentValueMacro that automates the generation of getter / setter implementations for EnvironmentValues.

 private struct MyEnvironmentKey: EnvironmentKey {
     static let defaultValue: String = "Default value"
 }

 extension EnvironmentValues {
+    @EnvironmentValue(for: MyEnvironmentKey.self)
+    var myCustomValue: String
-    var myCustomValue: String {
-        get { self[MyEnvironmentKey.self] }
-        set { self[MyEnvironmentKey.self] = newValue }
-    }
 }

@rockname rockname force-pushed the new-accessor-macro-example branch 2 times, most recently from fd25603 to 295068d Compare March 24, 2024 11:16
@rockname rockname force-pushed the new-accessor-macro-example branch from 295068d to b7252d2 Compare March 24, 2024 23:06
@rockname rockname marked this pull request as ready for review March 24, 2024 23:12
@rockname
Copy link
Contributor Author

@swift-ci Please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks for adding the example 👍🏽


return [
"""
get { self[\(raw: argument.expression)] }
Copy link
Member

Choose a reason for hiding this comment

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

I don’t think you need to use raw: here or in the setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: a283c96

//
//===----------------------------------------------------------------------===//

import SwiftUI
Copy link
Member

Choose a reason for hiding this comment

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

The example won’t compile on Linux and Windows if it depends on SwiftUI.

We’ve got two options here: Either wrap the macro in #if canImport(SwiftUI) or change it so it doesn’t depend on SwiftUI. Which one would you prefer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for proposing two options!
I'll choose the #if canImport(SwiftUI) option this time 🙏

b795cd8

@@ -47,3 +47,7 @@ runMemberMacrosPlayground()
// MARK: - Peer Macros

runPeerMacrosPlayground()

// MARKL - Accessor Macros
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// MARKL - Accessor Macros
// MARK: - Accessor Macros

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 🙈 : aaefc0e

Comment on lines 24 to 27
private struct MyEnvironmentKey: EnvironmentKey {
static let defaultValue: String = "Default value"
}

Copy link
Member

Choose a reason for hiding this comment

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

Since the expansion of the macro is syntactic and doesn’t actually try to resolve MyEnvironmentKey, you could remove the declaration of MyEnvironmentKey.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 74a3d68

Copy link
Member

@ahoppen ahoppen 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. Looks good to me 👍🏽

@ahoppen ahoppen enabled auto-merge (squash) March 26, 2024 08:02
@ahoppen
Copy link
Member

ahoppen commented Mar 26, 2024

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Mar 26, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 1481235 into swiftlang:main Mar 26, 2024
@rockname rockname deleted the new-accessor-macro-example branch March 26, 2024 23:10
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.

2 participants