From c893caa3041a88dcac4a685fc4455fbc93bf0968 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Wed, 16 Sep 2020 22:31:24 +0200 Subject: [PATCH] x.HasFlag(flags) is equal to (x & flags) == flags (RCS1096) (fix #720) --- ...rtHasFlagCallToBitwiseOperationAnalysis.cs | 17 +- .../InvocationExpressionRefactoring.cs | 1 + ...nvertHasFlagCallToBitwiseOperationTests.cs | 180 ++++++++++++++++++ ...asFlagCallToBitwiseOperationRefactoring.cs | 81 +++++--- 4 files changed, 251 insertions(+), 28 deletions(-) diff --git a/src/Common/CSharp/Analysis/ConvertHasFlagCallToBitwiseOperationAnalysis.cs b/src/Common/CSharp/Analysis/ConvertHasFlagCallToBitwiseOperationAnalysis.cs index 5c045f2334..5e8321e44c 100644 --- 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 index 5e5f282215..22511d095e 100644 --- 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 index 802a45f833..a1da4af5a7 100644 --- 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 index c6ec51625e..9e8a17c40c 100644 --- 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 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 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 constantValue = semanticModel.GetConstantValue(expression, cancellationToken); + + ulong value = SymbolUtility.GetEnumValueAsUInt64(constantValue.Value, enumTypeSymbol); + + bool isComposite = FlagsUtility.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();