Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SE: Move CollectionConstraint from S4158 to the engine (part 5) #8728

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ internal static class OperationDispatcher
{ OperationKindEx.Increment, new IncrementOrDecrement() },
{ OperationKindEx.InstanceReference, new InstanceReference() },
{ OperationKindEx.LocalReference, new LocalReference() },
{ OperationKindEx.MethodReference, new MethodReference() },
{ OperationKindEx.ObjectCreation, new ObjectCreation() },
{ OperationKindEx.ParameterReference, new ParameterReference() },
{ OperationKindEx.RecursivePattern, new RecursivePattern() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.ObjectModel;
using SonarAnalyzer.SymbolicExecution.Constraints;

namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;
Expand All @@ -43,6 +44,36 @@ internal static class CollectionTracker
KnownType.System_Collections_Generic_IDictionary_TKey_TValue,
KnownType.System_Collections_Immutable_IImmutableDictionary_TKey_TValue);

public static readonly HashSet<string> AddMethods = new()
{
nameof(ICollection<int>.Add),
nameof(List<int>.AddRange),
nameof(List<int>.Insert),
nameof(List<int>.InsertRange),
nameof(HashSet<int>.UnionWith),
nameof(HashSet<int>.SymmetricExceptWith), // This can add and/or remove items => It should remove all CollectionConstraints.
// However, just learning NotEmpty (and thus unlearning Empty) is good enough for now.
nameof(Queue<int>.Enqueue),
nameof(Stack<int>.Push),
nameof(Collection<int>.Insert),
"TryAdd"
};

public static readonly HashSet<string> RemoveMethods = new()
{
nameof(ICollection<int>.Remove),
nameof(List<int>.RemoveAll),
nameof(List<int>.RemoveAt),
nameof(List<int>.RemoveRange),
nameof(HashSet<int>.ExceptWith),
nameof(HashSet<int>.IntersectWith),
nameof(HashSet<int>.RemoveWhere),
nameof(Queue<int>.Dequeue),
nameof(Stack<int>.Pop)
};

private static readonly HashSet<string> AddOrRemoveMethods = [.. AddMethods, .. RemoveMethods];
Tim-Pohlmann marked this conversation as resolved.
Show resolved Hide resolved

public static CollectionConstraint ObjectCreationConstraint(ProgramState state, IObjectCreationOperationWrapper operation)
{
if (operation.Type.IsAny(CollectionTypes))
Expand All @@ -65,6 +96,14 @@ public static CollectionConstraint ArrayCreationConstraint(IArrayCreationOperati
? CollectionConstraint.Empty
: CollectionConstraint.NotEmpty;

public static ProgramState LearnFrom(ProgramState state, IMethodReferenceOperationWrapper operation) =>
operation.Instance is not null
&& operation.Instance.Type.DerivesOrImplementsAny(CollectionTypes)
&& AddOrRemoveMethods.Contains(operation.Method.Name)
&& state[operation.Instance] is { } value
? state.SetOperationAndSymbolValue(operation.Instance, value.WithoutConstraint<CollectionConstraint>())
: state;

public static ProgramState LearnFrom(ProgramState state, IPropertyReferenceOperationWrapper operation, ISymbol instanceSymbol)
{
if (operation.Instance is not null
Expand All @@ -83,7 +122,7 @@ public static ProgramState LearnFrom(ProgramState state, IPropertyReferenceOpera
state = state.SetOperationConstraint(operation, NumberConstraint.From(0, null));
}
}
if (operation.Property.IsIndexer)
else if (operation.Property.IsIndexer)
{
state = state.SetOperationConstraint(operation.Instance, CollectionConstraint.NotEmpty);
if (instanceSymbol is not null)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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.
*/

namespace SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors;

internal sealed class MethodReference : SimpleProcessor<IMethodReferenceOperationWrapper>
{
protected override IMethodReferenceOperationWrapper Convert(IOperation operation) =>
IMethodReferenceOperationWrapper.FromOperation(operation);

protected override ProgramState Process(SymbolicContext context, IMethodReferenceOperationWrapper operation) =>
CollectionTracker.LearnFrom(context.State, operation);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could in-line this, but I think it's good to keep it in the tracker.
Mainly because:

  • I can hide the hashset properties
  • Almost everything is centralized there for CollectionConstraint.

}
Original file line number Diff line number Diff line change
Expand Up @@ -84,53 +84,13 @@ public abstract class EmptyCollectionsShouldNotBeEnumeratedBase : SymbolicRuleCh
nameof(Dictionary<int, int>.TryGetValue)
};

private static readonly HashSet<string> AddMethods = new()
{
nameof(ICollection<int>.Add),
nameof(List<int>.AddRange),
nameof(List<int>.Insert),
nameof(List<int>.InsertRange),
nameof(HashSet<int>.UnionWith),
nameof(HashSet<int>.SymmetricExceptWith), // This can add and/or remove items => It should remove all CollectionConstraints.
// However, just learning NotEmpty (and thus unlearning Empty) is good enough for now.
nameof(Queue<int>.Enqueue),
nameof(Stack<int>.Push),
nameof(Collection<int>.Insert),
"TryAdd"
};

private static readonly HashSet<string> RemoveMethods = new()
{
nameof(ICollection<int>.Remove),
nameof(List<int>.RemoveAll),
nameof(List<int>.RemoveAt),
nameof(List<int>.RemoveRange),
nameof(HashSet<int>.ExceptWith),
nameof(HashSet<int>.IntersectWith),
nameof(HashSet<int>.RemoveWhere),
nameof(Queue<int>.Dequeue),
nameof(Stack<int>.Pop)
};

private readonly HashSet<IOperation> emptyAccess = new();
private readonly HashSet<IOperation> nonEmptyAccess = new();

protected override ProgramState PreProcessSimple(SymbolicContext context)
{
var operation = context.Operation.Instance;
if (operation.AsInvocation() is { } invocation)
{
return ProcessInvocation(context, invocation);
}
else if (operation.AsMethodReference() is { Instance: not null } methodReference)
{
return ProcessAddMethod(context.State, methodReference.Method, methodReference.Instance) ?? context.State;
}
else
{
return context.State;
}
}
protected override ProgramState PreProcessSimple(SymbolicContext context) =>
context.Operation.Instance.AsInvocation() is { } invocation
? ProcessInvocation(context, invocation)
: context.State;

public override void ExecutionCompleted()
{
Expand Down Expand Up @@ -176,12 +136,12 @@ bool HasFilteringPredicate() =>
}

private static ProgramState ProcessAddMethod(ProgramState state, IMethodSymbol method, IOperation instance) =>
AddMethods.Contains(method.Name)
CollectionTracker.AddMethods.Contains(method.Name)
? SetOperationAndSymbolConstraint(state, instance, CollectionConstraint.NotEmpty)
: null;

private static ProgramState ProcessRemoveMethod(ProgramState state, IMethodSymbol method, IOperation instance) =>
RemoveMethods.Contains(method.Name)
CollectionTracker.RemoveMethods.Contains(method.Name)
? SetOperationAndSymbolValue(state, instance, (state[instance] ?? SymbolicValue.Empty).WithoutConstraint(CollectionConstraint.NotEmpty))
: null;

Expand Down
Loading