Skip to content

Commit

Permalink
Fix S2583/S2589 FP: Tuple deconstruction assignment (#8461)
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource authored Dec 19, 2023
1 parent 30fc307 commit cfee18a
Show file tree
Hide file tree
Showing 17 changed files with 304 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ OperationKindEx.PropertyReference when operation.ToPropertyReference() is { Prop
internal static IConversionOperationWrapper? AsConversion(this IOperation operation) =>
operation.As(OperationKindEx.Conversion, IConversionOperationWrapper.FromOperation);

internal static IDeclarationExpressionOperationWrapper? AsDeclarationExpression(this IOperation operation) =>
operation.As(OperationKindEx.DeclarationExpression, IDeclarationExpressionOperationWrapper.FromOperation);

internal static IDeclarationPatternOperationWrapper? AsDeclarationPattern(this IOperation operation) =>
operation.As(OperationKindEx.DeclarationPattern, IDeclarationPatternOperationWrapper.FromOperation);

Expand All @@ -69,6 +72,9 @@ OperationKindEx.PropertyReference when operation.ToPropertyReference() is { Prop
internal static IPropertyReferenceOperationWrapper? AsPropertyReference(this IOperation operation) =>
operation.As(OperationKindEx.PropertyReference, IPropertyReferenceOperationWrapper.FromOperation);

internal static ITupleOperationWrapper? AsTuple(this IOperation operation) =>
operation.As(OperationKindEx.Tuple, ITupleOperationWrapper.FromOperation);

internal static IAwaitOperationWrapper ToAwait(this IOperation operation) =>
IAwaitOperationWrapper.FromOperation(operation);

Expand Down Expand Up @@ -123,6 +129,9 @@ internal static IParameterReferenceOperationWrapper ToParameterReference(this IO
internal static IEventReferenceOperationWrapper ToEventReference(this IOperation operation) =>
IEventReferenceOperationWrapper.FromOperation(operation);

internal static ITupleOperationWrapper ToTuple(this IOperation operation) =>
ITupleOperationWrapper.FromOperation(operation);

internal static IUnaryOperationWrapper ToUnary(this IOperation operation) =>
IUnaryOperationWrapper.FromOperation(operation);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ internal static class OperationDispatcher
{ OperationKindEx.CompoundAssignment, new CompoundAssignment() },
{ OperationKindEx.Conversion, new Conversion() },
{ OperationKindEx.DeclarationPattern, new DeclarationPattern() },
{ OperationKindEx.DeconstructionAssignment, new DeconstructionAssignment() },
{ OperationKindEx.Decrement, new IncrementOrDecrement() },
{ OperationKindEx.DefaultValue, new DefaultValue() },
{ OperationKindEx.DelegateCreation, new NotNullOperation() },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 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 DeconstructionAssignment : SimpleProcessor<IDeconstructionAssignmentOperationWrapper>
{
protected override IDeconstructionAssignmentOperationWrapper Convert(IOperation operation) =>
IDeconstructionAssignmentOperationWrapper.FromOperation(operation);

protected override ProgramState Process(SymbolicContext context, IDeconstructionAssignmentOperationWrapper assignment)
{
var operationValues = TupleElementValues(context.State, assignment.Target, assignment.Value);
var newState = context.State;
foreach (var (tupleMember, value) in operationValues)
{
newState = newState.SetOperationAndSymbolValue(tupleMember, value);
}
return newState;
}

private static IEnumerable<OperationValue> TupleElementValues(ProgramState state, IOperation target, IOperation value)
{
var operationValues = new List<OperationValue>();
var leftTupleElements = TupleElements(Unwrap(target, state).ToTuple(), state);
// If the right side is a tuple, then every symbol/constraint is copied to the left side.
if (Unwrap(value, state).AsTuple() is { } rightSideTuple)
{
var rightTupleElements = TupleElements(rightSideTuple, state);
for (var i = 0; i < leftTupleElements.Length; i++)
{
var leftTupleMember = leftTupleElements[i];
var rightTupleMember = rightTupleElements[i];
if (leftTupleMember.Kind == OperationKindEx.Discard)
{
continue;
}
else if (leftTupleMember.AsTuple() is { } nestedTuple)
{
operationValues.AddRange(TupleElementValues(state, nestedTuple.WrappedOperation, rightTupleMember));
}
else
{
operationValues.Add(new(leftTupleMember, state[rightTupleMember]));
}
}
}
// If the right side is not a tuple, then every member of the left side tuple is set to empty.
else
{
operationValues.AddRange(leftTupleElements
.Where(x => x.Kind != OperationKindEx.Discard)
.Select(x => new OperationValue(x, SymbolicValue.Empty)));
}
return operationValues;
}

private static IOperation[] TupleElements(ITupleOperationWrapper operation, ProgramState state) =>
operation.Elements.Select(x => Unwrap(x, state)).ToArray();

private static IOperation Unwrap(IOperation operation, ProgramState state)
{
var unwrapped = state.ResolveCaptureAndUnwrapConversion(operation);
return unwrapped.AsDeclarationExpression()?.Expression ?? unwrapped;
}

private sealed record OperationValue(IOperation Operation, SymbolicValue Value);
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ protected ProgramState(ProgramState original) // Custom record override constr
Exceptions = original.Exceptions;
}

public ProgramState SetOperationAndSymbolValue(IOperation operation, SymbolicValue value)
{
var newState = SetOperationValue(operation, value);
if (operation.TrackedSymbol(this) is { } symbol)
{
newState = newState.SetSymbolValue(symbol, value);
}
return newState;
}

public ProgramState SetOperationValue(IOperationWrapper operation, SymbolicValue value) =>
operation is null
? throw new ArgumentNullException(nameof(operation))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 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.Roslyn;
using SonarAnalyzer.UnitTest.TestFramework.SymbolicExecution;

namespace SonarAnalyzer.UnitTest.SymbolicExecution.Roslyn;

public partial class ProgramStateTest
{
[TestMethod]
public void SetOperationAndSymbolValue_TrackedSymbol()
{
var operations = TestHelper.CompileCfgBodyCS("bool b = true;").Blocks[1].Operations;
var localReference = operations[0].ChildOperations.First().ToLocalReference();
var symbol = localReference.Local;
var sut = ProgramState.Empty;

sut[localReference].Should().BeNull();
sut[symbol].Should().BeNull();

sut = sut.SetOperationAndSymbolValue(localReference.WrappedOperation, SymbolicValue.True);

sut[localReference].Should().Be(SymbolicValue.True);
sut[symbol].Should().Be(SymbolicValue.True);
}

[TestMethod]
public void SetOperationAndSymbolValue_NotTrackedSymbol()
{
var operations = TestHelper.CompileCfgBodyCS("""var texts = new string[] { }; texts[42] = string.Empty;""").Blocks[1].Operations;
var arrayElementReference = operations[1].ChildOperations.First().ChildOperations.First().ToArrayElementReference();
var sut = ProgramState.Empty;

sut[arrayElementReference].Should().BeNull();

sut = sut.SetOperationAndSymbolValue(arrayElementReference.WrappedOperation, SymbolicValue.NotNull);

sut[arrayElementReference].Should().Be(SymbolicValue.NotNull);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,35 @@ public static void DoSomething()
else if (myStruct.y) { } // FN
}
}

// https://github.com/SonarSource/sonar-dotnet/issues/7057
public class Repro_7057
{
private (string, int) SomeTuple() => ("hello", 1);
private string SomeString() => "hello";

public void WithTuple()
{
string text1 = null;
(text1, var (text2, _)) = (SomeString(), SomeTuple());
if (text1 == null) // Compliant
{
Console.WriteLine();
}
if (text2 == null) // Compliant
{
Console.WriteLine();
}

string text3 = null;
((text3, _), var (text4, _)) = ((null, 42), ("hello", 42));
if (text3 == null) // Noncompliant
{
Console.WriteLine();
}
if (text4 == null) // Noncompliant
{
Console.WriteLine(); // Secondary
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ public void GoGoGo()
{
var tmp = 0;
var flag = true;
while (flag) // Noncompliant
while (flag) // Compliant
{
(flag, tmp) = (false, 5);
}
Expand Down Expand Up @@ -329,7 +329,7 @@ public void MutedNull()
{
var tmp = 0;
var flag = "x";
while (flag != null) // Noncompliant
while (flag != null) // Compliant
{
(flag, tmp) = (null, 5);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ public void ParenthesizedVariableDesignation_Nested(object arg)
public void NestedDeconstructionAssignment()
{
var (a, (b, _)) = (true, (true, true));
if (a) { } // FN
if (b) { } // FN
if (a) { } // Noncompliant
if (b) { } // Noncompliant
}

int UsingDeclaration_Null()
Expand Down Expand Up @@ -330,7 +330,7 @@ public void AssignmentTarget(bool arg)
{
}

if (b) // FN
if (b) // Noncompliant
{
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ public object InitWithTupleAssignment
{
var tmp = 0;
var flag = true;
while (flag) // Noncompliant
while (flag) // Compliant
{
(flag, tmp) = (false, 5);
}
o = value; // Secondary
o = value;
}
}
}
Expand Down
Loading

0 comments on commit cfee18a

Please sign in to comment.