From b6ce681f3999e3854dfd749327f01ffbe906c4c1 Mon Sep 17 00:00:00 2001
From: Pavel Mikula
 <57188685+pavel-mikula-sonarsource@users.noreply.github.com>
Date: Fri, 16 Jun 2023 15:33:28 +0200
Subject: [PATCH] Migrate S4158: Do not report on method references (#7453)

---
 ...ptyCollectionsShouldNotBeEnumeratedBase.cs | 50 ++++++++++---------
 .../EmptyCollectionsShouldNotBeEnumerated.cs  | 15 +++++-
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
index 4e2d6ac9ecc..09f8a9d6a47 100644
--- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
+++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
@@ -129,9 +129,9 @@ protected override ProgramState PreProcessSimple(SymbolicContext context)
         {
             return ProcessInvocation(context, invocation);
         }
-        else if (operation.AsMethodReference() is { } methodReference)
+        else if (operation.AsMethodReference() is { Instance: not null } methodReference)
         {
-            return ProcessMethod(context, methodReference.Method, methodReference.Instance);
+            return ProcessAddMethod(context.State, methodReference.Method, methodReference.Instance);
         }
         else if (operation.AsPropertyReference() is { Property.IsIndexer: true } indexer)
         {
@@ -163,33 +163,37 @@ public override void ExecutionCompleted()
 
     private ProgramState ProcessInvocation(SymbolicContext context, IInvocationOperationWrapper invocation)
     {
-        return invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))
-            && SizeConstraint(context.State, invocation.Instance ?? invocation.Arguments[0].ToArgument().Value, HasFilteringPredicate()) is { } constraint
-                ? context.SetOperationConstraint(constraint)
-                : ProcessMethod(context, invocation.TargetMethod, invocation.Instance);
-
-        bool HasFilteringPredicate() =>
-            invocation.Arguments.Any(x => x.ToArgument().Parameter.Type.Is(KnownType.System_Func_T_TResult));
-    }
-
-    private ProgramState ProcessMethod(SymbolicContext context, IMethodSymbol method, IOperation instance)
-    {
-        var state = context.State;
-        if (instance is null)
+        if (invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))
+            && SizeConstraint(context.State, invocation.Instance ?? invocation.Arguments[0].ToArgument().Value, HasFilteringPredicate()) is { } constraint)
         {
-            return state;
+            return context.SetOperationConstraint(constraint);
         }
-        if (RaisingMethods.Contains(method.Name))
+        else if (invocation.Instance is { } instance)
         {
-            if (state[instance]?.HasConstraint(CollectionConstraint.Empty) is true)
-            {
-                emptyAccess.Add(context.Operation.Instance);
-            }
-            else
+            if (RaisingMethods.Contains(invocation.TargetMethod.Name))
             {
-                nonEmptyAccess.Add(context.Operation.Instance);
+                if (context.State[instance]?.HasConstraint(CollectionConstraint.Empty) is true)
+                {
+                    emptyAccess.Add(context.Operation.Instance);
+                }
+                else
+                {
+                    nonEmptyAccess.Add(context.Operation.Instance);
+                }
             }
+            return ProcessAddMethod(context.State, invocation.TargetMethod, instance);
+        }
+        else
+        {
+            return context.State;
         }
+
+        bool HasFilteringPredicate() =>
+            invocation.Arguments.Any(x => x.ToArgument().Parameter.Type.Is(KnownType.System_Func_T_TResult));
+    }
+
+    private static ProgramState ProcessAddMethod(ProgramState state, IMethodSymbol method, IOperation instance)
+    {
         if (AddMethods.Contains(method.Name))
         {
             state = state.SetOperationConstraint(instance, CollectionConstraint.NotEmpty);
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs
index 7d481b52b12..1a332ed336a 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs
@@ -702,7 +702,7 @@ public void AddPassedAsParameter()
         list.Clear();   // Compliant
 
         list = new List<int>();
-        DoSomething(list.Clear); // Noncompliant
+        DoSomething(list.Clear); // We don't raise here to avoid FPs
 
         DoSomething(StaticMethodWithoutInstance);
 
@@ -717,7 +717,7 @@ public void AddPassedAsParameter()
         list.Clear();    // Compliant, but will break when we learn Empty from Clear()
 
         list = new List<int>();
-        Action clear = list.Clear;  // Noncompliant FP
+        Action clear = list.Clear;  // We don't raise here to avoid FPs
         clear();                    // FN
         clear();                    // FN
 
@@ -728,6 +728,17 @@ public void AddPassedAsParameter()
         clear();                    // FN
     }
 
+    public void AddPassedAsParameter_Dictionary()   // Reproducer from Peach
+    {
+        var d = new Dictionary<int, int>();
+        AddSomething(
+            d.ContainsKey,  // Compliant
+            (k, v) => d[k] = v);
+
+        void AddSomething(Func<int, bool> containsKey, Action<int, int> add)
+        { }
+    }
+
     private static void StaticMethodWithoutInstance() { }
 
     public void Count()