relax more conditions; improve ccov

analyzers/src/SonarAnalyzer.CSharp/Rules/MagicNumberShouldNotBeUsed.cs

@@ -61,52 +61,49 @@ protected override void Initialize(SonarAnalysisContext context) =>
 
         private static bool IsExceptionToTheRule(LiteralExpressionSyntax literalExpression) =>
             NotConsideredAsMagicNumbers.Contains(literalExpression.Token.ValueText)
-            // It's ok to use magic numbers as part of a variable declaration
             || literalExpression.FirstAncestorOrSelf<VariableDeclarationSyntax>() != null
-            // It's ok to use magic numbers as part of a parameter declaration
             || literalExpression.FirstAncestorOrSelf<ParameterSyntax>() != null
-            // It's ok to use magic numbers as part of an enum declaration
             || literalExpression.FirstAncestorOrSelf<EnumMemberDeclarationSyntax>() != null
-            // It's ok to use magic numbers in the GetHashCode method. Note that I am only checking the method name of the sake of simplicity
             || literalExpression.FirstAncestorOrSelf<MethodDeclarationSyntax>()?.Identifier.ValueText == nameof(object.GetHashCode)
-            // It's ok to use magic numbers in pragma directives
             || literalExpression.FirstAncestorOrSelf<PragmaWarningDirectiveTriviaSyntax>() != null
-            // It's ok to use magic numbers in property declaration
             || IsInsideProperty(literalExpression)
-            || IsNamedArgument(literalExpression)
-            || IsSingleOrNamedAttributeArgument(literalExpression)
-            || IsSingleDigitInToleratedComparisons(literalExpression);
+            || IsSingleDigitInToleratedComparisons(literalExpression)
+            || IsToleratedArgument(literalExpression);
+
+        // Inside property we consider magic numbers as exceptions in the following cases:
+        //   - A {get; set;} = MAGIC_NUMBER
+        //   - A { get { return MAGIC_NUMBER; } }
+        private static bool IsInsideProperty(SyntaxNode node)
+        {
+            if (node.FirstAncestorOrSelf<PropertyDeclarationSyntax>() == null)
+            {
+                return false;
+            }
+            var parent = node.Parent;
+            return parent is ReturnStatementSyntax || parent is EqualsValueClauseSyntax;
+        }
 
         private static bool IsSingleDigitInToleratedComparisons(LiteralExpressionSyntax literalExpression) =>
             literalExpression.Parent is BinaryExpressionSyntax binaryExpression
-            && binaryExpression.IsAnyKind(SyntaxKind.EqualsExpression, SyntaxKind.LessThanExpression, SyntaxKind.LessThanOrEqualExpression)
             && IsSingleDigit(literalExpression.Token.ValueText)
             && ToStringContainsAnyAcceptedNames(binaryExpression);
 
-        private static bool IsSingleDigit(string text) => byte.TryParse(text, out var result) && result <= 9;
+        private static bool IsToleratedArgument(LiteralExpressionSyntax literalExpression) =>
+            IsToleratedMethodArgument(literalExpression)
+            || IsSingleOrNamedAttributeArgument(literalExpression);
 
-        private static bool IsNamedArgument(LiteralExpressionSyntax literalExpression) =>
+        // Named argument or constructor argument.
+        private static bool IsToleratedMethodArgument(LiteralExpressionSyntax literalExpression) =>
             literalExpression.Parent is ArgumentSyntax arg
-            && arg.NameColon is not null;
+            && (arg.NameColon is not null || arg.Parent.Parent is ObjectCreationExpressionSyntax);
 
         private static bool IsSingleOrNamedAttributeArgument(LiteralExpressionSyntax literalExpression) =>
             literalExpression.Parent is AttributeArgumentSyntax arg
             && (arg.NameColon is not null
                 || arg.NameEquals is not null
                 || (arg.Parent is AttributeArgumentListSyntax argList && argList.Arguments.Count == 1));
 
-        // Inside property we consider magic numbers as exceptions in the following cases:
-        //   - A {get; set;} = MAGIC_NUMBER
-        //   - A { get { return MAGIC_NUMBER; } }
-        private static bool IsInsideProperty(SyntaxNode node)
-        {
-            if (node.FirstAncestorOrSelf<PropertyDeclarationSyntax>() == null)
-            {
-                return false;
-            }
-            var parent = node.Parent;
-            return parent is ReturnStatementSyntax || parent is EqualsValueClauseSyntax;
-        }
+        private static bool IsSingleDigit(string text) => byte.TryParse(text, out var result) && result <= 9;
 
         private static bool ToStringContainsAnyAcceptedNames(SyntaxNode syntaxNode)
         {

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/MagicNumberShouldNotBeUsed.cs

@@ -7,7 +7,7 @@ namespace Tests.Diagnostics
     public class FooAttribute : Attribute
     {
         public int Baz { get; set; }
-        public FooAttribute(int Bar, int Bar2 = 0)
+        public FooAttribute(int Bar = 0, int Bar2 = 0)
         {
         }
     }
@@ -17,6 +17,7 @@ public class FooBar
         public int Size { get; set; }
         public int COUNT { get; set; }
         public int length { get; set; }
+        public int baz { get; set; }
     }
 
     public class ValidUseCases
@@ -34,15 +35,22 @@ public static double FooProp
             get { return 2; }
         }
 
+        public ValidUseCases(int x, int y) { }
+
         public ValidUseCases(string s, FooBar foo)
         {
             int i1 = 0;
             int i2 = -1;
             int i3 = 1;
             int i4 = 42;
-            var list = new List<string>(42); // Compliant, class constructor
-            var bigInteger = new IntPtr(1612342); // Compliant, class constructor
-            var g = new Guid(0xA, 0xB, 0xC, new Byte[] { 0, 1, 2, 3, 4, 5, 6, 7 }); // Compliant, class constructor
+            var list = new List<string>(42); // Compliant, set to variable
+            var bigInteger = new IntPtr(1612342); // compliant, set to variable
+            var g = new Guid(0xA, 0xB, 0xC, new Byte[] { 0, 1, 2, 3, 4, 5, 6, 7 }); // Compliant, set in a variable
+
+            Foo(new IntPtr(123456)); // Compliant, class constructor
+            var byteArrayName = new Byte[] { 0, 1, 2, 3, 4, 5, 6, 7 };
+            Foo(new Guid(0xA, 0xB, 0xC, byteArrayName)); // Compliant, magic numbers set directly as constructor params
+
             long[] values1 = { 30L, 40L }; // Compliant, array initialisation
             int[] values2 = new int[] { 100, 200 }; // Compliant, array initialisation
             Console.WriteLine(value: 12); // Compliant, named argument
@@ -54,29 +62,46 @@ public ValidUseCases(string s, FooBar foo)
 
             var result = list.Count == 1; // Compliant, single digit for Count
             result = list.Count < 2; // Compliant, single digit for Count
+            result = list.Count <= 2; // Compliant
+            result = list.Count != 2; // Compliant
+            result = list.Count > 2; // Compliant
             result = s.Length == 3; // Compliant, single digit for Length
             result = foo.Size == 4; // Compliant, single digit for Size
-            result = foo.COUNT == 8; // Compliant, single digit for Size
-            result = foo.length == 9; // Compliant, single digit for Size
+            result = foo.COUNT == 8; // Compliant
+            result = foo.length == 9; // Compliant
 
             const int VAL = 15;
 
             Console.Write("test");
+
+            // we tolerate magic constants sent to constructors
+            new ValidUseCases(100, 300);
         }
 
         public override int GetHashCode()
         {
             return MY_VALUE * 397;
         }
 
+        [Foo(Bar: 42)] // Compliant, explicit attribute argument names
+        public void Foo1() { }
+
+        [Foo(Baz = 43)] // Compliant, explicit attribute argument names
+        public void Foo2() { }
+
         [Foo(Bar: 42, Baz = 43)] // Compliant, explicit attribute argument names
-        public void Foo(int value = 42)
+        public void Foo3() { }
+
+        public void Foo(int value = 42) // compliant, default value for argument
         {
             var x = -1 < 1;
         }
 
         [Foo(42)] // Compliant, attribute with only one argument
         public static int Bar(int value) => 0;
+
+        public static void Foo(IntPtr x) { }
+        public static void Foo(Guid x) { }
     }
 
     public enum MyEnum
@@ -93,6 +118,8 @@ public static double FooProp
             get { return Math.Sqrt(4); } // Noncompliant
         }
 
+        public WrongUseCases(int x, int y) { }
+
         public WrongUseCases(List<int> list, string s, FooBar foo)
         {
             Console.WriteLine(12); // Noncompliant
@@ -104,11 +131,17 @@ public WrongUseCases(List<int> list, string s, FooBar foo)
 
             var array = new string[10];
             array[5] = "test"; // Noncompliant
+            Foo(new int[] { 100 }); // Noncompliant, array with magic numbers should have a decent name
+
+
+            new WrongUseCases(100, Foo(200, 300)); // Noncompliant {{Assign this magic number '200' to a well-named (variable|constant), and use the (variable|constant) instead.}}
+            // Noncompliant@-1 {{Assign this magic number '300' to a well-named (variable|constant), and use the (variable|constant) instead.}}
 
             var result = list.Count == 99;
             result = list.Count < 400; // Noncompliant
             result = s.Length == 121; // Noncompliant
             result = foo.Size == 472; // Noncompliant
+            result = foo.baz == 4; // Noncompliant
             var x = 1;
             result = x == 2; // Noncompliant
         }
@@ -119,6 +152,9 @@ public int GetValue()
         {
             return 13; // Noncompliant
         }
+
+        public static void Foo(int[] array) { }
+        public static int Foo(int x, int y) => 1;
     }
 }