[Refactoring] Don't suggest names that are used in a parent scope when falling back to numbered names.

ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs

@@ -119,7 +119,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			string firstSuggestion = null;
 			foreach (var name in NamingHelper.GenerateNameProposals(type)) {
 				firstSuggestion = firstSuggestion ?? name;
-				if (!UsedVariableNames.Contains(name) && LookupVariable(name) == null)
+				if (NameIsUnused(name))
 					return name;
 			}
 			// If we get here, all of the standard suggestions are already used.
@@ -128,10 +128,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			string proposedName;
 			do {
 				proposedName = firstSuggestion + counter++;
-			} while (UsedVariableNames.Contains(proposedName));
+			} while (!NameIsUnused(proposedName));
 			return proposedName;
 		}
 
+		bool NameIsUnused(string name)
+		{
+			return !UsedVariableNames.Contains(name) && LookupVariable(name) == null;
+		}
+
 		/// <summary>
 		/// Generates a variable name for a variable of the specified type.
 		/// </summary>

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

@@ -32,33 +32,34 @@ using System.Linq;
 namespace ICSharpCode.NRefactory.CSharp.Refactoring
 {
 
-	[TestFixture]
+		[TestFixture]
-	public class NamingHelperTests
+		public class NamingHelperTests
-	{
-		RefactoringContext MakeContext(string input, bool expectErrors = false)
 		{
-			var context = TestRefactoringContext.Create (input, expectErrors);
+				RefactoringContext MakeContext(string input, bool expectErrors = false)
-			return context;
+				{
-		}
+						var context = TestRefactoringContext.Create(input, expectErrors);
+						return context;
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameTest()
+				public void GenerateVariableNameTest()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
 	{ $ }
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.AreEqual("i", name);
+						Assert.NotNull(name);
-		}
+						Assert.AreEqual("i", name);
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameIgnoresNamesUsedPreviouslyInScope()
+				public void GenerateVariableNameIgnoresNamesUsedPreviouslyInScope()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -66,16 +67,17 @@ class A
 		int i;
 		$
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.NotNull(name);
-		}
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameIgnoresNamesUsedLaterInScope()
+				public void GenerateVariableNameIgnoresNamesUsedLaterInScope()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -83,16 +85,17 @@ class A
 		$
 		int i;
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.NotNull(name);
-		}
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameIgnoresNamesUsedInNestedScope()
+				public void GenerateVariableNameIgnoresNamesUsedInNestedScope()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -100,16 +103,17 @@ class A
 		$
 		for (int i; i < 0; i++);
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.NotNull(name);
-		}
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameInForIgnoresIterator()
+				public void GenerateVariableNameInForIgnoresIterator()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -118,47 +122,70 @@ class A
 			$
 	}
 }", true);
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.NotNull(name);
+						Assert.NotNull(name);
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
-		}
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameInMethodIgnoresParameters()
+				public void GenerateVariableNameInMethodIgnoresParameters()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F(int i)
 	{
 		$
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.NotNull(name);
-		}
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameInForInitializerList()
+				public void GenerateVariableNameInForInitializerList()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F(int i)
 	{
 		for($ ; i < 0; i++);
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+						Assert.NotNull(name);
-		}
+						Assert.IsFalse(name == "i", "i was already used and should not be proposed.");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameShouldNotIgnoreBasedOnMethodCallIdentifiers()
+				public void GenerateVariableNameWithNumberedVariableInParentBlock()
+				{
+						var context = MakeContext(@"
+class A
+{
+	void F()
+	{
+		int i2;
 		{
-			var context = MakeContext(@"
+			int i, j, k;
+			$
+		}
+	}
+}"
+						);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
+						Assert.NotNull(name);
+						Assert.AreEqual("i3", name);
+				}
+
+				[Test]
+				public void GenerateVariableNameShouldNotIgnoreBasedOnMethodCallIdentifiers()
+				{
+						var context = MakeContext(@"
 class B
 {
 	void i()
@@ -173,17 +200,18 @@ class A
 		for($ ;;)
 			B.i();
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.IsTrue(name == "i");
+						Assert.NotNull(name);
-		}
+						Assert.IsTrue(name == "i");
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameIgnoresLinqIdentifiers()
+				public void GenerateVariableNameIgnoresLinqIdentifiers()
-		{
+				{
-			// Snippet tests that identifiers from in, into and let clauses are found
+						// Snippet tests that identifiers from in, into and let clauses are found
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -191,17 +219,18 @@ class A
 		$
 		var ints = from i in new int [] {} group i by i % 2 into j let k = 2 select j.Count() + k;
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.AreEqual("i2", name);
+						Assert.NotNull(name);
-		}
+						Assert.AreEqual("i2", name);
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameIgnoresFixedVariables()
+				public void GenerateVariableNameIgnoresFixedVariables()
-		{
+				{
-			// Snippet tests that identifiers from in, into and let clauses are found
+						// Snippet tests that identifiers from in, into and let clauses are found
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	unsafe void F()
@@ -209,16 +238,17 @@ class A
 		$
 		fixed (int i = 13) {}
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.AreEqual("j", name);
+						Assert.NotNull(name);
-		}
+						Assert.AreEqual("j", name);
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameFallsBackToNumbering()
+				public void GenerateVariableNameFallsBackToNumbering()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
@@ -226,26 +256,28 @@ class A
 		int i, j, k;
 		$
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new PrimitiveType("int"));
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new PrimitiveType("int"));
-			Assert.AreEqual("i2", name);
+						Assert.NotNull(name);
-		}
+						Assert.AreEqual("i2", name);
+				}
 
-		[Test]
+				[Test]
-		public void GenerateVariableNameForComposedType()
+				public void GenerateVariableNameForComposedType()
-		{
+				{
-			var context = MakeContext(@"
+						var context = MakeContext(@"
 class A
 {
 	void F()
 	{
 		$
 	}
-}");
+}"
-			var name = context.GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" });
+						);
-			Assert.NotNull(name);
+						var name = context.GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" });
-			Assert.AreEqual("variableNameGenerationTester", name);
+						Assert.NotNull(name);
+						Assert.AreEqual("variableNameGenerationTester", name);
+				}
 		}
-	}
 }