diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/ElementOrderCodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/ElementOrderCodeFixProvider.cs index fed1d807a..69e4c98c7 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/ElementOrderCodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/ElementOrderCodeFixProvider.cs @@ -15,6 +15,7 @@ namespace StyleCop.Analyzers.OrderingRules using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; + using Settings.ObjectModel; /// /// Implements code fixes for element ordering rules. @@ -30,8 +31,7 @@ internal class ElementOrderCodeFixProvider : CodeFixProvider SA1202ElementsMustBeOrderedByAccess.DiagnosticId, SA1203ConstantsMustAppearBeforeFields.DiagnosticId, SA1204StaticElementsMustAppearBeforeInstanceElements.DiagnosticId, - SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.DiagnosticId, - SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.DiagnosticId); + SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.DiagnosticId); /// public override FixAllProvider GetFixAllProvider() @@ -57,7 +57,8 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken) { - var orderingChecks = await GetEnabledRulesForDocumentAsync(document, cancellationToken).ConfigureAwait(false); + var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, cancellationToken); + var elementOrder = settings.OrderingRules.ElementOrder; var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var indentationOptions = IndentationOptions.FromDocument(document); @@ -67,68 +68,57 @@ private static async Task GetTransformedDocumentAsync(Document documen return document; } - syntaxRoot = UpdateSyntaxRoot(memberDeclaration, orderingChecks, syntaxRoot, indentationOptions); + syntaxRoot = UpdateSyntaxRoot(memberDeclaration, elementOrder, syntaxRoot, indentationOptions); return document.WithSyntaxRoot(syntaxRoot); } - private static async Task GetEnabledRulesForDocumentAsync(Document document, CancellationToken cancellationToken) - { - SemanticModel semanticModel; - if (!document.TryGetSemanticModel(out semanticModel)) - { - semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - } - - return ElementOrderingChecks.GetElementOrderingChecksForSemanticModel(semanticModel); - } - - private static SyntaxNode UpdateSyntaxRoot(MemberDeclarationSyntax memberDeclaration, ElementOrderingChecks checks, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) + private static SyntaxNode UpdateSyntaxRoot(MemberDeclarationSyntax memberDeclaration, ImmutableArray elementOrder, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) { var parentDeclaration = memberDeclaration.Parent; - var memberToMove = new MemberOrderHelper(memberDeclaration, checks); + var memberToMove = new MemberOrderHelper(memberDeclaration, elementOrder); if (parentDeclaration is TypeDeclarationSyntax) { - return HandleTypeDeclaration(memberToMove, (TypeDeclarationSyntax)parentDeclaration, checks, syntaxRoot, indentationOptions); + return HandleTypeDeclaration(memberToMove, (TypeDeclarationSyntax)parentDeclaration, elementOrder, syntaxRoot, indentationOptions); } if (parentDeclaration is NamespaceDeclarationSyntax) { - return HandleNamespaceDeclaration(memberToMove, (NamespaceDeclarationSyntax)parentDeclaration, checks, syntaxRoot, indentationOptions); + return HandleNamespaceDeclaration(memberToMove, (NamespaceDeclarationSyntax)parentDeclaration, elementOrder, syntaxRoot, indentationOptions); } if (parentDeclaration is CompilationUnitSyntax) { - return HandleCompilationUnitDeclaration(memberToMove, (CompilationUnitSyntax)parentDeclaration, checks, syntaxRoot, indentationOptions); + return HandleCompilationUnitDeclaration(memberToMove, (CompilationUnitSyntax)parentDeclaration, elementOrder, syntaxRoot, indentationOptions); } return syntaxRoot; } - private static SyntaxNode HandleTypeDeclaration(MemberOrderHelper memberOrder, TypeDeclarationSyntax typeDeclarationNode, ElementOrderingChecks checks, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) + private static SyntaxNode HandleTypeDeclaration(MemberOrderHelper memberOrder, TypeDeclarationSyntax typeDeclarationNode, ImmutableArray elementOrder, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) { - return OrderMember(memberOrder, typeDeclarationNode.Members, checks, syntaxRoot, indentationOptions); + return OrderMember(memberOrder, typeDeclarationNode.Members, elementOrder, syntaxRoot, indentationOptions); } - private static SyntaxNode HandleCompilationUnitDeclaration(MemberOrderHelper memberOrder, CompilationUnitSyntax compilationUnitDeclaration, ElementOrderingChecks checks, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) + private static SyntaxNode HandleCompilationUnitDeclaration(MemberOrderHelper memberOrder, CompilationUnitSyntax compilationUnitDeclaration, ImmutableArray elementOrder, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) { - return OrderMember(memberOrder, compilationUnitDeclaration.Members, checks, syntaxRoot, indentationOptions); + return OrderMember(memberOrder, compilationUnitDeclaration.Members, elementOrder, syntaxRoot, indentationOptions); } - private static SyntaxNode HandleNamespaceDeclaration(MemberOrderHelper memberOrder, NamespaceDeclarationSyntax namespaceDeclaration, ElementOrderingChecks checks, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) + private static SyntaxNode HandleNamespaceDeclaration(MemberOrderHelper memberOrder, NamespaceDeclarationSyntax namespaceDeclaration, ImmutableArray elementOrder, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) { - return OrderMember(memberOrder, namespaceDeclaration.Members, checks, syntaxRoot, indentationOptions); + return OrderMember(memberOrder, namespaceDeclaration.Members, elementOrder, syntaxRoot, indentationOptions); } - private static SyntaxNode OrderMember(MemberOrderHelper memberOrder, SyntaxList members, ElementOrderingChecks checks, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) + private static SyntaxNode OrderMember(MemberOrderHelper memberOrder, SyntaxList members, ImmutableArray elementOrder, SyntaxNode syntaxRoot, IndentationOptions indentationOptions) { var memberIndex = members.IndexOf(memberOrder.Member); MemberOrderHelper target = default(MemberOrderHelper); for (var i = memberIndex - 1; i >= 0; --i) { - var orderHelper = new MemberOrderHelper(members[i], checks); + var orderHelper = new MemberOrderHelper(members[i], elementOrder); if (orderHelper.Priority < memberOrder.Priority) { target = orderHelper; @@ -276,7 +266,8 @@ protected override async Task FixAllInDocumentAsync(FixAllContext fi } var indentationOptions = IndentationOptions.FromDocument(document); - var orderingChecks = await GetEnabledRulesForDocumentAsync(document, fixAllContext.CancellationToken).ConfigureAwait(false); + var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, fixAllContext.CancellationToken); + var elementOrder = settings.OrderingRules.ElementOrder; var syntaxRoot = await document.GetSyntaxRootAsync().ConfigureAwait(false); var trackedDiagnosticMembers = new List(); @@ -296,7 +287,7 @@ protected override async Task FixAllInDocumentAsync(FixAllContext fi foreach (var member in trackedDiagnosticMembers) { var memberDeclaration = syntaxRoot.GetCurrentNode(member); - syntaxRoot = UpdateSyntaxRoot(memberDeclaration, orderingChecks, syntaxRoot, indentationOptions); + syntaxRoot = UpdateSyntaxRoot(memberDeclaration, elementOrder, syntaxRoot, indentationOptions); } return syntaxRoot; diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1202UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1202UnitTests.cs index 5cd6632a7..fc40d30a1 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1202UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1202UnitTests.cs @@ -112,7 +112,7 @@ public async Task TestClassOrderingAsync() public class TestClass2 { } "; - var expected = this.CSharpDiagnostic().WithLocation(2, 14).WithArguments("public", "classes", "internal"); + var expected = this.CSharpDiagnostic().WithLocation(2, 14).WithArguments("public", "internal"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -157,10 +157,10 @@ public async Task TestPropertiesAsync() DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 22).WithArguments("protected", "properties", "private"), - this.CSharpDiagnostic().WithLocation(5, 31).WithArguments("protected internal", "properties", "protected"), - this.CSharpDiagnostic().WithLocation(6, 21).WithArguments("internal", "properties", "protected internal"), - this.CSharpDiagnostic().WithLocation(7, 19).WithArguments("public", "properties", "internal") + this.CSharpDiagnostic().WithLocation(4, 22).WithArguments("protected", "private"), + this.CSharpDiagnostic().WithLocation(5, 31).WithArguments("protected internal", "protected"), + this.CSharpDiagnostic().WithLocation(6, 21).WithArguments("internal", "protected internal"), + this.CSharpDiagnostic().WithLocation(7, 19).WithArguments("public", "internal") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -198,10 +198,10 @@ public void TestMethod5() { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 20).WithArguments("protected", "methods", "private"), - this.CSharpDiagnostic().WithLocation(5, 29).WithArguments("protected internal", "methods", "protected"), - this.CSharpDiagnostic().WithLocation(6, 19).WithArguments("internal", "methods", "protected internal"), - this.CSharpDiagnostic().WithLocation(7, 17).WithArguments("public", "methods", "internal") + this.CSharpDiagnostic().WithLocation(4, 20).WithArguments("protected", "private"), + this.CSharpDiagnostic().WithLocation(5, 29).WithArguments("protected internal", "protected"), + this.CSharpDiagnostic().WithLocation(6, 19).WithArguments("internal", "protected internal"), + this.CSharpDiagnostic().WithLocation(7, 17).WithArguments("public", "internal") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -234,7 +234,7 @@ public async Task TestProtectedInternalBeforePublicAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 5).WithArguments("public", "events", "protected internal"); + var expected = this.CSharpDiagnostic().WithLocation(4, 5).WithArguments("public", "protected internal"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -263,7 +263,7 @@ public async Task TestProtectedBeforePublicAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "fields", "protected"); + var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "protected"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -291,7 +291,7 @@ public event System.Action TestEvent2 { add { } remove { } } } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 32).WithArguments("public", "events", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 32).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -320,7 +320,7 @@ internal event System.Action TestEvent2 { add { } remove { } } } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 34).WithArguments("internal", "events", "protected"); + var expected = this.CSharpDiagnostic().WithLocation(4, 34).WithArguments("internal", "protected"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -349,7 +349,7 @@ public async Task TestPrivateBeforeInternalAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 28).WithArguments("internal", "delegates", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 28).WithArguments("internal", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -378,7 +378,7 @@ protected internal void TestMethod2() { } } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 29).WithArguments("protected internal", "methods", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 29).WithArguments("protected internal", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -407,7 +407,7 @@ internal protected void TestMethod2() { } } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 29).WithArguments("protected internal", "methods", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 29).WithArguments("protected internal", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -437,7 +437,7 @@ public async Task TestMembersWithoutAccessModifiersAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "fields", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -464,7 +464,7 @@ public async Task TestClassesWithoutAccessModifiersAsync() public class TestClass2 { } "; - var expected = this.CSharpDiagnostic().WithLocation(2, 14).WithArguments("public", "classes", "internal"); + var expected = this.CSharpDiagnostic().WithLocation(2, 14).WithArguments("public", "internal"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -491,7 +491,7 @@ public async Task TestOnlyFirstViolationReportedAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "fields", "private"); + var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -526,7 +526,7 @@ public void B() } "; - var expected = this.CSharpDiagnostic().WithLocation(7, 17).WithArguments("public", "methods", "private"); + var expected = this.CSharpDiagnostic().WithLocation(7, 17).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -565,10 +565,10 @@ public static void TestMethod5() { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 27).WithArguments("protected", "methods", "private"), - this.CSharpDiagnostic().WithLocation(5, 36).WithArguments("protected internal", "methods", "protected"), - this.CSharpDiagnostic().WithLocation(6, 26).WithArguments("internal", "methods", "protected internal"), - this.CSharpDiagnostic().WithLocation(7, 24).WithArguments("public", "methods", "internal") + this.CSharpDiagnostic().WithLocation(4, 27).WithArguments("protected", "private"), + this.CSharpDiagnostic().WithLocation(5, 36).WithArguments("protected internal", "protected"), + this.CSharpDiagnostic().WithLocation(6, 26).WithArguments("internal", "protected internal"), + this.CSharpDiagnostic().WithLocation(7, 24).WithArguments("public", "internal") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -604,8 +604,8 @@ public async Task TestConstOrderingAsync() DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("protected", "fields", "private"), - this.CSharpDiagnostic().WithLocation(5, 16).WithArguments("public", "fields", "protected") + this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("protected", "private"), + this.CSharpDiagnostic().WithLocation(5, 16).WithArguments("public", "protected") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -637,7 +637,7 @@ internal class TestClass3 { } } "; - var expected = this.CSharpDiagnostic().WithLocation(5, 20).WithArguments("internal", "classes", "private"); + var expected = this.CSharpDiagnostic().WithLocation(5, 20).WithArguments("internal", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -671,8 +671,8 @@ public class TestClass2 { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 17).WithArguments("public", "enums", "internal"), - this.CSharpDiagnostic().WithLocation(6, 18).WithArguments("public", "classes", "internal") + this.CSharpDiagnostic().WithLocation(4, 17).WithArguments("public", "internal"), + this.CSharpDiagnostic().WithLocation(6, 18).WithArguments("public", "internal") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -733,7 +733,7 @@ public MyClass(int a) } "; - var expected = this.CSharpDiagnostic().WithLocation(8, 12).WithArguments("public", "constructors", "private"); + var expected = this.CSharpDiagnostic().WithLocation(8, 12).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -812,9 +812,9 @@ void IA.TestMethod1() { } DiagnosticResult[] expected = { // explicit interface events are considered private by StyleCop - this.CSharpDiagnostic().WithLocation(34, 12).WithArguments("public", "properties", "protected"), - this.CSharpDiagnostic().WithLocation(41, 15).WithArguments("public", "indexers", "protected"), - this.CSharpDiagnostic().WithLocation(45, 13).WithArguments("public", "methods", "protected") + this.CSharpDiagnostic().WithLocation(34, 12).WithArguments("public", "protected"), + this.CSharpDiagnostic().WithLocation(41, 15).WithArguments("public", "protected"), + this.CSharpDiagnostic().WithLocation(45, 13).WithArguments("public", "protected") }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -892,7 +892,7 @@ void TestInterface.SomeMethod() } "; - var expected = this.CSharpDiagnostic().WithLocation(13, 24).WithArguments("public", "methods", "private"); + var expected = this.CSharpDiagnostic().WithLocation(13, 24).WithArguments("public", "private"); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1203UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1203UnitTests.cs index 02eda551f..651ba9f1e 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1203UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1203UnitTests.cs @@ -14,7 +14,7 @@ namespace StyleCop.Analyzers.Test.OrderingRules public class SA1203UnitTests : CodeFixVerifier { - private bool suppressSA1202 = false; + private bool orderByAccess = true; [Fact] public async Task TestNoDiagnosticAsync() @@ -101,7 +101,7 @@ public class Foo private int Baz = 1; private const int Bar = 2; }"; - var firstDiagnostic = this.CSharpDiagnostic().WithLocation(5, 23).WithArguments("private"); + var firstDiagnostic = this.CSharpDiagnostic().WithLocation(5, 23); await this.VerifyCSharpDiagnosticAsync(testCode, firstDiagnostic, CancellationToken.None).ConfigureAwait(false); var fixedTestCode = @" @@ -123,7 +123,7 @@ public struct Foo private int baz; private const int Bar = 2; }"; - var firstDiagnostic = this.CSharpDiagnostic().WithLocation(5, 23).WithArguments("private"); + var firstDiagnostic = this.CSharpDiagnostic().WithLocation(5, 23); await this.VerifyCSharpDiagnosticAsync(testCode, firstDiagnostic, CancellationToken.None).ConfigureAwait(false); var fixedTestCode = @" @@ -146,7 +146,7 @@ public class Foo private int Baz = 1; private const int FooBar = 2; }"; - var firstDiagnostic = this.CSharpDiagnostic().WithLocation(6, 23).WithArguments("private"); + var firstDiagnostic = this.CSharpDiagnostic().WithLocation(6, 23); await this.VerifyCSharpDiagnosticAsync(testCode, firstDiagnostic, CancellationToken.None).ConfigureAwait(false); var fixedTestCode = @" @@ -171,7 +171,7 @@ public class Test const int Test3 = 3; }"; - var expected = this.CSharpDiagnostic().WithLocation(5, 15).WithArguments("private"); + var expected = this.CSharpDiagnostic().WithLocation(5, 15); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -182,7 +182,7 @@ public class Test private int Test1 = 1; const int Test3 = 3; }"; - expected = this.CSharpDiagnostic().WithLocation(6, 15).WithArguments("private"); + expected = this.CSharpDiagnostic().WithLocation(6, 15); await this.VerifyCSharpDiagnosticAsync(testCodeAfterFix1, expected, CancellationToken.None).ConfigureAwait(false); var fixCode = @" @@ -240,8 +240,8 @@ public class Foo var diagnosticResults = new[] { - this.CSharpDiagnostic().WithLocation(10, 25).WithArguments("public"), - this.CSharpDiagnostic().WithLocation(14, 25).WithArguments("public") + this.CSharpDiagnostic().WithLocation(10, 25), + this.CSharpDiagnostic().WithLocation(14, 25) }; await this.VerifyCSharpDiagnosticAsync(testCode, diagnosticResults, CancellationToken.None).ConfigureAwait(false); await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); @@ -289,8 +289,8 @@ public async Task TestCodeFixAsync() var diagnosticResults = new[] { - this.CSharpDiagnostic().WithLocation(9, 26).WithArguments("private"), - this.CSharpDiagnostic().WithLocation(13, 25).WithArguments("public") + this.CSharpDiagnostic().WithLocation(9, 26), + this.CSharpDiagnostic().WithLocation(13, 25) }; await this.VerifyCSharpDiagnosticAsync(testCode, diagnosticResults, CancellationToken.None).ConfigureAwait(false); await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); @@ -340,8 +340,8 @@ public async Task TestCodeFixWithCommentsAsync() var diagnosticResults = new[] { - this.CSharpDiagnostic().WithLocation(10, 26).WithArguments("private"), - this.CSharpDiagnostic().WithLocation(14, 25).WithArguments("public") + this.CSharpDiagnostic().WithLocation(10, 26), + this.CSharpDiagnostic().WithLocation(14, 25) }; await this.VerifyCSharpDiagnosticAsync(testCode, diagnosticResults, CancellationToken.None).ConfigureAwait(false); await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); @@ -367,7 +367,7 @@ public async Task TestOnlyLeadingWhitespaceIsMovedAsync() var diagnosticResults = new[] { - this.CSharpDiagnostic().WithLocation(7, 14).WithArguments("private"), + this.CSharpDiagnostic().WithLocation(7, 14), }; await this.VerifyCSharpDiagnosticAsync(testCode, diagnosticResults, CancellationToken.None).ConfigureAwait(false); @@ -385,13 +385,14 @@ public async Task TestOnlyLeadingWhitespaceIsMovedAsync() } /// - /// Verifies that the code fix will move the non constant fields before the constant ones with SA1202 suppressed. + /// Verifies that the code fix will move the non-constant fields before the constant ones when element access is + /// not considered for ordering. /// /// A representing the asynchronous unit test. [Fact] - public async Task TestCodeFixWithSA1202SuppressedAsync() + public async Task TestCodeFixWithoutOrderByAccessAsync() { - this.suppressSA1202 = true; + this.orderByAccess = false; var testCode = @"public class Foo { @@ -427,14 +428,37 @@ public async Task TestCodeFixWithSA1202SuppressedAsync() var diagnosticResults = new[] { - this.CSharpDiagnostic().WithLocation(9, 26).WithArguments("private"), - this.CSharpDiagnostic().WithLocation(13, 25).WithArguments("public") + this.CSharpDiagnostic().WithLocation(9, 26), + this.CSharpDiagnostic().WithLocation(13, 25) }; await this.VerifyCSharpDiagnosticAsync(testCode, diagnosticResults, CancellationToken.None).ConfigureAwait(false); await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false); } + protected override string GetSettings() + { + if (!this.orderByAccess) + { + return @" +{ + ""settings"": { + ""orderingRules"": { + ""elementOrder"": [ + ""kind"", + ""constant"", + ""static"", + ""readonly"", + ] + } + } +} +"; + } + + return base.GetSettings(); + } + protected override IEnumerable GetCSharpDiagnosticAnalyzers() { yield return new SA1203ConstantsMustAppearBeforeFields(); @@ -444,13 +468,5 @@ protected override CodeFixProvider GetCSharpCodeFixProvider() { return new ElementOrderCodeFixProvider(); } - - protected override IEnumerable GetDisabledDiagnostics() - { - if (this.suppressSA1202) - { - yield return SA1202ElementsMustBeOrderedByAccess.DiagnosticId; - } - } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1204UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1204UnitTests.cs index 85a5fe83b..21cfb6b49 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1204UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1204UnitTests.cs @@ -116,7 +116,7 @@ public static class TestClass2 { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(2, 21).WithArguments("public", "classes") + this.CSharpDiagnostic().WithLocation(2, 21) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -150,9 +150,9 @@ public static void TestMethod2() { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 23).WithArguments("public", "fields"), - this.CSharpDiagnostic().WithLocation(6, 23).WithArguments("public", "properties"), - this.CSharpDiagnostic().WithLocation(8, 24).WithArguments("public", "methods") + this.CSharpDiagnostic().WithLocation(4, 23), + this.CSharpDiagnostic().WithLocation(6, 23), + this.CSharpDiagnostic().WithLocation(8, 24) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -194,9 +194,9 @@ public static void TestMethod2() { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(4, 23).WithArguments("public", "fields"), - this.CSharpDiagnostic().WithLocation(6, 23).WithArguments("public", "properties"), - this.CSharpDiagnostic().WithLocation(8, 24).WithArguments("public", "methods") + this.CSharpDiagnostic().WithLocation(4, 23), + this.CSharpDiagnostic().WithLocation(6, 23), + this.CSharpDiagnostic().WithLocation(8, 24) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -235,7 +235,7 @@ public static event System.Action TestEvent4 { add { } remove { } } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(5, 5).WithArguments("public", "events") + this.CSharpDiagnostic().WithLocation(5, 5) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -273,7 +273,7 @@ internal static class TestClass3 { } } "; - var expected = this.CSharpDiagnostic().WithLocation(5, 27).WithArguments("internal", "classes"); + var expected = this.CSharpDiagnostic().WithLocation(5, 27); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -304,8 +304,8 @@ internal static class TestClass4 { } DiagnosticResult[] expected = { - this.CSharpDiagnostic().WithLocation(2, 21).WithArguments("public", "classes"), - this.CSharpDiagnostic().WithLocation(4, 23).WithArguments("internal", "classes"), + this.CSharpDiagnostic().WithLocation(2, 21), + this.CSharpDiagnostic().WithLocation(4, 23), }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -335,7 +335,7 @@ public async Task TestMembersWithoutAccessModifiersAsync() } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 19).WithArguments("private", "fields"); + var expected = this.CSharpDiagnostic().WithLocation(4, 19); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -363,7 +363,7 @@ class TestClass1 { } static class TestClass2 { } "; - var expected = this.CSharpDiagnostic().WithLocation(3, 14).WithArguments("internal", "classes"); + var expected = this.CSharpDiagnostic().WithLocation(3, 14); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -389,7 +389,7 @@ public class TestClass1 { } public static class TestClass2 { } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 21).WithArguments("public", "classes"); + var expected = this.CSharpDiagnostic().WithLocation(4, 21); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -423,7 +423,7 @@ public class TestClass1 { } public static class TestClass2 { } "; - var expected = this.CSharpDiagnostic().WithLocation(11, 21).WithArguments("public", "classes"); + var expected = this.CSharpDiagnostic().WithLocation(11, 21); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -458,7 +458,7 @@ static class TestClass2 { } } "; - var expected = this.CSharpDiagnostic().WithLocation(4, 18).WithArguments("internal", "classes"); + var expected = this.CSharpDiagnostic().WithLocation(4, 18); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -504,7 +504,7 @@ public MyClass2() } "; - var expected = this.CSharpDiagnostic().WithLocation(8, 12).WithArguments("public", "constructors"); + var expected = this.CSharpDiagnostic().WithLocation(8, 12); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1214UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1214UnitTests.cs index a59474b4f..e4034c5a4 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1214UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1214UnitTests.cs @@ -95,7 +95,7 @@ public class Foo var expected = new[] { - this.CSharpDiagnostic().WithLocation(5, 33).WithArguments("private") + this.CSharpDiagnostic().WithLocation(5, 33) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -135,7 +135,7 @@ public class Foo var expected = new[] { - this.CSharpDiagnostic().WithLocation(4, 58).WithArguments("private") + this.CSharpDiagnostic().WithLocation(4, 58) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -172,8 +172,18 @@ public class Foo private int i = 0; private readonly int j = 0; }"; + var fixedCode = @" +public class Foo +{ + private readonly int j = 0; + private int i = 0; +}"; - await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + var expected = this.CSharpDiagnostic().WithLocation(5, 26); + + await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); + await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false); } [Fact] @@ -188,7 +198,7 @@ public struct Foo var expected = new[] { - this.CSharpDiagnostic().WithLocation(5, 33).WithArguments("private") + this.CSharpDiagnostic().WithLocation(5, 33) }; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -232,8 +242,8 @@ public class FooInner var expected = new[] { - this.CSharpDiagnostic().WithLocation(11, 33).WithArguments("public"), - this.CSharpDiagnostic().WithLocation(18, 37).WithArguments("private") + this.CSharpDiagnostic().WithLocation(11, 33), + this.CSharpDiagnostic().WithLocation(18, 37) // line 21 should be reported by SA1201 }; @@ -310,7 +320,7 @@ public async Task TestStaticFollowedByReadOnlyAtDifferentAccessLevelAsync() protected override IEnumerable GetCSharpDiagnosticAnalyzers() { - yield return new SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements(); + yield return new SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements(); } protected override CodeFixProvider GetCSharpCodeFixProvider() diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1215UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1215UnitTests.cs index 49fafe813..cfff4b6f0 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1215UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1215UnitTests.cs @@ -13,7 +13,7 @@ namespace StyleCop.Analyzers.Test.OrderingRules using Xunit; /// - /// Unit tests for . + /// Unit tests for . /// public class SA1215UnitTests : CodeFixVerifier { @@ -120,7 +120,7 @@ public async Task TestReadonlyOrderingInClassAsync() } "; - DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("public"); + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -148,7 +148,7 @@ public async Task TestReadonlyOrderingInStructAsync() } "; - DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("public"); + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25); await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); @@ -199,7 +199,7 @@ public async Task TestConstBeforeReadonlyAsync() /// protected override IEnumerable GetCSharpDiagnosticAnalyzers() { - yield return new SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements(); + yield return new SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements(); } protected override CodeFixProvider GetCSharpCodeFixProvider() diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/ElementOrderingChecks.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/ElementOrderingChecks.cs deleted file mode 100644 index 8ee8f0d25..000000000 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/ElementOrderingChecks.cs +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -namespace StyleCop.Analyzers.Helpers -{ - using Microsoft.CodeAnalysis; - using OrderingRules; - - /// - /// Contains information about enabled element order rules. - /// - internal struct ElementOrderingChecks - { - /// - /// Initializes a new instance of the struct. - /// - /// Indicates whether element type should be checked. - /// Indicates whether access level should be checked. - /// Indicates whether const modifiers should be checked. - /// Indicates whether static modifiers should be checked. - /// Indicates whether readonly modifiers should be checked. - internal ElementOrderingChecks(bool checkElementType, bool checkAccessLevel, bool checkConst, bool checkStatic, bool checkReadonly) - { - this.ElementType = checkElementType; - this.AccessLevel = checkAccessLevel; - this.Const = checkConst; - this.Static = checkStatic; - this.Readonly = checkReadonly; - } - - /// - /// Gets a value indicating whether the element type should be checked. - /// - /// Indicates whether element type should be checked. - internal bool ElementType { get; } - - /// - /// Gets a value indicating whether the access level should be checked. - /// - /// Indicates whether access level should be checked. - internal bool AccessLevel { get; } - - /// - /// Gets a value indicating whether the const modifier should be checked. - /// - /// Indicates whether const modifier should be checked. - internal bool Const { get; } - - /// - /// Gets a value indicating whether the static modifier should be checked. - /// - /// Indicates whether static modifier should be checked. - internal bool Static { get; } - - /// - /// Gets a value indicating whether the readonly modifier should be checked. - /// - /// Indicates whether readonly modifier should be checked. - internal bool Readonly { get; } - - /// - /// Creates a configured instance of the struct for the given semantic model. - /// - /// The semantic model. - /// The created . - internal static ElementOrderingChecks GetElementOrderingChecksForSemanticModel(SemanticModel semanticModel) - { - var checkReadonly = !semanticModel.Compilation.IsAnalyzerSuppressed(SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.DiagnosticId) - || !semanticModel.Compilation.IsAnalyzerSuppressed(SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.DiagnosticId); - return new ElementOrderingChecks( - !semanticModel.Compilation.IsAnalyzerSuppressed(SA1201ElementsMustAppearInTheCorrectOrder.DiagnosticId), - !semanticModel.Compilation.IsAnalyzerSuppressed(SA1202ElementsMustBeOrderedByAccess.DiagnosticId), - !semanticModel.Compilation.IsAnalyzerSuppressed(SA1203ConstantsMustAppearBeforeFields.DiagnosticId), - !semanticModel.Compilation.IsAnalyzerSuppressed(SA1204StaticElementsMustAppearBeforeInstanceElements.DiagnosticId), - checkReadonly); - } - } -} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/MemberOrderHelper.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/MemberOrderHelper.cs index e1d473294..8381d567d 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/MemberOrderHelper.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/MemberOrderHelper.cs @@ -8,6 +8,7 @@ namespace StyleCop.Analyzers.Helpers using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; + using StyleCop.Analyzers.Settings.ObjectModel; /// /// Helper for dealing with member priority. @@ -31,53 +32,66 @@ internal struct MemberOrderHelper SyntaxKind.FieldDeclaration, SyntaxKind.NamespaceDeclaration); - private readonly ModifierFlags modifierFlags; - private readonly int elementPriority; - private readonly AccessLevel accessibilty; - /// /// Initializes a new instance of the struct. /// /// The member to wrap. - /// The element ordering checks. - internal MemberOrderHelper(MemberDeclarationSyntax member, ElementOrderingChecks checks) + /// The element ordering traits. + internal MemberOrderHelper(MemberDeclarationSyntax member, ImmutableArray elementOrder) { this.Member = member; var modifiers = member.GetModifiers(); var type = member.Kind(); type = type == SyntaxKind.EventFieldDeclaration ? SyntaxKind.EventDeclaration : type; - this.elementPriority = checks.ElementType ? TypeMemberOrder.IndexOf(type) : 0; - this.modifierFlags = GetModifierFlags(modifiers, checks); - if (checks.AccessLevel) + this.Priority = 0; + foreach (OrderingTrait trait in elementOrder) { - if ((type == SyntaxKind.ConstructorDeclaration && this.modifierFlags.HasFlag(ModifierFlags.Static)) - || (type == SyntaxKind.MethodDeclaration && ((MethodDeclarationSyntax)member).ExplicitInterfaceSpecifier != null) - || (type == SyntaxKind.PropertyDeclaration && ((PropertyDeclarationSyntax)member).ExplicitInterfaceSpecifier != null) - || (type == SyntaxKind.IndexerDeclaration && ((IndexerDeclarationSyntax)member).ExplicitInterfaceSpecifier != null)) - { - this.accessibilty = AccessLevel.Public; - } - else + switch (trait) { - this.accessibilty = AccessLevelHelper.GetAccessLevel(modifiers); - if (this.accessibilty == AccessLevel.NotSpecified) + case OrderingTrait.Kind: + // 4 bits are required to store this. + this.Priority <<= 4; + this.Priority |= TypeMemberOrder.IndexOf(type) & 0x0F; + break; + + case OrderingTrait.Accessibility: + // 3 bits are required to store this. + this.Priority <<= 3; + this.Priority |= (int)GetAccessLevelForOrdering(member, modifiers) & 0x07; + break; + + case OrderingTrait.Constant: + this.Priority <<= 1; + if (modifiers.Any(SyntaxKind.ConstKeyword)) + { + this.Priority |= 1; + } + + break; + + case OrderingTrait.Static: + this.Priority <<= 1; + if (modifiers.Any(SyntaxKind.StaticKeyword)) + { + this.Priority |= 1; + } + + break; + + case OrderingTrait.Readonly: + this.Priority <<= 1; + if (modifiers.Any(SyntaxKind.ReadOnlyKeyword)) { - if (member.Parent.IsKind(SyntaxKind.CompilationUnit) || member.Parent.IsKind(SyntaxKind.NamespaceDeclaration)) - { - this.accessibilty = AccessLevel.Internal; - } - else - { - this.accessibilty = AccessLevel.Private; - } + this.Priority |= 1; } + + break; + + default: + continue; } } - else - { - this.accessibilty = AccessLevel.Public; - } } [Flags] @@ -105,7 +119,7 @@ private enum ModifierFlags } /// - /// The wrapped member. + /// Gets the wrapped member. /// /// /// The wrapped member. @@ -113,63 +127,42 @@ private enum ModifierFlags internal MemberDeclarationSyntax Member { get; } /// - /// The priority for this member. - /// - /// - /// The priority for this member. - /// - internal int Priority - { - get - { - var priority = this.ModifierPriority; - - // accessibility is more important than the modifiers - priority += (this.AccessibilityPriority + 1) * 100; - - // element type is more important than accessibility - priority += (this.elementPriority + 1) * 1000; - return priority; - } - } - - /// - /// The priority for this member only from accessibility. - /// - /// - /// The priority for this member. - /// - internal int AccessibilityPriority => (int)this.accessibilty; - - /// - /// The priority for this member only from modifiers. + /// Gets the priority for this member. /// /// /// The priority for this member. /// - internal int ModifierPriority => (int)this.modifierFlags; + internal int Priority { get; } - private static ModifierFlags GetModifierFlags(SyntaxTokenList syntax, ElementOrderingChecks checks) + internal static AccessLevel GetAccessLevelForOrdering(SyntaxNode member, SyntaxTokenList modifiers) { - var flags = ModifierFlags.None; - if (checks.Const && syntax.Any(SyntaxKind.ConstKeyword)) + SyntaxKind type = member.Kind(); + + AccessLevel accessibility; + if ((type == SyntaxKind.ConstructorDeclaration && modifiers.Any(SyntaxKind.StaticKeyword)) + || (type == SyntaxKind.MethodDeclaration && ((MethodDeclarationSyntax)member).ExplicitInterfaceSpecifier != null) + || (type == SyntaxKind.PropertyDeclaration && ((PropertyDeclarationSyntax)member).ExplicitInterfaceSpecifier != null) + || (type == SyntaxKind.IndexerDeclaration && ((IndexerDeclarationSyntax)member).ExplicitInterfaceSpecifier != null)) { - flags |= ModifierFlags.Const; + accessibility = AccessLevel.Public; } else { - if (checks.Static && syntax.Any(SyntaxKind.StaticKeyword)) + accessibility = AccessLevelHelper.GetAccessLevel(modifiers); + if (accessibility == AccessLevel.NotSpecified) { - flags |= ModifierFlags.Static; - } - - if (checks.Readonly && syntax.Any(SyntaxKind.ReadOnlyKeyword)) - { - flags |= ModifierFlags.Readonly; + if (member.Parent.IsKind(SyntaxKind.CompilationUnit) || member.Parent.IsKind(SyntaxKind.NamespaceDeclaration)) + { + accessibility = AccessLevel.Internal; + } + else + { + accessibility = AccessLevel.Private; + } } } - return flags; + return accessibility; } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.Designer.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.Designer.cs index b6db8f303..2839c796b 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.Designer.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.Designer.cs @@ -70,6 +70,114 @@ internal static string ElementOrderCodeFix { } } + /// + /// Looks up a localized string similar to An element within a C# code file is out of order in relation to the other elements in the code.. + /// + internal static string SA1201Description { + get { + return ResourceManager.GetString("SA1201Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A {0} should not follow a {1}. + /// + internal static string SA1201MessageFormat { + get { + return ResourceManager.GetString("SA1201MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Elements must appear in the correct order. + /// + internal static string SA1201Title { + get { + return ResourceManager.GetString("SA1201Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to An element within a C# code file is out of order in relation to other elements in the code.. + /// + internal static string SA1202Description { + get { + return ResourceManager.GetString("SA1202Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to '{0}' members must come before '{1}' members. + /// + internal static string SA1202MessageFormat { + get { + return ResourceManager.GetString("SA1202MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Elements must be ordered by access. + /// + internal static string SA1202Title { + get { + return ResourceManager.GetString("SA1202Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A constant field is placed beneath a non-constant field.. + /// + internal static string SA1203Description { + get { + return ResourceManager.GetString("SA1203Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Constant fields must appear before non-constant fields. + /// + internal static string SA1203MessageFormat { + get { + return ResourceManager.GetString("SA1203MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Constants must appear before fields. + /// + internal static string SA1203Title { + get { + return ResourceManager.GetString("SA1203Title", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A static element is positioned beneath an instance element.. + /// + internal static string SA1204Description { + get { + return ResourceManager.GetString("SA1204Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Static members must appear before non-static members. + /// + internal static string SA1204MessageFormat { + get { + return ResourceManager.GetString("SA1204MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Static elements must appear before instance elements. + /// + internal static string SA1204Title { + get { + return ResourceManager.GetString("SA1204Title", resourceCulture); + } + } + /// /// Looks up a localized string similar to Add access modifier. /// @@ -96,7 +204,34 @@ internal static string SA1213CodeFix { return ResourceManager.GetString("SA1213CodeFix", resourceCulture); } } - + + /// + /// Looks up a localized string similar to A readonly field is positioned beneath a non-readonly field.. + /// + internal static string SA1214Description { + get { + return ResourceManager.GetString("SA1214Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Readonly fields must appear before non-readonly fields. + /// + internal static string SA1214MessageFormat { + get { + return ResourceManager.GetString("SA1214MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Readonly fields must appear before non-readonly fields. + /// + internal static string SA1214Title { + get { + return ResourceManager.GetString("SA1214Title", resourceCulture); + } + } + /// /// Looks up a localized string similar to Reorder using statements. /// diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.resx b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.resx index f4cb34e74..6cd8ffb1b 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.resx +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/OrderingResources.resx @@ -120,6 +120,42 @@ Fix element order + + An element within a C# code file is out of order in relation to the other elements in the code. + + + A {0} should not follow a {1} + + + Elements must appear in the correct order + + + An element within a C# code file is out of order in relation to other elements in the code. + + + '{0}' members must come before '{1}' members + + + Elements must be ordered by access + + + A constant field is placed beneath a non-constant field. + + + Constant fields must appear before non-constant fields + + + Constants must appear before fields + + + A static element is positioned beneath an instance element. + + + Static members must appear before non-static members + + + Static elements must appear before instance elements + Add access modifier @@ -129,6 +165,15 @@ Fix accessor order + + A readonly field is positioned beneath a non-readonly field. + + + Readonly fields must appear before non-readonly fields + + + Readonly fields must appear before non-readonly fields + Reorder using statements diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1201ElementsMustAppearInTheCorrectOrder.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1201ElementsMustAppearInTheCorrectOrder.cs index 675446344..2e197803f 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1201ElementsMustAppearInTheCorrectOrder.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1201ElementsMustAppearInTheCorrectOrder.cs @@ -11,6 +11,7 @@ namespace StyleCop.Analyzers.OrderingRules using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; + using Settings.ObjectModel; /// /// An element within a C# code file is out of order in relation to the other elements in the code. @@ -110,10 +111,10 @@ internal class SA1201ElementsMustAppearInTheCorrectOrder : DiagnosticAnalyzer /// The ID for diagnostics produced by the analyzer. /// public const string DiagnosticId = "SA1201"; - private const string Title = "Elements must appear in the correct order"; - private const string MessageFormat = "A {0} should not follow a {1}."; - private const string Description = "An element within a C# code file is out of order in relation to the other elements in the code."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1201.md"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(OrderingResources.SA1201Title), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(OrderingResources.SA1201MessageFormat), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(OrderingResources.SA1201Description), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1201.md"; private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); @@ -169,9 +170,9 @@ internal class SA1201ElementsMustAppearInTheCorrectOrder : DiagnosticAnalyzer }; private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action CompilationUnitAction = HandleCompilationUnit; - private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; + private static readonly Action CompilationUnitAction = HandleCompilationUnit; + private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; + private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; /// public override ImmutableArray SupportedDiagnostics { get; } = @@ -190,28 +191,49 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); } - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { - var typeDeclaration = context.Node as TypeDeclarationSyntax; + var elementOrder = settings.OrderingRules.ElementOrder; + int kindIndex = elementOrder.IndexOf(OrderingTrait.Kind); + if (kindIndex < 0) + { + return; + } + + var typeDeclaration = (TypeDeclarationSyntax)context.Node; - HandleMemberList(context, typeDeclaration.Members, TypeMemberOrder); + HandleMemberList(context, elementOrder, kindIndex, typeDeclaration.Members, TypeMemberOrder); } - private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context) + private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { - var compilationUnit = context.Node as CompilationUnitSyntax; + var elementOrder = settings.OrderingRules.ElementOrder; + int kindIndex = elementOrder.IndexOf(OrderingTrait.Kind); + if (kindIndex < 0) + { + return; + } + + var compilationUnit = (CompilationUnitSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, OuterOrder); + HandleMemberList(context, elementOrder, kindIndex, compilationUnit.Members, OuterOrder); } - private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { - var compilationUnit = context.Node as NamespaceDeclarationSyntax; + var elementOrder = settings.OrderingRules.ElementOrder; + int kindIndex = elementOrder.IndexOf(OrderingTrait.Kind); + if (kindIndex < 0) + { + return; + } + + var compilationUnit = (NamespaceDeclarationSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, OuterOrder); + HandleMemberList(context, elementOrder, kindIndex, compilationUnit.Members, OuterOrder); } - private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxList members, ImmutableArray order) + private static void HandleMemberList(SyntaxNodeAnalysisContext context, ImmutableArray elementOrder, int kindIndex, SyntaxList members, ImmutableArray order) { for (int i = 0; i < members.Count - 1; i++) { @@ -226,6 +248,46 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxLi continue; } + bool compareKind = true; + for (int j = 0; compareKind && j < kindIndex; j++) + { + switch (elementOrder[j]) + { + case OrderingTrait.Accessibility: + if (MemberOrderHelper.GetAccessLevelForOrdering(members[i + 1], members[i + 1].GetModifiers()) + != MemberOrderHelper.GetAccessLevelForOrdering(members[i], members[i].GetModifiers())) + { + compareKind = false; + } + + continue; + + case OrderingTrait.Constant: + case OrderingTrait.Readonly: + // Only fields may be marked const or readonly, and all fields have the same kind. + continue; + + case OrderingTrait.Static: + bool currentIsStatic = members[i].GetModifiers().Any(SyntaxKind.StaticKeyword); + bool nextIsStatic = members[i + 1].GetModifiers().Any(SyntaxKind.StaticKeyword); + if (currentIsStatic != nextIsStatic) + { + compareKind = false; + } + + continue; + + case OrderingTrait.Kind: + default: + continue; + } + } + + if (!compareKind) + { + continue; + } + var elementSyntaxKind = members[i].Kind(); elementSyntaxKind = elementSyntaxKind == SyntaxKind.EventFieldDeclaration ? SyntaxKind.EventDeclaration : elementSyntaxKind; int index = order.IndexOf(elementSyntaxKind); diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1202ElementsMustBeOrderedByAccess.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1202ElementsMustBeOrderedByAccess.cs index ba37d0507..e6b858f1c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1202ElementsMustBeOrderedByAccess.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1202ElementsMustBeOrderedByAccess.cs @@ -4,13 +4,13 @@ namespace StyleCop.Analyzers.OrderingRules { using System; - using System.Collections.Generic; using System.Collections.Immutable; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Settings.ObjectModel; /// /// An element within a C# code file is out of order within regard to access level, in relation to other elements in @@ -42,10 +42,10 @@ internal class SA1202ElementsMustBeOrderedByAccess : DiagnosticAnalyzer /// The ID for diagnostics produced by the analyzer. /// public const string DiagnosticId = "SA1202"; - private const string Title = "Elements must be ordered by access"; - private const string MessageFormat = "All {0} {1} must come before {2} {1}."; - private const string Description = "An element within a C# code file is out of order within regard to access level, in relation to other elements in the code."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1202.md"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(OrderingResources.SA1202Title), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(OrderingResources.SA1202MessageFormat), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(OrderingResources.SA1202Description), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1202.md"; private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); @@ -53,27 +53,25 @@ internal class SA1202ElementsMustBeOrderedByAccess : DiagnosticAnalyzer private static readonly ImmutableArray TypeDeclarationKinds = ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); - private static readonly Dictionary MemberNames = new Dictionary - { - [SyntaxKind.DelegateDeclaration] = "delegates", - [SyntaxKind.EnumDeclaration] = "enums", - [SyntaxKind.InterfaceDeclaration] = "interfaces", - [SyntaxKind.StructDeclaration] = "structs", - [SyntaxKind.ClassDeclaration] = "classes", - [SyntaxKind.FieldDeclaration] = "fields", - [SyntaxKind.ConstructorDeclaration] = "constructors", - [SyntaxKind.EventDeclaration] = "events", - [SyntaxKind.PropertyDeclaration] = "properties", - [SyntaxKind.IndexerDeclaration] = "indexers", - [SyntaxKind.MethodDeclaration] = "methods", - [SyntaxKind.ConversionOperatorDeclaration] = "conversions", - [SyntaxKind.OperatorDeclaration] = "operators" - }; + private static readonly ImmutableHashSet MemberKinds = ImmutableHashSet.Create( + SyntaxKind.DelegateDeclaration, + SyntaxKind.EnumDeclaration, + SyntaxKind.InterfaceDeclaration, + SyntaxKind.StructDeclaration, + SyntaxKind.ClassDeclaration, + SyntaxKind.FieldDeclaration, + SyntaxKind.ConstructorDeclaration, + SyntaxKind.EventDeclaration, + SyntaxKind.PropertyDeclaration, + SyntaxKind.IndexerDeclaration, + SyntaxKind.MethodDeclaration, + SyntaxKind.ConversionOperatorDeclaration, + SyntaxKind.OperatorDeclaration); private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action CompilationUnitAction = HandleCompilationUnit; - private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; + private static readonly Action CompilationUnitAction = HandleCompilationUnit; + private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; + private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; /// public override ImmutableArray SupportedDiagnostics { get; } = @@ -92,31 +90,56 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); } - private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context) + private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int accessibilityIndex = elementOrder.IndexOf(OrderingTrait.Accessibility); + if (accessibilityIndex < 0) + { + return; + } + var compilationUnit = (CompilationUnitSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, AccessLevel.Internal); + HandleMemberList(context, elementOrder, accessibilityIndex, compilationUnit.Members, AccessLevel.Internal); } - private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int accessibilityIndex = elementOrder.IndexOf(OrderingTrait.Accessibility); + if (accessibilityIndex < 0) + { + return; + } + var compilationUnit = (NamespaceDeclarationSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, AccessLevel.Internal); + HandleMemberList(context, elementOrder, accessibilityIndex, compilationUnit.Members, AccessLevel.Internal); } - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int accessibilityIndex = elementOrder.IndexOf(OrderingTrait.Accessibility); + if (accessibilityIndex < 0) + { + return; + } + var typeDeclaration = (TypeDeclarationSyntax)context.Node; - HandleMemberList(context, typeDeclaration.Members, AccessLevel.Private); + HandleMemberList(context, elementOrder, accessibilityIndex, typeDeclaration.Members, AccessLevel.Private); } - private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxList members, AccessLevel defaultAccessLevel) + private static void HandleMemberList(SyntaxNodeAnalysisContext context, ImmutableArray elementOrder, int accessibilityIndex, SyntaxList members, AccessLevel defaultAccessLevel) { + MemberDeclarationSyntax previousMember = null; var previousSyntaxKind = SyntaxKind.None; var previousAccessLevel = AccessLevel.NotSpecified; + bool previousIsConst = false; + bool previousIsReadonly = false; + bool previousIsStatic = false; foreach (var member in members) { @@ -125,41 +148,79 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxLi // if the SyntaxKind of this member (e.g. SyntaxKind.IncompleteMember) will not // be handled, skip early. - if (!MemberNames.ContainsKey(currentSyntaxKind)) + if (!MemberKinds.Contains(currentSyntaxKind)) { continue; } - AccessLevel currentAccessLevel; var modifiers = member.GetModifiers(); - if ((currentSyntaxKind == SyntaxKind.ConstructorDeclaration && modifiers.Any(SyntaxKind.StaticKeyword)) - || (currentSyntaxKind == SyntaxKind.MethodDeclaration && (member as MethodDeclarationSyntax)?.ExplicitInterfaceSpecifier != null) - || (currentSyntaxKind == SyntaxKind.PropertyDeclaration && (member as PropertyDeclarationSyntax)?.ExplicitInterfaceSpecifier != null) - || (currentSyntaxKind == SyntaxKind.IndexerDeclaration && (member as IndexerDeclarationSyntax)?.ExplicitInterfaceSpecifier != null)) - { - currentAccessLevel = AccessLevel.Public; - } - else - { - currentAccessLevel = AccessLevelHelper.GetAccessLevel(modifiers); - currentAccessLevel = currentAccessLevel == AccessLevel.NotSpecified ? defaultAccessLevel : currentAccessLevel; - } + AccessLevel currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(member, modifiers); + bool currentIsConst = modifiers.Any(SyntaxKind.ConstKeyword); + bool currentIsReadonly = modifiers.Any(SyntaxKind.ReadOnlyKeyword); + bool currentIsStatic = modifiers.Any(SyntaxKind.StaticKeyword); - if (previousAccessLevel != AccessLevel.NotSpecified - && currentSyntaxKind == previousSyntaxKind - && currentAccessLevel > previousAccessLevel) + if (previousAccessLevel != AccessLevel.NotSpecified) { - context.ReportDiagnostic( - Diagnostic.Create( - Descriptor, - NamedTypeHelpers.GetNameOrIdentifierLocation(member), - AccessLevelHelper.GetName(currentAccessLevel), - MemberNames[currentSyntaxKind], - AccessLevelHelper.GetName(previousAccessLevel))); + bool compareAccessLevel = true; + for (int j = 0; compareAccessLevel && j < accessibilityIndex; j++) + { + switch (elementOrder[j]) + { + case OrderingTrait.Kind: + if (previousSyntaxKind != currentSyntaxKind) + { + compareAccessLevel = false; + } + + continue; + + case OrderingTrait.Constant: + if (previousIsConst != currentIsConst) + { + compareAccessLevel = false; + } + + continue; + + case OrderingTrait.Readonly: + if (previousIsReadonly != currentIsReadonly) + { + compareAccessLevel = false; + } + + continue; + + case OrderingTrait.Static: + if (previousIsStatic != currentIsStatic) + { + compareAccessLevel = false; + } + + continue; + + case OrderingTrait.Accessibility: + default: + continue; + } + } + + if (compareAccessLevel && currentAccessLevel > previousAccessLevel) + { + context.ReportDiagnostic( + Diagnostic.Create( + Descriptor, + NamedTypeHelpers.GetNameOrIdentifierLocation(member), + AccessLevelHelper.GetName(currentAccessLevel), + AccessLevelHelper.GetName(previousAccessLevel))); + } } + previousMember = member; previousSyntaxKind = currentSyntaxKind; previousAccessLevel = currentAccessLevel; + previousIsConst = currentIsConst; + previousIsReadonly = currentIsReadonly; + previousIsStatic = currentIsStatic; } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1203ConstantsMustAppearBeforeFields.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1203ConstantsMustAppearBeforeFields.cs index 4dcf1e4db..cd0ee41d5 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1203ConstantsMustAppearBeforeFields.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1203ConstantsMustAppearBeforeFields.cs @@ -10,6 +10,7 @@ namespace StyleCop.Analyzers.OrderingRules using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; + using Settings.ObjectModel; /// /// A constant field is placed beneath a non-constant field. @@ -26,10 +27,10 @@ internal class SA1203ConstantsMustAppearBeforeFields : DiagnosticAnalyzer /// The ID for diagnostics produced by the analyzer. /// public const string DiagnosticId = "SA1203"; - private const string Title = "Constants must appear before fields"; - private const string MessageFormat = "All {0} constants must appear before {0} fields"; - private const string Description = "A constant field is placed beneath a non-constant field."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1203.md"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(OrderingResources.SA1203Title), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(OrderingResources.SA1203MessageFormat), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(OrderingResources.SA1203Description), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1203.md"; private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); @@ -38,7 +39,7 @@ internal class SA1203ConstantsMustAppearBeforeFields : DiagnosticAnalyzer ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; + private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; /// public override ImmutableArray SupportedDiagnostics { get; } = @@ -55,12 +56,21 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); } - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int constantIndex = elementOrder.IndexOf(OrderingTrait.Constant); + if (constantIndex < 0) + { + return; + } + var typeDeclaration = (TypeDeclarationSyntax)context.Node; var members = typeDeclaration.Members; var previousFieldConstant = true; + var previousFieldStatic = false; + var previousFieldReadonly = false; var previousAccessLevel = AccessLevel.NotSpecified; foreach (var member in members) @@ -71,16 +81,60 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) continue; } + AccessLevel currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(field, field.Modifiers); bool currentFieldConstant = field.Modifiers.Any(SyntaxKind.ConstKeyword); - var currentAccessLevel = AccessLevelHelper.GetAccessLevel(field.Modifiers); - currentAccessLevel = currentAccessLevel == AccessLevel.NotSpecified ? AccessLevel.Private : currentAccessLevel; + bool currentFieldReadonly = currentFieldConstant || field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword); + bool currentFieldStatic = currentFieldConstant || field.Modifiers.Any(SyntaxKind.StaticKeyword); + bool compareConst = true; + for (int j = 0; compareConst && j < constantIndex; j++) + { + switch (elementOrder[j]) + { + case OrderingTrait.Accessibility: + if (currentAccessLevel != previousAccessLevel) + { + compareConst = false; + } + + continue; + + case OrderingTrait.Readonly: + if (currentFieldReadonly != previousFieldReadonly) + { + compareConst = false; + } + + continue; + + case OrderingTrait.Static: + if (currentFieldStatic != previousFieldStatic) + { + compareConst = false; + } + + continue; + + case OrderingTrait.Kind: + // Only fields may be marked const, and all fields have the same kind. + continue; + + case OrderingTrait.Constant: + default: + continue; + } + } - if (currentAccessLevel == previousAccessLevel && !previousFieldConstant && currentFieldConstant) + if (compareConst) { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(member), AccessLevelHelper.GetName(currentAccessLevel))); + if (!previousFieldConstant && currentFieldConstant) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(member))); + } } previousFieldConstant = currentFieldConstant; + previousFieldReadonly = currentFieldReadonly; + previousFieldStatic = currentFieldStatic; previousAccessLevel = currentAccessLevel; } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1204StaticElementsMustAppearBeforeInstanceElements.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1204StaticElementsMustAppearBeforeInstanceElements.cs index 7a4773e07..04d660a1b 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1204StaticElementsMustAppearBeforeInstanceElements.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1204StaticElementsMustAppearBeforeInstanceElements.cs @@ -4,7 +4,6 @@ namespace StyleCop.Analyzers.OrderingRules { using System; - using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; @@ -12,6 +11,7 @@ namespace StyleCop.Analyzers.OrderingRules using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Settings.ObjectModel; /// /// A static element is positioned beneath an instance element of the same type. @@ -29,10 +29,10 @@ internal class SA1204StaticElementsMustAppearBeforeInstanceElements : Diagnostic /// analyzer. /// public const string DiagnosticId = "SA1204"; - private const string Title = "Static elements must appear before instance elements"; - private const string MessageFormat = "All {0} static {1} must appear before {0} non-static {1}."; - private const string Description = "A static element is positioned beneath an instance element of the same type."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1204.md"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(OrderingResources.SA1204Title), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(OrderingResources.SA1204MessageFormat), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(OrderingResources.SA1204Description), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1204.md"; private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); @@ -41,26 +41,9 @@ internal class SA1204StaticElementsMustAppearBeforeInstanceElements : Diagnostic ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action CompilationUnitAction = HandleCompilationUnit; - private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; - - private static readonly Dictionary MemberNames = new Dictionary - { - [SyntaxKind.DelegateDeclaration] = "delegates", - [SyntaxKind.EnumDeclaration] = "enums", - [SyntaxKind.InterfaceDeclaration] = "interfaces", - [SyntaxKind.StructDeclaration] = "structs", - [SyntaxKind.ClassDeclaration] = "classes", - [SyntaxKind.FieldDeclaration] = "fields", - [SyntaxKind.ConstructorDeclaration] = "constructors", - [SyntaxKind.EventDeclaration] = "events", - [SyntaxKind.PropertyDeclaration] = "properties", - [SyntaxKind.IndexerDeclaration] = "indexers", - [SyntaxKind.MethodDeclaration] = "methods", - [SyntaxKind.ConversionOperatorDeclaration] = "conversions", - [SyntaxKind.OperatorDeclaration] = "operators" - }; + private static readonly Action CompilationUnitAction = HandleCompilationUnit; + private static readonly Action NamespaceDeclarationAction = HandleNamespaceDeclaration; + private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; /// public override ImmutableArray SupportedDiagnostics { get; } = @@ -79,70 +62,126 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); } - private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context) + private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int staticIndex = elementOrder.IndexOf(OrderingTrait.Static); + if (staticIndex < 0) + { + return; + } + var compilationUnit = (CompilationUnitSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, AccessLevel.Internal); + HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members); } - private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int staticIndex = elementOrder.IndexOf(OrderingTrait.Static); + if (staticIndex < 0) + { + return; + } + var compilationUnit = (NamespaceDeclarationSyntax)context.Node; - HandleMemberList(context, compilationUnit.Members, AccessLevel.Internal); + HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members); } - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) + private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) { + var elementOrder = settings.OrderingRules.ElementOrder; + int staticIndex = elementOrder.IndexOf(OrderingTrait.Static); + if (staticIndex < 0) + { + return; + } + var typeDeclaration = (TypeDeclarationSyntax)context.Node; - HandleMemberList(context, typeDeclaration.Members, AccessLevel.Private); + HandleMemberList(context, elementOrder, staticIndex, typeDeclaration.Members); } - private static void HandleMemberList(SyntaxNodeAnalysisContext context, SyntaxList members, AccessLevel defaultAccessLevel) + private static void HandleMemberList(SyntaxNodeAnalysisContext context, ImmutableArray elementOrder, int staticIndex, SyntaxList members) { var previousSyntaxKind = SyntaxKind.None; var previousAccessLevel = AccessLevel.NotSpecified; var previousMemberStatic = true; + var previousMemberConstant = false; + var previousMemberReadonly = false; + foreach (var member in members) { + var modifiers = member.GetModifiers(); + var currentSyntaxKind = member.Kind(); currentSyntaxKind = currentSyntaxKind == SyntaxKind.EventFieldDeclaration ? SyntaxKind.EventDeclaration : currentSyntaxKind; - var modifiers = member.GetModifiers(); - var currentMemberStatic = modifiers.Any(SyntaxKind.StaticKeyword); - var currentMemberConst = modifiers.Any(SyntaxKind.ConstKeyword); - AccessLevel currentAccessLevel; - if ((currentSyntaxKind == SyntaxKind.ConstructorDeclaration && modifiers.Any(SyntaxKind.StaticKeyword)) - || (currentSyntaxKind == SyntaxKind.MethodDeclaration && (member as MethodDeclarationSyntax)?.ExplicitInterfaceSpecifier != null) - || (currentSyntaxKind == SyntaxKind.PropertyDeclaration && (member as PropertyDeclarationSyntax)?.ExplicitInterfaceSpecifier != null) - || (currentSyntaxKind == SyntaxKind.IndexerDeclaration && (member as IndexerDeclarationSyntax)?.ExplicitInterfaceSpecifier != null)) - { - currentAccessLevel = AccessLevel.Public; - } - else + var currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(member, modifiers); + bool currentMemberConstant = modifiers.Any(SyntaxKind.ConstKeyword); + bool currentMemberReadonly = currentMemberConstant || modifiers.Any(SyntaxKind.ReadOnlyKeyword); + bool currentMemberStatic = currentMemberConstant || modifiers.Any(SyntaxKind.StaticKeyword); + bool compareStatic = true; + for (int j = 0; compareStatic && j < staticIndex; j++) { - currentAccessLevel = AccessLevelHelper.GetAccessLevel(member.GetModifiers()); - currentAccessLevel = currentAccessLevel == AccessLevel.NotSpecified ? defaultAccessLevel : currentAccessLevel; + switch (elementOrder[j]) + { + case OrderingTrait.Accessibility: + if (currentAccessLevel != previousAccessLevel) + { + compareStatic = false; + } + + continue; + + case OrderingTrait.Readonly: + if (currentMemberReadonly != previousMemberReadonly) + { + compareStatic = false; + } + + continue; + + case OrderingTrait.Constant: + if (currentMemberConstant != previousMemberConstant) + { + compareStatic = false; + } + + continue; + + case OrderingTrait.Kind: + if (previousSyntaxKind != currentSyntaxKind) + { + compareStatic = false; + } + + continue; + + case OrderingTrait.Static: + default: + continue; + } } - if (currentSyntaxKind == previousSyntaxKind - && currentAccessLevel == previousAccessLevel - && !previousMemberStatic - && currentMemberStatic - && !currentMemberConst) + if (compareStatic) { - context.ReportDiagnostic( - Diagnostic.Create( - Descriptor, - NamedTypeHelpers.GetNameOrIdentifierLocation(member), - AccessLevelHelper.GetName(currentAccessLevel), - MemberNames[currentSyntaxKind])); + if (currentMemberStatic && !previousMemberStatic) + { + context.ReportDiagnostic( + Diagnostic.Create( + Descriptor, + NamedTypeHelpers.GetNameOrIdentifierLocation(member), + AccessLevelHelper.GetName(currentAccessLevel))); + } } previousSyntaxKind = currentSyntaxKind; previousAccessLevel = currentAccessLevel; - previousMemberStatic = currentMemberStatic || currentMemberConst; + previousMemberStatic = currentMemberStatic; + previousMemberConstant = currentMemberConstant; + previousMemberReadonly = currentMemberReadonly; } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.cs new file mode 100644 index 000000000..708a57814 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.cs @@ -0,0 +1,151 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.OrderingRules +{ + using System; + using System.Collections.Immutable; + using Microsoft.CodeAnalysis; + using Microsoft.CodeAnalysis.CSharp; + using Microsoft.CodeAnalysis.CSharp.Syntax; + using Microsoft.CodeAnalysis.Diagnostics; + using StyleCop.Analyzers.Helpers; + using StyleCop.Analyzers.Settings.ObjectModel; + + /// + /// An readonly element is positioned beneath a non-readonly element. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + internal class SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements : DiagnosticAnalyzer + { + /// + /// The ID for diagnostics produced by the + /// analyzer. + /// + public const string DiagnosticId = "SA1214"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(OrderingResources.SA1214Title), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(OrderingResources.SA1214MessageFormat), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(OrderingResources.SA1214Description), OrderingResources.ResourceManager, typeof(OrderingResources)); + private static readonly string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1214.md"; + + private static readonly DiagnosticDescriptor Descriptor = + new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); + + private static readonly ImmutableArray TypeDeclarationKinds = + ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); + + private static readonly Action CompilationStartAction = HandleCompilationStart; + private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptor); + + /// + public override void Initialize(AnalysisContext context) + { + context.RegisterCompilationStartAction(CompilationStartAction); + } + + private static void HandleCompilationStart(CompilationStartAnalysisContext context) + { + context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); + } + + private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings) + { + var elementOrder = settings.OrderingRules.ElementOrder; + int readonlyIndex = elementOrder.IndexOf(OrderingTrait.Readonly); + if (readonlyIndex < 0) + { + return; + } + + var typeDeclaration = (TypeDeclarationSyntax)context.Node; + + // This variable is null when the previous member is not a field. + FieldDeclarationSyntax previousField = null; + var previousFieldConst = true; + var previousFieldStatic = false; + var previousFieldReadonly = false; + var previousAccessLevel = AccessLevel.NotSpecified; + foreach (var member in typeDeclaration.Members) + { + FieldDeclarationSyntax field = member as FieldDeclarationSyntax; + if (field == null) + { + previousField = null; + continue; + } + + var modifiers = member.GetModifiers(); + var currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(member, modifiers); + bool currentFieldConst = modifiers.Any(SyntaxKind.ConstKeyword); + bool currentFieldStatic = currentFieldConst || modifiers.Any(SyntaxKind.StaticKeyword); + bool currentFieldReadonly = currentFieldConst || modifiers.Any(SyntaxKind.ReadOnlyKeyword); + if (previousField == null) + { + previousField = field; + previousFieldConst = currentFieldConst; + previousFieldStatic = currentFieldStatic; + previousFieldReadonly = currentFieldReadonly; + previousAccessLevel = currentAccessLevel; + continue; + } + + bool compareReadonly = true; + for (int j = 0; compareReadonly && j < readonlyIndex; j++) + { + switch (elementOrder[j]) + { + case OrderingTrait.Kind: + // This analyzer only ever looks at sequences of fields. + continue; + + case OrderingTrait.Accessibility: + if (previousAccessLevel != currentAccessLevel) + { + compareReadonly = false; + } + + continue; + + case OrderingTrait.Constant: + if (previousFieldConst != currentFieldConst) + { + compareReadonly = false; + } + + continue; + + case OrderingTrait.Static: + if (previousFieldStatic != currentFieldStatic) + { + compareReadonly = false; + } + + continue; + + case OrderingTrait.Readonly: + default: + continue; + } + } + + if (compareReadonly) + { + if (currentFieldReadonly && !previousFieldReadonly) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(member))); + } + } + + previousField = field; + previousFieldConst = currentFieldConst; + previousFieldStatic = currentFieldStatic; + previousFieldReadonly = currentFieldReadonly; + previousAccessLevel = currentAccessLevel; + } + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.cs deleted file mode 100644 index 1e25a742a..000000000 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.cs +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -namespace StyleCop.Analyzers.OrderingRules -{ - using System; - using System.Collections.Immutable; - using System.Linq; - using Microsoft.CodeAnalysis; - using Microsoft.CodeAnalysis.CSharp; - using Microsoft.CodeAnalysis.CSharp.Syntax; - using Microsoft.CodeAnalysis.Diagnostics; - using StyleCop.Analyzers.Helpers; - - /// - /// A static readonly element is positioned beneath a static non-readonly element of the same type. - /// - /// - /// A violation of this rule occurs when a static readonly element is positioned beneath a static non-readonly - /// element of the same type. - /// - [DiagnosticAnalyzer(LanguageNames.CSharp)] - internal class SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements : DiagnosticAnalyzer - { - /// - /// The ID for diagnostics produced by the - /// analyzer. - /// - public const string DiagnosticId = "SA1214"; - private const string Title = "Static readonly elements must appear before static non-readonly elements"; - private const string MessageFormat = "All {0} static readonly fields must appear before {0} static non-readonly fields."; - private const string Description = "A static readonly element is positioned beneath a static non-readonly element of the same type."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1214.md"; - - private static readonly DiagnosticDescriptor Descriptor = - new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); - - private static readonly ImmutableArray TypeDeclarationKinds = - ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); - - private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; - - /// - public override ImmutableArray SupportedDiagnostics { get; } = - ImmutableArray.Create(Descriptor); - - /// - public override void Initialize(AnalysisContext context) - { - context.RegisterCompilationStartAction(CompilationStartAction); - } - - private static void HandleCompilationStart(CompilationStartAnalysisContext context) - { - context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); - } - - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) - { - var typeDeclaration = (TypeDeclarationSyntax)context.Node; - - AnalyzeType(context, typeDeclaration); - } - - private static void AnalyzeType(SyntaxNodeAnalysisContext context, TypeDeclarationSyntax typeDeclaration) - { - var previousFieldReadonly = true; - var previousAccessLevel = AccessLevel.NotSpecified; - var previousMemberStatic = true; - foreach (var member in typeDeclaration.Members) - { - var field = member as FieldDeclarationSyntax; - if (field == null) - { - previousFieldReadonly = true; - continue; - } - - var currentFieldReadonly = field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword); - var currentAccessLevel = AccessLevelHelper.GetAccessLevel(field.Modifiers); - currentAccessLevel = currentAccessLevel == AccessLevel.NotSpecified ? AccessLevel.Private : currentAccessLevel; - var currentMemberStatic = field.Modifiers.Any(SyntaxKind.StaticKeyword); - if (currentAccessLevel == previousAccessLevel - && currentMemberStatic - && previousMemberStatic - && currentFieldReadonly - && !previousFieldReadonly) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(field), AccessLevelHelper.GetName(currentAccessLevel))); - } - - previousFieldReadonly = currentFieldReadonly; - previousAccessLevel = currentAccessLevel; - previousMemberStatic = currentMemberStatic; - } - } - } -} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.cs deleted file mode 100644 index 17144d768..000000000 --- a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.cs +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -namespace StyleCop.Analyzers.OrderingRules -{ - using System; - using System.Collections.Immutable; - using Microsoft.CodeAnalysis; - using Microsoft.CodeAnalysis.CSharp; - using Microsoft.CodeAnalysis.CSharp.Syntax; - using Microsoft.CodeAnalysis.Diagnostics; - using StyleCop.Analyzers.Helpers; - - /// - /// An instance readonly element is positioned beneath an instance non-readonly element of the same type. - /// - /// - /// A violation of this rule occurs when an instance readonly element is positioned beneath an instance - /// non-readonly element of the same type. - /// - [DiagnosticAnalyzer(LanguageNames.CSharp)] - internal class SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements : DiagnosticAnalyzer - { - /// - /// The ID for diagnostics produced by the - /// analyzer. - /// - public const string DiagnosticId = "SA1215"; - private const string Title = "Instance readonly elements must appear before instance non-readonly elements"; - private const string MessageFormat = "All {0} readonly fields must appear before {0} non-readonly fields."; - private const string Description = "An instance readonly element is positioned beneath an instance non-readonly element of the same type."; - private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1215.md"; - - private static readonly DiagnosticDescriptor Descriptor = - new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); - - private static readonly ImmutableArray TypeDeclarationKinds = - ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration); - - private static readonly Action CompilationStartAction = HandleCompilationStart; - private static readonly Action TypeDeclarationAction = HandleTypeDeclaration; - - /// - public override ImmutableArray SupportedDiagnostics { get; } = - ImmutableArray.Create(Descriptor); - - /// - public override void Initialize(AnalysisContext context) - { - context.RegisterCompilationStartAction(CompilationStartAction); - } - - private static void HandleCompilationStart(CompilationStartAnalysisContext context) - { - context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds); - } - - private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context) - { - var typeDeclaration = (TypeDeclarationSyntax)context.Node; - - var previousFieldReadonly = true; - var previousAccessLevel = AccessLevel.NotSpecified; - var previousMemberStaticOrConst = true; - foreach (var member in typeDeclaration.Members) - { - var field = member as FieldDeclarationSyntax; - if (field == null) - { - continue; - } - - var currentFieldReadonly = field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword); - var currentAccessLevel = AccessLevelHelper.GetAccessLevel(field.Modifiers); - currentAccessLevel = currentAccessLevel == AccessLevel.NotSpecified ? AccessLevel.Private : currentAccessLevel; - var currentMemberStaticOrConst = field.Modifiers.Any(SyntaxKind.StaticKeyword) || field.Modifiers.Any(SyntaxKind.ConstKeyword); - if (currentAccessLevel == previousAccessLevel - && !currentMemberStaticOrConst - && !previousMemberStaticOrConst - && currentFieldReadonly - && !previousFieldReadonly) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(field), AccessLevelHelper.GetName(currentAccessLevel))); - } - - previousFieldReadonly = currentFieldReadonly; - previousAccessLevel = currentAccessLevel; - previousMemberStaticOrConst = currentMemberStaticOrConst; - } - } - } -} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingSettings.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingSettings.cs index 63e915a2b..4daab28da 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingSettings.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingSettings.cs @@ -3,11 +3,26 @@ namespace StyleCop.Analyzers.Settings.ObjectModel { + using System.Collections.Immutable; using Newtonsoft.Json; [JsonObject(MemberSerialization.OptIn)] internal class OrderingSettings { + private static readonly ImmutableArray DefaultElementOrder = + ImmutableArray.Create( + OrderingTrait.Kind, + OrderingTrait.Accessibility, + OrderingTrait.Constant, + OrderingTrait.Static, + OrderingTrait.Readonly); + + /// + /// This is the backing field for the property. + /// + [JsonProperty("elementOrder", DefaultValueHandling = DefaultValueHandling.Ignore)] + private ImmutableArray.Builder elementOrder; + /// /// This is the backing field for the property. /// @@ -26,10 +41,19 @@ internal class OrderingSettings [JsonConstructor] protected internal OrderingSettings() { + this.elementOrder = ImmutableArray.CreateBuilder(); this.systemUsingDirectivesFirst = true; this.usingDirectivesPlacement = UsingDirectivesPlacement.InsideNamespace; } + public ImmutableArray ElementOrder + { + get + { + return this.elementOrder.Count > 0 ? this.elementOrder.ToImmutable() : DefaultElementOrder; + } + } + public bool SystemUsingDirectivesFirst => this.systemUsingDirectivesFirst; diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingTrait.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingTrait.cs new file mode 100644 index 000000000..a06156984 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/ObjectModel/OrderingTrait.cs @@ -0,0 +1,33 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.Settings.ObjectModel +{ + internal enum OrderingTrait + { + /// + /// Elements are ordered according to their kind. + /// + Kind, + + /// + /// Code elements are ordered according to their declared accessibility. + /// + Accessibility, + + /// + /// Constant elements are ordered before non-constant elements. + /// + Constant, + + /// + /// Static elements are ordered before non-static elements. + /// + Static, + + /// + /// Readonly elements are ordered before non-readonly elements. + /// + Readonly + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json index a21793ed0..8a3eb34cf 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json @@ -29,6 +29,29 @@ "description": "Configuration for ordering rules (SA1200-)", "additionalProperties": false, "properties": { + "elementOrder": { + "type": "array", + "description": "Specifies the traits used for ordering elements within a document, along with their precedence.", + "default": [ + "kind", + "accessibility", + "constant", + "static", + "readonly" + ], + "items": { + "type": "string", + "description": "", + "enum": [ + "accessibility", + "kind", + "constant", + "static", + "readonly" + ] + }, + "uniqueItems": true + }, "systemUsingDirectivesFirst": { "type": "boolean", "description": "When true, System using directives must be placed before other using directives.", diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj b/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj index 5111681f6..90084bce7 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj +++ b/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj @@ -111,7 +111,6 @@ - @@ -227,8 +226,7 @@ - - + @@ -276,6 +274,7 @@ + diff --git a/documentation/Configuration.md b/documentation/Configuration.md index b96c151e9..1cf35d566 100644 --- a/documentation/Configuration.md +++ b/documentation/Configuration.md @@ -83,6 +83,69 @@ This section describes the features of ordering rules which can be configured in } ``` +### Element Order + +The following properties are used to configure element ordering in StyleCop Analyzers. + +| Property | Default Value | Summary | +| --- | --- | --- | +| `elementOrder` | `[ "kind", "accessibility", "constant", "static", "readonly" ]` | Specifies the traits used for ordering elements within a document, along with their precedence | + +The `elementOrder` property is an array of element traits. The ordering rules (SA1201, SA1202, SA1203, SA1204, SA1214, +and SA1215) evaluate these traits in the order they are defined to identify ordering problems, and the code fix uses +this property when reordering code elements. Any traits which are omitted from the array are ignored. The following +traits are supported: + +* `kind`: Elements are ordered according to their kind (see [SA1201](SA1201.md) for this predefined order) +* `accessibility`: Elements are ordered according to their declared accessibility (see [SA1202](SA1202.md) for this + predefined order) +* `constant`: Constant elements are ordered before non-constant elements +* `static`: Static elements are ordered before non-static elements +* `readonly`: Readonly elements are ordered before non-readonly elements + +This configuration property allows for a wide variety of ordering configurations, as shown in the following examples. + +#### Example: All Constants First + +The following example shows a customized element order where *all* constant fields are placed before non-constant +fields, regardless of accessibility. + +```json +{ + "settings": { + "orderingRules": { + "elementOrder": [ + "kind", + "constant", + "accessibility", + "static", + "readonly" + ] + } + } +} +``` + +#### Example: Ignore Accessibility + +The following example shows a customized element order where element accessibility is simply ignored, but other ordering +rules remain enforced. + +```json +{ + "settings": { + "orderingRules": { + "elementOrder": [ + "kind", + "constant", + "static", + "readonly" + ] + } + } +} +``` + ### Using Directives The following properties are used to configure using directives in StyleCop Analyzers. diff --git a/documentation/KnownChanges.md b/documentation/KnownChanges.md index ec50e7cf7..936541e08 100644 --- a/documentation/KnownChanges.md +++ b/documentation/KnownChanges.md @@ -26,6 +26,7 @@ lists each of these issues, along with a link to the issue where the decision wa | --- | --- | --- | | SA1109 | Block statements must not contain embedded regions | [#998](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/998) | | SA1126 | Prefix calls correctly | [#59](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/59) | +| SA1215 | Instance readonly elements must appear before instance non-readonly elements | [#1812](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/pull/1812) | | SA1409 | Remove unnecessary code | [#1058](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1058) | | SA1603 | Documentation must contain valid XML | [#1291](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1291) | | SA1628 | Documentation text must begin with a capital letter | [#1057](https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1057) | @@ -150,7 +151,13 @@ var a = new[] { 1, 2, 3 }.ToArray(); ## Ordering Rules -There are no known changes at this time. +### SA1214 + +StyleCop Classic only reports SA1214 for violations involving static fields. In StyleCop Analyzers, SA1214 and SA1215 +were merged into a single rule to improve the ability of users to customize the behavior of several ordering rules +involving members of a type. + +:warning: Violations reported as SA1215 in StyleCop Classic are reported as SA1214 in StyleCop Analyzers. ## Naming Rules diff --git a/documentation/SA1201.md b/documentation/SA1201.md index 8137d694d..318c60209 100644 --- a/documentation/SA1201.md +++ b/documentation/SA1201.md @@ -25,51 +25,29 @@ A violation of this rule occurs when the code elements within a file do not foll To comply with this rule, elements at the file root level or within a namespace must be positioned in the following order: -*Extern Alias Directives* - -*Using Directives* - -*Namespaces* - -*Delegates* - -*Enums* - -*Interfaces* - -*Structs* - -*Classes* - - +* Extern Alias Directives +* Using Directives +* Namespaces +* Delegates +* Enums +* Interfaces +* Structs +* Classes Within a class, struct, or interface, elements must be positioned in the following order: -*Fields* - -*Constructors* - -*Finalizers (Destructors)* - -*Delegates* - -*Events* - -*Enums* - -*Interfaces* - -*Properties* - -*Indexers* - -*Methods* - -*Structs* - -*Classes* - - +* Fields +* Constructors +* Finalizers (Destructors) +* Delegates +* Events +* Enums +* Interfaces +* Properties +* Indexers +* Methods +* Structs +* Classes* Complying with a standard ordering scheme based on element type can increase the readability and maintainability of the file and encourage code reuse. diff --git a/documentation/SA1202.md b/documentation/SA1202.md index a699ec751..7fd3f1560 100644 --- a/documentation/SA1202.md +++ b/documentation/SA1202.md @@ -17,27 +17,27 @@ ## Cause -An element within a C# code file is out of order within regard to access level, in relation to other elements in the code. +An element within a C# code file is out of order within regard to access level, in relation to other elements in the +code. ## Rule description -A violation of this rule occurs when the code elements within a file do not follow a standard ordering scheme based on access level. +A violation of this rule occurs when the code elements within a file do not follow a standard ordering scheme based on +access level. To comply with this rule, adjacent elements of the same type must be positioned in the following order by access level: -*public* +* public +* internal +* protected internal +* protected +* private -*internal* +:memo: Static constructors and explicitly implemented interface members are considered *public* for the purposes of this +rule. -*protected internal* - -*protected* - -*private* - - - -Complying with a standard ordering scheme based on access level can increase the readability and maintainability of the file and make it easier to identify the public interface that is being exposed from a class. +Complying with a standard ordering scheme based on access level can increase the readability and maintainability of the +file and make it easier to identify the public interface that is being exposed from a class. ## How to fix violations diff --git a/documentation/SA1214.md b/documentation/SA1214.md index 44064db82..612c61f43 100644 --- a/documentation/SA1214.md +++ b/documentation/SA1214.md @@ -3,7 +3,7 @@ - + @@ -17,23 +17,23 @@ ## Cause -A static readonly element is positioned beneath a static non-readonly element of the same type. +A readonly field is positioned beneath a non-readonly field. ## Rule description -A violation of this rule occurs when a static readonly element is positioned beneath a static non-readonly element of the same type. +A violation of this rule occurs when a readonly field is positioned beneath a non-readonly field. ## How to fix violations -To fix an instance of this violation, place all static readonly elements above all static non-readonly elements of the same type. +To fix an instance of this violation, place all readonly fields above all non-readonly fields. ## How to suppress violations ```csharp -[SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1214:StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements", Justification = "Reviewed.")] +[SuppressMessage("StyleCop.CSharp.OrderingRules", "SA1214:ReadonlyElementsMustAppearBeforeNonReadonlyElements", Justification = "Reviewed.")] ``` ```csharp -#pragma warning disable SA1214 // StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements -#pragma warning restore SA1214 // StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements +#pragma warning disable SA1214 // ReadonlyElementsMustAppearBeforeNonReadonlyElements +#pragma warning restore SA1214 // ReadonlyElementsMustAppearBeforeNonReadonlyElements ``` diff --git a/documentation/SA1215.md b/documentation/SA1215.md index ef0f6e6bf..a07b6a010 100644 --- a/documentation/SA1215.md +++ b/documentation/SA1215.md @@ -15,6 +15,9 @@
TypeNameSA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElementsSA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements
CheckId
+:warning: This rule has been intentionally omitted from StyleCop Analyzers. See [KnownChanges.md](KnownChanges.md) for +additional information. + ## Cause An instance readonly element is positioned beneath an instance non-readonly element of the same type.