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

S2436: Add support for record structs #5611

Merged
merged 24 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8fb4817
Fix tests
martin-strecker-sonarsource Apr 29, 2022
0c7d364
Fix C# 10 test
martin-strecker-sonarsource Apr 30, 2022
0001646
Add support for record struct to GetDeclarationTypeName
martin-strecker-sonarsource Apr 30, 2022
4d64608
Fix test call for C# 10
martin-strecker-sonarsource Apr 30, 2022
a9c78e0
Remove duplication and simplifiy GetEnclosingTypeName
martin-strecker-sonarsource Apr 30, 2022
136d2ca
Use IsRedundantPositionalRecordContext
martin-strecker-sonarsource Apr 30, 2022
bfbdca3
Simplify GetEnclosingTypeName
martin-strecker-sonarsource Apr 30, 2022
5c645cf
Allign wrapped arguments correctly.
martin-strecker-sonarsource Apr 30, 2022
154c11a
Fix error message for top levelstatement functions
martin-strecker-sonarsource Apr 30, 2022
278d630
Update ITs (change in the message only)
martin-strecker-sonarsource May 2, 2022
93ed614
Change JoinStr to filter empty/whitespace only text
martin-strecker-sonarsource May 3, 2022
3401884
Adopt whitespace test
martin-strecker-sonarsource May 3, 2022
9e8cadd
replace JoinIfNotWhitespace with call to JoinStr
martin-strecker-sonarsource May 3, 2022
66a8fea
Add test case
martin-strecker-sonarsource May 3, 2022
945d554
Rename test file
martin-strecker-sonarsource May 3, 2022
89c0108
Adopt test
martin-strecker-sonarsource May 3, 2022
3288583
Revert "Change JoinStr to filter empty/whitespace only text"
martin-strecker-sonarsource May 3, 2022
caf8e86
Rename and cleanup after revert
martin-strecker-sonarsource May 3, 2022
a3bf5cd
Rename
martin-strecker-sonarsource May 3, 2022
8c5f740
Re-factor
martin-strecker-sonarsource May 3, 2022
c4692e9
Fix failing test
martin-strecker-sonarsource May 3, 2022
ed1f940
Rename file
martin-strecker-sonarsource May 4, 2022
9672e34
Formatting and naming. Inline TypeKinds
martin-strecker-sonarsource May 4, 2022
3cc25a4
Remove filescoped namespaces
martin-strecker-sonarsource May 4, 2022
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
2 changes: 1 addition & 1 deletion analyzers/its/expected/Net5/Net5--net5.0-S2436.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"issues": [
{
"id": "S2436",
"message": "Reduce the number of generic parameters in the '.LocalBar' method to no more than the 3 authorized.",
"message": "Reduce the number of generic parameters in the 'LocalBar' method to no more than the 3 authorized.",
"location": {
"uri": "sources\Net5\Net5\Main.cs",
"region": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public static string GetDeclarationTypeName(this SyntaxNode node) =>
SyntaxKind.StructDeclaration => "struct",
SyntaxKind.InterfaceDeclaration => "interface",
SyntaxKindEx.RecordClassDeclaration => "record",
SyntaxKindEx.RecordStructDeclaration => "record struct",
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
_ => GetUnknownType(node.Kind())
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -56,20 +57,24 @@ protected override void Initialize(ParameterLoadingAnalysisContext context)
{
var typeDeclaration = (TypeDeclarationSyntax)c.Node;

if (c.ContainingSymbol.Kind != SymbolKind.NamedType
if (c.IsRedundantPositionalRecordContext()
|| typeDeclaration.TypeParameterList == null
|| typeDeclaration.TypeParameterList.Parameters.Count <= MaxNumberOfGenericParametersInClass)
{
return;
}

c.ReportIssue(Diagnostic.Create(Rule, typeDeclaration.Identifier.GetLocation(),
typeDeclaration.Identifier.ValueText, typeDeclaration.GetDeclarationTypeName(), MaxNumberOfGenericParametersInClass));
c.ReportIssue(Diagnostic.Create(Rule,
typeDeclaration.Identifier.GetLocation(),
typeDeclaration.Identifier.ValueText,
typeDeclaration.GetDeclarationTypeName(),
MaxNumberOfGenericParametersInClass));
},
SyntaxKind.ClassDeclaration,
SyntaxKind.StructDeclaration,
SyntaxKind.InterfaceDeclaration,
SyntaxKindEx.RecordClassDeclaration);
SyntaxKindEx.RecordClassDeclaration,
SyntaxKindEx.RecordStructDeclaration);

context.RegisterSyntaxNodeActionInNonGenerated(
c =>
Expand All @@ -81,43 +86,17 @@ protected override void Initialize(ParameterLoadingAnalysisContext context)
return;
}

c.ReportIssue(Diagnostic.Create(
Rule,
methodDeclaration.Identifier.GetLocation(),
$"{GetEnclosingTypeName(c.Node)}.{methodDeclaration.Identifier.ValueText}",
"method",
MaxNumberOfGenericParametersInMethod));
c.ReportIssue(Diagnostic.Create(Rule,
methodDeclaration.Identifier.GetLocation(),
new[] { EnclosingTypeName(c.Node), methodDeclaration.Identifier.ValueText }.JoinNonEmpty("."),
"method",
MaxNumberOfGenericParametersInMethod));
},
SyntaxKind.MethodDeclaration,
SyntaxKindEx.LocalFunctionStatement);
}

private static string GetEnclosingTypeName(SyntaxNode node)
{
var parent = node.Parent;

while (parent != null)
{
switch (parent.Kind())
{
case SyntaxKind.ClassDeclaration:
return ((ClassDeclarationSyntax)parent).Identifier.ValueText;

case SyntaxKind.StructDeclaration:
return ((StructDeclarationSyntax)parent).Identifier.ValueText;

case SyntaxKind.InterfaceDeclaration:
return ((InterfaceDeclarationSyntax)parent).Identifier.ValueText;

case SyntaxKindEx.RecordClassDeclaration:
return ((RecordDeclarationSyntaxWrapper)parent).Identifier.ValueText;

default:
parent = parent.Parent;
break;
}
}
return null;
}
private static string EnclosingTypeName(SyntaxNode node) =>
node.Ancestors().OfType<BaseTypeDeclarationSyntax>().FirstOrDefault()?.Identifier.ValueText;
}
}
17 changes: 12 additions & 5 deletions analyzers/src/SonarAnalyzer.Common/Helpers/EnumerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,29 +118,36 @@ public static int IndexOf<T>(this IEnumerable<T> enumerable, Func<T, bool> predi
?.index ?? -1;

/// <summary>
/// This is string.Join() as extension. Concatenates members of collection using specified separator between each member. Selector is used to extract string value from T for concatenation.
/// This is <see cref="string.Join"/> as extension. It concatenates the members of the collection using the specified <paramref name="separator"/> between each member.
/// <paramref name="selector"/> is used to convert <typeparamref name="T"/> to <see cref="string"/> for concatenation.
/// </summary>
public static string JoinStr<T>(this IEnumerable<T> enumerable, string separator, Func<T, string> selector) =>
string.Join(separator, enumerable.Select(x => selector(x)));

/// <summary>
/// This is string.Join() as extension. Concatenates members of collection using specified separator between each member. Selector is used to extract integer value from T for concatenation.
/// This is <see cref="string.Join"/> as extension. It concatenates the members of the collection using the specified <paramref name="separator"/> between each member.
/// <paramref name="selector"/> is used to convert <typeparamref name="T"/> to <see cref="int"/> for concatenation.
/// </summary>
public static string JoinStr<T>(this IEnumerable<T> enumerable, string separator, Func<T, int> selector) =>
string.Join(separator, enumerable.Select(x => selector(x)));

/// <summary>
/// This is string.Join() as extension. Concatenates members of string collection using specified separator between each member.
/// This is <see cref="string.Join"/> as extension. It concatenates the members of the collection using the specified <paramref name="separator"/> between each member.
/// </summary>
public static string JoinStr(this IEnumerable<string> enumerable, string separator) =>
JoinStr(enumerable, separator, x => x);

/// <summary>
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
/// This is string.Join() as extension. Concatenates members of int collection using specified separator between each member.
/// This is <see cref="string.Join"/> as extension. It concatenates the members of the collection using the specified <paramref name="separator"/> between each member.
/// </summary>
public static string JoinStr(this IEnumerable<int> enumerable, string separator) =>
JoinStr(enumerable, separator, x => x);


/// <summary>
/// Concatenates the members of a <see cref="string"/> collection using the specified <paramref name="separator"/> between each member.
/// Any whitespace or null member of the collection will be ignored.
/// </summary>
public static string JoinNonEmpty(this IEnumerable<string> enumerable, string separator) =>
string.Join(separator, enumerable.Where(x => !string.IsNullOrWhiteSpace(x)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

#pragma warning disable SA1122 // Use string.Empty for empty strings

using System;
using System.Collections.Generic;
using FluentAssertions;
Expand Down Expand Up @@ -131,14 +133,31 @@ public void JoinStr_Int()
new[] { 1, 22, 333 }.JoinStr(null).Should().Be("122333");
}

[TestMethod]
public void JoinNonEmpty()
{
Array.Empty<string>().JoinNonEmpty(", ").Should().Be("");
new[] { "a" }.JoinNonEmpty(", ").Should().Be("a");
new[] { "a", "bb", "ccc" }.JoinNonEmpty(", ").Should().Be("a, bb, ccc");
new[] { "a", "bb", "ccc" }.JoinNonEmpty(null).Should().Be("abbccc");
new[] { "a", "bb", "ccc" }.JoinNonEmpty("").Should().Be("abbccc");
new[] { null, "a", "b" }.JoinNonEmpty(".").Should().Be("a.b");
new[] { "a", null, "b" }.JoinNonEmpty(".").Should().Be("a.b");
new[] { "a", "b", null }.JoinNonEmpty(".").Should().Be("a.b");
new string[] { null, null, null }.JoinNonEmpty(".").Should().Be("");
new string[] { "", "", "" }.JoinNonEmpty(".").Should().Be("");
new string[] { "", "\t", " " }.JoinNonEmpty(".").Should().Be("");
new string[] { "a", "\t", "b" }.JoinNonEmpty(".").Should().Be("a.b");
}

[TestMethod]
public void WhereNotNull_Class()
{
var instance = new Object();
Array.Empty<object>().WhereNotNull().Should().BeEmpty();
new object[] { null, null, null }.WhereNotNull().Should().BeEmpty();
new object[] { 1, "a", instance }.WhereNotNull().Should().BeEquivalentTo(new object[] { 1, "a", instance });
new object[] { 1, "a", null }.WhereNotNull().Should().BeEquivalentTo(new object[] { 1, "a"});
new object[] { 1, "a", null }.WhereNotNull().Should().BeEquivalentTo(new object[] { 1, "a" });
}

[TestMethod]
Expand All @@ -162,3 +181,5 @@ public StructType(int count)
}
}
}

#pragma warning restore SA1122 // Use string.Empty for empty strings
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,27 @@ namespace SonarAnalyzer.UnitTest.Rules
[TestClass]
public class TooManyGenericParametersTest
{
private readonly VerifierBuilder builder = new VerifierBuilder<TooManyGenericParameters>();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

[TestMethod]
public void TooManyGenericParameters_DefaultValues() =>
OldVerifier.VerifyAnalyzer(@"TestCases\TooManyGenericParameters_DefaultValues.cs", new TooManyGenericParameters());
builder.AddPaths("TooManyGenericParameters.DefaultValues.cs").Verify();

[TestMethod]
public void TooManyGenericParameters_CustomValues() =>
OldVerifier.VerifyAnalyzer(@"TestCases\TooManyGenericParameters_CustomValues.cs",
new TooManyGenericParameters { MaxNumberOfGenericParametersInClass = 4, MaxNumberOfGenericParametersInMethod = 4 });
new VerifierBuilder()
.AddAnalyzer(() => new TooManyGenericParameters { MaxNumberOfGenericParametersInClass = 4, MaxNumberOfGenericParametersInMethod = 4 })
.AddPaths("TooManyGenericParameters.CustomValues.cs")
.Verify();

#if NET
[TestMethod]
public void TooManyGenericParameters_CSharp9() =>
OldVerifier.VerifyAnalyzerFromCSharp9Console(@"TestCases\TooManyGenericParameters.CSharp9.cs", new TooManyGenericParameters());
builder.AddPaths("TooManyGenericParameters.CSharp9.cs").WithTopLevelStatements().Verify();

[TestMethod]
public void TooManyGenericParameters_CSharp10() =>
OldVerifier.VerifyAnalyzerFromCSharp10Console(@"TestCases\TooManyGenericParameters.CSharp10.cs", new TooManyGenericParameters());
builder.AddPaths("TooManyGenericParameters.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify();
#endif
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
void LocalFoo<T1, T2, T3>() { }
void LocalBar<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the '.LocalBar' method to no more than the 3 authorized.}}

record struct RecordStruct
record struct RecordStruct
{
public void Foo<T1, T2, T3>() { }
public void Foo<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the '.Foo' method to no more than the 3 authorized.}}
// Although an issue is raised, the above message is not correct. There should be 'RecordStruct.Foo' instead of '.Foo'.
public void Foo<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the 'RecordStruct.Foo' method to no more than the 3 authorized.}}
// ^^^
}

record struct PositionalRecordStruct(int SomeProperty)
{
public void Foo<T1, T2, T3>() { }
public void Foo<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the '.Foo' method to no more than the 3 authorized.}}
// Although an issue is raised, the above message is not correct. There should be 'PositionalRecordStruct.Foo' instead of '.Foo'.
public void Foo<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the 'PositionalRecordStruct.Foo' method to no more than the 3 authorized.}}
}

record struct RecordStruct<T1, T2> { }
record struct RecordStruct<T1, T2, T3> { } // FN
record struct RecordStruct<T1, T2, T3> { } // Noncompliant {{Reduce the number of generic parameters in the 'RecordStruct' record struct to no more than the 2 authorized.}}
// ^^^^^^^^^^^^

record struct PositionalRecordStruct<T1, T2>(int SomeProperty) { }
record struct PositionalRecordStruct<T1, T2, T3>(int SomeProperty) { } // FN
record struct PositionalRecordStruct<T1, T2, T3>(int SomeProperty) { } // Noncompliant {{Reduce the number of generic parameters in the 'PositionalRecordStruct' record struct to no more than the 2 authorized.}}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
void LocalFoo<T1, T2, T3>() { }
void LocalBar<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the '.LocalBar' method to no more than the 3 authorized.}}
void LocalBar<T1, T2, T3, T4>() { } // Noncompliant {{Reduce the number of generic parameters in the 'LocalBar' method to no more than the 3 authorized.}}

record Record
{
Expand Down