From 243b39310e051bb90fe0aca34bbaf90d47c2670c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mike=20Kr=C3=BCger?= <mkrueger@xamarin.com>
Date: Wed, 30 Jan 2013 09:32:29 +0100
Subject: [PATCH] Fixed some convert foreach to for action bugs.

---
 .../CodeActions/ConvertForeachToForAction.cs  | 49 ++++++++++++++++---
 .../CodeActions/ConvertForeachToForTests.cs   | 31 ++++++++++++
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs
index 6444cdd5af..53a4162c6a 100644
--- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs
+++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs
@@ -37,30 +37,63 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 	[ContextAction("Convert 'foreach' loop to 'for'", Description = "Works on 'foreach' loops that allow direct access to its elements.")]
 	public class ConvertForeachToForAction : ICodeActionProvider
 	{
+		static string[] VariableNames = new string[] { "i", "j", "k", "l", "n", "m", "x", "y", "z"};
+		static string[] CollectionNames = new string[] { "list" };
+
+		static string GetName(ICSharpCode.NRefactory.CSharp.Resolver.CSharpResolver state, string[] variableNames)
+		{
+			for (int i = 0; i < 1000; i++) {
+				foreach (var vn in variableNames) {
+					string id = i > 0 ? vn + i : vn;
+					var rr = state.LookupSimpleNameOrTypeName(id, new IType[0], NameLookupMode.Expression);
+					if (rr.IsError) 
+						return id;
+				}
+			}
+			return null;
+		}
+
 		public IEnumerable<CodeAction> GetActions(RefactoringContext context)
 		{
 			var foreachStatement = GetForeachStatement(context);
 			if (foreachStatement == null) {
 				yield break;
 			}
+
+			var state = context.GetResolverStateBefore (foreachStatement.EmbeddedStatement);
+			string name = GetName(state, VariableNames);
+			if (name == null) // very unlikely, but just in case ...
+				yield break;
+
 			yield return new CodeAction(context.TranslateString("Convert 'foreach' loop to 'for'"), script => {
 				var result = context.Resolve(foreachStatement.InExpression);
 				var countProperty = GetCountProperty(result.Type);
-				
+
 				// TODO: use another variable name if 'i' is already in use
-				var initializer = new VariableDeclarationStatement(new PrimitiveType("int"), "i", new PrimitiveExpression(0));
-				var id1 = new IdentifierExpression("i");
+				var initializer = new VariableDeclarationStatement(new PrimitiveType("int"), name, new PrimitiveExpression(0));
+				var id1 = new IdentifierExpression(name);
 				var id2 = id1.Clone();
 				var id3 = id1.Clone();
-				
+				var inExpression = foreachStatement.InExpression;
+				Statement declarationStatement = null;
+				if (inExpression is ObjectCreateExpression || inExpression is ArrayCreateExpression) {
+					string listName = GetName(state, CollectionNames) ?? "col";
+					declarationStatement = new VariableDeclarationStatement (
+						new PrimitiveType ("var"),
+						listName,
+						inExpression.Clone ()
+					);
+					inExpression = new IdentifierExpression (listName);
+				}
+
 				var variableDeclarationStatement = new VariableDeclarationStatement(
 					foreachStatement.VariableType.Clone(),
 					foreachStatement.VariableName,
-					new IndexerExpression(foreachStatement.InExpression.Clone(), id3)
-				);
+					new IndexerExpression(inExpression.Clone(), id3)
+					);
 				var forStatement = new ForStatement() {
 					Initializers = { initializer },
-					Condition = new BinaryOperatorExpression (id1, BinaryOperatorType.LessThan, new MemberReferenceExpression (foreachStatement.InExpression.Clone (), countProperty)),
+					Condition = new BinaryOperatorExpression (id1, BinaryOperatorType.LessThan, new MemberReferenceExpression (inExpression.Clone (), countProperty)),
 					Iterators = { new ExpressionStatement (new UnaryOperatorExpression (UnaryOperatorType.PostIncrement, id2)) },
 					EmbeddedStatement = new BlockStatement {
 						variableDeclarationStatement
@@ -79,6 +112,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 				} else {
 					forStatement.EmbeddedStatement.AddChild (foreachStatement.EmbeddedStatement.Clone (), BlockStatement.StatementRole);
 				}
+				if (declarationStatement != null)
+					script.InsertBefore (foreachStatement, declarationStatement);
 				script.Replace (foreachStatement, forStatement);
 				script.Link (initializer.Variables.First ().NameToken, id1, id2, id3);
 			});
diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs
index 89c7ed3ec3..00b3f86f0b 100644
--- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs
+++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs
@@ -113,5 +113,36 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 				"}"
 			);
 		}
+
+		/// <summary>
+		/// Bug 9876 - Convert to for loop created invalid code if iteration variable is called i
+		/// </summary>
+		[Test]
+		public void TestBug9876 ()
+		{
+			Test<ConvertForeachToForAction> (@"class TestClass
+{
+	void TestMethod ()
+	{
+		$foreach (var i in new[] { 1, 2, 3 }) {
+			Console.WriteLine (i);
+		}
+	}
+}", @"class TestClass
+{
+	void TestMethod ()
+	{
+		var list = new[] {
+			1,
+			2,
+			3
+		};
+		for (int j = 0; j < list.Length; j++) {
+			var i = list [j];
+			Console.WriteLine (i);
+		}
+	}
+}");
+		}
 	}
 }
\ No newline at end of file