Unlearn NotNull on Remove

SonarSource · Aug 23, 2023 · 2e97af4 · 2e97af4
1 parent 86e1ba7
commit 2e97af4
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 4 deletions.
diff --git a/...r.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/...r.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
@@ -101,14 +101,27 @@ public abstract class EmptyCollectionsShouldNotBeEnumeratedBase : SymbolicRuleCh
         nameof(List<int>.AddRange),
         nameof(List<int>.Insert),
         nameof(List<int>.InsertRange),
-        nameof(HashSet<int>.SymmetricExceptWith),
         nameof(HashSet<int>.UnionWith),
+        nameof(HashSet<int>.SymmetricExceptWith),   // TODO needs to remove CollectionConstraint
         nameof(Queue<int>.Enqueue),
         nameof(Stack<int>.Push),
         nameof(Collection<int>.Insert),
         "TryAdd"
     };
 
+    private static readonly HashSet<string> RemoveMethods = new()
+    {
+        nameof(ICollection<int>.Remove),
+        nameof(List<int>.RemoveAll),
+        nameof(List<int>.RemoveAt),
+        nameof(List<int>.RemoveRange),
+        nameof(HashSet<int>.ExceptWith),
+        nameof(HashSet<int>.IntersectWith),
+        nameof(HashSet<int>.RemoveWhere),
+        nameof(Queue<int>.Dequeue),
+        nameof(Stack<int>.Pop)
+    };
+
     private readonly HashSet<IOperation> emptyAccess = new();
     private readonly HashSet<IOperation> nonEmptyAccess = new();
 
@@ -130,7 +143,7 @@ protected override ProgramState PreProcessSimple(SymbolicContext context)
         }
         else if (operation.AsMethodReference() is { Instance: not null } methodReference)
         {
-            return ProcessAddMethod(context.State, methodReference.Method, methodReference.Instance);
+            return ProcessAddMethod(context.State, methodReference.Method, methodReference.Instance) ?? context.State;
         }
         else if (operation.AsPropertyReference() is { Property.IsIndexer: true } indexer)
         {
@@ -180,7 +193,8 @@ private ProgramState ProcessInvocation(SymbolicContext context, IInvocationOpera
                     nonEmptyAccess.Add(context.Operation.Instance);
                 }
             }
-            return ProcessAddMethod(context.State, invocation.TargetMethod, instance);
+            return ProcessAddMethod(context.State, invocation.TargetMethod, instance)
+                ?? ProcessRemoveMethod(context.State, invocation.TargetMethod, instance);
         }
         else
         {
@@ -200,6 +214,21 @@ private static ProgramState ProcessAddMethod(ProgramState state, IMethodSymbol m
             {
                 state = state.SetSymbolConstraint(symbol, CollectionConstraint.NotEmpty);
             }
+            return state;
+        }
+        return null;
+    }
+
+    private static ProgramState ProcessRemoveMethod(ProgramState state, IMethodSymbol method, IOperation instance)
+    {
+        if (RemoveMethods.Contains(method.Name))
+        {
+            var value = (state[instance] ?? SymbolicValue.Empty).WithoutConstraint(CollectionConstraint.NotEmpty);
+            state = state.SetOperationValue(instance, value);
+            if (instance.TrackedSymbol(state) is { } symbol)
+            {
+                state = state.SetSymbolValue(symbol, value);
+            }
         }
         return state;
     }

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/CollectionConstraintTests.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/CollectionConstraintTests.cs
@@ -52,7 +52,7 @@ void Foo({{type}}<int> items)
                     if (items.Count == 0)   // Noncompliant 2589 always false
                         return;
                     items.{{remove}};       // Removes NotNull
-                    if (items.Count == 0)   // Noncompliant FP
+                    if (items.Count == 0)   // Compliant
                         return;
                     Console.WriteLine();
                 }