fix incorrect FirstOrDefault usage #4580
Conversation
| @@ -41,7 +41,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) | |||
| return; | |||
| } | |||
|
|
|||
| ClassDeclarationSyntax declaration = syntaxToken.Parent.AncestorsAndSelf().OfType<ClassDeclarationSyntax>().FirstOrDefault(); | |||
| ClassDeclarationSyntax declaration = syntaxToken.Parent.AncestorsAndSelf().OfType<ClassDeclarationSyntax>().First(); | |||
There was a problem hiding this comment.
I think it's possible to not have an ancestor ClassDeclarationSyntax (though very very unlikely anyone will hit that case).
But here is the scenario in mind:
[TestClass]
internal record TestClass1
{
}I think both First and FirstOrDefault are not the right thing to do. Instead, we should be looking for TypeDeclarationSyntax rather than ClassDeclarationSyntax, and in this case, First makes sense.
There was a problem hiding this comment.
if that is the case, then we would get a null ref inside the call to FixClassDeclarationAsync since is does not do a null check when accessing classDeclaration.Modifiers.
unless i am missing something?
There was a problem hiding this comment.
@SimonCropp Yes, exactly what I meant. I think this is already broken if the test class is a record and will have NullReferenceException currently.
There was a problem hiding this comment.
We have a simple test for class named WhenClassIsInternalAndTestClass_Diagnostic, if you duplicate it, change class to record, I expect it to fail. The fix will be to look for TypeDeclarationSyntax instead of ClassDeclarationSyntax. Let me know please if you'd like to fix this bug, and if you need any additional info.
Again, thank you for all your contributions! ❤️
No description provided.