[Refactoring] Make NamingHelper less horrible and remove the helper from RefactoringContext.

ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs

@@ -129,7 +129,62 @@ namespace ICSharpCode.NRefactory.CSharp.Completion
 			}
 			return Enumerable.Empty<ICompletionData>();
 		}
-
+		
+		IEnumerable<string> GenerateNameProposals(AstType type)
+		{
+			if (type is PrimitiveType) {
+				var pt = (PrimitiveType)type;
+				switch (pt.Keyword) {
+					case "object":
+						yield return "o";
+						yield return "obj";
+						break;
+					case "bool":
+						yield return "b";
+						yield return "pred";
+						break;
+					case "double":
+					case "float":
+					case "decimal":
+						yield return "d";
+						yield return "f";
+						yield return "m";
+						break;
+					default:
+						yield return "i";
+						yield return "j";
+						yield return "k";
+						break;
+				}
+				yield break;
+			}
+			string name;
+			if (type is SimpleType) {
+				name = ((SimpleType)type).Identifier;
+			} else if (type is MemberType) {
+				name = ((MemberType)type).MemberName;
+			} else {
+				yield break;
+			}
+			
+			var names = WordParser.BreakWords(name);
+			
+			var possibleName = new StringBuilder();
+			for (int i = 0; i < names.Count; i++) {
+				possibleName.Length = 0;
+				for (int j = i; j < names.Count; j++) {
+					if (string.IsNullOrEmpty(names [j])) {
+						continue;
+					}
+					if (j == i) { 
+						names [j] = Char.ToLower(names [j] [0]) + names [j].Substring(1);
+					}
+					possibleName.Append(names [j]);
+				}
+				yield return possibleName.ToString();
+			}
+		}
+		
 		IEnumerable<ICompletionData> HandleMemberReferenceCompletion(ExpressionResult expr)
 		{
 			if (expr == null) 
@@ -354,7 +409,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion
 						if (parent.Variables.Count != 1)
 							return DefaultControlSpaceItems(isAsExpression, controlSpace);
 					
-						foreach (var possibleName in NamingHelper.GenerateNameProposals (parent.Type)) {
+						foreach (var possibleName in GenerateNameProposals (parent.Type)) {
 							if (possibleName.Length > 0) {
 								proposeNameList.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString()));
 							}
@@ -1064,7 +1119,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion
 			}
 			if (node is Identifier && node.Parent is ForeachStatement) {
 				var foreachStmt = (ForeachStatement)node.Parent;
-				foreach (var possibleName in NamingHelper.GenerateNameProposals (foreachStmt.VariableType)) {
+				foreach (var possibleName in GenerateNameProposals (foreachStmt.VariableType)) {
 					if (possibleName.Length > 0) {
 						wrapper.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString()));
 					}
@@ -1082,7 +1137,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion
 				// Try Parameter name case 
 				var param = node.Parent as ParameterDeclaration;
 				if (param != null) {
-					foreach (var possibleName in NamingHelper.GenerateNameProposals (param.Type)) {
+					foreach (var possibleName in GenerateNameProposals (param.Type)) {
 						if (possibleName.Length > 0) {
 							wrapper.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString()));
 						}

ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/IterateViaForeachAction.cs

@@ -116,9 +116,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 
 		static ForeachStatement MakeForeach(Expression node, IType type, RefactoringContext context)
 		{
+			var namingHelper = new NamingHelper(context);
 			return new ForeachStatement() {
 				VariableType = new SimpleType("var"),
-				VariableName = context.GenerateVariableName(type),
+				VariableName = namingHelper.GenerateVariableName(type),
 				InExpression = node.Clone(),
 				EmbeddedStatement = new BlockStatement()
 			};

ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs

@@ -33,17 +33,17 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 {
 	public class NamingHelper
 	{
-		ISet<string> UsedVariableNames;
+		ISet<string> usedVariableNames;
 		RefactoringContext context;
 
 		public NamingHelper(RefactoringContext context)
 		{
 			this.context = context;
-			var astNode = context.GetNode<Statement>();
-			if (UsedVariableNames == null) {
+			if (usedVariableNames == null) {
 				var visitor = new VariableFinderVisitor();
+				var astNode = context.GetNode<Statement>();
 				astNode.AcceptVisitor(visitor);
-				UsedVariableNames = visitor.VariableNames;
+				usedVariableNames = visitor.VariableNames;
 			}
 		}
 
@@ -88,21 +88,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			}
 
 			var names = WordParser.BreakWords(name);
-
-			var possibleName = new StringBuilder();
-			for (int i = 0; i < names.Count; i++) {
-				possibleName.Length = 0;
-				for (int j = i; j < names.Count; j++) {
-					if (string.IsNullOrEmpty(names [j])) {
-						continue;
-					}
-					if (j == i) { 
-						names [j] = Char.ToLower(names [j] [0]) + names [j].Substring(1);
-					}
-					possibleName.Append(names [j]);
-				}
-				yield return possibleName.ToString();
+			if (names.Count > 0) {
+				names [0] = Char.ToLower(names [0] [0]) + names [0].Substring(1);
 			}
+			yield return string.Join("", names);
 		}
 
 		/// <summary>
@@ -119,8 +108,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			string firstSuggestion = null;
 			foreach (var name in NamingHelper.GenerateNameProposals(type)) {
 				firstSuggestion = firstSuggestion ?? name;
-				if (NameIsUnused(name))
+				if (NameIsUnused(name)) {
+					usedVariableNames.Add(name);
 					return name;
+				}
 			}
 			// If we get here, all of the standard suggestions are already used.
 			// This will at least be the second variable named based on firstSuggestion, so start at 2
@@ -129,12 +120,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			do {
 				proposedName = firstSuggestion + counter++;
 			} while (!NameIsUnused(proposedName));
+			usedVariableNames.Add(proposedName);
 			return proposedName;
 		}
 
 		bool NameIsUnused(string name)
 		{
-			return !UsedVariableNames.Contains(name) && LookupVariable(name) == null;
+			return !usedVariableNames.Contains(name) && LookupVariable(name) == null;
 		}
 
 		/// <summary>
@@ -192,8 +184,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 
 		IVariable LookupVariable(string name)
 		{
-			var astNode = context.GetNode<BlockStatement>();
-			var resolverState = context.Resolver.GetResolverStateAfter(astNode.LastChild.PrevSibling);
+			var blockStatement = context.GetNode<BlockStatement>();
+			var resolverState = context.GetResolverStateAfter(blockStatement.RBraceToken.PrevSibling);
 			var simpleNameRR = resolverState.ResolveSimpleName(name, new List<IType>()) as LocalResolveResult;
 			if (simpleNameRR == null)
 				return null;

ICSharpCode.NRefactory.CSharp/Refactoring/RefactoringContext.cs

@@ -154,21 +154,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 		{
 			return baseName + (number > 0 ? (number + 1).ToString () : "");
 		}
-
-		NamingHelper namingHelper = null;
-		public string GenerateVariableName(AstType type)
-		{
-			if (namingHelper == null)
-				namingHelper = new NamingHelper(this);
-			return namingHelper.GenerateVariableName(type);
-		}
-
-		public string GenerateVariableName(IType type)
-		{
-			if (namingHelper == null)
-				namingHelper = new NamingHelper(this);
-			return namingHelper.GenerateVariableName(type);
-		}
 		#endregion
 	}
 }

ICSharpCode.NRefactory.Tests/CSharp/Refactoring/NamingHelperTests.cs

@@ -27,11 +27,9 @@ using NUnit.Framework;
 using ICSharpCode.NRefactory.CSharp.Refactoring;
 using ICSharpCode.NRefactory.CSharp.CodeActions;
 using ICSharpCode.NRefactory.CSharp;
-using System.Linq;
 
 namespace ICSharpCode.NRefactory.CSharp.Refactoring
 {
-
 		[TestFixture]
 		public class NamingHelperTests
 		{
@@ -51,7 +49,7 @@ class A
 	{ $ }
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.AreEqual("i", name);
 				}
@@ -69,7 +67,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -87,7 +85,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -105,7 +103,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -122,7 +120,7 @@ class A
 			$
 	}
 }", true);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -139,7 +137,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -156,7 +154,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
 				}
@@ -177,7 +175,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.AreEqual("i3", name);
 				}
@@ -202,7 +200,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.IsTrue(name == "i");
 				}
@@ -221,7 +219,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.AreEqual("i2", name);
 				}
@@ -229,7 +227,6 @@ class A
 				[Test]
 				public void GenerateVariableNameIgnoresFixedVariables()
 				{
-						// Snippet tests that identifiers from in, into and let clauses are found
 						var context = MakeContext(@"
 class A
 {
@@ -240,7 +237,7 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.AreEqual("j", name);
 				}
@@ -258,13 +255,13 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int"));
 						Assert.NotNull(name);
 						Assert.AreEqual("i2", name);
 				}
-
+		
 				[Test]
-				public void GenerateVariableNameForComposedType()
+				public void GenerateVariableNameForCustomType()
 				{
 						var context = MakeContext(@"
 class A
@@ -275,9 +272,29 @@ class A
 	}
 }"
 						);
-						var name = context.GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" });
+						var name = new NamingHelper(context).GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" });
 						Assert.NotNull(name);
 						Assert.AreEqual("variableNameGenerationTester", name);
 				}
+		
+				[Test]
+				public void GenerateVariableNameDoesNotRepeatNames()
+				{
+						var context = MakeContext(@"
+class A
+{
+	void F()
+	{
+		$
+	}
+}"
+						);
+						var namingHelper = new NamingHelper(context);
+						var name1 = namingHelper.GenerateVariableName(new PrimitiveType("int"));
+						var name2 = namingHelper.GenerateVariableName(new PrimitiveType("int"));
+						Assert.NotNull(name1);
+						Assert.NotNull(name2);
+						Assert.AreNotEqual(name1, name2, "The generated names should not repeat.");
+				}
 		}
 }