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

S3059: Add support for record structs #5612

Merged
merged 8 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
32 changes: 22 additions & 10 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/TypeMemberVisibility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand All @@ -37,59 +38,70 @@ public sealed class TypeMemberVisibility : SonarDiagnosticAnalyzer
private const string MessageFormat = "Types should not have members with visibility set higher than the type's visibility";

private static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorBuilder.GetDescriptor(DiagnosticId, MessageFormat, RspecStrings.ResourceManager);
private static readonly SyntaxKind[] TypeKinds = { SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration, SyntaxKind.EnumDeclaration, SyntaxKindEx.RecordClassDeclaration };
private static readonly SyntaxKind[] TypeKinds =
{
SyntaxKind.ClassDeclaration,
SyntaxKind.EnumDeclaration,
SyntaxKindEx.RecordClassDeclaration,
SyntaxKindEx.RecordStructDeclaration,
SyntaxKind.StructDeclaration,
};
private static readonly SyntaxKind[] MemberDeclarationKinds =
{
SyntaxKind.ClassDeclaration,
SyntaxKind.ConstructorDeclaration,
SyntaxKind.DelegateDeclaration,
SyntaxKind.EnumDeclaration,
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
SyntaxKind.EventDeclaration,
SyntaxKind.EventFieldDeclaration,
SyntaxKind.EnumDeclaration,
SyntaxKind.FieldDeclaration,
SyntaxKind.IndexerDeclaration,
SyntaxKind.InterfaceDeclaration,
SyntaxKind.MethodDeclaration,
SyntaxKind.PropertyDeclaration,
SyntaxKindEx.RecordClassDeclaration,
SyntaxKind.StructDeclaration
SyntaxKindEx.RecordStructDeclaration,
SyntaxKind.StructDeclaration,
};

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterSyntaxNodeActionInNonGenerated(c =>
{
if (c.IsRedundantPositionalRecordContext())
{
return;
}
var typeDeclaration = (BaseTypeDeclarationSyntax)c.Node;
var secondaryLocations = GetInvalidMemberLocations(typeDeclaration, c.SemanticModel);
if (c.ContainingSymbol.Kind == SymbolKind.NamedType
&& secondaryLocations.Any())
var secondaryLocations = GetInvalidMemberLocations(c.SemanticModel, typeDeclaration);
if (secondaryLocations.Any())
{
c.ReportIssue(Diagnostic.Create(Rule, typeDeclaration.Identifier.GetLocation(), secondaryLocations));
}
},
TypeKinds);

private static List<Location> GetInvalidMemberLocations(BaseTypeDeclarationSyntax type, SemanticModel semanticModel)
private static List<Location> GetInvalidMemberLocations(SemanticModel semanticModel, BaseTypeDeclarationSyntax type)
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
{
var parentType = GetParentType(type);
if (parentType == null && type.Modifiers.AnyOfKind(SyntaxKind.InternalKeyword))
if (parentType is null && type.Modifiers.AnyOfKind(SyntaxKind.InternalKeyword))
{
return type.DescendantNodes()
.Where(node => node.IsAnyKind(MemberDeclarationKinds))
.OfType<MemberDeclarationSyntax>()
// We skip overridden methods since they need to keep the public visibility (if present)
.Where(declaration => declaration.Modifiers().AnyOfKind(SyntaxKind.PublicKeyword)
&& !declaration.Modifiers().AnyOfKind(SyntaxKind.OverrideKeyword)
&& !IsInterfaceImplementation(declaration, semanticModel))
&& !IsInterfaceImplementation(semanticModel, declaration))
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
.Select(declaration => declaration.Modifiers().Single(modifier => modifier.IsKind(SyntaxKind.PublicKeyword)).GetLocation())
.ToList();
}

return new List<Location>();
}

private static bool IsInterfaceImplementation(MemberDeclarationSyntax declaration, SemanticModel semanticModel) =>
private static bool IsInterfaceImplementation(SemanticModel semanticModel, MemberDeclarationSyntax declaration) =>
semanticModel.GetDeclaredSymbol(declaration)?.GetInterfaceMember() != null;

private static BaseTypeDeclarationSyntax GetParentType(SyntaxNode node) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,27 @@ namespace SonarAnalyzer.UnitTest.Rules
[TestClass]
public class TypeMemberVisibilityTest
{
private static readonly VerifierBuilder Builder = new VerifierBuilder<TypeMemberVisibility>();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

[TestMethod]
public void TypeMemberVisibility_CS() =>
OldVerifier.VerifyAnalyzer(@"TestCases\TypeMemberVisibility.cs", new TypeMemberVisibility());
Builder.AddPaths("TypeMemberVisibility.cs").Verify();

#if NET

[TestMethod]
public void TypeMemberVisibility_CSharp9() =>
OldVerifier.VerifyAnalyzerFromCSharp9Library(@"TestCases\TypeMemberVisibility.CSharp9.cs", new TypeMemberVisibility());
Builder.AddPaths("TypeMemberVisibility.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify();

[TestMethod]
public void TypeMemberVisibility_CSharp10() =>
OldVerifier.VerifyAnalyzerFromCSharp10Library(@"TestCases\TypeMemberVisibility.CSharp10.cs", new TypeMemberVisibility());
Builder.AddPaths("TypeMemberVisibility.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify();

[TestMethod]
public void TypeMemberVisibility_CSharpPreview() =>
OldVerifier.VerifyAnalyzerCSharpPreviewLibrary(@"TestCases\TypeMemberVisibility.CSharpPreview.cs", new TypeMemberVisibility());
Builder.AddPaths("TypeMemberVisibility.CSharpPreview.cs").WithOptions(ParseOptionsHelper.CSharpPreview).Verify();

#endif

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@

namespace Tests.Diagnostics
{
internal record struct Noncompliant // FN
internal record struct Noncompliant // Noncompliant {{Types should not have members with visibility set higher than the type's visibility}}
// ^^^^^^^^^^^^
{
public static decimal A = 3.14m;
static public decimal A = 3.14m; // Secondary
// ^^^^^^
private decimal E = 1m;

public int PropertyA { get; }
public int PropertyA { get; } // Secondary
private int PropertyE { get; }

public int GetA() => 1;
public int GetA() => 1; // Secondary
private int GetE() => 1;
}

internal record struct NoncompliantPositionalRecord(string Property) // FN
internal record struct NoncompliantPositionalRecord(string Property) // Noncompliant
{
public static decimal A = 3.14m;
public static decimal A = 3.14m; // Secondary
}
}