Issue with Array Handling in `@AccountKey` Macro in `AccountKeyMacro.swift`

[NikolaiMadlener]

Problem

When trying to create an @AccountKey with an array type, for example, by passing as: [String], the method fails with the following diagnostics error:
'@AccountKey' failed to parse the meta type expression in argument 'as'

@pauljohanneskraft found that the underlying problem seems to be in SpeziAccount > SpeziAcountMacros > AccountKeyMacro.swift : line 119, where metaTypeTypeNameArgument is called. If an array is passed, the method fails with the above mentioned error. It seems like after a recent update, the Abstract Syntax Tree (AST) structure seems to have changed, causing this failure.

Solution

This can be worked around by defining a typealias for the array type. However, we should invest in a real solution to the problem.

Additional context

No response

Code of Conduct

• I agree to follow this project's Code of Conduct and Contributing Guidelines

[pauljohanneskraft]

A bit more detail describing the problem (this only revolves around specifying [String] in the as parameter of the macro):

In AccountKeyMacro.expansion(of:providingPeersOf:in:) the valueTypeInitializer object is not an IdentifierTypeSyntax anymore, but instead a ArrayTypeSyntax. Further, metaTypeTypeNameArgument(name:) extracts an ArrayExprSyntax for the name "as", but only MemberAccessExprSyntax and DeclReferenceExprSyntax are actually returned.

Unfortunately, I didn't find an easy way to compare a ArrayTypeSyntax to an ArrayExprSyntax, since there is no name property (different to the IdentifierTypeSyntax type).

In general, I'm wondering: What was the rationale for having this as property and not simply using the property's value? Optimally, we could simply get rid of it and solely rely on the property's type, right?

[PSchmiedmayer]

CC @Supereg here for some input 🚀

[bauer-andreas]

In general, I'm wondering: What was the rationale for having this as property and not simply using the property's value? Optimally, we could simply get rid of it and solely rely on the property's type, right?

We use it for overload resolution to be able to provide different defaults/overloads depending on the values conformance’s. Just have a look at all the macro definitions. To my knowledge you currently cannot be generic over the value type of the property a macro is attached to.

[pauljohanneskraft]

Oh, okay - makes sense - thank you @Supereg !

We could technically open that up and only ensure the generic constraint inside the macro expansion, but that would probably be a lot of work, so let's stick to having this as property for now.

What about allowing the other Syntax types as well though? I assume that in some previous version of Swift the syntax node type of an array type also was of type IdentifierTypeSyntax and not ArrayTypeSyntax, so we may want to extend the code to also allow ArrayTypeSyntax here or would you prefer another approach?

[bauer-andreas]

We could technically open that up and only ensure the generic constraint inside the macro expansion, but that would probably be a lot of work, so let's stick to having this as property for now.

Not sure if we could easily check that? The whole point is that you know which overload to choose from and not confuse the user. But happy to see what you have in mind. I’m all in in simplifying this, if possible.

What about allowing the other Syntax types as well though? I assume that in some previous version of Swift the syntax node type of an array type also was of type IdentifierTypeSyntax and not ArrayTypeSyntax, so we may want to extend the code to also allow ArrayTypeSyntax here or would you prefer another approach?

Yes, seems like the way arrays are parsed changed. We should fix that and add a unit test for this to catch future regressions.