Fixed bug in extract method.

ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs

@@ -27,6 +27,7 @@ using System;
 using System.Collections.Generic;
 using ICSharpCode.NRefactory.TypeSystem;
 using System.Linq;
+using ICSharpCode.NRefactory.Semantics;
 
 namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 {
@@ -40,7 +41,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 	
 	public class VariableUsageAnalyzation : DepthFirstAstVisitor
 	{
-//		readonly RefactoringContext context;
+		readonly RefactoringContext context;
 		readonly List<IVariable> usedVariables;
 		
 		Dictionary<IVariable, VariableState> states = new Dictionary<IVariable, VariableState> ();
@@ -50,7 +51,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 		
 		public VariableUsageAnalyzation (RefactoringContext context, List<IVariable> usedVariables)
 		{
-//			this.context = context;
+			this.context = context;
 			this.usedVariables = usedVariables;
 		}
 
@@ -95,7 +96,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 		public override void VisitIdentifierExpression(IdentifierExpression identifierExpression)
 		{
 			if (startLocation.IsEmpty || startLocation <= identifierExpression.StartLocation && identifierExpression.EndLocation <= endLocation) {
-				SetState (identifierExpression.Identifier, VariableState.Used);
+				var result = context.Resolve(identifierExpression) as LocalResolveResult;
+				if (result != null && usedVariables.Contains (result.Variable))
+					SetState (identifierExpression.Identifier, VariableState.Used);
 			}
 		}
 		
@@ -103,8 +106,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 		{
 			if (startLocation.IsEmpty || startLocation <= assignmentExpression.StartLocation && assignmentExpression.EndLocation <= endLocation) {
 				var left = assignmentExpression.Left as IdentifierExpression;
-				if (left != null)
-					SetState(left.Identifier, VariableState.Changed);
+				if (left != null) {
+					var result = context.Resolve(left) as LocalResolveResult;
+					if (result != null && usedVariables.Contains (result.Variable))
+						SetState(left.Identifier, VariableState.Changed);
+				}
 			}
 			base.VisitAssignmentExpression (assignmentExpression);
 		}
@@ -113,8 +119,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 		{
 			if (startLocation.IsEmpty || startLocation <= directionExpression.StartLocation && directionExpression.EndLocation <= endLocation) {
 				var expr = directionExpression.Expression as IdentifierExpression;
-				if (expr != null)
-					SetState(expr.Identifier, VariableState.Changed);
+				if (expr != null) {
+					var result = context.Resolve(expr) as LocalResolveResult;
+					if (result != null && usedVariables.Contains (result.Variable))
+						SetState(expr.Identifier, VariableState.Changed);
+				}
 			}
 			base.VisitDirectionExpression (directionExpression);
 		}
@@ -122,7 +131,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 		public override void VisitVariableInitializer(VariableInitializer variableInitializer)
 		{
 			if (startLocation.IsEmpty || startLocation <= variableInitializer.StartLocation && variableInitializer.EndLocation <= endLocation) {
-				SetState(variableInitializer.Name, variableInitializer.Initializer.IsNull ?  VariableState.None : VariableState.Changed);
+				var result = context.Resolve(variableInitializer) as LocalResolveResult;
+				if (result != null && usedVariables.Contains (result.Variable)) {
+					SetState(variableInitializer.Name, variableInitializer.Initializer.IsNull ?  VariableState.None : VariableState.Changed);
+				}
 			}
 
 			base.VisitVariableInitializer(variableInitializer);
@@ -134,8 +146,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod
 				if (unaryOperatorExpression.Operator == UnaryOperatorType.Increment || unaryOperatorExpression.Operator == UnaryOperatorType.Decrement ||
 					unaryOperatorExpression.Operator == UnaryOperatorType.PostIncrement || unaryOperatorExpression.Operator == UnaryOperatorType.PostDecrement) {
 					var expr = unaryOperatorExpression.Expression as IdentifierExpression;
-					if (expr != null)
-						SetState(expr.Identifier, VariableState.Changed);
+					if (expr != null) {
+						var result = context.Resolve(expr) as LocalResolveResult;
+						if (result != null && usedVariables.Contains (result.Variable))
+							SetState(expr.Identifier, VariableState.Changed);
+					}
 				}
 			}
 			base.VisitUnaryOperatorExpression (unaryOperatorExpression);

ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs

@@ -31,7 +31,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 	[TestFixture]
 	public class ExtractMethodTests : ContextActionTestBase
 	{
-		[Test()]
+		[Test]
 		public void SimpleArgument()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -56,7 +56,8 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 }
 ");
 		}
-		[Test()]
+
+		[Test]
 		public void NoArgument()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -82,7 +83,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 ");
 		}
 
-		[Test()]
+		[Test]
 		public void ExtractMethodResultStatementTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -112,7 +113,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 ");
 		}
 		
-		[Test()]
+		[Test]
 		public void ExtractMethodResultExpressionTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -140,7 +141,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 ");
 		}
 		
-		[Test()]
+		[Test]
 		public void ExtractMethodStaticResultStatementTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -168,7 +169,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 ");
 		}
 		
-		[Test()]
+		[Test]
 		public void ExtractMethodStaticResultExpressionTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -194,7 +195,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 ");
 		}
 		
-		[Test()]
+		[Test]
 		public void ExtractMethodMultiVariableTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -229,7 +230,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 		/// <summary>
 		/// Bug 607990 - "Extract Method" refactoring sometimes tries to pass in unnecessary parameter depending on selection
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug607990()
 		{
 			Test<ExtractMethodAction>(@"using System;
@@ -261,7 +262,7 @@ class TestClass
 		/// <summary>
 		/// Bug 616193 - Extract method passes param with does not exists any more in main method
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug616193()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -296,7 +297,7 @@ class TestClass
 		/// <summary>
 		/// Bug 616199 - Extract method forgets to return a local var which is used in main method
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug616199()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -326,7 +327,7 @@ class TestClass
 		/// <summary>
 		/// Bug 666271 - "Extract Method" on single line adds two semi-colons in method, none in replaced text
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug666271()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -354,7 +355,7 @@ class TestClass
 		/// <summary>
 		/// Bug 693944 - Extracted method returns void instead of the correct type
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug693944()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -379,7 +380,7 @@ class TestClass
 		}
 		
 		[Ignore("Fix me!")]
-		[Test()]
+		[Test]
 		public void ExtractMethodMultiVariableWithLocalReturnVariableTest()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -418,7 +419,7 @@ class TestClass
 		/// <summary>
 		/// Bug 8835 - missing "extract method" in the code editor
 		/// </summary>
-		[Test()]
+		[Test]
 		public void TestBug8835 ()
 		{
 			Test<ExtractMethodAction>(@"class TestClass
@@ -443,6 +444,49 @@ class TestClass
 }
 ");
 		}
+
+		/// <summary>
+		/// Bug 9881 - Extract method is broken
+		/// </summary>
+		[Test]
+		public void TestBug9881()
+		{
+			Test<ExtractMethodAction>(@"using System.Linq;
+class TestClass
+{
+	void Foo ()
+	{
+		var val = Enumerable.Range (0, 10).ToList ();
+		<-for (int j = 0; j < val.Count; j++) {
+			int i = val [j];
+			Console.WriteLine (i);
+		}->
+
+		foreach (var i in val) {
+			Console.WriteLine (i + 2);
+		}
+	}
+}", @"using System.Linq;
+class TestClass
+{
+	static void NewMethod (System.Collections.Generic.List<int> val)
+	{
+		for (int j = 0; j < val.Count; j++) {
+			int i = val [j];
+			Console.WriteLine (i);
+		}
+	}
+	void Foo ()
+	{
+		var val = Enumerable.Range (0, 10).ToList ();
+		NewMethod (val);
+
+		foreach (var i in val) {
+			Console.WriteLine (i + 2);
+		}
+	}
+}");
+		}
 	}
 }
 

ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs

@@ -262,6 +262,10 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
 			var parser = new CSharpParser();
 			var unit = parser.Parse(content, "program.cs");
 			if (!expectErrors) {
+				if (parser.HasErrors) {
+					Console.WriteLine (content);
+					Console.WriteLine ("----");
+				}
 				foreach (var error in parser.Errors) {
 					Console.WriteLine(error.Message);
 				}