[CodeIssue] Added AccessToModifiedClosureIssue and AccessToDisposedClosureIssue

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

@@ -256,6 +256,10 @@
     <Compile Include="Refactoring\CodeActions\PutInsideUsingAction.cs" />
     <Compile Include="Refactoring\CodeActions\SplitDeclarationListAction.cs" />
     <Compile Include="Refactoring\CodeActions\UseStringFormatAction.cs" />
+    <Compile Include="Refactoring\CodeIssues\AccessToClosureIssues\AccessToClosureIssue.cs" />
+    <Compile Include="Refactoring\CodeIssues\AccessToClosureIssues\AccessToDisposedClosureIssue.cs" />
+    <Compile Include="Refactoring\CodeIssues\AccessToClosureIssues\AccessToModifiedClosureIssue.cs" />
+    <Compile Include="Refactoring\CodeIssues\AccessToClosureIssues\LocalVariableNamePicker.cs" />
     <Compile Include="Refactoring\CodeIssues\ExplicitConversionInForEachIssue.cs" />
     <Compile Include="Refactoring\DocumentScript.cs" />
     <Compile Include="Refactoring\CodeActions\ExtractAnonymousMethodAction.cs" />

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs

@@ -0,0 +1,329 @@
+// 
+// AccessToClosureIssue.cs
+// 
+// Author:
+//      Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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 System.Linq;
+using ICSharpCode.NRefactory.CSharp.Analysis;
+using ICSharpCode.NRefactory.CSharp.Resolver;
+using ICSharpCode.NRefactory.Semantics;
+using ICSharpCode.NRefactory.TypeSystem;
+
+namespace ICSharpCode.NRefactory.CSharp.Refactoring
+{
+	public abstract class AccessToClosureIssue : ICodeIssueProvider
+	{
+		static FindReferences refFinder = new FindReferences ();
+		static ControlFlowGraphBuilder cfgBuilder = new ControlFlowGraphBuilder ();
+
+		public string Title
+		{ get; private set; }
+
+		protected AccessToClosureIssue (string title)
+		{
+			Title = title;
+		}
+
+		public IEnumerable<CodeIssue> GetIssues (BaseRefactoringContext context)
+		{
+			var unit = context.RootNode as CompilationUnit;
+			if (unit == null)
+				return Enumerable.Empty<CodeIssue> ();
+			return new GatherVisitor (context, unit, this).GetIssues ();
+		}
+
+		protected virtual bool IsTargetVariable (IVariable variable)
+		{
+			return true;
+		}
+
+		protected abstract NodeKind GetNodeKind (AstNode node);
+
+		protected virtual bool CanReachModification (ControlFlowNode node, Statement start,
+												  IDictionary<Statement, IList<Node>> modifications)
+		{
+			return node.NextStatement != null && node.NextStatement != start &&
+				   modifications.ContainsKey (node.NextStatement);
+		}
+
+		protected abstract IEnumerable<CodeAction> GetFixes (BaseRefactoringContext context, Node env, 
+															 string variableName, AstType variableType);
+
+		#region GatherVisitor
+
+		class GatherVisitor : GatherVisitorBase
+		{
+			CompilationUnit unit;
+			string title;
+			AccessToClosureIssue issueProvider;
+
+			public GatherVisitor (BaseRefactoringContext context, CompilationUnit unit,
+								  AccessToClosureIssue issueProvider)
+				: base (context)
+			{
+				this.title = context.TranslateString (issueProvider.Title);
+				this.unit = unit;
+				this.issueProvider = issueProvider;
+			}
+
+			public override void VisitVariableInitializer (VariableInitializer variableInitializer)
+			{
+				var variableDecl = variableInitializer.Parent as VariableDeclarationStatement;
+				if (variableDecl == null)
+					return;
+
+				CheckVariable (((LocalResolveResult)ctx.Resolve (variableInitializer)).Variable, 
+							   variableDecl.Type, 
+							   variableDecl.GetParent<Statement> ());
+				base.VisitVariableInitializer (variableInitializer);
+			}
+
+			public override void VisitForeachStatement (ForeachStatement foreachStatement)
+			{
+				CheckVariable (((LocalResolveResult)ctx.Resolve (foreachStatement.VariableNameToken)).Variable,
+							   foreachStatement.VariableType, 
+							   foreachStatement);
+				base.VisitForeachStatement (foreachStatement);
+			}
+
+			public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration)
+			{
+				var parent = parameterDeclaration.Parent;
+				Statement body = null;
+				if (parent is MethodDeclaration) {
+					body = ((MethodDeclaration)parent).Body;
+				} else if (parent is AnonymousMethodExpression) {
+					body = ((AnonymousMethodExpression)parent).Body;
+				} else if (parent is LambdaExpression) {
+					body = ((LambdaExpression)parent).Body as Statement;
+				}
+				if (body != null)
+					CheckVariable (((LocalResolveResult)ctx.Resolve (parameterDeclaration)).Variable, 
+								   parameterDeclaration.Type, body);
+				base.VisitParameterDeclaration (parameterDeclaration);
+			}
+
+			void FindLocalReferences (IVariable variable, FoundReferenceCallback callback)
+			{
+				refFinder.FindLocalReferences (variable, ctx.ParsedFile, unit, ctx.Compilation, callback, 
+											   ctx.CancellationToken);
+			}
+
+			void CheckVariable (IVariable variable, AstType type, Statement env)
+			{
+				if (!issueProvider.IsTargetVariable (variable))
+					return;
+
+				var root = new Environment (env, env);
+				var envLookup = new Dictionary<AstNode, Environment> ();
+				envLookup [env] = root;
+
+				FindLocalReferences (variable, (astNode, resolveResult) => 
+					AddNode (envLookup, new Node (astNode, issueProvider.GetNodeKind (astNode))));
+
+				root.SortChildren ();
+				CollectIssues (root, variable.Name, type);
+			}
+
+			void CollectIssues (Environment env, string variableName, AstType variableType)
+			{
+				IList<ControlFlowNode> cfg = null;
+				IDictionary<Statement, IList<Node>> modifications = null;
+
+				if (env.Body != null) {
+					cfg = cfgBuilder.BuildControlFlowGraph (env.Body);
+					modifications = new Dictionary<Statement, IList<Node>> ();
+					foreach (var node in env.Children) {
+						if (node.Kind == NodeKind.Modification || node.Kind == NodeKind.ReferenceAndModification) {
+							IList<Node> nodes;
+							if (!modifications.TryGetValue (node.ContainingStatement, out nodes))
+								modifications [node.ContainingStatement] = nodes = new List<Node> ();
+							nodes.Add (node);
+						}
+					}
+				}
+
+				foreach (var child in env.GetChildEnvironments ()) {
+					if (!child.IssueCollected && cfg != null && 
+						CanReachModification (cfg, child, modifications))
+						CollectAllIssues (child, variableName, variableType);
+
+					CollectIssues (child, variableName, variableType);
+				}
+			}
+
+			void CollectAllIssues (Environment env, string variableName, AstType variableType)
+			{
+				var fixes = issueProvider.GetFixes (ctx, env, variableName, variableType).ToArray ();
+				env.IssueCollected = true;
+
+				foreach (var child in env.Children) {
+					if (child is Environment) {
+						CollectAllIssues ((Environment)child, variableName, variableType);
+					} else {
+						if (child.Kind != NodeKind.Modification)
+							AddIssue (child.AstNode, title, fixes);
+						// stop marking references after the variable is modified in current environment
+						if (child.Kind != NodeKind.Reference)
+							break;
+					}
+				}
+			}
+
+			void AddNode (IDictionary<AstNode, Environment> envLookup, Node node)
+			{
+				var astParent = node.AstNode.Parent;
+				var path = new List<AstNode> ();
+				while (astParent != null) {
+					Environment parent;
+					if (envLookup.TryGetValue (astParent, out parent)) {
+						parent.Children.Add (node);
+						return;
+					}
+
+					if (astParent is LambdaExpression) {
+						parent = new Environment (astParent, ((LambdaExpression)astParent).Body as Statement);
+					} else if (astParent is AnonymousMethodExpression) {
+						parent = new Environment (astParent, ((AnonymousMethodExpression)astParent).Body);
+					}
+
+					path.Add (astParent);
+					if (parent != null) {
+						foreach (var astNode in path)
+							envLookup [astNode] = parent;
+						path.Clear ();
+
+						parent.Children.Add (node);
+						node = parent;
+					}
+					astParent = astParent.Parent;
+				}
+			}
+
+			bool CanReachModification (IEnumerable<ControlFlowNode> cfg, Environment env,
+									   IDictionary<Statement, IList<Node>> modifications)
+			{
+				if (modifications.Count == 0)
+					return false;
+
+				var start = env.ContainingStatement;
+				if (modifications.ContainsKey (start) &&
+					modifications [start].Any (v => v.AstNode.StartLocation > env.AstNode.EndLocation))
+					return true;
+
+				var stack = new Stack<ControlFlowNode> (cfg.Where (node => node.NextStatement == start));
+				var visitedNodes = new HashSet<ControlFlowNode> (stack);
+				while (stack.Count > 0) {
+					var node = stack.Pop ();
+					if (issueProvider.CanReachModification (node, start, modifications))
+						return true;
+					foreach (var edge in node.Outgoing) {
+						if (visitedNodes.Add (edge.To))
+							stack.Push (edge.To);
+					}
+				}
+				return false;
+			}
+
+		}
+
+		#endregion
+
+		#region Node
+
+		protected enum NodeKind
+		{
+			Reference,
+			Modification,
+			ReferenceAndModification,
+			Environment,
+		}
+
+		protected class Node
+		{
+			public AstNode AstNode
+			{ get; private set; }
+
+			public NodeKind Kind
+			{ get; private set; }
+
+			public Statement ContainingStatement
+			{ get; private set; }
+
+			public Node (AstNode astNode, NodeKind kind)
+			{
+				AstNode = astNode;
+				Kind = kind;
+				ContainingStatement = astNode.GetParent<Statement> ();
+			}
+
+			public virtual IEnumerable<Node> GetAllReferences ()
+			{
+				yield return this;
+			}
+		}
+
+		protected class Environment : Node
+		{
+			public Statement Body
+			{ get; private set; }
+
+			public bool IssueCollected
+			{ get; set; }
+
+			public List<Node> Children
+			{ get; private set; }
+
+			public Environment (AstNode astNode, Statement body)
+				: base (astNode, NodeKind.Environment)
+			{
+				Body = body;
+				Children = new List<Node> ();
+			}
+
+			public override IEnumerable<Node> GetAllReferences ()
+			{
+				return Children.SelectMany (child => child.GetAllReferences ());
+			}
+
+			public IEnumerable<Environment> GetChildEnvironments ()
+			{
+				return from child in Children
+					   where child is Environment
+					   select (Environment)child;
+			}
+
+			public void SortChildren ()
+			{
+				Children.Sort ((x, y) => x.AstNode.StartLocation.CompareTo(y.AstNode.StartLocation));
+				foreach (var env in GetChildEnvironments ())
+					env.SortChildren ();
+			}
+		}
+
+		#endregion
+	}
+}

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToDisposedClosureIssue.cs

@@ -0,0 +1,94 @@
+// 
+// AccessToDisposedClosureIssue.cs
+// 
+// Author:
+//      Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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 System.Linq;
+using ICSharpCode.NRefactory.CSharp.Analysis;
+using ICSharpCode.NRefactory.TypeSystem;
+
+namespace ICSharpCode.NRefactory.CSharp.Refactoring
+{
+
+	[IssueDescription ("Access to disposed closure variable",
+					   Description = "Access to closure variable from anonymous method when the variable is" + 
+									 " disposed externally",
+					   Category = IssueCategories.CodeQualityIssues,
+					   Severity = Severity.Warning,
+					   IssueMarker = IssueMarker.Underline)]
+	public class AccessToDisposedClosureIssue : AccessToClosureIssue
+	{
+		public AccessToDisposedClosureIssue ()
+			: base ("Access to disposed closure")
+		{
+		}
+
+		protected override bool IsTargetVariable (IVariable variable)
+		{
+			return variable.Type.GetAllBaseTypeDefinitions ().Any (t => t.KnownTypeCode == KnownTypeCode.IDisposable);
+		}
+
+		protected override NodeKind GetNodeKind (AstNode node)
+		{
+			if (node.Parent is UsingStatement)
+				return NodeKind.ReferenceAndModification;
+
+			if (node.Parent is VariableDeclarationStatement && node.Parent.Parent is UsingStatement)
+				return NodeKind.Modification;
+
+			var memberRef = node.Parent as MemberReferenceExpression;
+			if (memberRef != null && memberRef.Parent is InvocationExpression && memberRef.MemberName == "Dispose")
+				return NodeKind.ReferenceAndModification;
+
+			return NodeKind.Reference;
+		}
+
+		protected override bool CanReachModification (ControlFlowNode node, Statement start,
+												   IDictionary<Statement, IList<Node>> modifications)
+		{
+			if (base.CanReachModification (node, start, modifications))
+				return true;
+
+			if (node.NextStatement != start) {
+				var usingStatement = node.PreviousStatement as UsingStatement;
+				if (usingStatement != null) {
+					if (modifications.ContainsKey(usingStatement))
+						return true;
+					if (usingStatement.ResourceAcquisition is Statement &&
+						modifications.ContainsKey ((Statement)usingStatement.ResourceAcquisition))
+						return true;
+				}
+			}
+			return false;
+		}
+
+		protected override IEnumerable<CodeAction> GetFixes (BaseRefactoringContext context, Node env,
+															 string variableName, AstType variableType)
+		{
+			yield break;
+		}
+	}
+}

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToModifiedClosureIssue.cs

@@ -0,0 +1,113 @@
+// 
+// AccessToModifiedClosureIssue.cs
+//  
+// Author:
+//      Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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 System.Linq;
+
+namespace ICSharpCode.NRefactory.CSharp.Refactoring
+{
+	[IssueDescription ("Access to modified closure variable",
+					   Description = "Access to closure variable from anonymous method when the variable is modified " +
+									 "externally",
+					   Category = IssueCategories.CodeQualityIssues,
+					   Severity = Severity.Warning,
+					   IssueMarker = IssueMarker.Underline)]	
+	public class AccessToModifiedClosureIssue : AccessToClosureIssue
+	{
+		public AccessToModifiedClosureIssue ()
+			: base ("Access to modified closure")
+		{
+		}
+
+		protected override NodeKind GetNodeKind (AstNode node)
+		{
+			var assignment = node.GetParent<AssignmentExpression> ();
+			if (assignment != null && assignment.Left == node) {
+				if (assignment.Operator == AssignmentOperatorType.Assign) 
+					return NodeKind.Modification;
+				return NodeKind.ReferenceAndModification;
+			}
+			var unaryExpr = node.GetParent<UnaryOperatorExpression> ();
+			if (unaryExpr != null && unaryExpr.Expression == node &&
+				(unaryExpr.Operator == UnaryOperatorType.Increment ||
+				 unaryExpr.Operator == UnaryOperatorType.PostIncrement ||
+				 unaryExpr.Operator == UnaryOperatorType.Decrement ||
+				 unaryExpr.Operator == UnaryOperatorType.PostDecrement)) {
+				return NodeKind.ReferenceAndModification;
+			}
+			if (node.Parent is ForeachStatement)
+				return NodeKind.Modification;
+
+			return NodeKind.Reference;
+		}
+
+		protected override IEnumerable<CodeAction> GetFixes (BaseRefactoringContext context, Node env,
+															 string variableName, AstType variableType)
+		{
+			var containingStatement = env.ContainingStatement;
+
+			// we don't give a fix for these cases since the general fix may not work
+			// lambda in while/do-while/for condition
+			if (containingStatement is WhileStatement || containingStatement is DoWhileStatement ||
+				containingStatement is ForStatement)
+				yield break;
+			// lambda in for initializer/iterator
+			if (containingStatement.Parent is ForStatement &&
+				((ForStatement)containingStatement.Parent).EmbeddedStatement != containingStatement)
+				yield break;
+
+			Action<Script> action = script =>
+			{
+				var newName = LocalVariableNamePicker.PickSafeName (containingStatement.GetParent<MethodDeclaration> (),
+															Enumerable.Range (1, 100).Select (i => variableName + i));
+				var variableDecl = new VariableDeclarationStatement (variableType.Clone (), newName, 
+																	 new IdentifierExpression (variableName));
+				
+				if (containingStatement.Parent is BlockStatement || containingStatement.Parent is SwitchSection) {
+					script.InsertBefore (containingStatement, variableDecl);
+				} else {
+					var offset = script.GetCurrentOffset (containingStatement.StartLocation);
+					script.InsertBefore (containingStatement, variableDecl);
+					script.InsertText (offset, "{");
+					script.InsertText (script.GetCurrentOffset (containingStatement.EndLocation), "}");
+					script.FormatText (containingStatement.Parent);
+				}
+
+				var textNodes = new List<AstNode> ();
+				textNodes.Add (variableDecl.Variables.First ().NameToken);
+
+				foreach (var reference in env.GetAllReferences ()) {
+					var identifier = new IdentifierExpression (newName);
+					script.Replace (reference.AstNode, identifier);
+					textNodes.Add (identifier);
+				}
+				script.Link (textNodes.ToArray ());
+			};
+			yield return new CodeAction (context.TranslateString ("Copy to local variable"), action);
+		}
+	}
+}

ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/LocalVariableNamePicker.cs

@@ -0,0 +1,70 @@
+// 
+// NamePicker.cs
+// 
+// Author:
+//      Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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.Collections.Generic;
+using System.Linq;
+
+namespace ICSharpCode.NRefactory.CSharp.Refactoring
+{
+	public static class LocalVariableNamePicker
+	{
+		public static string PickSafeName (MethodDeclaration method, IEnumerable<string> candidates)
+		{
+			var existingNames = new VariableNameCollector ().Collect (method);
+			return candidates.FirstOrDefault (name => !existingNames.Contains (name));
+		}
+
+		class VariableNameCollector : DepthFirstAstVisitor
+		{
+			private ISet<string> variableNames = new HashSet<string> ();
+
+			public ISet<string> Collect (MethodDeclaration methodDecl)
+			{
+				variableNames.Clear ();
+				methodDecl.AcceptVisitor (this);
+				return variableNames;
+			}
+
+			public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration)
+			{
+				variableNames.Add (parameterDeclaration.Name);
+				base.VisitParameterDeclaration (parameterDeclaration);
+			}
+
+			public override void VisitVariableInitializer (VariableInitializer variableInitializer)
+			{
+				variableNames.Add (variableInitializer.Name);
+				base.VisitVariableInitializer (variableInitializer);
+			}
+
+			public override void VisitForeachStatement (ForeachStatement foreachStatement)
+			{
+				variableNames.Add (foreachStatement.VariableName);
+				base.VisitForeachStatement (foreachStatement);
+			}
+		}
+	}
+}

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

@@ -0,0 +1,106 @@
+// 
+// AccessToDisposedClosureTests.cs
+// 
+// Author:
+//      Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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 ICSharpCode.NRefactory.CSharp.CodeActions;
+using ICSharpCode.NRefactory.CSharp.Refactoring;
+using NUnit.Framework;
+
+namespace ICSharpCode.NRefactory.CSharp.CodeIssues
+{
+	public class AccessToDisposedClosureTests : InspectionActionTestBase
+	{
+		void Test (string input, int issueCount)
+		{
+			TestRefactoringContext context;
+			var issues = GetIssues (new AccessToDisposedClosureIssue (), input, out context);
+			Assert.AreEqual (issueCount, issues.Count);
+		}
+
+		[Test]
+		public void TestUsing ()
+		{
+			string input = @"
+using System;
+class TestClass : IDisposable
+{
+	public void Dispose () { }
+}
+class TestClass2
+{
+	void TestMethod ()
+	{
+		using (var x = new TestClass ()) {
+			Action a = () => x.ToString ();
+		}
+	}
+}";
+			Test (input, 1);
+		}
+
+		[Test]
+		public void TestUsing2 ()
+		{
+			string input = @"
+using System;
+class TestClass : IDisposable
+{
+	public void Dispose () { }
+}
+class TestClass2
+{
+	void TestMethod ()
+	{
+		var x = new TestClass ();
+		using (x) {
+			Action a = () => x.ToString ();
+		}
+	}
+}";
+			Test (input, 1);
+		}
+
+		[Test]
+		public void TestDispose ()
+		{
+			string input = @"
+using System;
+class TestClass : IDisposable
+{
+	public void Dispose () { }
+}
+class TestClass2
+{
+	void TestMethod ()
+	{
+		var x = new TestClass ();
+		Action a = () => x.ToString ();
+		x.Dispose();	
+	}
+}";
+			Test (input, 1);
+		}
+	}
+}

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

@@ -0,0 +1,753 @@
+// 
+// AccessToModifiedClosureTests.cs
+//  
+// Author:
+//       Mansheng Yang <lightyang0@gmail.com>
+// 
+// Copyright (c) 2012 Mansheng Yang <lightyang0@gmail.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 ICSharpCode.NRefactory.CSharp.CodeActions;
+using ICSharpCode.NRefactory.CSharp.Refactoring;
+using NUnit.Framework;
+
+namespace ICSharpCode.NRefactory.CSharp.CodeIssues
+{
+	[TestFixture]
+	public class AccessToModifiedClosureTests : InspectionActionTestBase
+	{
+		static void Test(string input, int issueCount, string output = null, int fixIndex = -1)
+		{
+			TestRefactoringContext context;
+			var issues = GetIssues (new AccessToModifiedClosureIssue (), input, out context);
+			Assert.AreEqual (issueCount, issues.Count);
+			if (issueCount > 0) {
+				if (output != null) {
+					if (fixIndex == -1)
+						CheckFix (context, issues, output);
+					else
+						CheckFix (context, issues [fixIndex], output);
+				} else {
+					foreach (var issue in issues)
+						Assert.AreEqual (0, issue.Actions.Count);
+				}
+			}
+		}
+
+		[Test]
+		public void TestForeachVariable ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		System.Func<int, int> f;
+		foreach (var i in a)
+			f = new System.Func<int, int> (x => x + i);
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		System.Func<int, int> f;
+		foreach (var i in a) {
+			var i1 = i;
+			f = new System.Func<int, int> (x => x + i1);
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestFor ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		for (int i = 0; i < 10; i++) {
+			var f = new System.Func<int, int> (x => x + i);
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		for (int i = 0; i < 10; i++) {
+			int i1 = i;
+			var f = new System.Func<int, int> (x => x + i1);
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+	
+		[Test]
+		public void TestForInitializer ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int j)
+	{
+		int i;
+		for (i = 0; j < 10; j++) {
+			var f = new System.Func<int, int> (x => x + i);
+		}
+	}
+}";
+			Test (input, 0);
+		}
+
+		[Test]
+		public void TestForBody ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		for (int i = 0; i < 10;) {
+			var f = new System.Func<int, int> (delegate (int x) { return x + i; });
+			i++;
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		for (int i = 0; i < 10;) {
+			int i1 = i;
+			var f = new System.Func<int, int> (delegate (int x) { return x + i1; });
+			i++;
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+		
+		[Test]
+		public void TestWhileBody ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i < 10) {
+			i++;
+			var f = new System.Func<int, int> (x => x + i);
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i < 10) {
+			i++;
+			int i1 = i;
+			var f = new System.Func<int, int> (x => x + i1);
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+		
+		[Test]
+		public void TestWhileCondition ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i++ < 10) {
+			var f = new System.Func<int, int> (delegate (int x) { return x + i; });
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i++ < 10) {
+			int i1 = i;
+			var f = new System.Func<int, int> (delegate (int x) { return x + i1; });
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestDoWhileBody ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		do {
+			i += 1;
+			var f = new System.Func<int, int> (x => x + i);
+		} while (i < 10);
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		do {
+			i += 1;
+			int i1 = i;
+			var f = new System.Func<int, int> (x => x + i1);
+		} while (i < 10);
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestDoWhileCondition ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i++ < 10) {
+			var f = new System.Func<int, int> (x => x + i);
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		while (i++ < 10) {
+			int i1 = i;
+			var f = new System.Func<int, int> (x => x + i1);
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestMultipleLambdas ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			var f = new System.Func<int, int> (x => x + i);
+			var f2 = new System.Func<int, bool> (x => x + i > 10);
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			var i1 = i;
+			var f = new System.Func<int, int> (x => x + i1);
+			var i1 = i;
+			var f2 = new System.Func<int, bool> (x => x + i1 > 10);
+		}
+	}
+}";
+			Test (input, 2, output);
+		}
+
+		[Test]
+		public void TestFixAllInLambda ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] array)
+	{
+		foreach (var i in array) {
+			var f = new System.Func<int, int> (x => {
+				int a = i + 1;
+				int b = i + 2;
+				return a + b + i;
+			});
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] array)
+	{
+		foreach (var i in array) {
+			var i1 = i;
+			var f = new System.Func<int, int> (x => {
+				int a = i1 + 1;
+				int b = i1 + 2;
+				return a + b + i1;
+			});
+		}
+	}
+}";
+			Test (input, 3, output, 0);
+		}
+		
+		[Test]
+		public void TestModifiedInLambda ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] array)
+	{
+		foreach (var i in array) {
+			var f = new System.Func<int, int> (x => {
+				i = 0;
+				int a = i + 1;
+				int b = i + 2;
+				return a + b + i;
+			});
+		}
+	}
+}";
+			Test (input, 0);
+			input = @"
+class TestClass
+{
+	void TestMethod (int[] array)
+	{
+		foreach (var i in array) {
+			var f = new System.Func<int, int> (x => {
+				i++;
+				int a = i + 1;
+				i = 3;
+				int b = i + 2;
+				return a + b + i;
+			});
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] array)
+	{
+		foreach (var i in array) {
+			var i1 = i;
+			var f = new System.Func<int, int> (x => {
+				i1++;
+				int a = i1 + 1;
+				i1 = 3;
+				int b = i1 + 2;
+				return a + b + i1;
+			});
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+		
+		[Test]
+		public void TestNestedLambda ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			var f = new System.Func<int, int> (x => {
+				var f2 =  new System.Func<int, int> (y => y - i);
+				return f2(x) + i;
+			});
+		}
+	}
+}";
+			var output1 = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			var i1 = i;
+			var f = new System.Func<int, int> (x => {
+				var f2 =  new System.Func<int, int> (y => y - i1);
+				return f2(x) + i1;
+			});
+		}
+	}
+}";
+			Test (input, 2, output1, 1);
+			var output2 = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			var f = new System.Func<int, int> (x => {
+				var i1 = i;
+				var f2 =  new System.Func<int, int> (y => y - i1);
+				return f2(x) + i;
+			});
+		}
+	}
+}";
+			Test (input, 2, output2, 0);
+		}
+	
+		[Test]
+		public void TestMultipleVariables ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			foreach (var j in a) {
+				var f = new System.Func<int, int> (x => x + i + j);
+			}
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			foreach (var j in a) {
+				var i1 = i;
+				var j1 = j;
+				var f = new System.Func<int, int> (x => x + i1 + j1);
+			}
+		}
+	}
+}";
+			Test (input, 2, output);
+		}
+
+		[Test]
+		public void TestModificationAfterLambdaDecl ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		System.Func<int, int> f = x => i + x;
+		i += 1;
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		int i1 = i;
+		System.Func<int, int> f = x => i1 + x;
+		i += 1;
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestModificationBeforeLambdaDecl ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		i += 1;
+		System.Func<int, int> f = x => i + x;
+	}
+}";
+			Test (input, 0);
+		}
+
+		[Test]
+		public void TestUnreachable ()
+		{
+			var returnCase = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		System.Func<int, int> f = x => i + x;
+		return;
+		i += 1;
+	}
+}";
+			var ifCase = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		System.Func<int, int> f;
+		if (i > 0)
+			f = x => i + x;
+		else
+			i += 1;
+	}
+}";
+			Test (returnCase, 0);
+			Test (ifCase, 0);
+		}
+
+		[Test]
+		public void TestParameter ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int i)
+	{
+		System.Func<int, int> f = x => i + x;
+		i += 1;
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int i)
+	{
+		int i1 = i;
+		System.Func<int, int> f = x => i1 + x;
+		i += 1;
+	}
+}";
+			Test (input, 1, output);
+		}
+
+		[Test]
+		public void TestModificationInSameStatement ()
+		{
+			var input1 = @"
+class TestClass
+{
+	void TestMethod2 (int b, System.Func<int, int> a)
+	{
+	    TestMethod2 (b++, c => c + b);
+	}
+}";
+			var input2 = @"
+class TestClass
+{
+	void TestMethod3 (System.Func<int, int> a, int b)
+	{
+	    TestMethod3 (c => c + b, b++);
+	}
+}";
+			var output2 = @"
+class TestClass
+{
+	void TestMethod3 (System.Func<int, int> a, int b)
+	{
+		int b1 = b;
+	    TestMethod3 (c => c + b1, b++);
+	}
+}";
+			Test (input1, 0);
+			Test (input2, 1, output2);
+		}
+		
+		[Test]
+		public void TestInsertion ()
+		{
+			var input1 = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		System.Func<int, int> f = null;
+		if ((f = x => x + i) != null)
+			i++;
+	}
+}";
+			var output1 = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		int i = 0;
+		System.Func<int, int> f = null;
+		int i1 = i;
+		if ((f = x => x + i1) != null)
+			i++;
+	}
+}";
+			Test (input1, 1, output1);
+
+			var input2 = @"
+class TestClass
+{
+	void TestMethod (int k)
+	{
+		int i = 0;
+		System.Func<int, int> f = null;
+		switch (k) {
+		default:
+			f = x => x + i;
+			break;
+		}
+		i++;
+	}
+}";
+			var output2 = @"
+class TestClass
+{
+	void TestMethod (int k)
+	{
+		int i = 0;
+		System.Func<int, int> f = null;
+		switch (k) {
+		default:
+			int i1 = i;
+			f = x => x + i1;
+			break;
+		}
+		i++;
+	}
+}";
+			Test (input2, 1, output2);
+		}
+
+		[Test]
+		public void TestLambdaInLoopCondition ()
+		{
+			var forCase = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		System.Func<int, int> f = null;
+		for (int i = 0; i < 10 && ((f = x => x + i) != null); i++) {
+		}
+	}
+}";
+			var whileCase = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		System.Func<int, int> f = null;
+		int i = 0;
+		while (i < 10 && (f = x => x + i) != null) i++;
+	}
+}";
+			var doWhileCase = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		System.Func<int, int> f = null;
+		int i = 0;
+		do { i++; } while (i < 10 && (f = x => x + i) != null);
+	}
+}";
+			Test (forCase, 1);
+			Test (whileCase, 1);
+			Test (doWhileCase, 1);
+		}
+
+		[Test]
+		public void TestLambdaInForIterator ()
+		{
+
+			var input = @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		System.Func<int, int> f = null;
+		for (int i = 0; i < 10; i++, f = x => x + i) {
+		}
+	}
+}";
+			Test (input, 1);
+		}
+
+		[Test]
+		public void TestExistingVariableName ()
+		{
+			var input = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			int i1;
+			var f = new System.Func<int, int> (x => x + i);
+		}
+	}
+}";
+			var output = @"
+class TestClass
+{
+	void TestMethod (int[] a)
+	{
+		foreach (var i in a) {
+			int i1;
+			var i2 = i;
+			var f = new System.Func<int, int> (x => x + i2);
+		}
+	}
+}";
+			Test (input, 1, output);
+		}
+		
+	}
+}

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

@@ -98,6 +98,8 @@
     <Compile Include="CSharp\CodeActions\PutInsideUsingTests.cs" />
     <Compile Include="CSharp\CodeActions\SplitDeclarationListTests.cs" />
     <Compile Include="CSharp\CodeActions\UseStringFormatTests.cs" />
+    <Compile Include="CSharp\CodeIssues\AccessToDisposedClosureTests.cs" />
+    <Compile Include="CSharp\CodeIssues\AccessToModifiedClosureTests.cs" />
     <Compile Include="CSharp\CodeIssues\ExplicitConversionInForEachIssueTests.cs" />
     <Compile Include="CSharp\CSharpAmbienceTests.cs" />
     <Compile Include="CSharp\CodeDomConvertVisitorTests.cs" />