Merge pull request #17825 from CyrusNajmabadi/noCollectionInitializer…

…WhenVarIsReferenced Don't offer 'use collection initializer' if the collection being initialized is referenced during initialization.
dotnet · Mar 14, 2017 · ea0043b · ea0043b
2 parents a192dc6 + f1ae59b
commit ea0043b
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 6 deletions.
diff --git a/src/EditorFeatures/CSharpTest/UseCollectionInitializer/UseCollectionInitializerTests.cs b/src/EditorFeatures/CSharpTest/UseCollectionInitializer/UseCollectionInitializerTests.cs
@@ -699,6 +699,57 @@ static void Main(string[] args)
         var myStringList = myStringArray?.ToList() ?? new [||]List<string>();
         myStringList.Add(""Done"");
     }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionInitializer)]
+        [WorkItem(17823, "https://github.com/dotnet/roslyn/issues/17823")]
+        public async Task TestMissingWhenReferencedInInitializer()
+        {
+            await TestMissingInRegularAndScriptAsync(
+@"
+using System.Collections.Generic;
+
+class C
+{
+    static void M()
+    {
+        var items = new [||]List<object>();
+        items[0] = items[0];
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionInitializer)]
+        [WorkItem(17823, "https://github.com/dotnet/roslyn/issues/17823")]
+        public async Task TestWhenReferencedInInitializer()
+        {
+            await TestInRegularAndScript1Async(
+@"
+using System.Collections.Generic;
+
+class C
+{
+    static void M()
+    {
+        var items = new [||]List<object>();
+        items[0] = 1;
+        items[1] = items[0];
+    }
+}",
+@"
+using System.Collections.Generic;
+
+class C
+{
+    static void M()
+    {
+        var items = new [||]List<object>
+        {
+            [0] = 1
+        };
+        items[1] = items[0];
+    }
 }");
         }
     }

diff --git a/...Core/Portable/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs b/...Core/Portable/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs
@@ -50,7 +50,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
             return SpecializedTasks.EmptyTask;
         }
 
-        protected override Task FixAllAsync(
+        protected override async Task FixAllAsync(
             Document document, ImmutableArray<Diagnostic> diagnostics, 
             SyntaxEditor editor, CancellationToken cancellationToken)
         {
@@ -78,13 +78,17 @@ protected override Task FixAllAsync(
             // care about so we can find them across each edit.
             var currentRoot = originalRoot.TrackNodes(originalObjectCreationNodes);
 
+            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            (semanticModel, currentRoot) = GetCurrentSemanticModelAndRoot(
+                semanticModel, currentRoot, cancellationToken);
+
             while (originalObjectCreationNodes.Count > 0)
             {
                 var originalObjectCreation = originalObjectCreationNodes.Pop();
                 var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single();
 
                 var analyzer = new ObjectCreationExpressionAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TVariableDeclaratorSyntax>(
-                    syntaxFacts, objectCreation);
+                    semanticModel, syntaxFacts, objectCreation, cancellationToken);
                 var matches = analyzer.Analyze();
                 if (matches == null || matches.Value.Length == 0)
                 {
@@ -103,11 +107,27 @@ protected override Task FixAllAsync(
                     subEditor.RemoveNode(match);
                 }
 
-                currentRoot = subEditor.GetChangedRoot();
+                var newRoot = subEditor.GetChangedRoot();
+                (semanticModel, currentRoot) = GetCurrentSemanticModelAndRoot(
+                    semanticModel, newRoot, cancellationToken);
             }
 
             editor.ReplaceNode(originalRoot, currentRoot);
-            return SpecializedTasks.EmptyTask;
+        }
+
+        private static (SemanticModel, SyntaxNode) GetCurrentSemanticModelAndRoot(
+            SemanticModel semanticModel, SyntaxNode newRoot, CancellationToken cancellationToken)
+        {
+            var oldSyntaxTree = semanticModel.SyntaxTree;
+            var newSyntaxTree = semanticModel.SyntaxTree.WithRootAndOptions(
+                newRoot, oldSyntaxTree.Options);
+
+            var newCompilation = semanticModel.Compilation.ReplaceSyntaxTree(oldSyntaxTree, newSyntaxTree);
+
+            var currentSemanticModel = newCompilation.GetSemanticModel(newSyntaxTree);
+            var currentRoot = currentSemanticModel.SyntaxTree.GetRoot(cancellationToken);
+
+            return (currentSemanticModel, currentRoot);
         }
 
         protected abstract TStatementSyntax GetNewStatement(

diff --git a/...e/Portable/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs b/...e/Portable/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs
@@ -63,10 +63,12 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
                 return;
             }
 
+            var semanticModel = context.SemanticModel;
             var objectCreationExpression = (TObjectCreationExpressionSyntax)context.Node;
             var language = objectCreationExpression.Language;
             var syntaxTree = objectCreationExpression.SyntaxTree;
             var cancellationToken = context.CancellationToken;
+
             var optionSet = context.Options.GetDocumentOptionSetAsync(syntaxTree, cancellationToken).GetAwaiter().GetResult();
             if (optionSet == null)
             {
@@ -89,7 +91,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
             }
 
             var analyzer = new ObjectCreationExpressionAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TVariableDeclaratorSyntax>(
-                GetSyntaxFactsService(), objectCreationExpression);
+                semanticModel, GetSyntaxFactsService(), objectCreationExpression, cancellationToken);
             var matches = analyzer.Analyze();
             if (matches == null || matches.Value.Length == 0)
             {

diff --git a/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs b/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs
@@ -2,7 +2,10 @@
 
 using System.Collections;
 using System.Collections.Immutable;
+using System.Linq;
+using System.Threading;
 using Microsoft.CodeAnalysis.LanguageServices;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.UseCollectionInitializer
 {
@@ -22,18 +25,25 @@ internal struct ObjectCreationExpressionAnalyzer<
         where TExpressionStatementSyntax : TStatementSyntax
         where TVariableDeclaratorSyntax : SyntaxNode
     {
+        private readonly SemanticModel _semanticModel;
         private readonly ISyntaxFactsService _syntaxFacts;
         private readonly TObjectCreationExpressionSyntax _objectCreationExpression;
+        private readonly CancellationToken _cancellationToken;
 
         private TStatementSyntax _containingStatement;
         private SyntaxNodeOrToken _valuePattern;
+        private ISymbol _variableSymbol;
 
         public ObjectCreationExpressionAnalyzer(
+            SemanticModel semanticModel,
             ISyntaxFactsService syntaxFacts,
-            TObjectCreationExpressionSyntax objectCreationExpression) : this()
+            TObjectCreationExpressionSyntax objectCreationExpression,
+            CancellationToken cancellationToken) : this()
         {
+            _semanticModel = semanticModel;
             _syntaxFacts = syntaxFacts;
             _objectCreationExpression = objectCreationExpression;
+            _cancellationToken = cancellationToken;
         }
 
         internal ImmutableArray<TExpressionStatementSyntax>? Analyze()
@@ -146,6 +156,21 @@ private bool TryAnalyzeIndexAssignment(
                 return false;
             }
 
+            // If we're initializing a variable, then we can't reference that variable on the right 
+            // side of the initialization.  Rewriting this into a collection initializer would lead
+            // to a definite-assignment error.
+            if (_variableSymbol != null)
+            {
+                foreach (var child in right.DescendantNodesAndSelf().OfType<TExpressionSyntax>())
+                {
+                    if (ValuePatternMatches(child) &&
+                        _variableSymbol.Equals(_semanticModel.GetSymbolInfo(child, _cancellationToken).GetAnySymbol()))
+                    {
+                        return false;
+                    }
+                }
+            }
+
             instance = _syntaxFacts.GetExpressionOfElementAccessExpression(left);
             return true;
         }
@@ -251,6 +276,7 @@ private bool TryInitializeVariableDeclarationCase()
             }
 
             _valuePattern = _syntaxFacts.GetIdentifierOfVariableDeclarator(containingDeclarator);
+            _variableSymbol = _semanticModel.GetDeclaredSymbol(containingDeclarator);
             return true;
         }
     }