x.HasFlag(flags) is equal to (x & flags) == flags (RCS1096) (fix #720)

dotnet · Sep 16, 2020 · c893caa · c893caa
1 parent 5c1706c
commit c893caa
Show file tree

Hide file tree

Showing 4 changed files with 251 additions and 28 deletions.
diff --git a/src/Common/CSharp/Analysis/ConvertHasFlagCallToBitwiseOperationAnalysis.cs b/src/Common/CSharp/Analysis/ConvertHasFlagCallToBitwiseOperationAnalysis.cs
@@ -1,6 +1,5 @@
 // Copyright (c) Josef Pihrt. All rights reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis;
@@ -41,12 +40,22 @@ public static bool IsFixable(
 
             IMethodSymbol methodSymbol = semanticModel.GetMethodSymbol(invocationInfo.InvocationExpression, cancellationToken);
 
-            return methodSymbol?.IsStatic == false
+            if (methodSymbol?.IsStatic == false
                 && methodSymbol.IsReturnType(SpecialType.System_Boolean)
                 && methodSymbol.HasSingleParameter(SpecialType.System_Enum)
                 && methodSymbol.IsContainingType(SpecialType.System_Enum)
-                && !semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken).HasMetadataName(MetadataNames.System_Enum)
-                && !semanticModel.GetTypeSymbol(invocationInfo.Arguments.Single().Expression, cancellationToken).HasMetadataName(MetadataNames.System_Enum);
+                && !semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken).HasMetadataName(MetadataNames.System_Enum))
+            {
+                ExpressionSyntax expression = invocationInfo.Arguments.Single().Expression;
+
+                if (!semanticModel.GetTypeSymbol(expression, cancellationToken).HasMetadataName(MetadataNames.System_Enum)
+                    && semanticModel.HasConstantValue(expression, cancellationToken))
+                {
+                    return true;
+                }
+            }
+
+            return false;
         }
 
         public static bool IsSuitableAsArgumentOfHasFlag(

diff --git a/src/Refactorings/CSharp/Refactorings/InvocationExpressionRefactoring.cs b/src/Refactorings/CSharp/Refactorings/InvocationExpressionRefactoring.cs
@@ -87,6 +87,7 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, In
                             return ConvertHasFlagCallToBitwiseOperationRefactoring.RefactorAsync(
                                 context.Document,
                                 invocationExpression,
+                                semanticModel,
                                 cancellationToken);
                         },
                         RefactoringIdentifiers.ConvertHasFlagCallToBitwiseOperation);

diff --git a/src/Tests/Analyzers.Tests/RCS1096ConvertHasFlagCallToBitwiseOperationTests.cs b/src/Tests/Analyzers.Tests/RCS1096ConvertHasFlagCallToBitwiseOperationTests.cs
@@ -47,6 +47,66 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_HasFlag_Flag()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ([|x.HasFlag(RegexOptions.Singleline)|]) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & RegexOptions.Singleline) != 0) { }
+    }
+}
+");
+        }
+
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_HasFlags()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ([|x.HasFlag(RegexOptions.Singleline | RegexOptions.Multiline)|]) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & (RegexOptions.Singleline | RegexOptions.Multiline)) == (RegexOptions.Singleline | RegexOptions.Multiline)) { }
+    }
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
         public async Task Test_HasFlag_Parentheses()
         {
@@ -107,6 +167,66 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_NotHasFlag_Flag()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if (![|x.HasFlag(RegexOptions.Singleline)|]) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & RegexOptions.Singleline) == 0) { }
+    }
+}
+");
+        }
+
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_NotHasFlags()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if (![|x.HasFlag(RegexOptions.Singleline | RegexOptions.Multiline)|]) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & (RegexOptions.Singleline | RegexOptions.Multiline)) != (RegexOptions.Singleline | RegexOptions.Multiline)) { }
+    }
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
         public async Task Test_HasFlag_EqualsTrue()
         {
@@ -137,6 +257,36 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_HasFlags_EqualsTrue()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ([|x.HasFlag(RegexOptions.Singleline | RegexOptions.Multiline)|] == true) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & (RegexOptions.Singleline | RegexOptions.Multiline)) == (RegexOptions.Singleline | RegexOptions.Multiline)) { }
+    }
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
         public async Task Test_HasFlag_EqualsFalse()
         {
@@ -167,6 +317,36 @@ void M()
 ");
         }
 
+        [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
+        public async Task Test_HasFlags_EqualsFalse()
+        {
+            await VerifyDiagnosticAndFixAsync(@"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ([|x.HasFlag(RegexOptions.Singleline | RegexOptions.Multiline)|] == false) { }
+    }
+}
+", @"
+using System.Text.RegularExpressions;
+
+class C
+{
+    void M()
+    {
+        var x = RegexOptions.Singleline | RegexOptions.Multiline;
+
+        if ((x & (RegexOptions.Singleline | RegexOptions.Multiline)) != (RegexOptions.Singleline | RegexOptions.Multiline)) { }
+    }
+}
+");
+        }
+
         [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ConvertHasFlagCallToBitwiseOperationOrViceVersa)]
         public async Task Test_HasFlag_WithTrivia()
         {

diff --git a/src/Workspaces.Common/CSharp/Refactorings/ConvertHasFlagCallToBitwiseOperationRefactoring.cs b/src/Workspaces.Common/CSharp/Refactorings/ConvertHasFlagCallToBitwiseOperationRefactoring.cs
@@ -14,54 +14,87 @@ internal static class ConvertHasFlagCallToBitwiseOperationRefactoring
     {
         public const string Title = "Use '&' operator";
 
+        public static async Task<Document> RefactorAsync(
+            Document document,
+            InvocationExpressionSyntax invocation,
+            CancellationToken cancellationToken = default)
+        {
+            SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+
+            return await RefactorAsync(document, invocation, semanticModel, cancellationToken).ConfigureAwait(false);
+        }
+
         public static Task<Document> RefactorAsync(
             Document document,
             InvocationExpressionSyntax invocation,
+            SemanticModel semanticModel,
             CancellationToken cancellationToken = default)
         {
+            ExpressionSyntax expression = invocation.ArgumentList.Arguments[0].Expression;
+
+            var enumTypeSymbol = (INamedTypeSymbol)semanticModel.GetTypeSymbol(expression, cancellationToken);
+
+            Optional<object> constantValue = semanticModel.GetConstantValue(expression, cancellationToken);
+
+            ulong value = SymbolUtility.GetEnumValueAsUInt64(constantValue.Value, enumTypeSymbol);
+
+            bool isComposite = FlagsUtility<ulong>.Instance.IsComposite(value);
+
             ParenthesizedExpressionSyntax parenthesizedExpression = ParenthesizedExpression(
                 BitwiseAndExpression(
                     ((MemberAccessExpressionSyntax)invocation.Expression).Expression.Parenthesize(),
-                    invocation.ArgumentList.Arguments[0].Expression.Parenthesize()).Parenthesize());
+                    expression.Parenthesize()).Parenthesize());
+
+            SyntaxKind binaryExpressionKind = (isComposite) ? SyntaxKind.EqualsExpression : SyntaxKind.NotEqualsExpression;
 
-            var binaryExpressionKind = SyntaxKind.NotEqualsExpression;
             SyntaxNode nodeToReplace = invocation;
 
             SyntaxNode parent = invocation.Parent;
 
             if (!parent.SpanContainsDirectives())
             {
-                SyntaxKind parentKind = parent.Kind();
-
-                if (parentKind == SyntaxKind.LogicalNotExpression)
-                {
-                    binaryExpressionKind = SyntaxKind.EqualsExpression;
-                    nodeToReplace = parent;
-                }
-                else if (parentKind == SyntaxKind.EqualsExpression)
+                switch (parent.Kind())
                 {
-                    ExpressionSyntax right = ((BinaryExpressionSyntax)parent).Right;
-
-                    if (right != null)
-                    {
-                        SyntaxKind rightKind = right.Kind();
-
-                        if (rightKind == SyntaxKind.TrueLiteralExpression)
+                    case SyntaxKind.LogicalNotExpression:
                         {
-                            binaryExpressionKind = SyntaxKind.NotEqualsExpression;
+                            binaryExpressionKind = (isComposite) ? SyntaxKind.NotEqualsExpression : SyntaxKind.EqualsExpression;
                             nodeToReplace = parent;
+                            break;
                         }
-                        else if (rightKind == SyntaxKind.FalseLiteralExpression)
+                    case SyntaxKind.EqualsExpression:
                         {
-                            binaryExpressionKind = SyntaxKind.EqualsExpression;
-                            nodeToReplace = parent;
+                            switch (((BinaryExpressionSyntax)parent).Right?.Kind())
+                            {
+                                case SyntaxKind.TrueLiteralExpression:
+                                    {
+                                        binaryExpressionKind = (isComposite) ? SyntaxKind.EqualsExpression : SyntaxKind.NotEqualsExpression;
+                                        nodeToReplace = parent;
+                                        break;
+                                    }
+                                case SyntaxKind.FalseLiteralExpression:
+                                    {
+                                        binaryExpressionKind = (isComposite) ? SyntaxKind.NotEqualsExpression : SyntaxKind.EqualsExpression;
+                                        nodeToReplace = parent;
+                                        break;
+                                    }
+                            }
+
+                            break;
                         }
-                    }
                 }
             }
 
-            ParenthesizedExpressionSyntax newNode = BinaryExpression(binaryExpressionKind, parenthesizedExpression, NumericLiteralExpression(0))
-                .WithTriviaFrom(nodeToReplace)
+            ExpressionSyntax right;
+            if (isComposite)
+            {
+                right = expression.Parenthesize();
+            }
+            else
+            {
+                right = NumericLiteralExpression(0);
+            }
+
+            ParenthesizedExpressionSyntax newNode = BinaryExpression(binaryExpressionKind, parenthesizedExpression, right).WithTriviaFrom(nodeToReplace)
                 .Parenthesize()
                 .WithFormatterAnnotation();