Merge pull request #15023 from CyrusNajmabadi/suppressUseThrowExpression

Don't offer Use-Throw-Expression if teh variable being checked is accessed before it is assigned.
Fixes #15018

src/EditorFeatures/CSharpTest/UseThrowExpression/UseThrowExpressionTests.cs

@@ -189,13 +189,34 @@ void M(string s, string t)
     {
         if (s == null)
         {
-            [|throw|] new ArgumentNullException(nameof(s)) };
+            [|throw|] new ArgumentNullException(nameof(s));
+        };
         s = ""something"";
         _s = s;
     }
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseThrowExpression)]
+        public async Task NotWithIntermediaryMemberAccess()
+        {
+            await TestMissingAsync(
+@"using System;
+
+class C
+{
+    void M(string s, string t)
+    {
+        if (s == null)
+        {
+            [|throw|] new ArgumentNullException(nameof(s));
+        };
+        s.ToString();
+        _s = s;
+    }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseThrowExpression)]
         public async Task TestNullCheckOnLeft()
         {

src/Features/CSharp/Portable/UseThrowExpression/CSharpUseThrowExpressionDiagnosticAnalyzer.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.UseThrowExpression;
 
 namespace Microsoft.CodeAnalysis.CSharp.UseThrowExpression
@@ -13,5 +15,8 @@ protected override bool IsSupported(ParseOptions options)
             var csOptions = (CSharpParseOptions)options;
             return csOptions.LanguageVersion >= LanguageVersion.CSharp7;
         }
+
+        protected override ISyntaxFactsService GetSyntaxFactsService()
+            => CSharpSyntaxFactsService.Instance;
     }
 }
\ No newline at end of file

src/Features/Core/Portable/UseThrowExpression/AbstractUseThrowExpressionDiagnosticAnalyzer.cs

@@ -7,6 +7,7 @@
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Semantics;
 using Microsoft.CodeAnalysis.Text;
 
@@ -101,31 +102,39 @@ private void AnalyzeOperation(OperationAnalysisContext context)
                 return;
             }
 
-            ISymbol localOrParameter;
-            if (!TryDecomposeIfCondition(ifOperation, out localOrParameter))
+            if (!TryDecomposeIfCondition(ifOperation, out var localOrParameter))
             {
                 return;
             }
 
-            IExpressionStatement expressionStatement;
-            IAssignmentExpression assignmentExpression;
             if (!TryFindAssignmentExpression(containingBlock, ifOperation, localOrParameter,
-                out expressionStatement, out assignmentExpression))
+                out var expressionStatement, out var assignmentExpression))
             {
                 return;
             }
 
             // We found an assignment using this local/parameter.  Now, just make sure there
-            // were no intervening writes between the check and the assignement.
-            var dataFlow = semanticModel.AnalyzeDataFlow(
-                ifOperation.Syntax, expressionStatement.Syntax);
+            // 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 (dataFlow.WrittenInside.Contains(localOrParameter))
+            if (expressionStatementIndex > ifOperationIndex + 1)
             {
-                return;
+                // 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;
+                }
             }
 
-            // Ok, there were no intervening writes.  This check+assignment can be simplified.
+            // Ok, there were no intervening writes or accesses.  This check+assignment can be simplified.
 
             var allLocations = ImmutableArray.Create(
                 ifOperation.Syntax.GetLocation(),
@@ -159,6 +168,8 @@ private void AnalyzeOperation(OperationAnalysisContext context)
             }
         }
 
+        protected abstract ISyntaxFactsService GetSyntaxFactsService();
+
         private bool TryFindAssignmentExpression(
             IBlockStatement containingBlock, IIfStatement ifOperation, ISymbol localOrParameter,
             out IExpressionStatement expressionStatement, out IAssignmentExpression assignmentExpression)
@@ -181,8 +192,7 @@ private void AnalyzeOperation(OperationAnalysisContext context)
                     continue;
                 }
 
-                ISymbol assignmentValue;
-                if (!TryGetLocalOrParameterSymbol(assignmentExpression.Value, out assignmentValue))
+                if (!TryGetLocalOrParameterSymbol(assignmentExpression.Value, out var assignmentValue))
                 {
                     continue;
                 }