Add checks for unexpected usages of hte collection.

src/EditorFeatures/CSharpTest/ConvertForToForEach/ConvertForToForEachTests.cs

@@ -91,6 +91,108 @@ void Test(string[] array)
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsConvertForToForEach)]
+        public async Task TestWarnIfCollectionPotentiallyMutated1()
+        {
+            await TestInRegularAndScript1Async(
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        [||]for (int i = 0; i < list.Count; i++)
+        {
+            Console.WriteLine(list[i]);
+            list.Add(null);
+        }
+    }
+}",
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        foreach (string {|Rename:v|} in list)
+        {
+            Console.WriteLine(v);
+            {|Warning:list|}.Add(null);
+        }
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsConvertForToForEach)]
+        public async Task TestWarnIfCollectionPotentiallyMutated2()
+        {
+            await TestInRegularAndScript1Async(
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        [||]for (int i = 0; i < list.Count; i++)
+        {
+            Console.WriteLine(list[i]);
+            list = null;
+        }
+    }
+}",
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        foreach (string {|Rename:v|} in list)
+        {
+            Console.WriteLine(v);
+            {|Warning:list|} = null;
+        }
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsConvertForToForEach)]
+        public async Task TestNoWarnIfCollectionPropertyAccess()
+        {
+            await TestInRegularAndScript1Async(
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        [||]for (int i = 0; i < list.Count; i++)
+        {
+            Console.WriteLine(list[i]);
+            Console.WriteLine(list.Count);
+        }
+    }
+}",
+@"using System;
+using System.Collections.Generic;
+
+class C
+{
+    void Test(IList<string> list)
+    {
+        foreach (string {|Rename:v|} in list)
+        {
+            Console.WriteLine(v);
+            Console.WriteLine(list.Count);
+        }
+    }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsConvertForToForEach)]
         public async Task TestNoWarnIfDoesNotCrossFunctionBoundary()
         {

src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs

@@ -421,24 +421,15 @@ public TestParameters WithFixProviderData(object fixProviderData)
 
             void TestAnnotations(ImmutableArray<TextSpan> expectedSpans, string annotationKind)
             {
-                var annotatedTokens = fixedRoot.GetAnnotatedNodesAndTokens(annotationKind).Select(n => (SyntaxToken)n).ToList();
+                var annotatedItems = fixedRoot.GetAnnotatedNodesAndTokens(annotationKind).OrderBy(s => s.SpanStart).ToList();
 
-                Assert.Equal(expectedSpans.Length, annotatedTokens.Count);
+                Assert.Equal(expectedSpans.Length, annotatedItems.Count);
 
-                if (expectedSpans.Length > 0)
+                for (var i = 0; i < Math.Min(expectedSpans.Length, annotatedItems.Count); i++)
                 {
-                    var expectedTokens = TokenUtilities.GetTokens(TokenUtilities.GetSyntaxRoot(expectedText, GetLanguage(), parseOptions));
-                    var actualTokens = TokenUtilities.GetTokens(fixedRoot);
-
-                    for (var i = 0; i < Math.Min(expectedTokens.Count, actualTokens.Count); i++)
-                    {
-                        var expectedToken = expectedTokens[i];
-                        var actualToken = actualTokens[i];
-
-                        var actualIsConflict = annotatedTokens.Contains(actualToken);
-                        var expectedIsConflict = expectedSpans.Contains(expectedToken.Span);
-                        Assert.Equal(expectedIsConflict, actualIsConflict);
-                    }
+                    var actual = annotatedItems[i].Span;
+                    var expected = expectedSpans[i];
+                    Assert.Equal(expected, actual);
                 }
             }
         }

src/EditorFeatures/VisualBasicTest/ConvertForToForEach/ConvertForToForEachTests.vb

-Imports Microsoft.CodeAnalysis.CodeRefactorings
+' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+Imports Microsoft.CodeAnalysis.CodeRefactorings
 Imports Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeRefactorings
 Imports Microsoft.CodeAnalysis.VisualBasic.ConvertForToForEach
 

src/Features/Core/Portable/ConvertForToForEach/AbstractConvertForToForEachCodeRefactoringProvider.cs

@@ -161,7 +161,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
             var bodyStatements = GetBodyStatements(forStatement);
             foreach (var statement in bodyStatements)
             {
-                if (iterationVariableIsUsedForMoreThanCollectionIndex(statement))
+                if (IterationVariableIsUsedForMoreThanCollectionIndex(statement))
                 {
                     return;
                 }
@@ -173,8 +173,10 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
                     document, forStatement, iterationVariable, collectionExpression,
                     containingType, collectionType.Type, iterationType, c)));
 
+            return;
+
             // local functions
-            bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
+            bool IterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
             {
                 if (syntaxFacts.IsIdentifierName(current))
                 {
@@ -215,7 +217,7 @@ bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
                 {
                     if (child.IsNode)
                     {
-                        if (iterationVariableIsUsedForMoreThanCollectionIndex(child.AsNode()))
+                        if (IterationVariableIsUsedForMoreThanCollectionIndex(child.AsNode()))
                         {
                             return true;
                         }
@@ -327,7 +329,7 @@ bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
             //      var x = list[i]   or
             //
             // If so, we'll use those as the iteration variables for the new foreach statement.
-            var (typeNode, foreachIdentifier, declarationStatement) = tryDeconstructInitialDeclaration();
+            var (typeNode, foreachIdentifier, declarationStatement) = TryDeconstructInitialDeclaration();
 
             if (typeNode == null)
             {
@@ -355,7 +357,7 @@ bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
             var foreachIdentifierReference = foreachIdentifier.WithoutAnnotations(RenameAnnotation.Kind).WithoutTrivia();
 
             // Walk the for statement, replacing any matches we find.
-            findAndReplaceMatches(forStatement);
+            FindAndReplaceMatches(forStatement);
 
             // Finally, remove the declaration statement if we found one.  Move all its leading
             // trivia to the next statement.
@@ -375,7 +377,7 @@ bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
             return document.WithSyntaxRoot(editor.GetChangedRoot());
 
             // local functions
-            (TTypeNode, SyntaxToken, TStatementSyntax) tryDeconstructInitialDeclaration()
+            (TTypeNode, SyntaxToken, TStatementSyntax) TryDeconstructInitialDeclaration()
             {
                 var bodyStatements = GetBodyStatements(forStatement);
 
@@ -407,40 +409,65 @@ bool iterationVariableIsUsedForMoreThanCollectionIndex(SyntaxNode current)
                 return default;
             }
 
-            void findAndReplaceMatches(SyntaxNode current)
+            void FindAndReplaceMatches(SyntaxNode current)
             {
-                if (syntaxFacts.AreEquivalent(current, indexExpression))
+                if (SemanticEquivalence.AreEquivalent(semanticModel, current, collectionExpression))
                 {
-                    // Found a match.  replace with iteration variable.
-                    var replacementToken = foreachIdentifierReference;
-
-                    if (semanticFacts.IsWrittenTo(semanticModel, current, cancellationToken))
+                    if (syntaxFacts.AreEquivalent(current.Parent, indexExpression))
                     {
-                        replacementToken = replacementToken.WithAdditionalAnnotations(
-                            WarningAnnotation.Create(FeaturesResources.Warning_colon_Collection_was_modified_during_iteration));
-                    }
+                        var indexMatch = current.Parent;
+                        // Found a match.  replace with iteration variable.
+                        var replacementToken = foreachIdentifierReference;
+
+                        if (semanticFacts.IsWrittenTo(semanticModel, indexMatch, cancellationToken))
+                        {
+                            replacementToken = replacementToken.WithAdditionalAnnotations(
+                                WarningAnnotation.Create(FeaturesResources.Warning_colon_Collection_was_modified_during_iteration));
+                        }
+
+                        if (CrossesFunctionBoundary(current))
+                        {
+                            replacementToken = replacementToken.WithAdditionalAnnotations(
+                                WarningAnnotation.Create(FeaturesResources.Warning_colon_Iteration_variable_crossed_function_boundary));
+                        }
 
-                    if (crossesFunctionBoundary(current))
+                        editor.ReplaceNode(
+                            indexMatch,
+                            generator.IdentifierName(replacementToken).WithTriviaFrom(indexMatch));
+                    }
+                    else
                     {
-                        replacementToken = replacementToken.WithAdditionalAnnotations(
-                            WarningAnnotation.Create(FeaturesResources.Warning_colon_Iteration_variable_crossed_function_boundary));
+                        // Collection was used for some other purpose.  If it's passed as an argument
+                        // to something, or is written to, or has a method invoked on it, we'll warn
+                        // that it's potentially changing and may break if you switch to a foreach loop.
+                        var shouldWarn = syntaxFacts.IsArgument(current.Parent);
+                        shouldWarn |= semanticFacts.IsWrittenTo(semanticModel, current, cancellationToken);
+                        shouldWarn |=
+                            syntaxFacts.IsAnyMemberAccessExpression(current.Parent) &&
+                            syntaxFacts.IsInvocationExpression(current.Parent.Parent);
+
+                        if (shouldWarn)
+                        {
+                            editor.ReplaceNode(
+                                current,
+                                (node, _) => node.WithAdditionalAnnotations(
+                                    WarningAnnotation.Create(FeaturesResources.Warning_colon_Iteration_variable_crossed_function_boundary)));
+                        }
                     }
 
-                    var replacement = generator.IdentifierName(replacementToken).WithTriviaFrom(current);
-
-                    editor.ReplaceNode(current, replacement);
+                    return;
                 }
 
                 foreach (var child in current.ChildNodesAndTokens())
                 {
                     if (child.IsNode)
                     {
-                        findAndReplaceMatches(child.AsNode());
+                        FindAndReplaceMatches(child.AsNode());
                     }
                 }
             }
 
-            bool crossesFunctionBoundary(SyntaxNode node)
+            bool CrossesFunctionBoundary(SyntaxNode node)
             {
                 var containingFunction = node.AncestorsAndSelf().FirstOrDefault(
                     n => syntaxFacts.IsLocalFunctionStatement(n) || syntaxFacts.IsAnonymousFunction(n));

src/Features/Core/Portable/FeaturesResources.Designer.cs

@@ -3698,6 +3698,15 @@ internal class FeaturesResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Warning: Collection may be modified during iteration..
+        /// </summary>
+        internal static string Warning_colon_Collection_may_be_modified_during_iteration {
+            get {
+                return ResourceManager.GetString("Warning_colon_Collection_may_be_modified_during_iteration", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Warning: Collection was modified during iteration..
         /// </summary>

src/Features/Core/Portable/FeaturesResources.resx

@@ -1343,4 +1343,7 @@ This version used in: {2}</value>
   <data name="Warning_colon_Iteration_variable_crossed_function_boundary" xml:space="preserve">
     <value>Warning: Iteration variable crossed function boundary.</value>
   </data>
-</root>
+  <data name="Warning_colon_Collection_may_be_modified_during_iteration" xml:space="preserve">
+    <value>Warning: Collection may be modified during iteration.</value>
+  </data>
+</root>
\ No newline at end of file

src/Features/Core/Portable/IntroduceVariable/AbstractIntroduceVariableService.cs

@@ -271,7 +271,7 @@ where NodeMatchesExpression(originalSemanticModel, currentSemanticModel, syntaxF
                 if (allOccurrences &&
                     this.CanReplace(nodeInCurrent))
                 {
-                    return SemanticEquivalence.AreSemanticallyEquivalent(
+                    return SemanticEquivalence.AreEquivalent(
                         originalSemanticModel, currentSemanticModel, expressionInOriginal, nodeInCurrent);
                 }
             }

src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf

@@ -2000,6 +2000,11 @@ Tato verze se používá zde: {2}.</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.de.xlf

@@ -2000,6 +2000,11 @@ Diese Version wird verwendet in: {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.es.xlf

@@ -2000,6 +2000,11 @@ Esta versión se utiliza en: {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf

@@ -2000,6 +2000,11 @@ Version utilisée dans : {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.it.xlf

@@ -2000,6 +2000,11 @@ Questa versione è usata {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf

@@ -2000,6 +2000,11 @@ This version used in: {2}</source>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf

@@ -2000,6 +2000,11 @@ This version used in: {2}</source>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf

@@ -2000,6 +2000,11 @@ Ta wersja jest używana wersja: {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf

@@ -2000,6 +2000,11 @@ Essa versão é usada no: {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf

@@ -2000,6 +2000,11 @@ This version used in: {2}</source>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf

@@ -2000,6 +2000,11 @@ Bu sürüm şurada kullanılır: {2}</target>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf

@@ -2000,6 +2000,11 @@ This version used in: {2}</source>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf

@@ -2000,6 +2000,11 @@ This version used in: {2}</source>
         <target state="new">Warning: Iteration variable crossed function boundary.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_colon_Collection_may_be_modified_during_iteration">
+        <source>Warning: Collection may be modified during iteration.</source>
+        <target state="new">Warning: Collection may be modified during iteration.</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
\ No newline at end of file

src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs

@@ -691,6 +691,9 @@ public SyntaxNode GetExpressionOfArgument(SyntaxNode node)
         public RefKind GetRefKindOfArgument(SyntaxNode node)
             => (node as ArgumentSyntax).GetRefKind();
 
+        public bool IsArgument(SyntaxNode node)
+            => node.Kind() == SyntaxKind.Argument;
+
         public bool IsSimpleArgument(SyntaxNode node)
         {
             var argument = node as ArgumentSyntax;

src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs

@@ -166,6 +166,7 @@ internal interface ISyntaxFactsService : ILanguageService
         /// no named params, no omitted args).
         /// </summary>
         bool IsSimpleArgument(SyntaxNode node);
+        bool IsArgument(SyntaxNode node);
         RefKind GetRefKindOfArgument(SyntaxNode node);
 
         void GetNameAndArityOfSimpleName(SyntaxNode node, out string name, out int arity);

src/Workspaces/Core/Portable/Shared/Extensions/SemanticEquivalence.cs

@@ -9,7 +9,10 @@ namespace Microsoft.CodeAnalysis.Shared.Extensions
 {
     internal static class SemanticEquivalence
     {
-        public static bool AreSemanticallyEquivalent(
+        public static bool AreEquivalent(SemanticModel semanticModel, SyntaxNode node1, SyntaxNode node2)
+            => AreEquivalent(semanticModel, semanticModel, node1, node2);
+
+        public static bool AreEquivalent(
             SemanticModel semanticModel1,
             SemanticModel semanticModel2,
             SyntaxNode node1,

src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb

@@ -633,6 +633,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return RefKind.None
         End Function
 
+        Public Function IsArgument(node As SyntaxNode) As Boolean Implements ISyntaxFactsService.IsArgument
+            Return TypeOf node Is ArgumentSyntax
+        End Function
+
         Public Function IsSimpleArgument(node As SyntaxNode) As Boolean Implements ISyntaxFactsService.IsSimpleArgument
             Dim argument = TryCast(node, ArgumentSyntax)
             Return argument IsNot Nothing AndAlso Not argument.IsNamed AndAlso Not argument.IsOmitted