diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs index 5b06d87ebb..6ce291b7fc 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs @@ -94,7 +94,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring where block != null select block).LastOrDefault(); - if (mostNestedBlockStatement != null) { + if (mostNestedBlockStatement != null && mostNestedBlockStatement != rootNode) { AddIssue(variableDeclarationStatement, context.TranslateString("Variable could be moved to a nested scope"), GetActions(variableDeclarationStatement, mostNestedBlockStatement)); } @@ -112,7 +112,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { var previousPath = GetPath(assumedRoot, leaves.First()); int lowestIndex = previousPath.Count - 1; - foreach (var leaf in leaves) { + foreach (var leaf in leaves.Skip(1)) { var currentPath = GetPath(assumedRoot, leaf); lowestIndex = GetLowestCommonAncestorIndex(previousPath, currentPath, lowestIndex); previousPath = currentPath; @@ -123,7 +123,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring int GetLowestCommonAncestorIndex(IList path1, IList path2, int maxIndex) { var max = Math.Min(Math.Min(path1.Count, path2.Count), maxIndex); - for (int i = 0; i < max; i++) { + for (int i = 0; i <= max; i++) { if (path1 [i] != path2 [i]) return i - 1; } @@ -136,7 +136,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring do { reversePath.Add(to); to = to.Parent; - } while (to != from); + } while (to != from.Parent); reversePath.Reverse(); return reversePath; } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs index 4fb00f9f2f..b834dc149c 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeTests.cs @@ -193,6 +193,24 @@ class A Assert.AreEqual(0, issues.Count); } + [Test] + public void IgnoresVariablesOnlyUsedInOneScope() + { + var input = @" +class A +{ + void F() + { + A a = new A(); + a.F(); + a.F(); + } +}"; + TestRefactoringContext context; + var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context); + Assert.AreEqual(0, issues.Count); + } + [Test] public void DoesNotSuggestMovingIntoLoop() { @@ -206,6 +224,25 @@ class A val = 3; } } +}"; + TestRefactoringContext context; + var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context); + Assert.AreEqual(0, issues.Count); + } + + [Test] + public void DoesNotSuggestMovingPastDependentCondition() + { + var input = @" +class A +{ + void F() + { + int val = 2; + if (val == 2 || val == 1) { + val = 3; + } + } }"; TestRefactoringContext context; var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context);