-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix crash in use-null-prop fixer #79340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment
return null; | ||
|
||
return (analysisResult.Value.ConditionPartToCheck, analysisResult.Value.WhenPartToCheck); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
broke analysis into two parts. the pure, stateless analysis portino, and the part that does that and reports diagnostics. the former will be used by the fixer when batch fixing.
var ifStatement = (TIfStatementSyntax)context.Node; | ||
var analysisResultOpt = AnalyzeIfStatement( | ||
context.SemanticModel, referenceEqualsMethod, ifStatement, cancellationToken); | ||
if (analysisResultOpt is not IfStatementAnalysisResult analysisResult) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing happened to the if-statement analyzer.
@@ -32,7 +33,7 @@ internal abstract class AbstractUseNullPropagationCodeFixProvider< | |||
TElementBindingExpressionSyntax, | |||
TIfStatementSyntax, | |||
TExpressionStatementSyntax, | |||
TElementBindingArgumentListSyntax> : SyntaxEditorBasedCodeFixProvider | |||
TElementBindingArgumentListSyntax> : ForkingSyntaxEditorBasedCodeFixProvider<SyntaxNode> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is hte major change. the amount of mutation that the fixer would perform (Especially with nested constructs) made this too difficult to be a normal SyntaxEditor fixer. So it got changes to a Forking one. That means it will fix, and then if it hits another diagnostic that contains a span it already fixed, it will instead form and attempt to reanalyze that outer construct to see if and how it can still be fixed.
var whenPart = root.FindNode(diagnostic.AdditionalLocations[2].SourceSpan, getInnermostNodeForTie: true); | ||
var conditionalExpressionParts = this.GetPartsOfConditionalExpression( | ||
semanticModel, conditionalExpression, cancellationToken); | ||
if (conditionalExpressionParts is not var (conditionalPart, whenPart)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now, instead of grabbing this data off the diagnostic, we go back and renanalyze to get hte up to date info int he current fork.
Fixes #79338