From 4a5d1e851b5a13687fbaaaa6b426964eeb7a5487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Mon, 16 Jul 2012 12:30:40 +0200 Subject: [PATCH 1/4] [CodeAction] Fixed most extract method tests. --- .../ICSharpCode.NRefactory.CSharp.csproj | 5 +- .../ExtractMethod/ExtractMethodAction.cs | 107 ++++++------- .../ExtractMethod/VariableLookupVisitor.cs | 23 +-- .../ExtractMethod/VariableUsageAnalyzation.cs | 146 ++++++++++++++++++ .../CSharp/CodeActions/ExtractMethodTests.cs | 19 +-- .../CodeActions/TestRefactoringContext.cs | 12 +- 6 files changed, 226 insertions(+), 86 deletions(-) create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index c730f6fa6b..f0b7c07dbe 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -14,7 +14,7 @@ false 10.0.0 2.0 - true + True ..\ICSharpCode.NRefactory.snk False File @@ -45,7 +45,7 @@ full - true + True @@ -376,6 +376,7 @@ + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs index 12a47472c2..65449a707a 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs @@ -80,11 +80,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod if (!StaticVisitor.UsesNotStaticMember(context, expression)) method.Modifiers |= Modifiers.Static; var task = script.InsertWithCursor(context.TranslateString("Extract method"), Script.InsertPosition.Before, method); - task.ContinueWith (delegate { + + Action replaceStatements = delegate { var target = new IdentifierExpression(methodName); script.Replace(expression, new InvocationExpression(target)); script.Link(target, method.NameToken); - }, TaskScheduler.FromCurrentSynchronizationContext ()); + }; + + if (task.IsCompleted) { + replaceStatements (null); + } else { + task.ContinueWith (replaceStatements, TaskScheduler.FromCurrentSynchronizationContext ()); + } }); } @@ -113,84 +120,68 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod var usedVariables = VariableLookupVisitor.Analyze(context, statements); - var extractedCodeAnalysis = new DefiniteAssignmentAnalysis( - (Statement)statements [0].Parent, - context.Resolver, - context.CancellationToken); + var inExtractedRegion = new VariableUsageAnalyzation (context, usedVariables); var lastStatement = statements [statements.Count - 1]; - extractedCodeAnalysis.SetAnalyzedRange(statements [0], lastStatement); - var statusAfterMethod = new List>(); - foreach (var variable in usedVariables) { - extractedCodeAnalysis.Analyze( - variable.Name, - DefiniteAssignmentStatus.PotentiallyAssigned, - context.CancellationToken); - statusAfterMethod.Add(Tuple.Create(variable, extractedCodeAnalysis.GetStatusAfter(lastStatement))); - } var stmt = statements [0].GetParent(); while (stmt.GetParent () != null) { stmt = stmt.GetParent(); } - var wholeCodeAnalysis = new DefiniteAssignmentAnalysis(stmt, context.Resolver, context.CancellationToken); - var statusBeforeMethod = new Dictionary(); - foreach (var variable in usedVariables) { - wholeCodeAnalysis.Analyze(variable.Name, DefiniteAssignmentStatus.PotentiallyAssigned, context.CancellationToken); - statusBeforeMethod [variable] = extractedCodeAnalysis.GetStatusBefore(statements [0]); - } + inExtractedRegion.SetAnalyzedRange(statements [0], lastStatement); + stmt.AcceptVisitor (inExtractedRegion); - var afterCodeAnalysis = new DefiniteAssignmentAnalysis(stmt, context.Resolver, context.CancellationToken); - var statusAtEnd = new Dictionary(); - afterCodeAnalysis.SetAnalyzedRange(lastStatement, stmt.Statements.Last(), false, true); + var beforeExtractedRegion = new VariableUsageAnalyzation (context, usedVariables); + beforeExtractedRegion.SetAnalyzedRange(statements [0].Parent, statements [0], true, false); + stmt.AcceptVisitor (beforeExtractedRegion); + var afterExtractedRegion = new VariableUsageAnalyzation (context, usedVariables); + afterExtractedRegion.SetAnalyzedRange(lastStatement, stmt.Statements.Last(), false, true); + stmt.AcceptVisitor (afterExtractedRegion); + usedVariables.Sort ((l, r) => l.Region.Begin.CompareTo (r.Region.Begin)); foreach (var variable in usedVariables) { - afterCodeAnalysis.Analyze(variable.Name, DefiniteAssignmentStatus.PotentiallyAssigned, context.CancellationToken); - statusBeforeMethod [variable] = extractedCodeAnalysis.GetStatusBefore(statements [0]); - } - var beforeVisitor = new VariableLookupVisitor(context); - beforeVisitor.SetAnalyzedRange(stmt, statements [0], true, false); - stmt.AcceptVisitor(beforeVisitor); - var afterVisitor = new VariableLookupVisitor(context); - afterVisitor.SetAnalyzedRange(lastStatement, stmt, false, true); - stmt.AcceptVisitor(afterVisitor); - - foreach (var status in statusAfterMethod) { - if (!beforeVisitor.UsedVariables.Contains(status.Item1) && !afterVisitor.UsedVariables.Contains(status.Item1)) + if (!(variable is IParameter) && !beforeExtractedRegion.Has (variable) && !afterExtractedRegion.Has (variable)) continue; - Expression argumentExpression = new IdentifierExpression(status.Item1.Name); + Expression argumentExpression = new IdentifierExpression(variable.Name); - ParameterModifier mod; - switch (status.Item2) { - case DefiniteAssignmentStatus.AssignedAfterTrueExpression: - case DefiniteAssignmentStatus.AssignedAfterFalseExpression: - case DefiniteAssignmentStatus.PotentiallyAssigned: - mod = ParameterModifier.Ref; - argumentExpression = new DirectionExpression(FieldDirection.Ref, argumentExpression); - break; - case DefiniteAssignmentStatus.DefinitelyAssigned: - if (statusBeforeMethod [status.Item1] != DefiniteAssignmentStatus.PotentiallyAssigned) - goto case DefiniteAssignmentStatus.PotentiallyAssigned; + ParameterModifier mod = ParameterModifier.None; + if (inExtractedRegion.GetStatus (variable) == VariableState.Changed) { + + if (beforeExtractedRegion.GetStatus (variable) == VariableState.None) { mod = ParameterModifier.Out; argumentExpression = new DirectionExpression(FieldDirection.Out, argumentExpression); - break; - // case DefiniteAssignmentStatus.Unassigned: - default: - mod = ParameterModifier.None; - break; + } else { + mod = ParameterModifier.Ref; + argumentExpression = new DirectionExpression(FieldDirection.Ref, argumentExpression); + } } - method.Parameters.Add( - new ParameterDeclaration(context.CreateShortType(status.Item1.Type), status.Item1.Name, mod)); + + method.Parameters.Add(new ParameterDeclaration(context.CreateShortType(variable.Type), variable.Name, mod)); invocation.Arguments.Add(argumentExpression); } var task = script.InsertWithCursor(context.TranslateString("Extract method"), Script.InsertPosition.Before, method); - task.ContinueWith (delegate { + Action replaceStatements = delegate { foreach (var node in statements.Skip (1)) { script.Remove(node); } - script.Replace(statements [0], new ExpressionStatement(invocation)); + foreach (var variable in usedVariables) { + if ((variable is IParameter) || beforeExtractedRegion.Has (variable) || !afterExtractedRegion.Has (variable)) + continue; + script.InsertBefore (statements [0], new VariableDeclarationStatement (context.CreateShortType(variable.Type), variable.Name)); + } + + var invocationStatement = new ExpressionStatement(invocation); + script.Replace(statements [0], invocationStatement); + + script.Link(target, method.NameToken); - }, TaskScheduler.FromCurrentSynchronizationContext ()); + }; + + if (task.IsCompleted) { + replaceStatements (null); + } else { + task.ContinueWith (replaceStatements, TaskScheduler.FromCurrentSynchronizationContext ()); + } }); } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableLookupVisitor.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableLookupVisitor.cs index 3613ebb2a7..0a8240bb7f 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableLookupVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableLookupVisitor.cs @@ -33,18 +33,22 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod class VariableLookupVisitor : DepthFirstAstVisitor { readonly RefactoringContext context; - + public List UsedVariables = new List (); - + TextLocation startLocation = TextLocation.Empty; TextLocation endLocation = TextLocation.Empty; - - + public VariableLookupVisitor (RefactoringContext context) { this.context = context; } + public bool Has (IVariable item1) + { + return UsedVariables.Contains (item1); + } + public void SetAnalyzedRange(AstNode start, AstNode end, bool startInclusive = true, bool endInclusive = true) { if (start == null) @@ -54,17 +58,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod startLocation = startInclusive ? start.StartLocation : start.EndLocation; endLocation = endInclusive ? end.EndLocation : end.StartLocation; } - + public override void VisitIdentifierExpression(IdentifierExpression identifierExpression) { if (startLocation.IsEmpty || startLocation <= identifierExpression.StartLocation && identifierExpression.EndLocation <= endLocation) { var result = context.Resolve(identifierExpression); var local = result as LocalResolveResult; - if (local != null && !UsedVariables.Contains(local.Variable)) + if (local != null && !UsedVariables.Contains(local.Variable)) { UsedVariables.Add(local.Variable); + } } } - + public override void VisitVariableDeclarationStatement(VariableDeclarationStatement variableDeclarationStatement) { base.VisitVariableDeclarationStatement(variableDeclarationStatement); @@ -78,14 +83,14 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod } } - + public static List Analyze(RefactoringContext context, Expression expression) { var visitor = new VariableLookupVisitor(context); expression.AcceptVisitor(visitor); return visitor.UsedVariables; } - + public static List Analyze(RefactoringContext context, List statements) { var visitor = new VariableLookupVisitor(context); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs new file mode 100644 index 0000000000..b759245440 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/VariableUsageAnalyzation.cs @@ -0,0 +1,146 @@ +// +// VariableUsageAnalyzation.cs +// +// Author: +// Mike Krüger +// +// Copyright (c) 2012 Xamarin Inc. (http://xamarin.com) +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +using System; +using System.Collections.Generic; +using ICSharpCode.NRefactory.TypeSystem; +using System.Linq; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod +{ + + public enum VariableState { + None, + Used, + Changed + } + + + public class VariableUsageAnalyzation : DepthFirstAstVisitor + { + readonly RefactoringContext context; + readonly List usedVariables; + + Dictionary states = new Dictionary (); + + TextLocation startLocation = TextLocation.Empty; + TextLocation endLocation = TextLocation.Empty; + + public VariableUsageAnalyzation (RefactoringContext context, List usedVariables) + { + this.context = context; + this.usedVariables = usedVariables; + } + + public bool Has(IVariable variable) + { + return states.ContainsKey (variable); + } + + public void SetAnalyzedRange(AstNode start, AstNode end, bool startInclusive = true, bool endInclusive = true) + { + if (start == null) + throw new ArgumentNullException("start"); + if (end == null) + throw new ArgumentNullException("end"); + startLocation = startInclusive ? start.StartLocation : start.EndLocation; + endLocation = endInclusive ? end.EndLocation : end.StartLocation; + states.Clear (); + } + + public VariableState GetStatus (IVariable variable) + { + VariableState state; + if (!states.TryGetValue (variable, out state)) + return VariableState.None; + return state; + } + + void SetState (string identifier, VariableState state) + { + var variable = usedVariables.FirstOrDefault (v => v.Name == identifier); + if (variable == null) + return; + VariableState oldState; + if (states.TryGetValue (variable, out oldState)) { + if (oldState < state) + states [variable] = state; + } else { + states [variable] = state; + } + } + + public override void VisitIdentifierExpression(IdentifierExpression identifierExpression) + { + if (startLocation.IsEmpty || startLocation <= identifierExpression.StartLocation && identifierExpression.EndLocation <= endLocation) { + SetState (identifierExpression.Identifier, VariableState.Used); + } + } + + public override void VisitAssignmentExpression(AssignmentExpression assignmentExpression) + { + if (startLocation.IsEmpty || startLocation <= assignmentExpression.StartLocation && assignmentExpression.EndLocation <= endLocation) { + var left = assignmentExpression.Left as IdentifierExpression; + if (left != null) + SetState(left.Identifier, VariableState.Changed); + } + base.VisitAssignmentExpression (assignmentExpression); + } + + public override void VisitDirectionExpression(DirectionExpression directionExpression) + { + if (startLocation.IsEmpty || startLocation <= directionExpression.StartLocation && directionExpression.EndLocation <= endLocation) { + var expr = directionExpression.Expression as IdentifierExpression; + if (expr != null) + SetState(expr.Identifier, VariableState.Changed); + } + base.VisitDirectionExpression (directionExpression); + } + + 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); + } + + base.VisitVariableInitializer(variableInitializer); + } + + public override void VisitUnaryOperatorExpression(UnaryOperatorExpression unaryOperatorExpression) + { + if (startLocation.IsEmpty || startLocation <= unaryOperatorExpression.StartLocation && unaryOperatorExpression.EndLocation <= endLocation) { + 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); + } + } + base.VisitUnaryOperatorExpression (unaryOperatorExpression); + } + + } +} + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs index 1a85e2b188..592948235a 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs @@ -28,7 +28,6 @@ using ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod; namespace ICSharpCode.NRefactory.CSharp.CodeActions { - [Ignore("FIXME!!")] [TestFixture] public class ExtractMethodTests : ContextActionTestBase { @@ -91,7 +90,6 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Ignore("FIXME!!")] [Test()] public void ExtractMethodStaticResultStatementTest() { @@ -146,7 +144,6 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - [Ignore("FIXME!!")] [Test()] public void ExtractMethodMultiVariableTest() { @@ -164,7 +161,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions ", @"class TestClass { int member; - void NewMethod (ref int j, int i, out int k) + void NewMethod (int i, ref int j, out int k) { j = i + j; k = j + member; @@ -185,7 +182,8 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions [Test()] public void TestBug607990() { - Test(@"class TestClass + Test(@"using System; +class TestClass { void TestMethod () { @@ -193,7 +191,8 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions obj1.ToString();-> } } -", @"class TestClass +", @"using System; +class TestClass { static void NewMethod () { @@ -212,7 +211,6 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions /// /// Bug 616193 - Extract method passes param with does not exists any more in main method /// - [Ignore("FIXME!!")] [Test()] public void TestBug616193() { @@ -248,7 +246,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions /// /// Bug 616199 - Extract method forgets to return a local var which is used in main method /// - [Ignore("FIXME!!")] + [Ignore("Fix me!")] [Test()] public void TestBug616199() { @@ -331,8 +329,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "); } - - [Ignore("FIXME!!")] + [Ignore("Fix me!")] [Test()] public void ExtractMethodMultiVariableWithLocalReturnVariableTest() { @@ -352,7 +349,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions ", @"class TestClass { int member; - void NewMethod (ref int j, int i, out int k, out int test) + void NewMethod (int i, ref int j, out int k, out int test) { j = i + j; k = j + member; diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs index 28d2f6163e..a3e6bb0ae4 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/TestRefactoringContext.cs @@ -105,9 +105,9 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions foreach (var node in nodes) { InsertBefore(entity, node); } - var t = new Task (() => {}); - t.RunSynchronously (); - return t; + var tcs = new TaskCompletionSource (); + tcs.SetResult (null); + return tcs.Task; } public override Task InsertWithCursor (string operation, ITypeDefinition parentType, IEnumerable nodes) @@ -121,9 +121,9 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions InsertText (startOffset, output.Text); output.RegisterTrackedSegments (this, startOffset); } - var t = new Task (() => {}); - t.RunSynchronously (); - return t; + var tcs = new TaskCompletionSource (); + tcs.SetResult (null); + return tcs.Task; } void Rename (AstNode node, string newName) From d4209d09dda75dc4a57b349becdcbc76ed1e8c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Mon, 16 Jul 2012 15:23:00 +0200 Subject: [PATCH 2/4] [CodeAction] Fixed extract method unit test. --- .../ExtractMethod/ExtractMethodAction.cs | 23 +++++++- .../CSharp/CodeActions/ExtractMethodTests.cs | 53 ++++++++++++++++++- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs index 65449a707a..4d3d15c2d3 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ExtractMethod/ExtractMethodAction.cs @@ -139,14 +139,26 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod afterExtractedRegion.SetAnalyzedRange(lastStatement, stmt.Statements.Last(), false, true); stmt.AcceptVisitor (afterExtractedRegion); usedVariables.Sort ((l, r) => l.Region.Begin.CompareTo (r.Region.Begin)); + + IVariable generatedReturnVariable = null; + foreach (var variable in usedVariables) { + if ((variable is IParameter) || beforeExtractedRegion.Has (variable) || !afterExtractedRegion.Has (variable)) + continue; + generatedReturnVariable = variable; + method.ReturnType = context.CreateShortType (variable.Type); + method.Body.Add (new ReturnStatement (new IdentifierExpression (variable.Name))); + break; + } + foreach (var variable in usedVariables) { if (!(variable is IParameter) && !beforeExtractedRegion.Has (variable) && !afterExtractedRegion.Has (variable)) continue; + if (variable == generatedReturnVariable) + continue; Expression argumentExpression = new IdentifierExpression(variable.Name); ParameterModifier mod = ParameterModifier.None; if (inExtractedRegion.GetStatus (variable) == VariableState.Changed) { - if (beforeExtractedRegion.GetStatus (variable) == VariableState.None) { mod = ParameterModifier.Out; argumentExpression = new DirectionExpression(FieldDirection.Out, argumentExpression); @@ -167,10 +179,17 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring.ExtractMethod foreach (var variable in usedVariables) { if ((variable is IParameter) || beforeExtractedRegion.Has (variable) || !afterExtractedRegion.Has (variable)) continue; + if (variable == generatedReturnVariable) + continue; script.InsertBefore (statements [0], new VariableDeclarationStatement (context.CreateShortType(variable.Type), variable.Name)); } + AstNode invocationStatement; - var invocationStatement = new ExpressionStatement(invocation); + if (generatedReturnVariable != null) { + invocationStatement = new VariableDeclarationStatement (new SimpleType ("var"), generatedReturnVariable.Name, invocation); + } else { + invocationStatement = new ExpressionStatement(invocation); + } script.Replace(statements [0], invocationStatement); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs index 592948235a..0dba8a94b1 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ExtractMethodTests.cs @@ -31,6 +31,56 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions [TestFixture] public class ExtractMethodTests : ContextActionTestBase { + [Test()] + public void SimpleArgument() + { + Test(@"class TestClass +{ + void TestMethod () + { + int i = 5; + <-Console.WriteLine (i);-> + } +} +", @"class TestClass +{ + static void NewMethod (int i) + { + Console.WriteLine (i); + } + void TestMethod () + { + int i = 5; + NewMethod (i); + } +} +"); + } + [Test()] + public void NoArgument() + { + Test(@"class TestClass +{ + void TestMethod () + { + int i = 5; + <-Console.WriteLine (""Hello World"");-> + } +} +", @"class TestClass +{ + static void NewMethod () + { + Console.WriteLine (""Hello World""); + } + void TestMethod () + { + int i = 5; + NewMethod (); + } +} +"); + } [Test()] public void ExtractMethodResultStatementTest() @@ -246,7 +296,6 @@ class TestClass /// /// Bug 616199 - Extract method forgets to return a local var which is used in main method /// - [Ignore("Fix me!")] [Test()] public void TestBug616199() { @@ -267,7 +316,7 @@ class TestClass } void TestMethod () { - string z = NewMethod (); + var z = NewMethod (); string ret = ""test1"" + z; } } From 461d5dabd04d69033690890518572ee0f6580e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 17 Jul 2012 08:26:57 +0200 Subject: [PATCH 3/4] [Completion] Fixed completion bug. --- .../Completion/CSharpCompletionEngineBase.cs | 2 + .../CodeCompletion/CodeCompletionBugTests.cs | 42 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngineBase.cs b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngineBase.cs index a2ec8ec116..3ffa14a9c5 100644 --- a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngineBase.cs +++ b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngineBase.cs @@ -801,6 +801,8 @@ namespace ICSharpCode.NRefactory.CSharp.Completion if (root == null) { return null; } + if (root is Accessor) + root = root.Parent; var csResolver = CompletionContextProvider.GetResolver (GetState(), root); var result = csResolver.Resolve(resolveNode); var state = csResolver.GetResolverStateBefore(resolveNode); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeCompletion/CodeCompletionBugTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeCompletion/CodeCompletionBugTests.cs index b2c5464ff5..a7bd3f474e 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeCompletion/CodeCompletionBugTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeCompletion/CodeCompletionBugTests.cs @@ -5241,7 +5241,7 @@ public class TestFoo public void TestBug4961() { CombinedProviderTest( -@"using System; + @"using System; using System.Collections.Generic; namespace EnumerationProblem @@ -5323,7 +5323,47 @@ $mc->$ }); } + /// + /// Bug 6146 - No intellisense on value keyword in property set method + /// + [Test()] + public void TestBug6146() + { + CombinedProviderTest( + @"using System; +public class FooBar +{ + public FooBar Foo { + set { + $value.$ + } + } +} +", provider => { + Assert.IsNotNull(provider.Find("Foo")); + }); + } + + + [Test()] + public void TestBug6146Case2() + { + CombinedProviderTest( + @"using System; +public class FooBar +{ + public FooBar Foo { + set { + $value.Foo.F$ + } + } +} + +", provider => { + Assert.IsNotNull(provider.Find("Foo")); + }); + } } From a4344c8737e1a8756d253e74d072142f0857f81a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 17 Jul 2012 17:43:03 +0200 Subject: [PATCH 4/4] Checked for possible null refrence exception. --- ICSharpCode.NRefactory.CSharp/Resolver/CSharpConversions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/CSharpConversions.cs b/ICSharpCode.NRefactory.CSharp/Resolver/CSharpConversions.cs index 14071753bf..359338129f 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/CSharpConversions.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/CSharpConversions.cs @@ -84,6 +84,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public bool Equals(TypePair other) { + if (this.FromType == null || this.ToType == null || other.FromType == null || other.ToType == null) + return false; return this.FromType.Equals(other.FromType) && this.ToType.Equals(other.ToType); }