-
Notifications
You must be signed in to change notification settings - Fork 222
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
New Rule T0009: Order the same member kind by its accessibility
- Loading branch information
1 parent
4e67e24
commit 5e74ac6
Showing
3 changed files
with
326 additions
and
0 deletions.
There are no files selected for viewing
111 changes: 111 additions & 0 deletions
111
analyzers/src/SonarAnalyzer.CSharp.Styling/Rules/MemberVisibilityOrdering.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
/* | ||
* SonarAnalyzer for .NET | ||
* Copyright (C) 2015-2024 SonarSource SA | ||
* mailto: contact AT sonarsource DOT com | ||
* | ||
* This program is free software; you can redistribute it and/or | ||
* modify it under the terms of the GNU Lesser General Public | ||
* License as published by the Free Software Foundation; either | ||
* version 3 of the License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
* Lesser General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Lesser General Public License | ||
* along with this program; if not, write to the Free Software Foundation, | ||
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
*/ | ||
|
||
namespace SonarAnalyzer.Rules.CSharp.Styling; | ||
|
||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
public sealed class MemberVisibilityOrdering : StylingAnalyzer | ||
{ | ||
public MemberVisibilityOrdering() : base("T0008", "Move this {0} {1} above the {2} ones.") { } | ||
|
||
protected override void Initialize(SonarAnalysisContext context) => | ||
context.RegisterNodeAction( | ||
ValidateMembers, | ||
SyntaxKind.ClassDeclaration, | ||
SyntaxKind.RecordDeclaration, | ||
SyntaxKind.RecordStructDeclaration, | ||
SyntaxKind.StructDeclaration); | ||
|
||
private void ValidateMembers(SonarSyntaxNodeReportingContext context) | ||
{ | ||
var type = (TypeDeclarationSyntax)context.Node; | ||
var members = new Dictionary<string, List<MemberInfo>>(); | ||
foreach (var member in type.Members) | ||
{ | ||
if (Category(member) is { } category && ReportingLocation(member) is { } location) | ||
{ | ||
members.GetOrAdd(category, x => []).Add(new(location, ComputeOrder(member))); | ||
} | ||
} | ||
foreach (var category in members.Keys) | ||
{ | ||
OrderInfo maxOrder = null; | ||
foreach (var member in members[category]) | ||
{ | ||
if (member.Order.Value < maxOrder?.Value) | ||
{ | ||
context.ReportIssue(Rule, member.Location, member.Order.Description, category, maxOrder.Description); | ||
} | ||
if (maxOrder is null || member.Order.Value > maxOrder.Value) | ||
{ | ||
maxOrder = member.Order; | ||
} | ||
} | ||
} | ||
} | ||
|
||
private static string Category(MemberDeclarationSyntax member) => | ||
member switch | ||
{ | ||
_ when member.Modifiers.Any(SyntaxKind.AbstractKeyword) => "Abstract Member", | ||
FieldDeclarationSyntax when member.Modifiers.Any(SyntaxKind.ConstKeyword) => "Constant", | ||
EnumDeclarationSyntax => "Enum", | ||
FieldDeclarationSyntax => "Field", | ||
DelegateDeclarationSyntax => "Delegate", | ||
EventFieldDeclarationSyntax => "Event", | ||
PropertyDeclarationSyntax => "Property", | ||
IndexerDeclarationSyntax => "Indexer", | ||
ConstructorDeclarationSyntax => "Constructor", | ||
MethodDeclarationSyntax => "Method", | ||
_ => null | ||
}; | ||
|
||
private static Location ReportingLocation(SyntaxNode node) => | ||
node switch | ||
{ | ||
EventFieldDeclarationSyntax eventField => eventField.Declaration.GetLocation(), | ||
FieldDeclarationSyntax field => field.Declaration.GetLocation(), | ||
_ => node.GetIdentifier()?.GetLocation() | ||
}; | ||
|
||
private static OrderInfo ComputeOrder(MemberDeclarationSyntax member) | ||
{ | ||
if (member.Modifiers.Any(SyntaxKind.ProtectedKeyword)) | ||
{ | ||
return new(2, "protected"); | ||
} | ||
else if (member.Modifiers.Any(SyntaxKind.PublicKeyword)) | ||
{ | ||
return new(1, "public"); | ||
} | ||
else if (member.Modifiers.Any(SyntaxKind.InternalKeyword)) | ||
{ | ||
return new(1, "internal"); | ||
} | ||
else // private or unspecified | ||
{ | ||
return new(3, "private"); | ||
} | ||
} | ||
|
||
private sealed record MemberInfo(Location Location, OrderInfo Order); | ||
|
||
private sealed record OrderInfo(int Value, string Description); | ||
} |
29 changes: 29 additions & 0 deletions
29
analyzers/tests/SonarAnalyzer.CSharp.Styling.Test/Rules/MemberVisibilityOrderingTest.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* SonarAnalyzer for .NET | ||
* Copyright (C) 2015-2024 SonarSource SA | ||
* mailto: contact AT sonarsource DOT com | ||
* | ||
* This program is free software; you can redistribute it and/or | ||
* modify it under the terms of the GNU Lesser General Public | ||
* License as published by the Free Software Foundation; either | ||
* version 3 of the License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
* Lesser General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Lesser General Public License | ||
* along with this program; if not, write to the Free Software Foundation, | ||
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | ||
*/ | ||
|
||
namespace SonarAnalyzer.CSharp.Styling.Test.Rules; | ||
|
||
[TestClass] | ||
public class MemberVisibilityOrderingTest | ||
{ | ||
[TestMethod] | ||
public void MemberVisibilityOrdering() => | ||
StylingVerifierBuilder.Create<MemberVisibilityOrdering>().AddPaths("MemberVisibilityOrdering.cs").Verify(); | ||
} |
186 changes: 186 additions & 0 deletions
186
analyzers/tests/SonarAnalyzer.CSharp.Styling.Test/TestCases/MemberVisibilityOrdering.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,186 @@ | ||
using System; | ||
|
||
public class ValidOrder | ||
{ | ||
public int publicOrInternal1; | ||
internal int publicOrInternal2; | ||
public int publicOrInternal3; | ||
internal int publicOrInternal4; | ||
|
||
protected int protectedVariant1; | ||
private protected int protectedVariant2; | ||
protected internal int protectedVariant3; | ||
protected int protectedVariant4; | ||
private protected int protectedVariant5; | ||
protected internal int protectedVariant6; | ||
|
||
private int private1; | ||
private int private2; | ||
} | ||
|
||
public class AbovePrivate | ||
{ | ||
private int private1; | ||
|
||
protected int protectedVariant1; // Noncompliant {{Move this protected Field above the private ones.}} | ||
private protected int protectedVariant2; // Noncompliant {{Move this protected Field above the private ones.}} | ||
protected internal int protectedVariant3; // Noncompliant {{Move this protected Field above the private ones.}} | ||
|
||
public int publicOrInternal1; // Noncompliant {{Move this public Field above the private ones.}} | ||
internal int publicOrInternal2; // Noncompliant {{Move this internal Field above the private ones.}} | ||
public int publicOrInternal3; // Noncompliant {{Move this public Field above the private ones.}} | ||
internal int publicOrInternal4; // Noncompliant {{Move this internal Field above the private ones.}} | ||
} | ||
|
||
public class AboveProtected | ||
{ | ||
protected int protectedVariant; | ||
|
||
public int publicOrInternal1; // Noncompliant {{Move this public Field above the protected ones.}} | ||
internal int publicOrInternal2; // Noncompliant {{Move this internal Field above the protected ones.}} | ||
} | ||
|
||
public class AboveProtectedInternal | ||
{ | ||
protected internal int protectedVariant; | ||
|
||
public int publicOrInternal1; // Noncompliant {{Move this public Field above the protected ones.}} | ||
internal int publicOrInternal2; // Noncompliant {{Move this internal Field above the protected ones.}} | ||
} | ||
|
||
public class AboveProtectedPrivate | ||
{ | ||
protected private int protectedVariant; | ||
|
||
public int publicOrInternal1; // Noncompliant {{Move this public Field above the protected ones.}} | ||
internal int publicOrInternal2; // Noncompliant {{Move this internal Field above the protected ones.}} | ||
} | ||
|
||
public abstract class CompliantClassFull | ||
{ | ||
public const string Constant1 = "C1"; | ||
private const string Constant2 = "C2"; | ||
|
||
public enum Enum1 | ||
{ | ||
None, | ||
All | ||
} | ||
|
||
private enum Enum2 | ||
{ | ||
None, | ||
Any, | ||
All | ||
} | ||
|
||
public readonly object field1 = new(); | ||
public static readonly object field2 = new(); | ||
private readonly object field3 = new(); | ||
private static readonly object field4, field5; | ||
|
||
public abstract int AbstractMethod1(); | ||
public abstract int AbstractProperty1 { get; } | ||
protected abstract int AbstractMethod2(); | ||
protected abstract int AbstractProperty2 { get; } | ||
|
||
public delegate void SomeDelegate1(); | ||
private delegate void SomeDelegate2(); | ||
|
||
public event EventHandler SomeEvent1; | ||
private event EventHandler SomeEvent2; | ||
|
||
public object Property1 { get; } = 42; | ||
private object Property2 => 42; | ||
|
||
public object this[int index] => 42; | ||
private object this[string name] | ||
{ | ||
get => 42; | ||
} | ||
|
||
public CompliantClassFull() { } | ||
private CompliantClassFull(int arg) { } | ||
|
||
~CompliantClassFull() { } // Not interesting | ||
|
||
public void Method1() { } | ||
private void Method2() { } | ||
|
||
public class Nested1 { } // Relative order of these types is not important | ||
private struct Nested2 { } | ||
public record Nested3 { } | ||
protected record struct Nested4 { } | ||
public record Nested5 { } | ||
public struct Nested6 { } | ||
public class Nested7 { } | ||
} | ||
|
||
|
||
public abstract class AllWrong | ||
{ | ||
private const string Constant1 = "C1"; | ||
public const string Constant2 = "C2"; // Noncompliant {{Move this public Constant above the private ones.}} | ||
|
||
private enum Enum1 | ||
{ | ||
None, | ||
All | ||
} | ||
|
||
public enum Enum2 // Noncompliant {{Move this public Enum above the private ones.}} | ||
{ | ||
None, | ||
Any, | ||
All | ||
} | ||
|
||
private readonly object field1 = new(); | ||
private static readonly object field2 = new(); | ||
public readonly object field3 = new(); // Noncompliant {{Move this public Field above the private ones.}} | ||
public static readonly object field4, field5; // Noncompliant {{Move this public Field above the private ones.}} | ||
|
||
protected abstract int AbstractMethod1(); | ||
protected abstract int AbstractProperty1 { get; } | ||
public abstract int AbstractMethod2(); // Noncompliant {{Move this public Abstract Member above the protected ones.}} | ||
public abstract int AbstractProperty2 { get; } // Noncompliant {{Move this public Abstract Member above the protected ones.}} | ||
|
||
private delegate void SomeDelegate1(); | ||
public delegate void SomeDelegate2(); // Noncompliant {{Move this public Delegate above the private ones.}} | ||
|
||
private event EventHandler SomeEvent1; | ||
public event EventHandler SomeEvent2; // Noncompliant {{Move this public Event above the private ones.}} | ||
|
||
private object Property1 { get; } = 42; | ||
public object Property2 => 42; // Noncompliant {{Move this public Property above the private ones.}} | ||
|
||
private object this[int index] => 42; | ||
public object this[string name] // Noncompliant {{Move this public Indexer above the private ones.}} | ||
{ | ||
get => 42; | ||
} | ||
|
||
private AllWrong() { } | ||
public AllWrong(int arg) { } // Noncompliant {{Move this public Constructor above the private ones.}} | ||
|
||
private void Method1() { } | ||
public void Method2() { } // Noncompliant {{Move this public Method above the private ones.}} | ||
} | ||
|
||
public record R | ||
{ | ||
private int shouldBeLast; | ||
public int shouldBeFirst; // Noncompliant {{Move this public Field above the private ones.}} | ||
} | ||
|
||
public record struct RS | ||
{ | ||
private int shouldBeLast; | ||
public int shouldBeFirst; // Noncompliant {{Move this public Field above the private ones.}} | ||
} | ||
|
||
public record struct S | ||
{ | ||
private int shouldBeLast; | ||
public int shouldBeFirst; // Noncompliant {{Move this public Field above the private ones.}} | ||
} |