LFT-1728 - Add support for CollectionExpression in async generator #963
Conversation
| with: | ||
| dotnet-version: | | ||
| 7.0.x | ||
| 9.0.x |
There was a problem hiding this comment.
Need to update for new C# 12 syntax
j3parker
changed the title
| SpreadElementSyntax sp => sp.WithExpression( Transform( sp.Expression ) ), | ||
| _ => UnhandledSyntax( elem ) | ||
| } ), | ||
| original.GetSeparators() |
There was a problem hiding this comment.
We're not passing the original separators for the other cases in the transformer, so perhaps something to correct later
There was a problem hiding this comment.
Yeah -- see note below.
One other reason we're not doing it in the other ones is TransformAll supports the "maybe transform" e.g. so we can delete cancellation token arguments. There needs to be some accounting for that otherwise .GetSeparators() will possibly insert extra separators/commas, so it needs a bit more thought.
| original.Select( elem => elem switch { | ||
| ExpressionElementSyntax ee => ee.WithExpression( Transform( ee.Expression ) ), | ||
| SpreadElementSyntax sp => sp.WithExpression( Transform( sp.Expression ) ), | ||
| _ => UnhandledSyntax( elem ) | ||
| } ), |
There was a problem hiding this comment.
Particular reason this isn't just original.Select( Transform ) or something, with the top-level Dispatch handling Spread?
There was a problem hiding this comment.
Yeah -- what I have currently on my desktop has that, with the move to TransformAll. I'll get to it.
| _ => UnhandledSyntax( elem ) | ||
| } ), | ||
| original.GetSeparators() | ||
| ); |
There was a problem hiding this comment.
Actually part of this is already in place with the TransformAll combinator.
It doesn't do original.GetSeparators(), though.
I'll do a follow-up PR to switch to that, but also add the separators. That will incur some noise because we will have to fix up the whitespace in unrelated tests.
No description provided.