diff --git a/src/Features/Core/Portable/UseThrowExpression/AbstractUseThrowExpressionDiagnosticAnalyzer.cs b/src/Features/Core/Portable/UseThrowExpression/AbstractUseThrowExpressionDiagnosticAnalyzer.cs index da067d1bddfa6c3f53623fde52bc65fad77ba96c..be5fa96735a292ed8be306ab1cacb7e97c65fb2a 100644 --- a/src/Features/Core/Portable/UseThrowExpression/AbstractUseThrowExpressionDiagnosticAnalyzer.cs +++ b/src/Features/Core/Portable/UseThrowExpression/AbstractUseThrowExpressionDiagnosticAnalyzer.cs @@ -85,7 +85,7 @@ private void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol { return; } - + var option = optionSet.GetOption(CodeStyleOptions.PreferThrowExpression, throwStatement.Language); if (!option.Value) { @@ -129,37 +129,16 @@ private void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol } if (!TryFindAssignmentExpression(containingBlock, ifOperation, localOrParameter, - out var expressionStatement, out var assignmentExpression)) + out var expressionStatement, out var assignmentExpression)) { return; } // We found an assignment using this local/parameter. Now, just make sure there // were no intervening accesses between the check and the assignment. - var statements = containingBlock.Statements; - var ifOperationIndex = statements.IndexOf(ifOperation); - var expressionStatementIndex = statements.IndexOf(expressionStatement); - - if (expressionStatementIndex > ifOperationIndex + 1) - { - // There are intermediary statements between the check and the assignment. - // Make sure they don't try to access the local. - var dataFlow = semanticModel.AnalyzeDataFlow( - statements[ifOperationIndex + 1].Syntax, - statements[expressionStatementIndex - 1].Syntax); - - if (dataFlow.ReadInside.Contains(localOrParameter) || - dataFlow.WrittenInside.Contains(localOrParameter)) - { - return; - } - } - - // Also, have to make sure there is no read/write of the local/parameter on the left - // of the assignment. For example: map[val.Id] = val; - var exprDataFlow = semanticModel.AnalyzeDataFlow(assignmentExpression.Target.Syntax); - if (exprDataFlow.ReadInside.Contains(localOrParameter) || - exprDataFlow.WrittenInside.Contains(localOrParameter)) + if (ValueIsAccessed( + semanticModel, ifOperation, containingBlock, + localOrParameter, expressionStatement, assignmentExpression)) { return; } @@ -197,6 +176,34 @@ private void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol } } + private static bool ValueIsAccessed(SemanticModel semanticModel, IIfStatement ifOperation, IBlockStatement containingBlock, ISymbol localOrParameter, IExpressionStatement expressionStatement, IAssignmentExpression assignmentExpression) + { + var statements = containingBlock.Statements; + var ifOperationIndex = statements.IndexOf(ifOperation); + var expressionStatementIndex = statements.IndexOf(expressionStatement); + + if (expressionStatementIndex > ifOperationIndex + 1) + { + // There are intermediary statements between the check and the assignment. + // Make sure they don't try to access the local. + var dataFlow = semanticModel.AnalyzeDataFlow( + statements[ifOperationIndex + 1].Syntax, + statements[expressionStatementIndex - 1].Syntax); + + if (dataFlow.ReadInside.Contains(localOrParameter) || + dataFlow.WrittenInside.Contains(localOrParameter)) + { + return true; + } + } + + // Also, have to make sure there is no read/write of the local/parameter on the left + // of the assignment. For example: map[val.Id] = val; + var exprDataFlow = semanticModel.AnalyzeDataFlow(assignmentExpression.Target.Syntax); + return exprDataFlow.ReadInside.Contains(localOrParameter) || + exprDataFlow.WrittenInside.Contains(localOrParameter); + } + protected abstract ISyntaxFactsService GetSyntaxFactsService(); protected abstract ISemanticFactsService GetSemanticFactsService();