Skip to content

Commit

Permalink
Fix S3878 FP: When input array is collection expression with spread o…
Browse files Browse the repository at this point in the history
…perator (#8970)
  • Loading branch information
gregory-paidis-sonarsource committed Mar 21, 2024
1 parent 81bdd15 commit 8bf481c
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 21 deletions.
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.CFG/ShimLayer/SyntaxKindEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,6 @@ public static class SyntaxKindEx
public const SyntaxKind InterpolatedMultiLineRawStringStartToken = (SyntaxKind)9073;
public const SyntaxKind InterpolatedRawStringEndToken = (SyntaxKind)9074;
public const SyntaxKind ScopedType = (SyntaxKind)9075;
public const SyntaxKind SpreadElement = (SyntaxKind)9078;
}
}
26 changes: 15 additions & 11 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ArrayPassedAsParams.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ public sealed class ArrayPassedAsParams : ArrayPassedAsParamsBase<SyntaxKind, Ar
SyntaxKindEx.ImplicitObjectCreationExpression
};

protected override ArgumentSyntax GetLastArgumentIfArrayCreation(SyntaxNode expression) =>
GetLastArgumentIfArrayCreation(GetArgumentListFromExpression(expression));
protected override ArgumentSyntax LastArgumentIfArrayCreation(SyntaxNode expression) =>
LastArgumentIfArrayCreation(ArgumentList(expression));

private static BaseArgumentListSyntax GetArgumentListFromExpression(SyntaxNode expression) =>
private static ArgumentSyntax LastArgumentIfArrayCreation(BaseArgumentListSyntax argumentList) =>
argumentList is { Arguments: { Count: > 0 } arguments }
&& arguments.Last() is var lastArgument
&& IsArrayCreation(lastArgument.Expression)
? lastArgument
: null;

private static BaseArgumentListSyntax ArgumentList(SyntaxNode expression) =>
expression switch
{
ObjectCreationExpressionSyntax { } creation => creation.ArgumentList,
Expand All @@ -45,19 +52,16 @@ expression switch
_ => null
};

private static ArgumentSyntax GetLastArgumentIfArrayCreation(BaseArgumentListSyntax argumentList) =>
argumentList is { Arguments: { Count: > 0 } arguments }
&& arguments.Last() is var lastArgument
&& IsArrayCreation(lastArgument.Expression)
? lastArgument
: null;

private static bool IsArrayCreation(ExpressionSyntax expression) =>
expression switch
{
ArrayCreationExpressionSyntax { Initializer: not null } => true,
ImplicitArrayCreationExpressionSyntax => true,
_ when CollectionExpressionSyntaxWrapper.IsInstance(expression) => true,
_ when CollectionExpressionSyntaxWrapper.IsInstance(expression) => !ContainsSpread((CollectionExpressionSyntaxWrapper)expression),
_ => false
};

// [x, y, ..z] is not possible to be passed as params
private static bool ContainsSpread(CollectionExpressionSyntaxWrapper expression) =>
expression.Elements.Any(x => x.SyntaxNode.IsKind(SyntaxKindEx.SpreadElement));
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ public abstract class ArrayPassedAsParamsBase<TSyntaxKind, TArgumentNode> : Sona
private readonly DiagnosticDescriptor rule;

protected abstract TSyntaxKind[] ExpressionKinds { get; }
protected abstract TArgumentNode GetLastArgumentIfArrayCreation(SyntaxNode expression);
protected abstract TArgumentNode LastArgumentIfArrayCreation(SyntaxNode expression);

protected ArrayPassedAsParamsBase() : base(DiagnosticId) =>
rule = Language.CreateDescriptor(DiagnosticId, MessageFormat);

protected sealed override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(Language.GeneratedCodeRecognizer, c =>
{
if (GetLastArgumentIfArrayCreation(c.Node) is { } lastArgument
if (LastArgumentIfArrayCreation(c.Node) is { } lastArgument
&& IsParamParameter(c.SemanticModel, c.Node, lastArgument))
{
c.ReportIssue(Diagnostic.Create(rule, lastArgument.GetLocation()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public sealed class ArrayPassedAsParams : ArrayPassedAsParamsBase<SyntaxKind, Ar
SyntaxKind.InvocationExpression
};

protected override ArgumentSyntax GetLastArgumentIfArrayCreation(SyntaxNode expression) =>
protected override ArgumentSyntax LastArgumentIfArrayCreation(SyntaxNode expression) =>
GetLastArgumentIfArrayCreation(GetArgumentListFromExpression(expression));

private static ArgumentListSyntax GetArgumentListFromExpression(SyntaxNode expression) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,23 @@
const int a = 1;
int[] array = [2, 3];

_ = new MyClass(1, [1, 2, 3]); // Noncompliant
_ = new MyClass(1, []); // Noncompliant
var class1 = new MyClass(1, [1, 2, 3]); // Noncompliant
_ = new MyClass(1, []); // Noncompliant

// repro for https://github.com/SonarSource/sonar-dotnet/issues/8510
_ = new MyClass(1, [a, .. array]); // Noncompliant FP
_ = new MyClass(1, [a, .. array]); // Compliant

_ = new MyClass2([1], [1, 2, 3]); // Noncompliant
_ = new MyClass2([1], [1, 2, 3]); // Noncompliant
_ = new MyClass2([1, 2, 3], 1);
_ = new MyClass3([1, 2, 3], [4, 5, 6]); // Noncompliant FP
_ = new MyClass3([[1, 2, 3], [4, 5, 6]]); // Noncompliant
_ = new MyClass3([[1, 2, 3], [4, 5, 6]]); // Noncompliant

_ = new MyClass3([1, 2, 3], [4, 5, 6]); // Noncompliant FP

_ = new MyClass4(class1, new(1, [1, .. array])); // Compliant
_ = new MyClass4([class1, new(1, [1, .. array])]); // Noncompliant, outer collection raises, despite the nested spread operator
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


class MyClass(int a, params int[] args);
class MyClass2(int[] a, params int[] args);
class MyClass3(params int[][] args);
class MyClass4(params MyClass[] args);

0 comments on commit 8bf481c

Please sign in to comment.