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 S2368 FP/FN: Don't raise for extension methods and raise for constructors #8152

Merged
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 @@ -98,6 +98,7 @@ public static ImmutableArray<SyntaxToken> DeclaringReferenceIdentifiers(this ISy
{
AttributeArgumentSyntax x => x.NameColon?.Name.Identifier,
BaseTypeDeclarationSyntax x => x.Identifier,
ConstructorDeclarationSyntax x => x.Identifier,
DelegateDeclarationSyntax x => x.Identifier,
EnumMemberDeclarationSyntax x => x.Identifier,
EventDeclarationSyntax x => x.Identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,16 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind, MethodDeclarationSyntax>
{

private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
namespace SonarAnalyzer.Rules.CSharp;

private static readonly ImmutableArray<SyntaxKind> kindsOfInterest = ImmutableArray.Create(SyntaxKind.MethodDeclaration);
public override ImmutableArray<SyntaxKind> SyntaxKindsOfInterest => kindsOfInterest;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

protected override SyntaxToken GetIdentifier(MethodDeclarationSyntax method) => method.Identifier;
protected override Location GetIssueLocation(SyntaxNode node) =>
Language.Syntax.NodeIdentifier(node)?.GetLocation();

protected override GeneratedCodeRecognizer GeneratedCodeRecognizer => CSharpGeneratedCodeRecognizer.Instance;
}
protected override string GetType(SyntaxNode node) =>
node is MethodDeclarationSyntax ? "method" : "constructor";
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,55 +18,63 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules
namespace SonarAnalyzer.Rules;

public abstract class PublicMethodWithMultidimensionalArrayBase<TSyntaxKind> : SonarDiagnosticAnalyzer<TSyntaxKind>
where TSyntaxKind : struct
{
public abstract class PublicMethodWithMultidimensionalArrayBase : SonarDiagnosticAnalyzer
private const string DiagnosticId = "S2368";
protected override string MessageFormat => "Make this {0} private or simplify its parameters to not use multidimensional/jagged arrays.";

protected abstract Location GetIssueLocation(SyntaxNode node);
protected abstract string GetType(SyntaxNode node);

protected PublicMethodWithMultidimensionalArrayBase() : base(DiagnosticId) { }

protected sealed override void Initialize(SonarAnalysisContext context)
{
protected const string DiagnosticId = "S2368";
protected const string MessageFormat = "Make this method private or simplify its parameters to not use multidimensional/jagged arrays.";
context.RegisterNodeAction(Language.GeneratedCodeRecognizer, AnalyzeDeclaration, Language.SyntaxKind.MethodDeclarations);
context.RegisterNodeAction(Language.GeneratedCodeRecognizer, AnalyzeDeclaration, Language.SyntaxKind.ConstructorDeclaration);
}

protected abstract GeneratedCodeRecognizer GeneratedCodeRecognizer { get; }
private void AnalyzeDeclaration(SonarSyntaxNodeReportingContext c)
{
if (c.SemanticModel.GetDeclaredSymbol(c.Node) is IMethodSymbol methodSymbol
&& methodSymbol.GetInterfaceMember() == null
&& methodSymbol.GetOverriddenMember() == null
&& methodSymbol.IsPubliclyAccessible()
&& MethodHasMultidimensionalArrayParameters(methodSymbol))
{
c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], GetIssueLocation(c.Node), GetType(c.Node)));
}
}

public abstract class PublicMethodWithMultidimensionalArrayBase<TLanguageKindEnum, TMethodSyntax> : PublicMethodWithMultidimensionalArrayBase
where TLanguageKindEnum : struct
where TMethodSyntax : SyntaxNode
private static bool MethodHasMultidimensionalArrayParameters(IMethodSymbol methodSymbol)
{
protected sealed override void Initialize(SonarAnalysisContext context)
if (methodSymbol.IsExtensionMethod)
{
context.RegisterNodeAction(
GeneratedCodeRecognizer,
c =>
for (var i = 1; i < methodSymbol.Parameters.Length; i++)
{
if (IsMultidimensionalArrayParameter(methodSymbol.Parameters[i]))
{
var method = (TMethodSyntax)c.Node;

if (!(c.SemanticModel.GetDeclaredSymbol(method) is IMethodSymbol methodSymbol) ||
methodSymbol.GetInterfaceMember() != null ||
methodSymbol.GetOverriddenMember() != null ||
!methodSymbol.IsPubliclyAccessible() ||
!MethodHasMultidimensionalArrayParameters(methodSymbol))
{
return;
}

var identifier = GetIdentifier(method);
c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], identifier.GetLocation()));
},
SyntaxKindsOfInterest.ToArray());
return true;
}
}
return false;
}
else
{
return methodSymbol.Parameters.Any(param => IsMultidimensionalArrayParameter(param));
}
}

private static bool MethodHasMultidimensionalArrayParameters(IMethodSymbol methodSymbol) =>
methodSymbol.Parameters.Any(param => param.Type is IArrayTypeSymbol arrayType
&& (arrayType.Rank > 1
|| IsJaggedArrayParam(param, arrayType)));

private static bool IsJaggedArrayParam(IParameterSymbol param, IArrayTypeSymbol arrayType) =>
param.IsParams
? arrayType.ElementType is IArrayTypeSymbol subType && subType.ElementType is IArrayTypeSymbol
: arrayType.ElementType is IArrayTypeSymbol;

protected abstract SyntaxToken GetIdentifier(TMethodSyntax method);
private static bool IsMultidimensionalArrayParameter(IParameterSymbol param) =>
param.Type is IArrayTypeSymbol arrayType
&& (arrayType.Rank > 1
|| IsJaggedArrayParam(param, arrayType));

public abstract ImmutableArray<TLanguageKindEnum> SyntaxKindsOfInterest { get; }
}
private static bool IsJaggedArrayParam(IParameterSymbol param, IArrayTypeSymbol arrayType) =>
param.IsParams
? arrayType.ElementType is IArrayTypeSymbol subType && subType.ElementType is IArrayTypeSymbol
: arrayType.ElementType is IArrayTypeSymbol;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,18 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.VisualBasic
{
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind, MethodStatementSyntax>
{
private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
namespace SonarAnalyzer.Rules.VisualBasic;

private static readonly ImmutableArray<SyntaxKind> kindsOfInterest = ImmutableArray.Create(SyntaxKind.SubStatement, SyntaxKind.FunctionStatement);
public override ImmutableArray<SyntaxKind> SyntaxKindsOfInterest => kindsOfInterest;
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;

protected override SyntaxToken GetIdentifier(MethodStatementSyntax method) => method.Identifier;
protected override Location GetIssueLocation(SyntaxNode node) =>
node is ConstructorBlockSyntax x
? x.SubNewStatement.NewKeyword.GetLocation()
: Language.Syntax.NodeIdentifier(node)?.GetLocation();

protected override GeneratedCodeRecognizer GeneratedCodeRecognizer => VisualBasicGeneratedCodeRecognizer.Instance;
}
protected override string GetType(SyntaxNode node) =>
node is MethodStatementSyntax ? "method" : "constructor";
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,37 @@
using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;

namespace SonarAnalyzer.UnitTest.Rules
namespace SonarAnalyzer.UnitTest.Rules;

[TestClass]
public class PublicMethodWithMultidimensionalArrayTest
{
[TestClass]
public class PublicMethodWithMultidimensionalArrayTest
{
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.PublicMethodWithMultidimensionalArray>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.PublicMethodWithMultidimensionalArray>();
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.PublicMethodWithMultidimensionalArray>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.PublicMethodWithMultidimensionalArray>();

[TestMethod]
public void PublicMethodWithMultidimensionalArray_CS() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.cs")
.Verify();
[TestMethod]
public void PublicMethodWithMultidimensionalArray_CS() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.cs")
.Verify();

#if NET

[TestMethod]
public void PublicMethodWithMultidimensionalArray_CSharp11() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp11.cs")
.WithOptions(ParseOptionsHelper.FromCSharp11)
.Verify();
[TestMethod]
public void PublicMethodWithMultidimensionalArray_CSharp11() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp11.cs")
.WithOptions(ParseOptionsHelper.FromCSharp11)
.Verify();

[TestMethod]
public void PublicMethodWithMultidimensionalArray_CSharp12() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp12.cs")
.WithOptions(ParseOptionsHelper.FromCSharp12)
.Verify();
[TestMethod]
public void PublicMethodWithMultidimensionalArray_CSharp12() =>
builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp12.cs")
.WithOptions(ParseOptionsHelper.FromCSharp12)
.Verify();

#endif

[TestMethod]
public void PublicMethodWithMultidimensionalArray_VB() =>
builderVB.AddPaths("PublicMethodWithMultidimensionalArray.vb")
.Verify();
}
[TestMethod]
public void PublicMethodWithMultidimensionalArray_VB() =>
builderVB.AddPaths("PublicMethodWithMultidimensionalArray.vb")
.Verify();
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class C5(int i); // Compliant, not a multi-dimensional array

public class Aliases(IntMatrix a) // FN
{
public Aliases(IntMatrix a, int i) : this(a) { } // FN
public Aliases(IntMatrix a, int i) : this(a) { } // Noncompliant

public void AMethod1(IntMatrix a) { } // Noncompliant
public void AMethod2(params IntMatrix a) { } // Compliant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,16 @@ namespace Repro_8083
{
public class Constructors
{
public Constructors(int[][] a) { } // FN, the ctor is publicly exposed
public Constructors(int[,] a) { } // FN
public Constructors(int[][] a) { } // Noncompliant {{Make this constructor private or simplify its parameters to not use multidimensional/jagged arrays.}}
public Constructors(int[,] a) { } // Noncompliant
public Constructors(params int[] a) { } // Compliant, params of int
public Constructors(params int[][][] a) { } // FN, params of int[][]
public Constructors(params int[][][] a) { } // Noncompliant
public Constructors(int i) { } // Compliant, not a multi-dimensional array
}
}

public static class ExtensionMethod
{
public static void Method1<T>(this T[,] dataMatrix, int a) { }
public static void Method2<T>(this T data, T[,] dataMatrix) { } // Noncompliant
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,23 @@ Namespace Tests.Diagnostics
End Sub

End Class

Public Class Constructors
Public Sub New(A As Integer()()) ' Noncompliant {{Make this constructor private or simplify its parameters to not use multidimensional/jagged arrays.}}
' ^^^
End Sub

Public Sub New(A As Integer(,)) ' Noncompliant
End Sub

Public Sub New(ParamArray A As Integer())
End Sub

Public Sub New(ParamArray A As Integer()()()) ' Noncompliant
End Sub

Public Sub New(I As Integer)
End Sub
End Class

End Namespace