Fix make static to not drop trivia #54216
Conversation
| @@ -18,12 +18,12 @@ internal abstract class AbstractMakeMemberStaticCodeFixProvider : SyntaxEditorBa | |||
| { | |||
| internal sealed override CodeFixCategory CodeFixCategory => CodeFixCategory.Compile; | |||
|
|
|||
| protected abstract bool IsValidMemberNode([NotNullWhen(true)] SyntaxNode? node); | |||
| protected abstract bool TryGetMemberDeclaration(SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? memberDeclaration); | |||
There was a problem hiding this comment.
There is no need for an abstract class here, only C# is implemented. But I didn't cleanup this.
There was a problem hiding this comment.
I think there's a lot of optimism that this could be in VB :)
Fine to leave for now I think
There was a problem hiding this comment.
@ryzngard The situation is the exact opposite in VB.
For C#, a static class requires all members to be static
For VB, a module requires all members to be non-Shared (they're Shared implicitly).
There is a code fix for the VB situation:
|
|
||
| if (node.IsKind(SyntaxKind.VariableDeclarator)) | ||
| { | ||
| memberDeclaration = node.FirstAncestorOrSelf<MemberDeclarationSyntax>(a => a.IsKind(SyntaxKind.FieldDeclaration) || a.IsKind(SyntaxKind.EventFieldDeclaration)); |
There was a problem hiding this comment.
instead of walking arbitrarily high, we should check for exactly the shapes we expect here. that prevents thigns like matching on a variable inside a lambda inside a field initializer. In this case, we jsut want to match node { Parent: VaraibleDeclaration { Parent: FieldDeclarationSyntax or EventFieldDeclarationSyntax } }
Youssef1313
force-pushed
the
make-static-trivia
branch
from
6b56c24
to
6258179
Compare
|
CI is stuck. Closing and reopening |
|
@CyrusNajmabadi Is this ready to merge? |
jinujoseph
added
the
Community
|
Ping @CyrusNajmabadi |
|
Rerunning tests and enabling automerge. Apologies for the delay @Youssef1313 |
|
@ryzngard Can you rerun the failing jobs? |
|
i got it. |
ghost
added this to the 
Fixes #54202
First commit is just a cleanup.
Second commit fixes the above issue as well as fixing dropping other modifiers.
Why the old code wasn't working? No idea tbh, I tried to dig into it but wasn't able to find the root cause. I found that SyntaxGenerator is more happy with declarations rather than declarators.