Fixed crash in RemoveUnnecessarySuppressions caused by multi-variable declaration
#78787
Conversation
dotnet-policy-service
bot
added
Community
| [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] | ||
| public async Task TestRemoveDiagnosticSuppression_Attribute_MultiVariableDeclaration(MultiVariableTestKind testKind) | ||
| { | ||
| var (keyword, type) = testKind switch |
There was a problem hiding this comment.
Don't overengineere, just make keyword and type test parameters directly
There was a problem hiding this comment.
Simplified with InlineDataAttribute.
...moveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs
Outdated
Show resolved
Hide resolved
|
Also please add "Fixes #78786" to your PR description, so that GH automatically links that issue with the PR and closes it when the PR is merged |
Co-authored-by: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com>
…laration with inline data.
…m/John-Leitch/roslyn into remove-diagnostic-suppression-fix
|
Unclear why we need a loop in the first place. If we're just going to break when we have multiple. Why not just process the first symbol only? |
I assumed it was there for a reason, but now that you mention it, I don't see why. None of the tests seem to get more than one symbol except the case I've added, so they all pass just checking the first. Not sure if there are uncovered case though. |
|
Ok. Can you change to no loop, with a comment explaining why that is desirable here? Thanks! |
…op and fixed partial method/property support in AbstractRemoveUnnecessaryInlineSuppressionsDiagnosticAnalyzer.
|
Updated as requested. However, upon doing some testing to confirm the safety of checking only the first symbol, I caught a variant of the issue that is triggered by partial methods and props. I've included a fix for that as well as some test coverage. |
|
Thanks! |
4 participants
This PR fixes multi-variable declaration support in
RemoveUnnecessarySuppressionsby preventing redundant evaluation of symbols from variable declaration nodes containing more than one variable.Fixes #78786