Use-throw-expression should not be offered when the 'if-block' has additional statements in it #19379
Conversation
…ditional statements in it.
|
Tagging @dotnet/roslyn-ide |
|
Tagging @MattGertz Customer scenario our 'use throw expressoin' analyzer is too aggressive and it gets offered in reasonable code where it shouldn't be offered. This is problematic especially as one can 'fix all' and might end up breaking code unintentionally. Bugs this fixes: Workarounds, if any None. Risk Low. Performance impact Low. Is this a regression from a previous update? No. Root cause analysis: In sufficient checks to only run on an exact user code pattern. How was the bug found? Customer reported. |
|
Approved pending tests and code reviews. |
|
retest windows_debug_vs-integration_prtest please |
| @@ -116,6 +116,12 @@ private void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol | |||
| return; | |||
| } | |||
|
|
|||
| if (!IsOnlyStatementOfIf(ifOperation, throwOperation)) | |||
There was a problem hiding this comment.
💭 It may be more efficient to roll this into GetContainingIfOperation.
|
|
||
| return ifOperation.IfTrueStatement is IBlockStatement block && | ||
| block.Statements.Length == 1 && | ||
| block.Statements[0].Syntax == throwStatement.Syntax; |
There was a problem hiding this comment.
💡 I believe the first and last subexpressions are redundant (always true). The full expression could be reduced to:
return ((IBlockStatement)ifOperation.IfTrueStatement).Statements.Length == 1;
|
Oh, I like the way that turned out 👍 |
Fixes #19377