Fix bug in CallSiteImplicitAllocationAnalyzer (#85)

* Fix bug in CallSiteImplicitAllocationAnalyzer

* Add test for #84

ClrHeapAllocationsAnalyzer.Test/CallSiteImplicitAllocationAnalyzerTests.cs

@@ -95,5 +95,35 @@ attr.HasFlag (FileAttributes.Directory);
 
             Assert.AreEqual(0, info.Allocations.Count);
           }
+
+        [TestMethod]
+        public void ParamsIsPrecededByOptionalParameters()
+        {
+            var sampleProgram = @"
+using System.IO;
+
+public class MyClass
+{
+    static class Demo
+    {
+        static void Fun1()
+        {
+            Fun2();
+            Fun2(args: """", i: 5);
+        }
+        static void Fun2(int i = 0, params object[] args)
+        {
+        }
+    }
+}";
+
+            var analyser = new CallSiteImplicitAllocationAnalyzer();
+            var info = ProcessCode(analyser, sampleProgram, ImmutableArray.Create(SyntaxKind.InvocationExpression));
+
+            Assert.AreEqual(1, info.Allocations.Count, "Should report 1 allocation.");
+            // Diagnostic: (11,13): warning HeapAnalyzerImplicitParamsRule: This call site is calling into a function with a 'params' parameter. This results in an array allocation
+            AssertEx.ContainsDiagnostic(info.Allocations, id: CallSiteImplicitAllocationAnalyzer.ParamsParameterRule.Id, line: 11, character: 13);
+        }
+
     }
 }

ClrHeapAllocationsAnalyzer/CallSiteImplicitAllocationAnalyzer.cs

@@ -30,49 +30,39 @@ namespace ClrHeapAllocationAnalyzer
             var cancellationToken = context.CancellationToken;
             string filePath = node.SyntaxTree.FilePath;
 
-            var invocationExpression = node as InvocationExpressionSyntax;
-
-            if (semanticModel.GetSymbolInfo(invocationExpression, cancellationToken).Symbol is IMethodSymbol methodInfo)
-            {
-                if (methodInfo.IsOverride)
-                {
-                    CheckNonOverridenMethodOnStruct(methodInfo, reportDiagnostic, invocationExpression, filePath);
-                }
-
-                if (methodInfo.Parameters.Length > 0 && invocationExpression.ArgumentList != null)
-                {
-                    var lastParam = methodInfo.Parameters[methodInfo.Parameters.Length - 1];
-                    if (lastParam.IsParams)
-                    {
-                        CheckParam(invocationExpression, methodInfo, semanticModel, reportDiagnostic, filePath, cancellationToken);
-                    }
-                }
-            }
-        }
-
-        [MethodImpl(MethodImplOptions.NoInlining)]
-        private static void CheckParam(InvocationExpressionSyntax invocationExpression, IMethodSymbol methodInfo, SemanticModel semanticModel, Action<Diagnostic> reportDiagnostic, string filePath, CancellationToken cancellationToken)
-        {
-            var arguments = invocationExpression.ArgumentList.Arguments;
-            if (arguments.Count == methodInfo.Parameters.Length - 1)
+            if (semanticModel.GetOperation(node, cancellationToken) is not IInvocationOperation invocationOperation)
             {
                 return;
             }
 
-            if (arguments.Count != methodInfo.Parameters.Length)
+            var targetMethod = invocationOperation.TargetMethod;
+
+            if (targetMethod.IsOverride)
             {
-                reportDiagnostic(Diagnostic.Create(ParamsParameterRule, invocationExpression.GetLocation(), EmptyMessageArgs));
-                HeapAllocationAnalyzerEventSource.Logger.ParamsAllocation(filePath);
+                CheckNonOverridenMethodOnStruct(targetMethod, reportDiagnostic, node, filePath);
             }
-            else
+
+            bool compilationHasSystemArrayEmpty = !semanticModel.Compilation.GetSpecialType(SpecialType.System_Array).GetMembers("Empty").IsEmpty;
+
+            // Loop on every argument because params argument may not be the last one.
+            //     static void Fun1() => Fun2(args: "", i: 5);
+            //     static void Fun2(int i = 0, params object[] args) {}
+            foreach (var argument in invocationOperation.Arguments)
             {
-                var lastIndex = arguments.Count - 1;
-                var lastArgumentTypeInfo = semanticModel.GetTypeInfo(arguments[lastIndex].Expression, cancellationToken);
-                if (lastArgumentTypeInfo.Type != null && !lastArgumentTypeInfo.Type.Equals(methodInfo.Parameters[lastIndex].Type))
+                if (argument.ArgumentKind != ArgumentKind.ParamArray)
                 {
-                    reportDiagnostic(Diagnostic.Create(ParamsParameterRule, invocationExpression.GetLocation(), EmptyMessageArgs));
-                    HeapAllocationAnalyzerEventSource.Logger.ParamsAllocation(filePath);
+                    continue;
                 }
+
+                bool isEmpty = (argument.Value as IArrayCreationOperation)?.Initializer.ElementValues.IsEmpty == true;
+
+                // Up to net45 the System.Array.Empty<T> singleton didn't existed so an empty params array was still causing some memory allocation.
+                if (argument.IsImplicit && (!isEmpty || !compilationHasSystemArrayEmpty))
+                {
+                    reportDiagnostic(Diagnostic.Create(ParamsParameterRule, node.GetLocation(), EmptyMessageArgs));
+                }
+
+                break;
             }
         }
 
@@ -90,4 +80,4 @@ namespace ClrHeapAllocationAnalyzer
             }
         }
     }
-}
+}