Skip to content

Commit c23e996

Browse files
sharwellsharwell
committed
Merge SA1214 and SA1215 into a single rule
This change is necessary to support fully-configurable element orderings.
1 parent c9cf96f commit c23e996

10 files changed

Lines changed: 191 additions & 230 deletions

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/OrderingRules/ElementOrderCodeFixProvider.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ internal class ElementOrderCodeFixProvider : CodeFixProvider
3131
SA1202ElementsMustBeOrderedByAccess.DiagnosticId,
3232
SA1203ConstantsMustAppearBeforeFields.DiagnosticId,
3333
SA1204StaticElementsMustAppearBeforeInstanceElements.DiagnosticId,
34-
SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.DiagnosticId,
35-
SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements.DiagnosticId);
34+
SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.DiagnosticId);
3635

3736
/// <inheritdoc/>
3837
public override FixAllProvider GetFixAllProvider()

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1214UnitTests.cs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public class Foo
9595

9696
var expected = new[]
9797
{
98-
this.CSharpDiagnostic().WithLocation(5, 33).WithArguments("private")
98+
this.CSharpDiagnostic().WithLocation(5, 33)
9999
};
100100

101101
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
@@ -135,7 +135,7 @@ public class Foo
135135

136136
var expected = new[]
137137
{
138-
this.CSharpDiagnostic().WithLocation(4, 58).WithArguments("private")
138+
this.CSharpDiagnostic().WithLocation(4, 58)
139139
};
140140

141141
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
@@ -172,8 +172,18 @@ public class Foo
172172
private int i = 0;
173173
private readonly int j = 0;
174174
}";
175+
var fixedCode = @"
176+
public class Foo
177+
{
178+
private readonly int j = 0;
179+
private int i = 0;
180+
}";
175181

176-
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
182+
var expected = this.CSharpDiagnostic().WithLocation(5, 26);
183+
184+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
185+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
186+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
177187
}
178188

179189
[Fact]
@@ -188,7 +198,7 @@ public struct Foo
188198

189199
var expected = new[]
190200
{
191-
this.CSharpDiagnostic().WithLocation(5, 33).WithArguments("private")
201+
this.CSharpDiagnostic().WithLocation(5, 33)
192202
};
193203

194204
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
@@ -232,8 +242,8 @@ public class FooInner
232242

233243
var expected = new[]
234244
{
235-
this.CSharpDiagnostic().WithLocation(11, 33).WithArguments("public"),
236-
this.CSharpDiagnostic().WithLocation(18, 37).WithArguments("private")
245+
this.CSharpDiagnostic().WithLocation(11, 33),
246+
this.CSharpDiagnostic().WithLocation(18, 37)
237247

238248
// line 21 should be reported by SA1201
239249
};
@@ -310,7 +320,7 @@ public async Task TestStaticFollowedByReadOnlyAtDifferentAccessLevelAsync()
310320

311321
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
312322
{
313-
yield return new SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements();
323+
yield return new SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements();
314324
}
315325

316326
protected override CodeFixProvider GetCSharpCodeFixProvider()

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1215UnitTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace StyleCop.Analyzers.Test.OrderingRules
1313
using Xunit;
1414

1515
/// <summary>
16-
/// Unit tests for <see cref="SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements"/>.
16+
/// Unit tests for <see cref="SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements"/>.
1717
/// </summary>
1818
public class SA1215UnitTests : CodeFixVerifier
1919
{
@@ -120,7 +120,7 @@ public async Task TestReadonlyOrderingInClassAsync()
120120
}
121121
";
122122

123-
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("public");
123+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25);
124124

125125
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
126126

@@ -148,7 +148,7 @@ public async Task TestReadonlyOrderingInStructAsync()
148148
}
149149
";
150150

151-
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25).WithArguments("public");
151+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 25);
152152

153153
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
154154

@@ -199,7 +199,7 @@ public async Task TestConstBeforeReadonlyAsync()
199199
/// <inheritdoc/>
200200
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
201201
{
202-
yield return new SA1215InstanceReadonlyElementsMustAppearBeforeInstanceNonReadonlyElements();
202+
yield return new SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements();
203203
}
204204

205205
protected override CodeFixProvider GetCSharpCodeFixProvider()

‎StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements.cs‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.OrderingRules
5+
{
6+
using System;
7+
using System.Collections.Immutable;
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.CSharp;
10+
using Microsoft.CodeAnalysis.CSharp.Syntax;
11+
using Microsoft.CodeAnalysis.Diagnostics;
12+
using StyleCop.Analyzers.Helpers;
13+
using StyleCop.Analyzers.Settings.ObjectModel;
14+
15+
/// <summary>
16+
/// An readonly element is positioned beneath a non-readonly element.
17+
/// </summary>
18+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
19+
internal class SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements : DiagnosticAnalyzer
20+
{
21+
/// <summary>
22+
/// The ID for diagnostics produced by the
23+
/// <see cref="SA1214ReadonlyElementsMustAppearBeforeNonReadonlyElements"/> analyzer.
24+
/// </summary>
25+
public const string DiagnosticId = "SA1214";
26+
private const string Title = "Readonly elements must appear before non-readonly elements";
27+
private const string MessageFormat = "Readonly fields must appear before non-readonly fields.";
28+
private const string Description = "A readonly field is positioned beneath a non-readonly field.";
29+
private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1214.md";
30+
31+
private static readonly DiagnosticDescriptor Descriptor =
32+
new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.OrderingRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
33+
34+
private static readonly ImmutableArray<SyntaxKind> TypeDeclarationKinds =
35+
ImmutableArray.Create(SyntaxKind.ClassDeclaration, SyntaxKind.StructDeclaration);
36+
37+
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
38+
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> TypeDeclarationAction = HandleTypeDeclaration;
39+
40+
/// <inheritdoc/>
41+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
42+
ImmutableArray.Create(Descriptor);
43+
44+
/// <inheritdoc/>
45+
public override void Initialize(AnalysisContext context)
46+
{
47+
context.RegisterCompilationStartAction(CompilationStartAction);
48+
}
49+
50+
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
51+
{
52+
context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, TypeDeclarationKinds);
53+
}
54+
55+
private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
56+
{
57+
var elementOrder = settings.OrderingRules.ElementOrder;
58+
int readonlyIndex = elementOrder.IndexOf(OrderingTrait.Readonly);
59+
if (readonlyIndex < 0)
60+
{
61+
return;
62+
}
63+
64+
var typeDeclaration = (TypeDeclarationSyntax)context.Node;
65+
66+
FieldDeclarationSyntax previousMember = null;
67+
var previousFieldConst = true;
68+
var previousFieldStatic = false;
69+
var previousFieldReadonly = false;
70+
var previousAccessLevel = AccessLevel.NotSpecified;
71+
foreach (var member in typeDeclaration.Members)
72+
{
73+
FieldDeclarationSyntax field = member as FieldDeclarationSyntax;
74+
if (field == null)
75+
{
76+
previousMember = null;
77+
continue;
78+
}
79+
80+
var modifiers = member.GetModifiers();
81+
var currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(member, modifiers);
82+
bool currentFieldConst = modifiers.Any(SyntaxKind.ConstKeyword);
83+
bool currentFieldStatic = modifiers.Any(SyntaxKind.StaticKeyword);
84+
bool currentFieldReadonly = modifiers.Any(SyntaxKind.ReadOnlyKeyword);
85+
if (previousMember == null)
86+
{
87+
previousMember = field;
88+
previousFieldConst = currentFieldConst;
89+
previousFieldStatic = currentFieldStatic;
90+
previousFieldReadonly = currentFieldReadonly;
91+
previousAccessLevel = currentAccessLevel;
92+
continue;
93+
}
94+
95+
bool compareReadonly = true;
96+
for (int j = 0; j < readonlyIndex; j++)
97+
{
98+
switch (elementOrder[j])
99+
{
100+
case OrderingTrait.Kind:
101+
// This analyzer only ever looks at sequences of fields.
102+
continue;
103+
104+
case OrderingTrait.Accessibility:
105+
if (previousAccessLevel != currentAccessLevel)
106+
{
107+
compareReadonly = false;
108+
}
109+
110+
continue;
111+
112+
case OrderingTrait.Constant:
113+
if (previousFieldConst || currentFieldConst)
114+
{
115+
compareReadonly = false;
116+
}
117+
118+
continue;
119+
120+
case OrderingTrait.Static:
121+
if (previousFieldStatic != currentFieldStatic)
122+
{
123+
compareReadonly = false;
124+
}
125+
126+
continue;
127+
128+
case OrderingTrait.Readonly:
129+
default:
130+
continue;
131+
}
132+
}
133+
134+
if (compareReadonly)
135+
{
136+
if ((currentFieldReadonly || currentFieldConst) && !previousFieldReadonly && !previousFieldConst)
137+
{
138+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(member), AccessLevelHelper.GetName(currentAccessLevel)));
139+
}
140+
}
141+
142+
previousMember = field;
143+
previousFieldConst = currentFieldConst;
144+
previousFieldStatic = currentFieldStatic;
145+
previousFieldReadonly = currentFieldReadonly;
146+
previousAccessLevel = currentAccessLevel;
147+
}
148+
}
149+
}
150+
}

‎StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1214StaticReadonlyElementsMustAppearBeforeStaticNonReadonlyElements.cs‎

Lines changed: 0 additions & 107 deletions
This file was deleted.

0 commit comments

Comments
 (0)