diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Sonar/Analyzers/EmptyCollectionsShouldNotBeEnumerated.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Sonar/Analyzers/EmptyCollectionsShouldNotBeEnumerated.cs index b28dd92c898..bb85485d117 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Sonar/Analyzers/EmptyCollectionsShouldNotBeEnumerated.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Sonar/Analyzers/EmptyCollectionsShouldNotBeEnumerated.cs @@ -18,7 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using SonarAnalyzer.SymbolicExecution.Sonar.Constraints; +using SonarAnalyzer.SymbolicExecution.Constraints; namespace SonarAnalyzer.SymbolicExecution.Sonar.Analyzers { diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CollectionConstraint.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CollectionConstraint.cs index 7f82e51b5de..150b979b05d 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CollectionConstraint.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CollectionConstraint.cs @@ -18,9 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using SonarAnalyzer.SymbolicExecution.Constraints; - -namespace SonarAnalyzer.SymbolicExecution.Sonar.Constraints; +namespace SonarAnalyzer.SymbolicExecution.Constraints; internal sealed class CollectionConstraint : SymbolicConstraint { diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs index 8fb263a6326..10d18c6b4ce 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs @@ -20,8 +20,7 @@ using System.Collections; using System.Collections.ObjectModel; -using System.Linq; -using SonarAnalyzer.SymbolicExecution.Sonar.Constraints; +using SonarAnalyzer.SymbolicExecution.Constraints; namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks; @@ -122,9 +121,9 @@ protected override ProgramState PreProcessSimple(SymbolicContext context) ? context.State.SetOperationConstraint(objectCreation, constraint) : context.State; } - else if (IsEmptyArray(operation)) + else if (operation.AsArrayCreation() is { } arrayCreation) { - return context.State.SetOperationConstraint(operation, CollectionConstraint.Empty); + return context.State.SetOperationConstraint(operation, arrayCreation.DimensionSizes.Any(x => x.ConstantValue.Value is 0) ? CollectionConstraint.Empty : CollectionConstraint.NotEmpty); } else if (operation.AsInvocation() is { } invocation) { @@ -134,6 +133,10 @@ protected override ProgramState PreProcessSimple(SymbolicContext context) { return ProcessMethod(context, methodReference.Method, methodReference.Instance); } + else if (operation.AsPropertyReference() is { } propertyReference && PropertyReferenceConstraint(context.State, propertyReference) is { } constraint) + { + return context.State.SetOperationConstraint(operation, constraint); + } else { return context.State; @@ -148,14 +151,6 @@ public override void ExecutionCompleted() } } - private static CollectionConstraint CollectionCreationConstraint(ProgramState state, IObjectCreationOperationWrapper objectCreation) => - objectCreation.Arguments.SingleOrDefault(x => x.ToArgument().Parameter.Type.DerivesOrImplements(KnownType.System_Collections_IEnumerable)) is { } collectionArgument - ? state[collectionArgument]?.Constraint() - : CollectionConstraint.Empty; - - private static bool IsEmptyArray(IOperation operation) => - operation.AsArrayCreation()?.DimensionSizes.Any(x => x.ConstantValue.Value is 0) ?? false; - private ProgramState ProcessMethod(SymbolicContext context, IMethodSymbol method, IOperation instance) { var state = context.State; @@ -184,4 +179,23 @@ private ProgramState ProcessMethod(SymbolicContext context, IMethodSymbol method } return state; } + + private static CollectionConstraint CollectionCreationConstraint(ProgramState state, IObjectCreationOperationWrapper objectCreation) => + objectCreation.Arguments.SingleOrDefault(x => x.ToArgument().Parameter.Type.DerivesOrImplements(KnownType.System_Collections_IEnumerable)) is { } collectionArgument + ? state[collectionArgument]?.Constraint() + : CollectionConstraint.Empty; + + private static NumberConstraint PropertyReferenceConstraint(ProgramState state, IPropertyReferenceOperationWrapper propertyReference) + { + if (propertyReference.Property.Name is nameof(Array.Length) or nameof(List.Count) + && state.ResolveCapture(propertyReference.Instance).TrackedSymbol() is { } symbol + && state[symbol]?.Constraint() is { } collection) + { + return collection == CollectionConstraint.Empty ? NumberConstraint.From(0) : NumberConstraint.From(1, null); + } + else + { + return null; + } + } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.CSharp8.cs index 6c5044612da..97f24d6267c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.CSharp8.cs @@ -153,3 +153,63 @@ public interface IDecoder { bool Convert(out int i3, out int i4); } + +// https://github.com/SonarSource/sonar-dotnet/issues/6179 +public class Repro_6179 +{ + public void FilledInLoop_FromAnotherCollection_FromArray() + { + var data = new[] { 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0 }; + var list = new List>(); + List currentBin = null; + for (var i = 0; i < data.Length; i++) + { + if (data[i] == 0) + { + if (currentBin != null) + { + list.Add(currentBin); + currentBin = null; + } + + continue; + } + + currentBin ??= new List(); + currentBin.Add(i); + } + + for (var i = 0; i < list.Count; i++) + { + list.ForEach(x => { }); // Compliant + } + } + + public void FilledInLoop_FromAnotherCollection_FromList() + { + var data = new List { 0, 0, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0 }; + var list = new List>(); + List currentBin = null; + for (var i = 0; i < data.Count; i++) + { + if (data[i] == 0) + { + if (currentBin != null) + { + list.Add(currentBin); + currentBin = null; + } + + continue; + } + + currentBin ??= new List(); + currentBin.Add(i); + } + + for (var i = 0; i < list.Count; i++) + { + list.ForEach(x => { }); // Compliant + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs index 618a0ca5b6e..01b8f39b19e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs @@ -498,6 +498,97 @@ public void HigherRank_And_Jagged_Array() array5.Clone(); // Compliant } + public void LearnConditions_Size(bool condition) + { + List isNull = null; + var empty = new List(); + var notEmpty = new List() { 1, 2, 3 }; + + if ((isNull ?? empty).Count == 0) + { + empty.Clear(); // Noncompliant + } + + if ((isNull ?? notEmpty).Count == 0) + { + empty.Clear(); // Compliant, unreachable + } + + if ((condition ? empty : empty).Count == 0) + { + empty.Clear(); // Noncompliant + } + else + { + empty.Clear(); // Compliant, unreachable + } + + if (empty.Count == 0) + { + empty.Clear(); // Noncompliant + } + else + { + empty.Clear(); // Compliant, unreachable + } + + if (empty.Count() == 0) + { + empty.Clear(); // Noncompliant + } + else + { + empty.Clear(); // Noncompliant FP, Should be Compliant, unreachable + } + + if (Enumerable.Count(empty) == 0) + { + empty.Clear(); // Noncompliant + } + else + { + empty.Clear(); // Noncompliant FP, Should be Compliant, unreachable + } + + notEmpty.Clear(); // Compliant, prevents LVA from throwing notEmpty away during reference capture + } + + public void LearnConditions_Size_Array(bool condition) + { + int[] isNull = null; + var empty = new int[0]; + var notEmpty = new[] { 1, 2, 3 }; + + if ((condition ? empty : empty).Length == 0) + { + empty.Clone(); // Noncompliant + } + else + { + empty.Clone(); // Compliant, unreachable + } + + if (empty.Length == 0) + { + empty.Clone(); // Noncompliant + } + else + { + empty.Clone(); // Compliant, unreachable + } + + if (empty.Count() == 0) + { + empty.Clone(); // Noncompliant + } + else + { + empty.Clone(); // Noncompliant FP, Should be Compliant, unreachable + } + + notEmpty.Clone(); // Compliant, prevents LVA from throwing notEmpty away during reference capture + } + void Foo(IEnumerable items) { } }