Remove temporary strings in source generators formatting #281
Conversation
Sergio0694
added
next preview ✈️
|
|
||
| // Adjust the visibility of the constructors based on whether the target type is abstract. | ||
| // If that is not the case, the constructors have to be declared as public and not protected. | ||
| if (!info.IsAbstract) | ||
| { | ||
| replaced = replaced.Replace("protected", "public"); | ||
| modifiedConstructor = modifiedConstructor.WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))); |
There was a problem hiding this comment.
What if the constructor has other modifiers as well, like unsafe? This would remove all of them. Or is this constructor not from user code but generated by the generator and it's guaranteed that it won't have other modifiers?
There was a problem hiding this comment.
If preserving other modifiers is needed, try something like this:
modifiedConstructor = modifiedConstructor.WithModifiers(
modifiedConstructor.Modifiers.Replace(
modifiedConstructor.Modifiers.First(token => token.Kind() is SyntaxKind.ProtectedKeyword),
SyntaxFactory.Token(SyntaxKind.PublicKeyword)));There was a problem hiding this comment.
The syntax tree is from the ObservableRecipient.cs embedded resource file, so yeah we can assume that protected is the only modifier that could possibly be there. These trees are not provided by code parsed from consumers 🙂
| } | ||
|
|
||
| builder.Add((ConstructorDeclarationSyntax)ParseMemberDeclaration(replaced)!); |
There was a problem hiding this comment.
Ugh it was being reparsed just to replace a token... I'm glad this is fixed 😄
There was a problem hiding this comment.
Yup, looking back at this it was indeed quite bad ahah
See dotnet/roslyn-analyzers#6017 and Sergio0694/ComputeSharp#297.
cc. @Neme12
PR Checklist