8000 assertMacroExpansion always passes empty array as `conformingTo` parameter of extension macros · Issue #2031 · swiftlang/swift-syntax · GitHub
[go: up one dir, main page]
Skip to content

assertMacroExpansion always passes empty array as conformingTo parameter of extension macros #2031

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.

Sign up for GitHub

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

Jump to bottom
Closed
stephencelis opened this issue Aug 8, 2023 · 8 comments · Fixed by #2327
Closed

assertMacroExpansion always passes empty array as conformingTo parameter of extension macros #2031

stephencelis opened this issue Aug 8, 2023 · 8 comments · Fixed by #2327
Labels
bug Something isn't working

Comments

@stephencelis
Copy link
Contributor

Description

Previously a bug with conformance macros:

This issue has regressed with the change to extension macros.

Steps to Reproduce

  1. Write a macro expansion test for an extension macro.
  2. Note that the expanded source does not include the extension.
@stephencelis stephencelis added the bug Something isn't working label Aug 8, 2023
@ahoppen
Copy link
Member
ahoppen commented Aug 8, 2023

Tracked in Apple’s issue tracker as rdar://113582092

@ahoppen
Copy link
Member
ahoppen commented Aug 8, 2023

@stephencelis Do you have an example where the extension is not included in the expanded source? Because the SendableExtensionMacro in swift-syntax’s test suite works just fine.

https://github.com/apple/swift-syntax/blob/5412040ebb79c529c16a8a8a6bbfe59baf06c5f6/Tests/SwiftSyntaxMacroExpansionTest/MacroSystemTests.swift#L1504-L1522

@stephencelis
Copy link
Contributor Author

@ahoppen This might be a documentation problem, but the first example of an extension macro I found was this one in the Swift repo, which starts:

https://github.com/apple/swift/blob/7699834154417cb6eea9fc6438cf5a4c3e589dee/lib/Macros/Sources/SwiftMacros/OptionSetMacro.swift#L135-L137

These three lines of code make the macro expansion testing tool never add the conformance, presumably because macro expansion testing is totally decoupled from the library's macro declaration that specifies the allowed conformances.

@ahoppen
Copy link
Member
ahoppen commented Aug 9, 2023

Ah, yes. That’s a current limitation of assertMacroExpansion, which, at the moment, always passes an empty list for conformingTo. We are aware of the issue and are looking into it.

@ahoppen ahoppen changed the title SyntaxProtocol.expand() does not expand extension macros assertMacroExpansion always passes empty array as conformingTo parameter of extension macros Aug 9, 2023
@soumyamahunt
Copy link
Contributor

@ahoppen if there is limitation in inferring conformance for a type, I would expect all the protocols that macro declares conformance for all provided. That way existing tests will not break when migrating to extension macro from member macro.

@ahoppen
Copy link
Member
ahoppen commented Aug 11, 2023

The problem is that in assertMacroExpansion, we currently don’t know which conformances the macro declaration (ie the thing after @attached) contains because assertMacroExpansion only has a mapping from the macro name to the type. We would need to design some way of passing that information in to assertMacroExpansion here.

@soumyamahunt soumyamahunt mentioned this issue Aug 16, 2023
Closed
@DougGregor
Copy link
Member

The simplest approach is to pass this information as an extra parameter through assertMacroExpansion, e.g.,

assertMacroExpansion( 
       """ 
       @AddSendableExtension 
       struct MyType { 
       } 
       """, 
       expandedSource: """ 
  
         struct MyType { 
         } 
  
         extension MyType: Sendable { 
         } 
         """, 
       macros: testMacros, 
       conformsTo: ["AddSendableExtension": ["Sendable"]],
       indentationWidth: indentationWidth 
     )

where the conformsTo parameter has a type like [String: [TypeSyntax]] so we can pass different sets of conformances to each macro.

There are more advanced approaches I could imagine that, for example, depend on passing the actual macro declarations into assertMacroExpansion, e.g., we could pass in this text

@attached(extension, conformances: Sendable)
macro AddSendableExtension() = ...

and then use the conformances in the attached macro role declaration to populate the conformances passed to the macro implementations. It's more automatic, but it's a lot more plumbing.

@soumyamahunt soumyamahunt mentioned this issue Oct 10, 2023
Closed
@soumyamahunt
Copy link
Contributor

@ahoppen anything finalized on the approach yet, I am thinking of creating PR with @DougGregor's suggestion.

@soumyamahunt soumyamahunt mentioned this issue Nov 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Macros] Introduce macro specification in assertMacroExpansion soumyamahunt/swift-syntax
4 participants
@stephencelis @DougGregor @ahoppen @soumyamahunt
0