[CodeIssues] Add VariableDeclaredInWideScopeIssue.

ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj

@@ -387,6 +387,7 @@
     <Compile Include="Refactoring\CodeActions\ConvertToInitializer\InitializerPath.cs" />
     <Compile Include="Refactoring\CodeIssues\CallToStaticMemberViaDerivedTypeIssue.cs" />
     <Compile Include="Refactoring\CodeActions\MoveToOuterScopeAction.cs" />
+    <Compile Include="Refactoring\CodeIssues\VariableDeclaredInWideScopeIssue.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\ICSharpCode.NRefactory\ICSharpCode.NRefactory.csproj">

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableDeclaredInWideScopeIssue.cs

@@ -0,0 +1,146 @@
+//
+// VariableDeclaredWideScopeIssue.cs
+//
+// Author:
+//       Simon Lindgren <simon.n.lindgren@gmail.com>
+//
+// Copyright (c) 2012 Simon Lindgren
+//
+// 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.Collections.Generic;
+using System.Linq;
+using System;
+
+namespace ICSharpCode.NRefactory.CSharp.Refactoring
+{
+	[IssueDescription("The variable can be declared in a nested scope",
+	                   Description = "Highlights variables that can be declared in a nested scope.",
+	                   Category = IssueCategories.Opportunities,
+	                   Severity = Severity.Suggestion)]
+	public class VariableDeclaredInWideScopeIssue : ICodeIssueProvider
+	{
+		#region ICodeIssueProvider implementation
+		public IEnumerable<CodeIssue> GetIssues(BaseRefactoringContext context)
+		{
+			return new GatherVisitor(context, this).GetIssues();
+		}
+		#endregion
+
+		class GatherVisitor : GatherVisitorBase
+		{
+			readonly BaseRefactoringContext context;
+			
+			public GatherVisitor(BaseRefactoringContext context, VariableDeclaredInWideScopeIssue inspector) : base (context)
+			{
+				this.context = context;
+			}
+
+			static IList<Type> loopStatements = new List<Type>() {
+				typeof(WhileStatement),
+				typeof(ForeachStatement),
+				typeof(ForStatement),
+				typeof(DoWhileStatement)
+			};
+		
+			public override void VisitVariableDeclarationStatement(VariableDeclarationStatement variableDeclarationStatement)
+			{
+				base.VisitVariableDeclarationStatement(variableDeclarationStatement);
+
+				if (variableDeclarationStatement.Variables.Count > 1)
+					return;
+
+				// Start at the parent node. Presumably this is a BlockStatement
+				var rootNode = variableDeclarationStatement.Parent;
+				var variableInitializer = variableDeclarationStatement.Variables.First();
+				var identifiers = from node in rootNode.Descendants
+					let identifier = node as IdentifierExpression
+					where identifier != null && identifier.Identifier == variableInitializer.Name
+						select node;
+
+				if (identifiers.Count() == 0)
+					// variable is not used
+					return;
+
+				AstNode lowestCommonAncestor = GetLowestCommonAncestor(rootNode, identifiers);
+				var path = GetPath(rootNode, lowestCommonAncestor);
+
+				var firstLoopStatement = (from node in path
+				                          where loopStatements.Contains(node.GetType())
+				                          select node).FirstOrDefault();
+				IList<AstNode> possibleDestinationsPath;
+				if (firstLoopStatement == null) {
+					possibleDestinationsPath = path;
+				} else {
+					possibleDestinationsPath = GetPath(rootNode, firstLoopStatement);
+				}
+				var mostNestedBlockStatement = (from node in possibleDestinationsPath
+				                                let block = node as BlockStatement
+				                                where block != null
+				                                select block).LastOrDefault();
+
+				if (mostNestedBlockStatement != null) {
+					AddIssue(variableDeclarationStatement, context.TranslateString("Variable could be moved to a nested scope"),
+					         GetActions(variableDeclarationStatement, mostNestedBlockStatement));
+				}
+			}
+
+			IEnumerable<CodeAction> GetActions(VariableDeclarationStatement declaration, BlockStatement insertTarget)
+			{
+				yield return new CodeAction(context.TranslateString("Move to nested scope"), script => {
+					script.Remove(declaration);
+					script.InsertBefore(insertTarget.Statements.First(), declaration.Clone());
+				});
+			}
+
+			AstNode GetLowestCommonAncestor(AstNode assumedRoot, IEnumerable<AstNode> leaves)
+			{
+				var previousPath = GetPath(assumedRoot, leaves.First());
+				int lowestIndex = previousPath.Count - 1;
+				foreach (var leaf in leaves) {
+					var currentPath = GetPath(assumedRoot, leaf);
+					lowestIndex = GetLowestCommonAncestorIndex(previousPath, currentPath, lowestIndex);
+					previousPath = currentPath;
+				}
+				return previousPath [lowestIndex];
+			}
+			
+			int GetLowestCommonAncestorIndex(IList<AstNode> path1, IList<AstNode> path2, int maxIndex)
+			{
+				var max = Math.Min(Math.Min(path1.Count, path2.Count), maxIndex);
+				for (int i = 0; i < max; i++) {
+					if (path1 [i] != path2 [i])
+						return i - 1;
+				}
+				return max;
+			}
+
+			IList<AstNode> GetPath(AstNode from, AstNode to)
+			{
+				var reversePath = new List<AstNode>();
+				do {
+					reversePath.Add(to);
+					to = to.Parent;
+				} while (to != from);
+				reversePath.Reverse();
+				return reversePath;
+			}
+		}
+	}
+}
+

ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/VariableDeclaredInWideScopeIssue.cs

@@ -0,0 +1,216 @@
+//
+// SetterDoesNotUseValueParameterTests.cs
+//
+// Author:
+//       Simon Lindgren <simon.n.lindgren@gmail.com>
+//
+// Copyright (c) 2012 Simon Lindgren
+//
+// 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 NUnit.Framework;
+using ICSharpCode.NRefactory.CSharp.Refactoring;
+using ICSharpCode.NRefactory.CSharp.CodeActions;
+
+namespace ICSharpCode.NRefactory.CSharp.CodeIssues
+{
+	public class VariableDeclaredInWideScopeTests : InspectionActionTestBase
+	{
+		[Test]
+		public void TestIf()
+		{
+			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);
+		}
+	}
+}");
+		}
+
+		[Test]
+		public void TestIfWithMultipleVariables()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2;
+		int val2;
+		if (true) {
+			val2 = 2;
+		} else {
+			val2 = 3;
+			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()
+	{
+		int val2;
+		if (true) {
+			val2 = 2;
+		} else {
+			int val = 2;
+			val2 = 3;
+			System.Console.WriteLine(val);
+		}
+	}
+}");
+		}
+		
+		[Test]
+		public void TestLoopNestedInIf()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2;
+		if (true) {
+			while (true) {
+				val = 2;
+			}
+		}
+	}
+}";
+			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;
+			while (true) {
+				val = 2;
+			}
+		}
+	}
+}");
+		}
+
+		[Test]
+		public void IgnoresMultiBranchIf()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2;
+		if (true) {
+			System.Console.WriteLine(val);
+		} else {
+			System.Console.WriteLine(val);
+		}
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+		
+		[Test]
+		public void IgnoresMultiVariableDeclaration()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2, val2 = 3;
+		if (true) {
+			System.Console.WriteLine(val);
+		}
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+		
+		[Test]
+		public void IgnoresUnusedVariables()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2;
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+		
+		[Test]
+		public void DoesNotSuggestMovingIntoLoop()
+		{
+			var input = @"
+class A
+{
+	void F()
+	{
+		int val = 2;
+		while (true) {
+			val = 3;
+		}
+	}
+}";
+			TestRefactoringContext context;
+			var issues = GetIssues(new VariableDeclaredInWideScopeIssue(), input, out context);
+			Assert.AreEqual(0, issues.Count);
+		}
+	}
+}
+

ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj

@@ -283,6 +283,7 @@
     <Compile Include="CSharp\CodeActions\ConvertToInitializer\ConvertToInitializerTests.cs" />
     <Compile Include="CSharp\CodeIssues\CallToStaticMemberViaDerivedTypeTests.cs" />
     <Compile Include="CSharp\CodeActions\MoveToOuterScopeTests.cs" />
+    <Compile Include="CSharp\CodeIssues\VariableDeclaredInWideScopeIssue.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\Mono.Cecil\Mono.Cecil.csproj">