diff --git a/docs/Rules/MA0192.md b/docs/Rules/MA0192.md index 31475d865..a2202fa8e 100644 --- a/docs/Rules/MA0192.md +++ b/docs/Rules/MA0192.md @@ -11,6 +11,12 @@ This rule reports patterns such as: - `(value & MyEnum.Flag1) != MyEnum.Flag1` - `(value & MyEnum.Flag1) is MyEnum.Flag1` - `(value & MyEnum.Flag1) is not MyEnum.Flag1` +- `(value & MyEnum.Flag1) == 0` +- `(value & MyEnum.Flag1) != 0` +- `(value & MyEnum.Flag1) is 0` +- `(value & MyEnum.Flag1) is not 0` + +For comparisons against `0`, the enum member used in the bitwise `&` must be a single-bit value (for example `1`, `2`, `4`, `8`, ...). Combined values are ignored. ## Non-compliant code diff --git a/src/Meziantou.Analyzer.CodeFixers/Rules/UseHasFlagMethodFixer.cs b/src/Meziantou.Analyzer.CodeFixers/Rules/UseHasFlagMethodFixer.cs index 748933841..2c297ac92 100644 --- a/src/Meziantou.Analyzer.CodeFixers/Rules/UseHasFlagMethodFixer.cs +++ b/src/Meziantou.Analyzer.CodeFixers/Rules/UseHasFlagMethodFixer.cs @@ -184,42 +184,53 @@ private static bool TryGetComparedOperand(IPatternOperation patternOperation, [N var rightOperand = bitwiseAndOperation.RightOperand.UnwrapImplicitConversionOperations(); comparedOperand = comparedOperand.UnwrapImplicitConversionOperations(); - if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation) && + if (TryGetEnumFlagReference(rightOperand, comparedOperand, out var flagOperation, out var comparedWithZero) && IsValidPattern(leftOperand, flagOperation) && leftOperand.Syntax is ExpressionSyntax enumValueExpression && flagOperation.Syntax is ExpressionSyntax flagExpression) { - return new(operationExpression, enumValueExpression, flagExpression, negate); + return new(operationExpression, enumValueExpression, flagExpression, comparedWithZero ? !negate : negate); } - if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation) && + if (TryGetEnumFlagReference(leftOperand, comparedOperand, out flagOperation, out comparedWithZero) && IsValidPattern(rightOperand, flagOperation) && rightOperand.Syntax is ExpressionSyntax enumValueExpression2 && flagOperation.Syntax is ExpressionSyntax flagExpression2) { - return new(operationExpression, enumValueExpression2, flagExpression2, negate); + return new(operationExpression, enumValueExpression2, flagExpression2, comparedWithZero ? !negate : negate); } return null; } - private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IFieldReferenceOperation? flagOperation) + private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand, [NotNullWhen(true)] out IFieldReferenceOperation? flagOperation, out bool comparedWithZero) { potentialFlag = potentialFlag.UnwrapImplicitConversionOperations(); comparedOperand = comparedOperand.UnwrapImplicitConversionOperations(); if (potentialFlag is IFieldReferenceOperation firstFieldReference && - comparedOperand is IFieldReferenceOperation secondFieldReference && firstFieldReference.Field.HasConstantValue && - secondFieldReference.Field.HasConstantValue && - firstFieldReference.Field.IsEqualTo(secondFieldReference.Field) && firstFieldReference.Field.ContainingType.IsEnumeration()) { - flagOperation = secondFieldReference; - return true; + if (comparedOperand is IFieldReferenceOperation secondFieldReference && + secondFieldReference.Field.HasConstantValue && + firstFieldReference.Field.IsEqualTo(secondFieldReference.Field)) + { + flagOperation = secondFieldReference; + comparedWithZero = false; + return true; + } + + if (comparedOperand.IsConstantZero() && IsSingleBitSet(firstFieldReference.Field.ConstantValue)) + { + flagOperation = firstFieldReference; + comparedWithZero = true; + return true; + } } flagOperation = null; + comparedWithZero = false; return false; } @@ -237,6 +248,23 @@ private static bool IsValidPattern(IOperation enumValueOperation, IOperation fla return enumValueOperation.Type.IsEqualTo(flagOperation.Type); } + private static bool IsSingleBitSet(object? value) + { + return value switch + { + null => false, + sbyte x => IsSingleBitSet((byte)x), + byte x => x > 0 && (x & (x - 1)) == 0, + short x => IsSingleBitSet((ushort)x), + ushort x => x > 0 && (x & (x - 1)) == 0, + int x => IsSingleBitSet((uint)x), + uint x => x > 0 && (x & (x - 1)) == 0, + long x => IsSingleBitSet((ulong)x), + ulong x => x > 0 && (x & (x - 1)) == 0, + _ => false, + }; + } + private sealed record HasFlagPattern(ExpressionSyntax OperationExpression, ExpressionSyntax EnumValueExpression, ExpressionSyntax FlagExpression, bool Negate) { public TextSpan OperationSpan => OperationExpression.Span; diff --git a/src/Meziantou.Analyzer/Internals/NumericHelpers.cs b/src/Meziantou.Analyzer/Internals/NumericHelpers.cs index 7297ad8c8..79a94b69e 100644 --- a/src/Meziantou.Analyzer/Internals/NumericHelpers.cs +++ b/src/Meziantou.Analyzer/Internals/NumericHelpers.cs @@ -20,4 +20,21 @@ public static bool IsZero(object? value) _ => false, }; } + + public static bool IsSingleBitSet(object? value) + { + return value switch + { + null => false, + sbyte x => IsSingleBitSet((byte)x), + byte x => x > 0 && (x & (x - 1)) == 0, + short x => IsSingleBitSet((ushort)x), + ushort x => x > 0 && (x & (x - 1)) == 0, + int x => IsSingleBitSet((uint)x), + uint x => x > 0 && (x & (x - 1)) == 0, + long x => IsSingleBitSet((ulong)x), + ulong x => x > 0 && (x & (x - 1)) == 0, + _ => false, + }; + } } diff --git a/src/Meziantou.Analyzer/Rules/UseHasFlagMethodAnalyzer.cs b/src/Meziantou.Analyzer/Rules/UseHasFlagMethodAnalyzer.cs index a59a983b4..850612796 100644 --- a/src/Meziantou.Analyzer/Rules/UseHasFlagMethodAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/UseHasFlagMethodAnalyzer.cs @@ -141,14 +141,22 @@ private static bool TryGetEnumFlagReference(IOperation potentialFlag, IOperation comparedOperand = comparedOperand.UnwrapImplicitConversionOperations(); if (potentialFlag is IFieldReferenceOperation firstFieldReference && - comparedOperand is IFieldReferenceOperation secondFieldReference && firstFieldReference.Field.HasConstantValue && - secondFieldReference.Field.HasConstantValue && - firstFieldReference.Field.IsEqualTo(secondFieldReference.Field) && firstFieldReference.Field.ContainingType.IsEnumeration()) { - flagOperation = secondFieldReference; - return true; + if (comparedOperand is IFieldReferenceOperation secondFieldReference && + secondFieldReference.Field.HasConstantValue && + firstFieldReference.Field.IsEqualTo(secondFieldReference.Field)) + { + flagOperation = secondFieldReference; + return true; + } + + if (comparedOperand.IsConstantZero() && NumericHelpers.IsSingleBitSet(firstFieldReference.Field.ConstantValue)) + { + flagOperation = firstFieldReference; + return true; + } } flagOperation = null; diff --git a/tests/Meziantou.Analyzer.Test/Rules/UseHasFlagMethodAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/UseHasFlagMethodAnalyzerTests.cs index 1c025384c..dbb48ff87 100644 --- a/tests/Meziantou.Analyzer.Test/Rules/UseHasFlagMethodAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/UseHasFlagMethodAnalyzerTests.cs @@ -186,6 +186,146 @@ class Sample .ValidateAsync(); } + [Fact] + public async Task EqualsZeroCheck_ReportDiagnostic() + { + await CreateProjectBuilder() + .WithSourceCode(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => [|(value & MyEnum.Flag1) == 0|]; + } + """) + .ShouldFixCodeWith(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => !value.HasFlag(MyEnum.Flag1); + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task NotEqualsZeroCheck_ReportDiagnostic() + { + await CreateProjectBuilder() + .WithSourceCode(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => [|(value & MyEnum.Flag1) != 0|]; + } + """) + .ShouldFixCodeWith(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => value.HasFlag(MyEnum.Flag1); + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task IsPatternZeroCheck_ReportDiagnostic() + { + await CreateProjectBuilder() + .WithSourceCode(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => [|(value & MyEnum.Flag1) is 0|]; + } + """) + .ShouldFixCodeWith(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => !value.HasFlag(MyEnum.Flag1); + } + """) + .ValidateAsync(); + } + + [Fact] + public async Task IsNotPatternZeroCheck_ReportDiagnostic() + { + await CreateProjectBuilder() + .WithSourceCode(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => [|(value & MyEnum.Flag1) is not 0|]; + } + """) + .ShouldFixCodeWith(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + } + + class Sample + { + bool M(MyEnum value) => value.HasFlag(MyEnum.Flag1); + } + """) + .ValidateAsync(); + } + [Fact] public async Task DifferentFlag_NoDiagnostic() { @@ -207,6 +347,28 @@ class Sample .ValidateAsync(); } + [Fact] + public async Task CombinedFlag_NotEqualsZero_NoDiagnostic() + { + await CreateProjectBuilder() + .WithSourceCode(""" + [System.Flags] + enum MyEnum + { + None = 0, + Flag1 = 1, + Flag2 = 2, + Flag1AndFlag2 = Flag1 | Flag2, + } + + class Sample + { + bool M(MyEnum value) => (value & MyEnum.Flag1AndFlag2) != 0; + } + """) + .ValidateAsync(); + } + [Fact] public async Task DifferentFlag_IsNotPattern_NoDiagnostic() {