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

Fix S3267 FN: Support list pattern #6414

Merged
merged 9 commits into from
Dec 6, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,30 @@
}
}
]
},
{
"id": "S3267",
"message": "Loops should be simplified with "LINQ" expressions",
"location": [
{
"uri": "sources\akka.net\src\contrib\cluster\Akka.DistributedData\Serialization\ReplicatorMessageSerializer.cs",
"region": {
"startLine": 313,
"startColumn": 31,
"endLine": 313,
"endColumn": 55
}
},
{
"uri": "sources\akka.net\src\contrib\cluster\Akka.DistributedData\Serialization\ReplicatorMessageSerializer.cs",
"region": {
"startLine": 315,
"startColumn": 21,
"endLine": 315,
"endColumn": 48
}
}
]
}
]
}
23 changes: 16 additions & 7 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/LoopsAndLinq.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,21 @@ node switch
_ => null
};

private static bool CanIfStatementBeMoved(IfStatementSyntax ifStatementSyntax) =>
ifStatementSyntax.Else == null
&& ifStatementSyntax.Condition is InvocationExpressionSyntax invocationExpressionSyntax
&& !invocationExpressionSyntax.DescendantNodes()
.OfType<ArgumentSyntax>()
.Any(argument => argument.RefOrOutKeyword.IsAnyKind(SyntaxKind.OutKeyword, SyntaxKind.RefKeyword));
private static bool CanIfStatementBeMoved(IfStatementSyntax ifStatementSyntax)
{
return ifStatementSyntax.Else == null && (ConditionValidIsPattern() || ConditionValidInvocation());

bool ConditionValidIsPattern() => (IsPatternExpressionSyntaxWrapper.IsInstance(ifStatementSyntax.Condition) || ifStatementSyntax.Condition.IsKind(SyntaxKind.IsExpression))
csaba-sagi-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
&& !ifStatementSyntax.Condition.DescendantNodes()
.Any(d => d.IsAnyKind(SyntaxKindEx.VarPattern,
SyntaxKindEx.SingleVariableDesignation,
SyntaxKindEx.ParenthesizedVariableDesignation));

bool ConditionValidInvocation() => ifStatementSyntax.Condition is InvocationExpressionSyntax invocationExpressionSyntax
&& !invocationExpressionSyntax.DescendantNodes()
.OfType<ArgumentSyntax>()
.Any(argument => argument.RefOrOutKeyword.IsAnyKind(SyntaxKind.OutKeyword, SyntaxKind.RefKeyword));
}

cristian-ambrosini-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
/// <remarks>
/// There are multiple scenarios where the code can be simplified using LINQ.
Expand Down Expand Up @@ -135,7 +144,7 @@ private static void CheckIfCanBeSimplifiedUsingSelect(ForEachStatementSyntax for
!(memberAccess.Parent is AssignmentExpressionSyntax assignment && assignment.Left == memberAccess);
}

private class UsageStats
private sealed class UsageStats
{
public int Count { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,35 @@

namespace Tests.Diagnostics
{
class MyClass
class IsPatternTests
{
void ListPattern(List<int[]> list)
{
foreach (int[] array in list) // FN
foreach (int[] array in list) // Noncompliant
{
if (array is [1, 2, 3]) // FN
if (array is [1, 2, 3]) // Secondary
{
Console.WriteLine("Test");
Console.WriteLine("Pattern match successful");
}
}

foreach (var array in list) // Compliant, do not raise on VarPattern in ListPattern
{
if (array is [1, var x, var z])
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following up on the previous comment, if someone re-wrote this as a Linq Where, the x and y variables would not be properly scoped inside the if statement. Same with is ... local

Console.WriteLine("Pattern match successful");
}

}

foreach (var array in list) // Compliant, do not raise on declaration statements in ListPattern
{
if (array is [1, ..] local)
{
Console.WriteLine("Pattern match successful");
}

}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,42 @@ public class Point
public int X { get; set; }
public int? Y { get; set; }
}

void IsPatterns(List<string> strings, List<Tuple<string, int>> tuples)
{
const string target = "42";

foreach (var s in strings) // Noncompliant
{
if (s is target) // Secondary
{
Console.WriteLine("Pattern match successful");
}
}

foreach (var s in strings) // Compliant, do not raise on VarPattern in IsPattern
{
if (s is var s2)
{
Console.WriteLine("Pattern match successful");
}
}

foreach (var s in strings) // Compliant, do not raise on SingleVariableDeclaration in IsPattern
{
if (s is { Length: 42 } str)
{
Console.WriteLine("Pattern match successful");
}
}

foreach (var t in tuples) // Compliant, do not raise on ParenthesizedVariableDeclaration in IsPattern
{
if (t is var (t1, t2))
{
Console.WriteLine("Pattern match successful");
}
}
}
}
}
11 changes: 11 additions & 0 deletions analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LoopsAndLinq.cs
Original file line number Diff line number Diff line change
Expand Up @@ -309,5 +309,16 @@ public class Point

public int GetX() => X;
}

void IsPattern(List<int> list)
{
foreach (var item in list) // Noncompliant
{
if (item is 42) // Secondary
{
Console.WriteLine("The meaning of Life.");
}
}
}
}
}