SE: Move CollectionConstraint from S4158 to the engine (part 1)

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Binary.Collections.cs

@@ -18,9 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-using Microsoft.CodeAnalysis;
 using SonarAnalyzer.SymbolicExecution.Constraints;
-using SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks;
 
 namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;
 
@@ -49,7 +47,7 @@ private ProgramState LearnBranchingCollectionConstraint(ProgramState state, IBin
 
         ISymbol InstanceOfCountProperty(IOperation operation) =>
             operation.AsPropertyReference() is { Instance: { } instance, Property.Name: nameof(Array.Length) or nameof(List<int>.Count) }
-            && instance.Type.DerivesOrImplementsAny(EmptyCollectionsShouldNotBeEnumeratedBase.TrackedCollectionTypes)
+            && instance.Type.DerivesOrImplementsAny(CollectionTracker.CollectionTypes)
             && instance.TrackedSymbol(state) is { } symbol
                 ? symbol
                 : null;

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs

@@ -0,0 +1,62 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2024 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using SonarAnalyzer.SymbolicExecution.Constraints;
+
+namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;
+
+internal static class CollectionTracker
+{
+    public static readonly ImmutableArray<KnownType> CollectionTypes = ImmutableArray.Create(
+        KnownType.System_Collections_Generic_List_T,
+        KnownType.System_Collections_Generic_IList_T,
+        KnownType.System_Collections_Immutable_IImmutableList_T,
+        KnownType.System_Collections_Generic_ICollection_T,
+        KnownType.System_Collections_Generic_HashSet_T,
+        KnownType.System_Collections_Generic_ISet_T,
+        KnownType.System_Collections_Immutable_IImmutableSet_T,
+        KnownType.System_Collections_Generic_Queue_T,
+        KnownType.System_Collections_Immutable_IImmutableQueue_T,
+        KnownType.System_Collections_Generic_Stack_T,
+        KnownType.System_Collections_Immutable_IImmutableStack_T,
+        KnownType.System_Collections_ObjectModel_ObservableCollection_T,
+        KnownType.System_Array,
+        KnownType.System_Collections_Immutable_IImmutableArray_T,
+        KnownType.System_Collections_Generic_Dictionary_TKey_TValue,
+        KnownType.System_Collections_Generic_IDictionary_TKey_TValue,
+        KnownType.System_Collections_Immutable_IImmutableDictionary_TKey_TValue);
+
+    public static CollectionConstraint ObjectCreationConstraint(ProgramState state, IObjectCreationOperationWrapper operation)
+    {
+        if (operation.Type.IsAny(CollectionTypes))
+        {
+            return operation.Arguments.SingleOrDefault(IsEnumerable) is { } argument
+                ? state.Constraint<CollectionConstraint>(argument)
+                : CollectionConstraint.Empty;
+        }
+        else
+        {
+            return null;
+        }
+
+        static bool IsEnumerable(IOperation operation) =>
+            operation.ToArgument().Parameter.Type.DerivesOrImplements(KnownType.System_Collections_IEnumerable);
+    }
+}

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ObjectCreation.cs

@@ -19,7 +19,6 @@
  */
 
 using SonarAnalyzer.SymbolicExecution.Constraints;
-using SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks;
 
 namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;
 
@@ -30,13 +29,7 @@ protected override IObjectCreationOperationWrapper Convert(IOperation operation)
 
     protected override ProgramState Process(SymbolicContext context, IObjectCreationOperationWrapper operation)
     {
-        if (operation.Type.IsAny(EmptyCollectionsShouldNotBeEnumeratedBase.TrackedCollectionTypes)
-            && CollectionCreationConstraint(context.State, operation) is { } constraint)
-        {
-            return context.SetOperationConstraint(constraint)
-                .SetOperationConstraint(operation, ObjectConstraint.NotNull);
-        }
-        else if (operation.Type.IsNullableValueType())
+        if (operation.Type.IsNullableValueType())
         {
             if (operation.Arguments.IsEmpty)
             {
@@ -53,15 +46,10 @@ protected override ProgramState Process(SymbolicContext context, IObjectCreation
         }
         else
         {
-            return context.SetOperationConstraint(ObjectConstraint.NotNull);
+            var newState = context.SetOperationConstraint(ObjectConstraint.NotNull);
+            return CollectionTracker.ObjectCreationConstraint(context.State, operation) is { } constraint
+                ? newState.SetOperationConstraint(operation, constraint)
+                : newState;
         }
     }
-
-    private static CollectionConstraint CollectionCreationConstraint(ProgramState state, IObjectCreationOperationWrapper operation) =>
-        operation.Arguments.SingleOrDefault(IsEnumerable) is { } argument
-            ? state.Constraint<CollectionConstraint>(argument)
-            : CollectionConstraint.Empty;
-
-    private static bool IsEnumerable(IOperation operation) =>
-            operation.ToArgument().Parameter.Type.DerivesOrImplements(KnownType.System_Collections_IEnumerable);
 }

analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs

@@ -21,6 +21,7 @@
 using System.Collections;
 using System.Collections.ObjectModel;
 using SonarAnalyzer.SymbolicExecution.Constraints;
+using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;
 
 namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks;
 
@@ -29,27 +30,6 @@ public abstract class EmptyCollectionsShouldNotBeEnumeratedBase : SymbolicRuleCh
     protected const string DiagnosticId = "S4158";
     protected const string MessageFormat = "Remove this call, the collection is known to be empty here.";
 
-    internal static readonly ImmutableArray<KnownType> TrackedCollectionTypes = new[]
-    {
-        KnownType.System_Collections_Generic_List_T,
-        KnownType.System_Collections_Generic_IList_T,
-        KnownType.System_Collections_Immutable_IImmutableList_T,
-        KnownType.System_Collections_Generic_ICollection_T,
-        KnownType.System_Collections_Generic_HashSet_T,
-        KnownType.System_Collections_Generic_ISet_T,
-        KnownType.System_Collections_Immutable_IImmutableSet_T,
-        KnownType.System_Collections_Generic_Queue_T,
-        KnownType.System_Collections_Immutable_IImmutableQueue_T,
-        KnownType.System_Collections_Generic_Stack_T,
-        KnownType.System_Collections_Immutable_IImmutableStack_T,
-        KnownType.System_Collections_ObjectModel_ObservableCollection_T,
-        KnownType.System_Array,
-        KnownType.System_Collections_Immutable_IImmutableArray_T,
-        KnownType.System_Collections_Generic_Dictionary_TKey_TValue,
-        KnownType.System_Collections_Generic_IDictionary_TKey_TValue,
-        KnownType.System_Collections_Immutable_IImmutableDictionary_TKey_TValue,
-    }.ToImmutableArray();
-
     protected static readonly HashSet<string> RaisingMethods = new()
     {
         nameof(IEnumerable<int>.GetEnumerator),
@@ -200,7 +180,7 @@ private ProgramState ProcessInvocation(SymbolicContext context, IInvocationOpera
                     nonEmptyAccess.Add(context.Operation.Instance);
                 }
             }
-            if (instance.Type.DerivesOrImplementsAny(TrackedCollectionTypes))
+            if (instance.Type.DerivesOrImplementsAny(CollectionTracker.CollectionTypes))
             {
                 return ProcessAddMethod(context.State, targetMethod, instance)
                     ?? ProcessRemoveMethod(context.State, targetMethod, instance)
@@ -260,7 +240,7 @@ private static NumberConstraint SizeConstraint(ProgramState state, IOperation in
                 return NumberConstraint.From(1, null);
             }
         }
-        return instance.Type.DerivesOrImplementsAny(TrackedCollectionTypes) ? NumberConstraint.From(0, null) : null;
+        return instance.Type.DerivesOrImplementsAny(CollectionTracker.CollectionTypes) ? NumberConstraint.From(0, null) : null;
     }
 
     private static ProgramState ProcessIndexerAccess(ProgramState state, IPropertyReferenceOperationWrapper propertyReference)

analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs

@@ -526,23 +526,31 @@ public void DynamicObjectCreation_SetsNotNull()
     [TestMethod]
     public void ObjectCreation_SetsNotNull()
     {
-        const string code = @"
-object assigned;
-var obj = new Object();
-var valueType = new Guid();
-var declared = new Exception();
-assigned = new EventArgs();
-
-Tag(""Declared"", declared);
-Tag(""Assigned"", assigned);
-Tag(""ValueType"", valueType);
-Tag(""Object"", obj);";
+        const string code = """
+            object assigned;
+            var obj = new Object();
+            var valueType = new Guid();
+            var declared = new Exception();
+            assigned = new EventArgs();
+            var collection1 = new List<int>();
+            collection1.Add(42);
+            var collection2 = new List<int>(collection1);
+
+            Tag("Declared", declared);
+            Tag("Assigned", assigned);
+            Tag("ValueType", valueType);
+            Tag("Object", obj);
+            Tag("Collection1", collection1);
+            Tag("Collection2", collection2);
+            """;
         var validator = SETestContext.CreateCS(code).Validator;
         validator.ValidateContainsOperation(OperationKind.ObjectCreation);
         validator.TagValue("Declared").Should().HaveOnlyConstraint(ObjectConstraint.NotNull);
         validator.TagValue("Assigned").Should().HaveOnlyConstraint(ObjectConstraint.NotNull);
         validator.TagValue("ValueType").Should().HaveOnlyConstraint(ObjectConstraint.NotNull);   // This is questionable, value types should not have ObjectConstraint
         validator.TagValue("Object").Should().HaveOnlyConstraint(ObjectConstraint.NotNull);
+        validator.TagValue("Collection1").Should().HaveOnlyConstraints(ObjectConstraint.NotNull, CollectionConstraint.Empty);
+        validator.TagValue("Collection2").Should().HaveOnlyConstraints(ObjectConstraint.NotNull, CollectionConstraint.Empty); // This should fail when the Invocation logic is moved to CollectionTracker
     }
 
     [TestMethod]