From 15473ce42f58c2654baf487629ddb5db7d27f40a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 21 Jan 2014 09:26:01 +0100 Subject: [PATCH] Fixed Issue #372 'False positive in "Convert to |= expression issue"'. --- .../Custom/ConvertIfToAndExpressionIssue.cs | 4 ++++ .../ConvertIfToOrExpressionIssue.cs | 12 ++++++++++++ .../ConvertIfToOrExpressionIssueTests.cs | 17 +++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/ConvertIfToAndExpressionIssue.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Custom/ConvertIfToAndExpressionIssue.cs index 172bb66b3..a80937182 100644 --- 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("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("condition").Single(); + if (!ConvertIfToOrExpressionIssue.CheckTarget(target, expr)) + return; AddIssue(new CodeIssue( ifElseStatement.IfToken, ctx.TranslateString("Convert to '&=' expresssion"), diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Synced/PracticesAndImprovements/ConvertIfToOrExpressionIssue.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Synced/PracticesAndImprovements/ConvertIfToOrExpressionIssue.cs index 315dd82d6..bad8fa3ce 100644 --- a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Synced/PracticesAndImprovements/ConvertIfToOrExpressionIssue.cs +++ b/ICSharpCode.NRefactory.CSharp.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 { 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("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("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 index b874c4a2e..d5fe187c6 100644 --- 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(@"class Foo +{ + public bool Enabled { get; set; } + + int Bar(Foo fileChangeWatcher) + { + if (fileChangeWatcher != null) + fileChangeWatcher.Enabled = true; + } +}"); + } + + + } }