Fixed Issue #372 'False positive in "Convert to |= expression issue"'.

icsharpcode · Jan 21, 2014 · 15473ce · 15473ce
1 parent e3d3917
commit 15473ce
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 0 deletions.
diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/ConvertIfToAndExpressionIssue.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/ConvertIfToAndExpressionIssue.cs
@@ -80,6 +80,8 @@ public override void VisitIfElseStatement(IfElseStatement ifElseStatement)
 						if (initializer != null && target is IdentifierExpression && ((IdentifierExpression)target).Identifier != initializer.Name)
 							return;
 						var expr = match.Get<Expression>("condition").Single();
+						if (!ConvertIfToOrExpressionIssue.CheckTarget(target, expr))
+							return;
 						AddIssue(new CodeIssue(
 							ifElseStatement.IfToken,
 							ctx.TranslateString("Convert to '&&' expresssion"),
@@ -106,6 +108,8 @@ public override void VisitIfElseStatement(IfElseStatement ifElseStatement)
 						return;
 					} else {
 						var expr = match.Get<Expression>("condition").Single();
+						if (!ConvertIfToOrExpressionIssue.CheckTarget(target, expr))
+							return;
 						AddIssue(new CodeIssue(
 							ifElseStatement.IfToken,
 							ctx.TranslateString("Convert to '&=' expresssion"),

diff --git a/...rp.Refactoring/CodeIssues/Synced/PracticesAndImprovements/ConvertIfToOrExpressionIssue.cs b/...rp.Refactoring/CodeIssues/Synced/PracticesAndImprovements/ConvertIfToOrExpressionIssue.cs
@@ -43,6 +43,14 @@ protected override IGatherVisitor CreateVisitor(BaseRefactoringContext context)
 			return new GatherVisitor(context);
 		}
 
+		internal static bool CheckTarget(Expression target, Expression expr)
+		{
+			return !target.DescendantNodesAndSelf().Any(
+				n => (n is IdentifierExpression || n is MemberReferenceExpression) && 
+				expr.DescendantNodesAndSelf().Any(n2 => ((INode)n).IsMatch(n2))
+			);
+		}
+
 		class GatherVisitor : GatherVisitorBase<ConvertIfToOrExpressionIssue>
 		{
 			public GatherVisitor (BaseRefactoringContext ctx) : base (ctx)
@@ -83,6 +91,8 @@ public override void VisitIfElseStatement(IfElseStatement ifElseStatement)
 						if (initializer == null || !(target is IdentifierExpression) || ((IdentifierExpression)target).Identifier != initializer.Name)
 							return;
 						var expr = match.Get<Expression>("condition").Single();
+						if (!CheckTarget(target, expr))
+							return;
 						AddIssue(new CodeIssue(
 							ifElseStatement.IfToken,
 							ctx.TranslateString("Convert to '||' expresssion"),
@@ -103,6 +113,8 @@ public override void VisitIfElseStatement(IfElseStatement ifElseStatement)
 						return;
 					} else {
 						var expr = match.Get<Expression>("condition").Single();
+						if (!CheckTarget(target, expr))
+							return;
 						AddIssue(new CodeIssue(
 							ifElseStatement.IfToken,
 							ctx.TranslateString("Convert to '|=' expresssion"),

diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ConvertIfToOrExpressionIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ConvertIfToOrExpressionIssueTests.cs
@@ -111,6 +111,23 @@ int Bar(int o)
 }");
 		}
 
+		[Test]
+		public void TestNullCheckBug ()
+		{
+			TestWrongContext<ConvertIfToOrExpressionIssue>(@"class Foo
+{
+	public bool Enabled { get; set; }
+
+	int Bar(Foo fileChangeWatcher)
+	{
+		if (fileChangeWatcher != null)
+			fileChangeWatcher.Enabled = true;
+	}
+}");
+		}
+
+
+
 	}
 }