Apply Tim's comments

SonarSource · Jul 4, 2024 · 24e22d7 · 24e22d7
1 parent 776f880
commit 24e22d7
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 49 deletions.
diff --git a/...onarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/...onarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs
@@ -43,7 +43,7 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation,
             && !invocation.Arguments.IsEmpty
             && state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value).Kind == OperationKindEx.InstanceReference);
 
-    public static IOperation TryGetInstance(this IInvocationOperationWrapper invocation, ProgramState state) =>
+    public static IOperation GetInstance(this IInvocationOperationWrapper invocation, ProgramState state) =>
         invocation.Instance
         ?? (invocation.TargetMethod.IsExtensionMethod
                 ? state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value)

diff --git a/...onarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs b/...onarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs
@@ -71,7 +71,7 @@ internal sealed partial class Invocation
         nameof(Enumerable.Zip),
     ];
 
-    private static readonly HashSet<string> ElemenetExistsCheckMethods =
+    private static readonly HashSet<string> ElementExistsCheckMethods =
     [
         nameof(Enumerable.Contains),
         nameof(Enumerable.Any),
@@ -81,7 +81,7 @@ internal sealed partial class Invocation
     private static ProgramState[] ProcessLinqEnumerableAndQueryable(ProgramState state, IInvocationOperationWrapper invocation)
     {
         var name = invocation.TargetMethod.Name;
-        var states = ProcessBoolCollectionMethods(state, invocation);
+        var states = ProcessElementExistsCheckMethods(state, invocation);
         if (ReturningNotNull.Contains(name))
         {
             return states.Select(x => x.SetOperationConstraint(invocation, ObjectConstraint.NotNull)).ToArray();
@@ -101,16 +101,17 @@ private static ProgramState[] ProcessLinqEnumerableAndQueryable(ProgramState sta
         }
     }
 
-    private static ProgramState[] ProcessBoolCollectionMethods(ProgramState state, IInvocationOperationWrapper invocation)
+    private static ProgramState[] ProcessElementExistsCheckMethods(ProgramState state, IInvocationOperationWrapper invocation)
     {
-        if (invocation.TryGetInstance(state) is { } instance && ElemenetExistsCheckMethods.Contains(invocation.TargetMethod.Name) && instance.TrackedSymbol(state) is { } instanceSymbol)
+        if (ElementExistsCheckMethods.Contains(invocation.TargetMethod.Name)  && invocation.GetInstance(state) is { } instance && instance.TrackedSymbol(state) is { } instanceSymbol)
         {
             return state[instanceSymbol]?.Constraint<CollectionConstraint>() switch
             {
                 CollectionConstraint constraint when constraint == CollectionConstraint.Empty => state.SetOperationConstraint(invocation, BoolConstraint.False).ToArray(),
-                CollectionConstraint constraint when constraint == CollectionConstraint.NotEmpty && HasNoParameters(invocation.TargetMethod) =>
-                    state.SetOperationConstraint(invocation, BoolConstraint.True).ToArray(),
-                CollectionConstraint constraint when constraint == CollectionConstraint.NotEmpty => state.ToArray(),
+                CollectionConstraint constraint when constraint == CollectionConstraint.NotEmpty =>
+                    HasNoParameters(invocation.TargetMethod)
+                        ? state.SetOperationConstraint(invocation, BoolConstraint.True).ToArray()
+                        : state.ToArray(),
                 _ when HasNoParameters(invocation.TargetMethod) =>
                 [
                     state.SetOperationConstraint(invocation, BoolConstraint.True).SetSymbolConstraint(instanceSymbol, CollectionConstraint.NotEmpty),

diff --git a/...yzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs b/...yzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs
@@ -79,7 +79,7 @@ internal sealed partial class Invocation : MultiProcessor<IInvocationOperationWr
         ];
 
     private static readonly ImmutableArray<KnownType> CollectionTypes = ImmutableArray.Create(
-        KnownType.System_Linq_Enumerable,              // Has all the extension methods for IEnumerable < T >
+        KnownType.System_Linq_Enumerable,              // Has all the extension methods for IEnumerable <T>
         KnownType.System_Linq_Queryable,               // Has it's own versions of several extension methods (like Any() with IQueryable<T> parameter)
         KnownType.System_Collections_Generic_IList_T); // Has it's own implementation of certain methods (like Contains())
 

diff --git a/...sts/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs b/...sts/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs
@@ -1009,11 +1009,11 @@ public void Invocation_LinqEnumerable_Unknown_Object(string expression)
     }
 
     [TestMethod]
-    public void Invocation_BoolCollectionMethod_NoParameters_NoConstrains()
+    public void Invocation_ElementExistsCheckMethods_NoParameters_NoConstrains()
     {
         var code = """
             var value = collection.Any();
-            Tag("Value", value);
+            Tag("Value");
             """;
         var enumerableValidator = SETestContext.CreateCS(code, $"IEnumerable<object> collection").Validator;
         var collectionSymbol = enumerableValidator.Symbol("collection");
@@ -1024,27 +1024,12 @@ public void Invocation_BoolCollectionMethod_NoParameters_NoConstrains()
     }
 
     [TestMethod]
-    public void Invocation_BoolCollectionMethod_NoParameters_CollectionEmpty()
-    {
-        var code = """
-            collection.Clear();
-            var value = collection.Any();
-            Tag("Value", value);
-            """;
-        var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
-        var collectionSymbol = enumerableValidator.Symbol("collection");
-        var valueSymbol = enumerableValidator.Symbol("value");
-        enumerableValidator.TagStates("Value").Should().HaveCount(1)
-            .And.ContainSingle(x => x[collectionSymbol].HasConstraint(CollectionConstraint.Empty) && x[valueSymbol].HasConstraint(BoolConstraint.False));
-    }
-
-    [TestMethod]
-    public void Invocation_BoolCollectionMethod_NoParameters_CollectionNotEmpty()
+    public void Invocation_ElementExistsCheckMethods_NoParameters_CollectionNotEmpty()
     {
         var code = """
             collection.Add(1);
             var value = collection.Any();
-            Tag("Value", value);
+            Tag("Value");
             """;
         var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
         var collectionSymbol = enumerableValidator.Symbol("collection");
@@ -1054,27 +1039,12 @@ public void Invocation_BoolCollectionMethod_NoParameters_CollectionNotEmpty()
     }
 
     [TestMethod]
-    public void Invocation_BoolCollectionMethod_Parameters_CollectionEmpty()
-    {
-        var code = """
-            collection.Clear();
-            var value = collection.Any(x => x is 1);
-            Tag("Value", value);
-            """;
-        var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
-        var collectionSymbol = enumerableValidator.Symbol("collection");
-        var valueSymbol = enumerableValidator.Symbol("value");
-        enumerableValidator.TagStates("Value").Should().HaveCount(1)
-            .And.ContainSingle(x => x[collectionSymbol].HasConstraint(CollectionConstraint.Empty) && x[valueSymbol].HasConstraint(BoolConstraint.False));
-    }
-
-    [TestMethod]
-    public void Invocation_BoolCollectionMethod_Parameters_CollectionNotEmpty()
+    public void Invocation_ElementExistsCheckMethods_Parameters_CollectionNotEmpty()
     {
         var code = """
             collection.Add(1);
             var value = collection.Any(x => x is 1);
-            Tag("Value", value);
+            Tag("Value");
             """;
         var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
         var collectionSymbol = enumerableValidator.Symbol("collection");
@@ -1088,11 +1058,11 @@ public void Invocation_BoolCollectionMethod_Parameters_CollectionNotEmpty()
     [DataRow("Any(x => x is 1)")]
     [DataRow("Exists(x => x is 1)")]
     [DataRow("Contains(1)")]
-    public void Invocation_BoolCollectionMethod_NoPrexistingConstrains(string expression)
+    public void Invocation_ElementExistsCheckMethods_NoPrexistingConstrains(string expression)
     {
         var code = $"""
             var value = collection.{expression};
-            Tag("Value", value);
+            Tag("Value");
             """;
         var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
         var collectionSymbol = enumerableValidator.Symbol("collection");
@@ -1102,15 +1072,16 @@ public void Invocation_BoolCollectionMethod_NoPrexistingConstrains(string expres
     }
 
     [DataTestMethod]
+    [DataRow("Any()")]
     [DataRow("Any(x => x is 1)")]
     [DataRow("Exists(x => x is 1)")]
     [DataRow("Contains(1)")]
-    public void Invocation_BoolCollectionMethod_CollectionAlreadyHasContraint(string expression)
+    public void Invocation_ElementExistsCheckMethods_CollectionAlreadyHasContraint(string expression)
     {
         var code = $"""
             collection.Clear();
             var value = collection.{expression};
-            Tag("Value", value);
+            Tag("Value");
             """;
         var enumerableValidator = SETestContext.CreateCS(code, $"List<int> collection").Validator;
         var collectionSymbol = enumerableValidator.Symbol("collection");