From ecf5aea33b61117f7024561cfc9e4910c64b83f4 Mon Sep 17 00:00:00 2001 From: Gregory Paidis <115458417+gregory-paidis-sonarsource@users.noreply.github.com> Date: Wed, 14 Feb 2024 14:04:07 +0100 Subject: [PATCH] SE: Move CollectionConstraint from S4158 to the engine (part 6) (#8733) --- .../its/expected/Net5/S2589-Net5-net5.0.json | 6 + .../Roslyn/OperationProcessors/Argument.cs | 4 +- .../OperationProcessors/ArrayCreation.cs | 2 +- .../OperationProcessors/CollectionTracker.cs | 101 ++++++++++++++-- .../Invocation.Enumerable.cs | 23 ++-- .../Roslyn/OperationProcessors/Invocation.cs | 8 +- .../OperationProcessors/ObjectCreation.cs | 6 +- ...ptyCollectionsShouldNotBeEnumeratedBase.cs | 89 ++------------ .../RoslynSymbolicExecutionTest.Invocation.cs | 112 ++++++++++++++++++ .../RoslynSymbolicExecutionTest.Operations.cs | 47 +++++++- .../Roslyn/ConditionEvaluatesToConstant.cs | 22 ++++ .../EmptyCollectionsShouldNotBeEnumerated.cs | 8 +- .../EmptyCollectionsShouldNotBeEnumerated.vb | 4 +- 13 files changed, 309 insertions(+), 123 deletions(-) diff --git a/analyzers/its/expected/Net5/S2589-Net5-net5.0.json b/analyzers/its/expected/Net5/S2589-Net5-net5.0.json index 308d3054057..03844659303 100644 --- a/analyzers/its/expected/Net5/S2589-Net5-net5.0.json +++ b/analyzers/its/expected/Net5/S2589-Net5-net5.0.json @@ -53,6 +53,12 @@ "Message": "Change this condition so that it does not always evaluate to \u0027True\u0027.", "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S3440.cs#L11", "Location": "Line 11 Position 17-18" + }, + { + "Id": "S2589", + "Message": "Change this condition so that it does not always evaluate to \u0027True\u0027.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Net5/Net5/S3981.cs#L12", + "Location": "Line 12 Position 17-62" } ] } \ No newline at end of file diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Argument.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Argument.cs index 1345073fbc9..83ca129a51f 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Argument.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Argument.cs @@ -44,7 +44,9 @@ private static ProgramState ProcessArgument(ProgramState state, IArgumentOperati { state = state.SetSymbolConstraint(notNullSymbol, ObjectConstraint.NotNull); } - if (argument.WrappedOperation.TrackedSymbol(state) is { } symbol + // Enumerable is static class with extensions, so the instance is passed as argument, but it does not get mutated + if (!argument.Parameter.ContainingType.Is(KnownType.System_Linq_Enumerable) + && argument.WrappedOperation.TrackedSymbol(state) is { } symbol && state[symbol] is { } symbolValue) { state = state.SetSymbolValue(symbol, symbolValue.WithoutConstraint()); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ArrayCreation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ArrayCreation.cs index 9b7bd05dc46..962b719c2b6 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ArrayCreation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ArrayCreation.cs @@ -28,6 +28,6 @@ protected override IArrayCreationOperationWrapper Convert(IOperation operation) IArrayCreationOperationWrapper.FromOperation(operation); protected override ProgramState Process(SymbolicContext context, IArrayCreationOperationWrapper operation) => - context.SetOperationConstraint(CollectionTracker.ArrayCreationConstraint(operation)) + CollectionTracker.LearnFrom(context.State, operation) .SetOperationConstraint(operation, ObjectConstraint.NotNull); } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs index 380ac5dafd4..170188f4dd5 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs @@ -44,8 +44,8 @@ internal static class CollectionTracker KnownType.System_Collections_Generic_IDictionary_TKey_TValue, KnownType.System_Collections_Immutable_IImmutableDictionary_TKey_TValue); - public static readonly HashSet AddMethods = new() - { + private static readonly HashSet AddMethods = + [ nameof(ICollection.Add), nameof(List.AddRange), nameof(List.Insert), @@ -57,10 +57,10 @@ internal static class CollectionTracker nameof(Stack.Push), nameof(Collection.Insert), "TryAdd" - }; + ]; - public static readonly HashSet RemoveMethods = new() - { + private static readonly HashSet RemoveMethods = + [ nameof(ICollection.Remove), nameof(List.RemoveAll), nameof(List.RemoveAt), @@ -70,31 +70,41 @@ internal static class CollectionTracker nameof(HashSet.RemoveWhere), nameof(Queue.Dequeue), nameof(Stack.Pop) - }; + ]; private static readonly HashSet AddOrRemoveMethods = [.. AddMethods, .. RemoveMethods]; - public static CollectionConstraint ObjectCreationConstraint(ProgramState state, IObjectCreationOperationWrapper operation) + public static ProgramState LearnFrom(ProgramState state, IObjectCreationOperationWrapper operation) { if (operation.Type.IsAny(CollectionTypes)) { - return operation.Arguments.SingleOrDefault(IsEnumerable) is { } argument - ? state.Constraint(argument) - : CollectionConstraint.Empty; + if (operation.Arguments.SingleOrDefault(IsEnumerable) is { } argument) + { + return state.Constraint(argument) is { } constraint + ? state.SetOperationConstraint(operation, constraint) + : state; + } + else + { + return state.SetOperationConstraint(operation, CollectionConstraint.Empty); + } } else { - return null; + return state; } static bool IsEnumerable(IOperation operation) => operation.ToArgument().Parameter.Type.DerivesOrImplements(KnownType.System_Collections_IEnumerable); } - public static CollectionConstraint ArrayCreationConstraint(IArrayCreationOperationWrapper operation) => - operation.DimensionSizes.Any(x => x.ConstantValue.Value is 0) + public static ProgramState LearnFrom(ProgramState state, IArrayCreationOperationWrapper operation) + { + var constraint = operation.DimensionSizes.Any(x => x.ConstantValue.Value is 0) ? CollectionConstraint.Empty : CollectionConstraint.NotEmpty; + return state.SetOperationConstraint(operation, constraint); + } public static ProgramState LearnFrom(ProgramState state, IMethodReferenceOperationWrapper operation) => operation.Instance is not null @@ -132,4 +142,69 @@ public static ProgramState LearnFrom(ProgramState state, IPropertyReferenceOpera } return state; } + + public static ProgramState LearnFrom(ProgramState state, IInvocationOperationWrapper invocation) + { + if (EnumerableCountConstraint(state, invocation) is { } constraint) + { + return state.SetOperationConstraint(invocation, constraint); + } + if (invocation.Instance is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) + { + var targetMethod = invocation.TargetMethod; + var symbolValue = state[instance] ?? SymbolicValue.Empty; + if (AddMethods.Contains(targetMethod.Name)) + { + return SetOperationAndSymbolValue(symbolValue.WithConstraint(CollectionConstraint.NotEmpty)); + } + else if (RemoveMethods.Contains(targetMethod.Name)) + { + return SetOperationAndSymbolValue(symbolValue.WithoutConstraint(CollectionConstraint.NotEmpty)); + } + else if (targetMethod.Name == nameof(ICollection.Clear)) + { + return SetOperationAndSymbolValue(symbolValue.WithConstraint(CollectionConstraint.Empty)); + } + } + return state; + + ProgramState SetOperationAndSymbolValue(SymbolicValue value) + { + if (instance.TrackedSymbol(state) is { } symbol) + { + state = state.SetSymbolValue(symbol, value); + } + return state.SetOperationValue(instance, value); + } + } + + private static NumberConstraint EnumerableCountConstraint(ProgramState state, IInvocationOperationWrapper invocation) + { + if (invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))) + { + var instance = invocation.Instance ?? invocation.Arguments[0].ToArgument().Value; + if (instance.TrackedSymbol(state) is { } symbol + && state[symbol]?.Constraint() is { } collection) + { + if (collection == CollectionConstraint.Empty) + { + return NumberConstraint.From(0); + } + else + { + return HasFilteringPredicate() + ? NumberConstraint.From(0, null) // nonEmpty.Count(predicate) can be Empty or NotEmpty + : NumberConstraint.From(1, null); + } + } + else + { + return NumberConstraint.From(0, null); + } + } + return null; + + bool HasFilteringPredicate() => + invocation.Arguments.Any(x => x.ToArgument().Parameter.Type.Is(KnownType.System_Func_T_TResult)); + } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs index 87affe35075..ac20227684e 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs @@ -24,7 +24,7 @@ namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; internal sealed partial class Invocation { - private static readonly HashSet EnumerableAndQueryableReturningNotNull = new() + private static readonly HashSet ReturningNotNull = new() { nameof(Enumerable.Append), nameof(Enumerable.AsEnumerable), @@ -71,28 +71,23 @@ internal sealed partial class Invocation nameof(Enumerable.Zip), }; - private static ProgramState[] ProcessLinqEnumerableAndQueryable(SymbolicContext context, IInvocationOperationWrapper invocation) + private static ProgramState[] ProcessLinqEnumerableAndQueryable(ProgramState state, IInvocationOperationWrapper invocation) { var name = invocation.TargetMethod.Name; - if (EnumerableAndQueryableReturningNotNull.Contains(name)) + if (ReturningNotNull.Contains(name)) { - return context.SetOperationConstraint(ObjectConstraint.NotNull).ToArray(); + return state.SetOperationConstraint(invocation, ObjectConstraint.NotNull).ToArray(); } - else if (name == nameof(Enumerable.FirstOrDefault) // ElementAtOrDefault is intentionally not supported. It's causing many FPs - || name == nameof(Enumerable.LastOrDefault) - || name == nameof(Enumerable.SingleOrDefault)) + // ElementAtOrDefault is intentionally not supported. It's causing many FPs + else if (name is nameof(Enumerable.FirstOrDefault) or nameof(Enumerable.LastOrDefault) or nameof(Enumerable.SingleOrDefault)) { return invocation.TargetMethod.ReturnType.IsReferenceType - ? new[] - { - context.SetOperationConstraint(ObjectConstraint.Null), - context.SetOperationConstraint(ObjectConstraint.NotNull), - } - : context.State.ToArray(); + ? [state.SetOperationConstraint(invocation, ObjectConstraint.Null), state.SetOperationConstraint(invocation, ObjectConstraint.NotNull)] + : state.ToArray(); } else { - return context.State.ToArray(); + return state.ToArray(); } } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs index 799f020d4a1..ef001dd0fbc 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs @@ -42,9 +42,9 @@ protected override ProgramState[] Process(SymbolicContext context, IInvocationOp } else if (invocation.Instance.TrackedSymbol(state) is { } symbol && !IsNullableGetValueOrDefault(invocation)) { - state = state.SetSymbolConstraint(symbol, ObjectConstraint.NotNull); + state = state.SetSymbolConstraint(symbol, ObjectConstraint.NotNull) + .SetOperationConstraint(invocation.Instance, ObjectConstraint.NotNull); } - if (invocation.HasThisReceiver(state)) { state = state.ResetFieldConstraints(); @@ -56,6 +56,8 @@ protected override ProgramState[] Process(SymbolicContext context, IInvocationOp { state = state.SetSymbolConstraint(instanceSymbol, ObjectConstraint.NotNull); } + state = CollectionTracker.LearnFrom(state, invocation); + return invocation switch { _ when IsNullableGetValueOrDefault(invocation) => ProcessNullableGetValueOrDefault(context, invocation).ToArray(), @@ -63,7 +65,7 @@ _ when invocation.TargetMethod.Is(KnownType.Microsoft_VisualBasic_Information, " _ when invocation.TargetMethod.Is(KnownType.System_Diagnostics_Debug, nameof(Debug.Assert)) => ProcessDebugAssert(context, invocation), _ when invocation.TargetMethod.Is(KnownType.System_Object, nameof(ReferenceEquals)) => ProcessReferenceEquals(context, invocation), _ when invocation.TargetMethod.Is(KnownType.System_Nullable_T, "get_HasValue") => ProcessNullableHasValue(state, invocation), - _ when invocation.TargetMethod.ContainingType.IsAny(KnownType.System_Linq_Enumerable, KnownType.System_Linq_Queryable) => ProcessLinqEnumerableAndQueryable(context, invocation), + _ when invocation.TargetMethod.ContainingType.IsAny(KnownType.System_Linq_Enumerable, KnownType.System_Linq_Queryable) => ProcessLinqEnumerableAndQueryable(state, invocation), _ when invocation.TargetMethod.Name == nameof(Equals) => ProcessEquals(context, invocation), _ when invocation.TargetMethod.ContainingType.Is(KnownType.System_String) => ProcessSystemStringInvocation(state, invocation), _ => ProcessArgumentAttributes(state, invocation), diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ObjectCreation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ObjectCreation.cs index 330e5c272ee..1a75393756b 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ObjectCreation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/ObjectCreation.cs @@ -46,10 +46,8 @@ protected override ProgramState Process(SymbolicContext context, IObjectCreation } else { - var newState = context.SetOperationConstraint(ObjectConstraint.NotNull); - return CollectionTracker.ObjectCreationConstraint(context.State, operation) is { } constraint - ? newState.SetOperationConstraint(operation, constraint) - : newState; + return CollectionTracker.LearnFrom(context.State, operation) + .SetOperationConstraint(operation, ObjectConstraint.NotNull); } } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs index 181c09d12fa..fc2bdda8532 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs @@ -21,7 +21,6 @@ using System.Collections; using System.Collections.ObjectModel; using SonarAnalyzer.SymbolicExecution.Constraints; -using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks; @@ -30,8 +29,8 @@ 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."; - protected static readonly HashSet RaisingMethods = new() - { + protected static readonly HashSet RaisingMethods = + [ nameof(IEnumerable.GetEnumerator), nameof(ICollection.CopyTo), nameof(ICollection.Clear), @@ -82,16 +81,11 @@ public abstract class EmptyCollectionsShouldNotBeEnumeratedBase : SymbolicRuleCh nameof(Dictionary.ContainsKey), nameof(Dictionary.ContainsValue), nameof(Dictionary.TryGetValue) - }; + ]; private readonly HashSet emptyAccess = new(); private readonly HashSet nonEmptyAccess = new(); - protected override ProgramState PreProcessSimple(SymbolicContext context) => - context.Operation.Instance.AsInvocation() is { } invocation - ? ProcessInvocation(context, invocation) - : context.State; - public override void ExecutionCompleted() { foreach (var operation in emptyAccess.Except(nonEmptyAccess)) @@ -100,82 +94,21 @@ public override void ExecutionCompleted() } } - private ProgramState ProcessInvocation(SymbolicContext context, IInvocationOperationWrapper invocation) + protected override ProgramState PreProcessSimple(SymbolicContext context) { - var targetMethod = invocation.TargetMethod; - if (targetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count)) - && SizeConstraint(context.State, invocation.Instance ?? invocation.Arguments[0].ToArgument().Value, HasFilteringPredicate()) is { } constraint) - { - return context.SetOperationConstraint(constraint); - } - else if (invocation.Instance is { } instance) + if (context.Operation.Instance.AsInvocation() is { } invocation + && invocation.Instance is { } instance + && RaisingMethods.Contains(invocation.TargetMethod.Name)) { - if (RaisingMethods.Contains(targetMethod.Name)) + if (context.State[instance]?.HasConstraint(CollectionConstraint.Empty) is true) { - if (context.State[instance]?.HasConstraint(CollectionConstraint.Empty) is true) - { - emptyAccess.Add(context.Operation.Instance); - } - else - { - nonEmptyAccess.Add(context.Operation.Instance); - } + emptyAccess.Add(context.Operation.Instance); } - if (instance.Type.DerivesOrImplementsAny(CollectionTracker.CollectionTypes)) + else { - return ProcessAddMethod(context.State, targetMethod, instance) - ?? ProcessRemoveMethod(context.State, targetMethod, instance) - ?? ProcessClearMethod(context.State, targetMethod, instance) - ?? context.State; + nonEmptyAccess.Add(context.Operation.Instance); } } 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) => - CollectionTracker.AddMethods.Contains(method.Name) - ? SetOperationAndSymbolConstraint(state, instance, CollectionConstraint.NotEmpty) - : null; - - private static ProgramState ProcessRemoveMethod(ProgramState state, IMethodSymbol method, IOperation instance) => - CollectionTracker.RemoveMethods.Contains(method.Name) - ? SetOperationAndSymbolValue(state, instance, (state[instance] ?? SymbolicValue.Empty).WithoutConstraint(CollectionConstraint.NotEmpty)) - : null; - - private static ProgramState ProcessClearMethod(ProgramState state, IMethodSymbol method, IOperation instance) => - method.Name == nameof(ICollection.Clear) - ? SetOperationAndSymbolConstraint(state, instance, CollectionConstraint.Empty) - : null; - - private static ProgramState SetOperationAndSymbolConstraint(ProgramState state, IOperation instance, SymbolicConstraint constraint) => - SetOperationAndSymbolValue(state, instance, (state[instance] ?? SymbolicValue.Empty).WithConstraint(constraint)); - - private static ProgramState SetOperationAndSymbolValue(ProgramState state, IOperation instance, SymbolicValue value) - { - state = state.SetOperationValue(instance, value); - if (instance.TrackedSymbol(state) is { } symbol) - { - state = state.SetSymbolValue(symbol, value); - } - return state; - } - - private static NumberConstraint SizeConstraint(ProgramState state, IOperation instance, bool hasFilteringPredicate = false) - { - if (instance.TrackedSymbol(state) is { } symbol && state[symbol]?.Constraint() is { } collection) - { - if (collection == CollectionConstraint.Empty) - { - return NumberConstraint.From(0); - } - else if (!hasFilteringPredicate) // nonEmpty.Count(predicate) can be Empty or NotEmpty - { - return NumberConstraint.From(1, null); - } - } - return instance.Type.DerivesOrImplementsAny(CollectionTracker.CollectionTypes) ? NumberConstraint.From(0, null) : null; } } diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs index d22dd5cfb6b..d23664243f7 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.cs @@ -1383,4 +1383,116 @@ public void Invocation_NullableGetHasValue_UntrackedSymbol() x => x.Should().HaveOnlyConstraints(ObjectConstraint.NotNull), // Because it's int x => x.Should().HaveOnlyConstraints(ObjectConstraint.NotNull, NumberConstraint.From(0))); } + + [TestMethod] + public void Invocation_EnumerableCount_SetsNumberConstraint() + { + const string code = """ + var x = list.Count(); + Tag("Before", x); + + list.Clear(); + x = list.Count(); + Tag("Clear", x); + + list.Add(42); + x = list.Count(); + Tag("Add", x); + + x = list.Count(_ => true); + Tag("Predicate", x); + + """; + var validator = SETestContext.CreateCS(code, "List list").Validator; + validator.ValidateContainsOperation(OperationKind.Invocation); + + validator.TagValue("Before").Should().HaveOnlyConstraints(NumberConstraint.From(0, null), ObjectConstraint.NotNull); + validator.TagValue("Clear").Should().HaveOnlyConstraints(NumberConstraint.From(0), ObjectConstraint.NotNull); + validator.TagValue("Add").Should().HaveOnlyConstraints(NumberConstraint.From(1, null), ObjectConstraint.NotNull); + validator.TagValue("Predicate").Should().HaveOnlyConstraints(NumberConstraint.From(0, null), ObjectConstraint.NotNull); + } + + [TestMethod] + public void Invocation_CollectionMethods_SetCollectionConstraint() + { + const string code = """ + var tag = "before"; + + list.Clear(); + tag = "clear"; + + list.Add(42); + tag = "add"; + + list.RemoveAt(0); + tag = "remove"; + + void Invoke(Action action) { } + """; + var validator = SETestContext.CreateCS(code, "List list", new PreserveTestCheck("list")).Validator; + validator.ValidateContainsOperation(OperationKind.Invocation); + + validator.TagValue("before", "list").Should().HaveNoConstraints(); + Verify("clear", ObjectConstraint.NotNull, CollectionConstraint.Empty); + Verify("add", ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); + Verify("remove", ObjectConstraint.NotNull); + + void Verify(string state, params SymbolicConstraint[] constraints) => + validator.TagValue(state, "list").Should().HaveOnlyConstraints(constraints); + } + + [TestMethod] + public void Invocation_CollectionArgument_RemovesCollectionConstraint() + { + const string code = """ + var tag = "before"; + + list.Add(42); + tag = "add"; + + Use(list); + tag = "argument"; + + void Use(List arg) { } + """; + var validator = SETestContext.CreateCS(code, "List list", new PreserveTestCheck("list")).Validator; + validator.ValidateContainsOperation(OperationKind.Argument); + + validator.TagValue("before", "list").Should().HaveNoConstraints(); + validator.TagValue("add", "list").Should().HaveOnlyConstraints(ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); + validator.TagValue("argument", "list").Should().HaveOnlyConstraint(ObjectConstraint.NotNull); + } + + [TestMethod] + public void MethodReference_RemovesCollectionConstraint() + { + const string code = """ + var tag = "before"; + + list.Clear(); + tag = "clear"; + + Invoke(list.Add); // remove CollectionConstraint, we don't know what Invoke does + tag = "methodReference1"; + + list.Add(42); + tag = "add"; + + Action action = list.RemoveAt; + tag = "methodReference2"; + + void Invoke(Action action) { } + """; + var validator = SETestContext.CreateCS(code, "List list", new PreserveTestCheck("list")).Validator; + validator.ValidateContainsOperation(OperationKindEx.MethodReference); + + validator.TagValue("before", "list").Should().HaveNoConstraints(); + Verify("clear", ObjectConstraint.NotNull, CollectionConstraint.Empty); + Verify("methodReference1", ObjectConstraint.NotNull); + Verify("add", ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); + Verify("methodReference2", ObjectConstraint.NotNull); + + void Verify(string state, params SymbolicConstraint[] constraints) => + validator.TagValue(state, "list").Should().HaveOnlyConstraints(constraints); + } } diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs index e30b2f843a8..6ecedd6a348 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Operations.cs @@ -546,15 +546,17 @@ public void ObjectCreation_SetsNotNull() var valueType = new Guid(); var declared = new Exception(); assigned = new EventArgs(); + var collection1 = new List(); collection1.Add(42); var collection2 = new List(collection1); + var collection3 = new List(list); tag = "tag"; """; - var preserved = new string[] { "obj", "valueType", "declared", "assigned", "collection1", "collection2" }; - var validator = SETestContext.CreateCS(code, new PreserveTestCheck(preserved)).Validator; + var preserved = new string[] { "obj", "valueType", "declared", "assigned", "collection1", "collection2", "collection3" }; + var validator = SETestContext.CreateCS(code, "List list", new PreserveTestCheck(preserved)).Validator; validator.ValidateContainsOperation(OperationKind.ObjectCreation); Verify("declared", ObjectConstraint.NotNull); @@ -562,7 +564,8 @@ public void ObjectCreation_SetsNotNull() Verify("valueType", ObjectConstraint.NotNull); // This is questionable, value types should not have ObjectConstraint Verify("obj", ObjectConstraint.NotNull); Verify("collection1", ObjectConstraint.NotNull); // The CollectionConstraint here is deleted when collection1 is used as an argument - Verify("collection2", ObjectConstraint.NotNull, CollectionConstraint.Empty); + Verify("collection2", ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); + Verify("collection3", ObjectConstraint.NotNull); void Verify(string symbol, params SymbolicConstraint[] constraints) => validator.TagValue("tag", symbol).Should().HaveOnlyConstraints(constraints); @@ -1075,6 +1078,44 @@ public void PropertyReference_AutoProperty_IsTracked() validator.TagValue("AfterReadReference").Should().HaveOnlyConstraints(ObjectConstraint.NotNull); } + [TestMethod] + public void PropertyReference_Count_HasNumericConstraint() + { + const string code = $""" + var x = collection.Count; + Tag("Before", x); + + collection.Add(42); + x = collection.Count; + Tag("AfterAdd", x); + + collection.Clear(); + x = collection.Count; + Tag("AfterClear", x); + """; + + var validator = SETestContext.CreateCS(code, $"List collection").Validator; + validator.ValidateContainsOperation(OperationKind.PropertyReference); + validator.TagValue("Before").Should().HaveOnlyConstraints(NumberConstraint.From(0, null), ObjectConstraint.NotNull); + validator.TagValue("AfterAdd").Should().HaveOnlyConstraints(NumberConstraint.From(1, null), ObjectConstraint.NotNull); + validator.TagValue("AfterClear").Should().HaveOnlyConstraints(NumberConstraint.From(0), ObjectConstraint.NotNull); + } + + [TestMethod] + public void PropertyReference_Indexer_SetsCollectionConstraint() + { + const string code = """ + var tag = "before"; + _ = list[42]; + tag = "after"; + """; + + var validator = SETestContext.CreateCS(code, "List list").Validator; + validator.ValidateContainsOperation(OperationKind.PropertyReference); + validator.TagValue("before", "list").Should().HaveNoConstraints(); + validator.TagValue("after", "list").Should().HaveOnlyConstraints(ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); + } + [TestMethod] public void PropertyReference_FullProperties_NotTracked() { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs index 5455fb868d3..8607581a7b8 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/ConditionEvaluatesToConstant.cs @@ -3085,6 +3085,28 @@ public void Method1(string s) } } + public class CollectionsCount + { + public void Method(List list) + { + if (list.Count() == 0) // Compliant + { } + + list.Clear(); + + if (list.Count() == 0) // Noncompliant + { } + + list.Add(42); + if (list.Count() >= 1) // Noncompliant + { } + + if (list.Count(Condition) >= 1) // Compliant, we don't know how many elements satisfy the condition + { } + } + + private bool Condition(int x) => true; + } } // https://github.com/SonarSource/sonar-dotnet/issues/3565 diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs index 77413f2e494..87b3598e94f 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs @@ -647,7 +647,7 @@ public void LearnConditions_Size(bool condition, List arg) if (empty.Count() == 0) { - empty.Clear(); // FN + empty.Clear(); // Noncompliant } else { @@ -656,7 +656,7 @@ public void LearnConditions_Size(bool condition, List arg) if (empty.Count(x => condition) == 0) { - empty.Clear(); // FN + empty.Clear(); // Noncompliant } else { @@ -674,7 +674,7 @@ public void LearnConditions_Size(bool condition, List arg) if (Enumerable.Count(empty) == 0) { - empty.Clear(); // FN + empty.Clear(); // Noncompliant } else { @@ -715,7 +715,7 @@ public void LearnConditions_Size_Array(bool condition) if (empty.Count() == 0) { - empty.Clone(); // FN + empty.Clone(); // Noncompliant } else { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb index 84a5540c252..5b6ba2c3ad6 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb @@ -233,7 +233,7 @@ Public Class CollectionTests Empty.Clear() ' Noncompliant End If If Enumerable.Count(Empty) = 0 Then - Empty.Clear() ' FN + Empty.Clear() ' Noncompliant Else Empty.Clear() ' Compliant, unreachable End If @@ -265,7 +265,7 @@ Public Class AdvancedTests List.All(Function(X) True) ' FN List.Any() ' FN Enumerable.Reverse(List) ' FN - List.Clear() ' FN, should raise, because the methods above should not reset the state + List.Clear() ' Noncompliant End Sub Public Sub PassingAsArgument_Removes_Constraints()