Merge pull request #14990 from CyrusNajmabadi/objectInitializerFix

Do not offer use-object intiializer on code that references the variable being initialized.
Fixes #14988

src/EditorFeatures/CSharpTest/UseObjectInitializer/UseObjectInitializerTests.cs

@@ -49,6 +49,101 @@ void M()
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)]
+        public async Task TestDoNotUpdateAssignmentThatReferencesInitializedValue1Async()
+        {
+            await TestAsync(
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        var c = [||]new C();
+        c.i = 1;
+        c.i = c.i + 1;
+    }
+}",
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        var c = new C()
+        {
+            i = 1
+        };
+        c.i = c.i + 1;
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)]
+        public async Task TestDoNotUpdateAssignmentThatReferencesInitializedValue2Async()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        var c = [||]new C();
+        c.i = c.i + 1;
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)]
+        public async Task TestDoNotUpdateAssignmentThatReferencesInitializedValue3Async()
+        {
+            await TestAsync(
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        C c;
+        c = [||]new C();
+        c.i = 1;
+        c.i = c.i + 1;
+    }
+}",
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        C c;
+        c = new C()
+        {
+            i = 1
+        };
+        c.i = c.i + 1;
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)]
+        public async Task TestDoNotUpdateAssignmentThatReferencesInitializedValue4Async()
+        {
+            await TestMissingAsync(
+@"class C
+{
+    int i;
+
+    void M()
+    {
+        C c;
+        c = [||]new C();
+        c.i = c.i + 1;
+    }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)]
         public async Task TestOnAssignmentExpression()
         {

src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs

@@ -7,6 +7,8 @@
 using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.CodeAnalysis.Options;
+using System;
+using System.Linq;
 
 namespace Microsoft.CodeAnalysis.UseObjectInitializer
 {
@@ -236,8 +238,8 @@ internal struct Analyzer<
                     break;
                 }
 
-                SyntaxNode left, right;
-                _syntaxFacts.GetPartsOfAssignmentStatement(statement, out left, out right);
+                _syntaxFacts.GetPartsOfAssignmentStatement(
+                    statement, out var left, out var right);
 
                 var rightExpression = right as TExpressionSyntax;
                 var leftMemberAccess = left as TMemberAccessExpressionSyntax;
@@ -252,6 +254,27 @@ internal struct Analyzer<
                     break;
                 }
 
+                // Don't offer this fix if the value we're initializing is itself referenced
+                // on the RHS of the assignment.  For example:
+                //
+                //      var v = new X();
+                //      v.Prop = v.Prop.WithSomething();
+                //
+                // Or with
+                //
+                //      v = new X();
+                //      v.Prop = v.Prop.WithSomething();
+                //
+                // In the first case, 'v' is being initialized, and so will not be available 
+                // in the object initializer we create.
+                // 
+                // In the second case we'd change semantics because we'd access the old value 
+                // before the new value got written.
+                if (ExpressionContainsValuePattern(rightExpression))
+                {
+                    break;
+                }
+
                 // found a match!
                 seenNames = seenNames ?? new HashSet<string>();
 
@@ -271,6 +294,22 @@ internal struct Analyzer<
             return matches.ToImmutableAndFree();
         }
 
+        private bool ExpressionContainsValuePattern(TExpressionSyntax expression)
+        {
+            foreach (var subExpression in expression.DescendantNodesAndSelf().OfType<TExpressionSyntax>())
+            {
+                if (!_syntaxFacts.IsNameOfMemberAccessExpression(subExpression))
+                {
+                    if (ValuePatternMatches(subExpression))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+
         private bool ValuePatternMatches(TExpressionSyntax expression)
         {
             if (_valuePattern.IsToken)
@@ -294,8 +333,8 @@ private bool TryInitializeAssignmentCase()
                 return false;
             }
 
-            SyntaxNode left, right;
-            _syntaxFacts.GetPartsOfAssignmentStatement(_containingStatement, out left, out right);
+            _syntaxFacts.GetPartsOfAssignmentStatement(
+                _containingStatement, out var left, out var right);
             if (right != _objectCreationExpression)
             {
                 return false;