Skip to content

Commit

Permalink
Extract logic to CollectionTracker
Browse files Browse the repository at this point in the history
  • Loading branch information
gregory-paidis-sonarsource committed Feb 7, 2024
1 parent 52eb017 commit 879d2d8
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
*/

using SonarAnalyzer.SymbolicExecution.Constraints;
using SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks;

namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;

Expand All @@ -30,8 +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)
if (CollectionTracker.ObjectCreationConstraint(context.State, operation) is { } constraint)
{
return context.SetOperationConstraint(constraint)
.SetOperationConstraint(operation, ObjectConstraint.NotNull);
Expand All @@ -57,11 +55,4 @@ protected override ProgramState Process(SymbolicContext context, IObjectCreation
}
}

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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 879d2d8

Please sign in to comment.