Merge pull request #127 from fightangel/assignment-fix

[CodeIssues]RedundantAssignmentIssue: ignore assignments in try blocks

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

@@ -42,12 +42,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 			var unit = context.RootNode as SyntaxTree;
 			if (unit == null)
 				return Enumerable.Empty<CodeIssue> ();
-			return new GatherVisitor (context, unit).GetIssues ();
+			return new GatherVisitor (context).GetIssues ();
 		}
 
 		class GatherVisitor : GatherVisitorBase
 		{
-			public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit)
+			public GatherVisitor (BaseRefactoringContext ctx)
 				: base (ctx)
 			{
 			}
@@ -170,6 +170,14 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 				return false;
 			}
 
+			static bool IsInsideTryBlock (AstNode node)
+			{
+				var tryCatchStatement = node.GetParent<TryCatchStatement> ();
+				if (tryCatchStatement == null)
+					return false;
+				return tryCatchStatement.TryBlock.Contains (node.StartLocation.Line,node.StartLocation.Column);
+			}
+
 			enum NodeState
 			{
 				None,
@@ -222,8 +230,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 					}
 				}
 
-				foreach (var node in assignments)
+				foreach (var node in assignments) {
-					ProcessNode (node, true, nodeStates);
+					// we do not analyze an assignment inside a try block as it can jump to any catch block or finally block
+					if (IsInsideTryBlock(node.References[0]))
+						continue;
+					ProcessNode(node, true, nodeStates);
+				}
 			}
 
 			void ProcessNode (VariableReferenceNode node, bool addIssue,

ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs

@@ -255,26 +255,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 						}
 						break;
 					}
-
-					// Hack for handling try ... catch ... finally.
-					var tryc = cfNode.NextStatement as TryCatchStatement;
-					if (tryc != null) {
-						VariableReferenceNode outNode = null;
-						foreach (var n in tryc.CatchClauses) {
-							var catchNode = variableReferenceGraphBuilder.Build(n.Body, references, refStatements, this.resolver);
-							(outNode ?? node).AddNextNode (catchNode);
-							outNode = catchNode;
-						}
-						if (!tryc.FinallyBlock.IsNull) {
-							var finallyNode = variableReferenceGraphBuilder.Build(tryc.FinallyBlock, references, refStatements, this.resolver);
-							(outNode ?? node).AddNextNode (finallyNode);
-							outNode = finallyNode;
-						}
-						if (outNode != null) {
-							nodeDict [cfNode] = outNode;
-							return outNode;
-						}
-					}
 				}
 				VariableReferenceNode result;
 				if (!nodeDict.TryGetValue (startNode, out result))

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

@@ -331,6 +331,48 @@ class TestClass
 			}
 		}
 	}
+}";
+			Test<RedundantAssignmentIssue> (input, 0);
+		}
+
+		[Test]
+		public void TestAssignmentInCatch ()
+		{
+			var input = @"using System;
+class TestClass
+{
+    void Test(TestClass a) { }
+
+	void TestMethod ()
+	{
+		var a = new TestClass ();
+		try {
+		} catch (Exception) {
+			a = null;
+		}
+        Test (a);
+	}
+}";
+			Test<RedundantAssignmentIssue> (input, 0);
+		}
+
+		[Test]
+		public void TestAssignmentBeforeTry ()
+		{
+			var input = @"using System;
+class TestClass
+{
+    void Test(TestClass a) { }
+
+	void TestMethod ()
+	{
+		var a = null;
+		try {
+			a = new TestClass ();
+		} catch (Exception) {
+		}
+        Test (a);
+	}
 }";
 			Test<RedundantAssignmentIssue> (input, 0);
 		}