Skip to content
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
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 @@ _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(expression) =>
_ => 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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


_ = 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
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case is why I do expression.Elements instead of expression.DescendantNodes()



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