From e29ddf05c832d09d5e4574e61cd8e97861960979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Wed, 30 Jan 2013 10:23:10 +0100 Subject: [PATCH] Fixed bug in extract method. --- .../ExtractMethod/VariableUsageAnalyzation.cs | 35 ++++++--- .../CSharp/CodeActions/ExtractMethodTests.cs | 72 +++++++++++++++---- .../CodeActions/TestRefactoringContext.cs | 4 ++ 3 files changed, 87 insertions(+), 24 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs index 631d700edb..7a132b4346 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs +++ b/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 usedVariables; Dictionary states = new Dictionary (); @@ -50,7 +51,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod public VariableUsageAnalyzation (RefactoringContext context, List 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); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs index a604b7eddc..4459cc94e7 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs +++ b/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(@"class TestClass @@ -56,7 +56,8 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions } "); } - [Test()] + + [Test] public void NoArgument() { Test(@"class TestClass @@ -82,7 +83,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Test()] + [Test] public void ExtractMethodResultStatementTest() { Test(@"class TestClass @@ -112,7 +113,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Test()] + [Test] public void ExtractMethodResultExpressionTest() { Test(@"class TestClass @@ -140,7 +141,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Test()] + [Test] public void ExtractMethodStaticResultStatementTest() { Test(@"class TestClass @@ -168,7 +169,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Test()] + [Test] public void ExtractMethodStaticResultExpressionTest() { Test(@"class TestClass @@ -194,7 +195,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Test()] + [Test] public void ExtractMethodMultiVariableTest() { Test(@"class TestClass @@ -229,7 +230,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions /// /// Bug 607990 - "Extract Method" refactoring sometimes tries to pass in unnecessary parameter depending on selection /// - [Test()] + [Test] public void TestBug607990() { Test(@"using System; @@ -261,7 +262,7 @@ class TestClass /// /// Bug 616193 - Extract method passes param with does not exists any more in main method /// - [Test()] + [Test] public void TestBug616193() { Test(@"class TestClass @@ -296,7 +297,7 @@ class TestClass /// /// Bug 616199 - Extract method forgets to return a local var which is used in main method /// - [Test()] + [Test] public void TestBug616199() { Test(@"class TestClass @@ -326,7 +327,7 @@ class TestClass /// /// Bug 666271 - "Extract Method" on single line adds two semi-colons in method, none in replaced text /// - [Test()] + [Test] public void TestBug666271() { Test(@"class TestClass @@ -354,7 +355,7 @@ class TestClass /// /// Bug 693944 - Extracted method returns void instead of the correct type /// - [Test()] + [Test] public void TestBug693944() { Test(@"class TestClass @@ -379,7 +380,7 @@ class TestClass } [Ignore("Fix me!")] - [Test()] + [Test] public void ExtractMethodMultiVariableWithLocalReturnVariableTest() { Test(@"class TestClass @@ -418,7 +419,7 @@ class TestClass /// /// Bug 8835 - missing "extract method" in the code editor /// - [Test()] + [Test] public void TestBug8835 () { Test(@"class TestClass @@ -443,6 +444,49 @@ class TestClass } "); } + + /// + /// Bug 9881 - Extract method is broken + /// + [Test] + public void TestBug9881() + { + Test(@"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 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); + } + } +}"); + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs index 308cabd459..bd34adcd4a 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs +++ b/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); }