CodeAnalyzer, Control Flag - Fixed false positive

blowin · Aug 5, 2022 · a5abfb2 · a5abfb2
1 parent bfaa398
commit a5abfb2
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 19 deletions.
diff --git a/BlowinCleanCode/BlowinCleanCode.Test/SingleResponsibility/ControlFlagFeatureTest.cs b/BlowinCleanCode/BlowinCleanCode.Test/SingleResponsibility/ControlFlagFeatureTest.cs
@@ -207,6 +207,37 @@ public void Run(bool dummy)
                 }
             }
         }
+    }")]
+        [InlineData(@"
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Diagnostics;
+
+    namespace ConsoleApplication1
+    {
+        class Holder : IDisposable
+        {
+            public Holder(bool v) {}
+
+            public void Dispose(){}
+        }
+
+        class Test
+        {
+            public void Run(bool dummy)
+            {
+                // Disable BCC4002
+                if(true)
+                {
+                    using(var holder = new Holder(dummy)){
+                        return;
+                    }
+                }
+            }
+        }
     }")]
         public async Task Valid(string test)
         {

diff --git a/BlowinCleanCode/BlowinCleanCode/Feature/SingleResponsibility/ControlFlagFeatureAnalyze.cs b/BlowinCleanCode/BlowinCleanCode/Feature/SingleResponsibility/ControlFlagFeatureAnalyze.cs
@@ -1,5 +1,4 @@
-using System.Collections.Generic;
-using System.Collections.Immutable;
+using System.Collections.Immutable;
 using System.Linq;
 using BlowinCleanCode.Extension;
 using BlowinCleanCode.Extension.SyntaxExtension;
@@ -31,9 +30,7 @@ protected override void Analyze(SyntaxNodeAnalysisContext context, MethodDeclara
             if (flagParameters.Length == 0)
                 return;
 
-            var descendant = syntaxNode.Body?.DescendantNodes() ??
-                             syntaxNode.ExpressionBody?.DescendantNodes() ??
-                             Enumerable.Empty<SyntaxNode>();
+            var descendant = syntaxNode.Body ?? (SyntaxNode)syntaxNode.ExpressionBody;
 
             foreach (var controlFlag in UseAsControlFlag(flagParameters, descendant, pts.Keyword.Kind()))
             {
@@ -42,17 +39,20 @@ protected override void Analyze(SyntaxNodeAnalysisContext context, MethodDeclara
             }
         }
 
-        private static ImmutableArray<SyntaxToken> UseAsControlFlag(ImmutableArray<string> flagParameters,
-            IEnumerable<SyntaxNode> body, SyntaxKind returnKeyword)
+        private static ImmutableArray<SyntaxToken> UseAsControlFlag(ImmutableArray<string> flagParameters, SyntaxNode body, SyntaxKind returnKeyword)
         {
+            if(body == null)
+                return ImmutableArray<SyntaxToken>.Empty;
+
             var result = ImmutableArray<SyntaxToken>.Empty;
-            foreach (var node in body)
+            foreach (var node in body.DescendantNodes())
             {
-                if(!IsCondition(node))
+                var condition = GetCondition(node);
+                if (condition == null)
                     continue;
 
-                var descendant = node
-                    .DescendantNodes(sn => sn.IsNot<InvocationExpressionSyntax>())
+                var descendant = condition
+                    .DescendantNodesAndSelf(sn => sn.IsAny<BinaryExpressionSyntax, ParenthesizedExpressionSyntax>())
                     .OfType<IdentifierNameSyntax>();
 
                 foreach (var nameSyntax in descendant)
@@ -75,8 +75,17 @@ private static ImmutableArray<SyntaxToken> UseAsControlFlag(ImmutableArray<strin
             return result;
         }
 
-        private static bool IsCondition(SyntaxNode node) => node.IsAny<IfStatementSyntax, ConditionalExpressionSyntax>();
-
+        private static ExpressionSyntax GetCondition(SyntaxNode node)
+        {
+            if (node is IfStatementSyntax ifStatement)
+                return ifStatement.Condition;
+
+            if (node is ConditionalExpressionSyntax conditionalExpression)
+                return conditionalExpression.Condition;
+
+            return null;
+        }
+
         private static bool ConditionForSingleReturn(SyntaxNode nameSyntax, SyntaxKind returnKeyword)
         {
             // Only for void method
@@ -85,10 +94,7 @@ private static bool ConditionForSingleReturn(SyntaxNode nameSyntax, SyntaxKind r
 
             foreach (var syntaxNode in nameSyntax.Ancestors())
             {
-                if (!IsCondition(syntaxNode)) 
-                    continue;
-
-                if (!(syntaxNode is IfStatementSyntax ifStatement)) 
+                if (!syntaxNode.Is<IfStatementSyntax>(out var ifStatement)) 
                     return false;
 
                 switch (ifStatement.Statement)
@@ -107,8 +113,10 @@ private static bool ConditionForSingleReturn(SyntaxNode nameSyntax, SyntaxKind r
 
         private static ImmutableArray<string> FlagParameters(ParameterListSyntax parameterList)
         {
-            var countOfParameters = parameterList?.Parameters.Count ?? 0;
-            if (countOfParameters == 0)
+            if(parameterList?.Parameters == null)
+                return ImmutableArray<string>.Empty;
+
+            if (parameterList.Parameters.Count == 0)
                 return ImmutableArray<string>.Empty;
 
             var result = ImmutableArray<string>.Empty;