From 35028aa29632155d08baca5cd4ad633950687a95 Mon Sep 17 00:00:00 2001 From: Costin Zaharia <56015273+costin-zaharia-sonarsource@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:30:50 +0200 Subject: [PATCH] Update S4070 to accept negative numbers (#8211) --- .../Rules/DoNotMarkEnumsWithFlags.cs | 53 +++++++----- .../TestCases/DoNotMarkEnumsWithFlags.cs | 80 +++++++++++++++++-- 2 files changed, 105 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/DoNotMarkEnumsWithFlags.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/DoNotMarkEnumsWithFlags.cs index 1d69d466eb9..682275c4170 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/DoNotMarkEnumsWithFlags.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/DoNotMarkEnumsWithFlags.cs @@ -18,16 +18,17 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Numerics; + namespace SonarAnalyzer.Rules.CSharp { [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DoNotMarkEnumsWithFlags : SonarDiagnosticAnalyzer { - internal const string DiagnosticId = "S4070"; + private const string DiagnosticId = "S4070"; private const string MessageFormat = "Remove the 'FlagsAttribute' from this enum."; - private static readonly DiagnosticDescriptor Rule = - DescriptorFactory.Create(DiagnosticId, MessageFormat); + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); @@ -52,7 +53,7 @@ protected override void Initialize(SonarAnalysisContext context) => .ToList(); var allValues = membersWithValues.Select(x => x.Value) - .OfType() + .OfType() .Distinct() .ToList(); @@ -67,35 +68,47 @@ protected override void Initialize(SonarAnalysisContext context) => } }, SyntaxKind.EnumDeclaration); - // The idea of this method is to get rid of invalid values for flags such as negative values and decimals - private static ulong? GetEnumValueOrDefault(IFieldSymbol enumMember) => - enumMember.HasConstantValue && ulong.TryParse(enumMember.ConstantValue.ToString(), out var longValue) - ? longValue + private static BigInteger? GetEnumValueOrDefault(IFieldSymbol enumMember) => + enumMember.HasConstantValue + ? enumMember.ConstantValue switch // BigInteger is used here to support enums with base type of ulong. Direct cast is used to avoid string conversion. + { + byte x => (BigInteger)x, + sbyte x => (BigInteger)x, + short x => (BigInteger)x, + ushort x => (BigInteger)x, + int x => (BigInteger)x, + uint x => (BigInteger)x, + long x => (BigInteger)x, + ulong x => (BigInteger)x, + _ => null + } : null; - private static bool IsValidFlagValue(ulong? enumValue, List allValues) => - enumValue.HasValue && (IsZeroOrPowerOfTwo(enumValue.Value) || IsCombinationOfOtherValues(enumValue.Value, allValues)); + private static bool IsValidFlagValue(BigInteger? enumValue, IEnumerable allValues) => + enumValue.HasValue + && (IsZeroOrPowerOfTwo(enumValue.Value) + || IsCombinationOfOtherValues(enumValue.Value, allValues)); - // See https://stackoverflow.com/questions/600293/how-to-check-if-a-number-is-a-power-of-2 - private static bool IsZeroOrPowerOfTwo(ulong value) => - (value & (value - 1)) == 0; + private static bool IsZeroOrPowerOfTwo(BigInteger value) => + value.IsZero + || BigInteger.Abs(value).IsPowerOfTwo; - private static bool IsCombinationOfOtherValues(ulong value, List otherValues) + private static bool IsCombinationOfOtherValues(BigInteger value, IEnumerable otherValues) { - var newValue = value; - foreach (var otherValue in otherValues.SkipWhile(v => value <= v)) + var currentValue = value; + foreach (var otherValue in otherValues.SkipWhile(x => value <= x)) { - if (otherValue <= newValue) + if (otherValue <= currentValue) { - newValue ^= otherValue; - if (newValue == 0) + currentValue ^= otherValue; + if (currentValue == 0) { return true; } } } - return newValue == 0; + return currentValue == 0; } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotMarkEnumsWithFlags.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotMarkEnumsWithFlags.cs index c0bff437d25..708946abc52 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotMarkEnumsWithFlags.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotMarkEnumsWithFlags.cs @@ -59,10 +59,15 @@ public enum Color4 } [Flags] - public enum Color5 : int // Noncompliant + public enum NegativeValues { - Value = -3, // Secondary - Other = -4 // Secondary + Default = 0, + A = -2, + B = -4, + C = 1 << 31, // It overflows and becomes negative: https://github.com/SonarSource/sonar-dotnet/issues/7991 + D = A | B, + E = 2, + F = D | E } [Flags] @@ -102,12 +107,71 @@ enum EnumFoo4 // Noncompliant N3 = N1 | N2, N5 = 5 // Secondary } +} - // Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/7991 - [FlagsAttribute] - public enum Repro_7991 // Noncompliant FP +namespace DifferentBaseType +{ + [Flags] + public enum EByte : byte { - None = 0, - Red = 1 << 31 // Secondary + A = 1, + B = 2, + C = A | B + } + + [Flags] + public enum ESByte : sbyte + { + A = -2, + B = 2, + C = A | B + } + + [Flags] + public enum EShort : short + { + A = -2, + B = 2, + C = A | B + } + + [Flags] + public enum EUShort : ushort + { + A = 1, + B = 2, + C = A | B + } + + [Flags] + public enum EInt : int + { + A = -2, + B = 2, + C = A | B + } + + [Flags] + public enum EUInt : uint + { + A = 1, + B = 2, + C = A | B + } + + [Flags] + public enum ELong : long + { + A = -2, + B = 2, + C = A | B + } + + [Flags] + public enum EULong : ulong + { + A = 1, + B = 2, + C = A | B } }