diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs index b43fe993fe..925a2c65bb 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs @@ -51,12 +51,14 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring this.context = context; } - static IList blockingStatements = new List() { + static IList moveTargetBlacklist = new List() { typeof(WhileStatement), typeof(ForeachStatement), typeof(ForStatement), typeof(DoWhileStatement), - typeof(TryCatchStatement) + typeof(TryCatchStatement), + typeof(AnonymousMethodExpression), + typeof(LambdaExpression) }; public override void VisitVariableDeclarationStatement(VariableDeclarationStatement variableDeclarationStatement) @@ -86,7 +88,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var path = GetPath(rootNode, lowestCommonAncestor); var firstLoopStatement = (from node in path - where blockingStatements.Contains(node.GetType()) + where moveTargetBlacklist.Contains(node.GetType()) select node).FirstOrDefault(); IList possibleDestinationsPath; if (firstLoopStatement == null) { @@ -94,22 +96,54 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } else { possibleDestinationsPath = GetPath(rootNode, firstLoopStatement); } - var mostNestedBlockStatement = (from node in possibleDestinationsPath - let block = node as BlockStatement - where block != null - select block).LastOrDefault(); + var mostNestedEmbeddedStatement = (from node in possibleDestinationsPath + let statement = node as Statement + let blockStatement = statement as BlockStatement + where statement != null && IsNestedScope(statement) + select statement).LastOrDefault(); - if (mostNestedBlockStatement != null && mostNestedBlockStatement != rootNode) { + if (mostNestedEmbeddedStatement != null && mostNestedEmbeddedStatement != rootNode) { AddIssue(variableDeclarationStatement, context.TranslateString("Variable could be moved to a nested scope"), - GetActions(variableDeclarationStatement, mostNestedBlockStatement)); + GetActions(variableDeclarationStatement, mostNestedEmbeddedStatement)); } } - IEnumerable GetActions(VariableDeclarationStatement declaration, BlockStatement insertTarget) + bool IsNestedScope(AstNode node) + { + var blockStatement = node as BlockStatement; + if (blockStatement != null) + return true; + + var statement = node as Statement; + if (statement == null) + return false; + + var role = node.Role; + if (role == Roles.EmbeddedStatement || + role == IfElseStatement.TrueRole || + role == IfElseStatement.FalseRole) { + return true; + } + return false; + } + + IEnumerable GetActions(VariableDeclarationStatement declaration, Statement targetScope) { yield return new CodeAction(context.TranslateString("Move to nested scope"), script => { + var blockStatement = targetScope as BlockStatement; + if (blockStatement == null) { + var newBlockStatement = new BlockStatement { + Statements = { + declaration.Clone(), + targetScope.Clone() + } + }; + script.Replace(targetScope, newBlockStatement); + script.FormatText(targetScope.Parent); + } else { + script.InsertBefore(blockStatement.Statements.First(), declaration.Clone()); + } script.Remove(declaration); - script.InsertBefore(insertTarget.Statements.First(), declaration.Clone()); }); } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs index a3d77036ac..201665e507 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs @@ -288,6 +288,60 @@ class A var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context); Assert.AreEqual(0, issues.Count); } + + [Test] + public void DoesNotSuggestMovingIntoClosure () + { + var input = @" +using System; +class A +{ + void F() + { + int valA = 2; + Action a = () => Console.WriteLine(valA); + + int valB = 2; + Action b = () => { Console.WriteLine(valB); }; + + int valC = 2; + Action c = delegate() { Console.WriteLine(valC); }; + } +}"; + TestRefactoringContext context; + var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context); + Assert.AreEqual(0, issues.Count); + } + + [Test] + public void TestEmbeddedNonBlockStatement () + { + var input = @" +class A +{ + void F() + { + int val = 2; + if (true) + System.Console.WriteLine (val); + } +}"; + TestRefactoringContext context; + var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context); + Assert.AreEqual(1, issues.Count); + + CheckFix(context, issues [0], @" +class A +{ + void F() + { + if (true) { + int val = 2; + System.Console.WriteLine (val); + } + } +}"); + } } }