Skip to content

Commit 399028f

Browse files
sharwellsharwell
committed
Address code review feedback
1 parent ccee420 commit 399028f

3 files changed

Lines changed: 23 additions & 21 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1202ElementsMustBeOrderedByAccess.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
137137
MemberDeclarationSyntax previousMember = null;
138138
var previousSyntaxKind = SyntaxKind.None;
139139
var previousAccessLevel = AccessLevel.NotSpecified;
140+
bool previousIsConst = false;
141+
bool previousIsReadonly = false;
142+
bool previousIsStatic = false;
140143

141144
foreach (var member in members)
142145
{
@@ -152,8 +155,11 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
152155

153156
var modifiers = member.GetModifiers();
154157
AccessLevel currentAccessLevel = MemberOrderHelper.GetAccessLevelForOrdering(member, modifiers);
158+
bool currentIsConst = modifiers.Any(SyntaxKind.ConstKeyword);
159+
bool currentIsReadonly = modifiers.Any(SyntaxKind.ReadOnlyKeyword);
160+
bool currentIsStatic = modifiers.Any(SyntaxKind.StaticKeyword);
155161

156-
if (previousMember != null && previousAccessLevel != AccessLevel.NotSpecified)
162+
if (previousAccessLevel != AccessLevel.NotSpecified)
157163
{
158164
bool compareAccessLevel = true;
159165
for (int j = 0; compareAccessLevel && j < accessibilityIndex; j++)
@@ -169,9 +175,6 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
169175
continue;
170176

171177
case OrderingTrait.Constant:
172-
// Only fields may be marked const
173-
bool previousIsConst = previousMember.IsKind(SyntaxKind.FieldDeclaration) && previousMember.GetModifiers().Any(SyntaxKind.ConstKeyword);
174-
bool currentIsConst = member.IsKind(SyntaxKind.FieldDeclaration) && modifiers.Any(SyntaxKind.ConstKeyword);
175178
if (previousIsConst != currentIsConst)
176179
{
177180
compareAccessLevel = false;
@@ -180,9 +183,6 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
180183
continue;
181184

182185
case OrderingTrait.Readonly:
183-
// Only fields may be marked readonly
184-
bool previousIsReadonly = previousMember.IsKind(SyntaxKind.FieldDeclaration) && previousMember.GetModifiers().Any(SyntaxKind.ReadOnlyKeyword);
185-
bool currentIsReadonly = member.IsKind(SyntaxKind.FieldDeclaration) && modifiers.Any(SyntaxKind.ReadOnlyKeyword);
186186
if (previousIsReadonly != currentIsReadonly)
187187
{
188188
compareAccessLevel = false;
@@ -191,8 +191,6 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
191191
continue;
192192

193193
case OrderingTrait.Static:
194-
bool previousIsStatic = previousMember.GetModifiers().Any(SyntaxKind.StaticKeyword);
195-
bool currentIsStatic = modifiers.Any(SyntaxKind.StaticKeyword);
196194
if (previousIsStatic != currentIsStatic)
197195
{
198196
compareAccessLevel = false;
@@ -220,6 +218,9 @@ private static void HandleMemberList(SyntaxNodeAnalysisContext context, Immutabl
220218
previousMember = member;
221219
previousSyntaxKind = currentSyntaxKind;
222220
previousAccessLevel = currentAccessLevel;
221+
previousIsConst = currentIsConst;
222+
previousIsReadonly = currentIsReadonly;
223+
previousIsStatic = currentIsStatic;
223224
}
224225
}
225226
}

‎StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1204StaticElementsMustAppearBeforeInstanceElements.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context, Sty
7373

7474
var compilationUnit = (CompilationUnitSyntax)context.Node;
7575

76-
HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members, AccessLevel.Internal);
76+
HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members);
7777
}
7878

7979
private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
@@ -87,7 +87,7 @@ private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context
8787

8888
var compilationUnit = (NamespaceDeclarationSyntax)context.Node;
8989

90-
HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members, AccessLevel.Internal);
90+
HandleMemberList(context, elementOrder, staticIndex, compilationUnit.Members);
9191
}
9292

9393
private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
@@ -101,10 +101,10 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
101101

102102
var typeDeclaration = (TypeDeclarationSyntax)context.Node;
103103

104-
HandleMemberList(context, elementOrder, staticIndex, typeDeclaration.Members, AccessLevel.Private);
104+
HandleMemberList(context, elementOrder, staticIndex, typeDeclaration.Members);
105105
}
106106

107-
private static void HandleMemberList(SyntaxNodeAnalysisContext context, ImmutableArray<OrderingTrait> elementOrder, int staticIndex, SyntaxList<MemberDeclarationSyntax> members, AccessLevel defaultAccessLevel)
107+
private static void HandleMemberList(SyntaxNodeAnalysisContext context, ImmutableArray<OrderingTrait> elementOrder, int staticIndex, SyntaxList<MemberDeclarationSyntax> members)
108108
{
109109
var previousSyntaxKind = SyntaxKind.None;
110110
var previousAccessLevel = AccessLevel.NotSpecified;

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
6363

6464
var typeDeclaration = (TypeDeclarationSyntax)context.Node;
6565

66-
FieldDeclarationSyntax previousMember = null;
66+
// This variable is null when the previous member is not a field.
67+
FieldDeclarationSyntax previousField = null;
6768
var previousFieldConst = true;
6869
var previousFieldStatic = false;
6970
var previousFieldReadonly = false;
@@ -73,7 +74,7 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
7374
FieldDeclarationSyntax field = member as FieldDeclarationSyntax;
7475
if (field == null)
7576
{
76-
previousMember = null;
77+
previousField = null;
7778
continue;
7879
}
7980

@@ -82,9 +83,9 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
8283
bool currentFieldConst = modifiers.Any(SyntaxKind.ConstKeyword);
8384
bool currentFieldStatic = modifiers.Any(SyntaxKind.StaticKeyword);
8485
bool currentFieldReadonly = modifiers.Any(SyntaxKind.ReadOnlyKeyword);
85-
if (previousMember == null)
86+
if (previousField == null)
8687
{
87-
previousMember = field;
88+
previousField = field;
8889
previousFieldConst = currentFieldConst;
8990
previousFieldStatic = currentFieldStatic;
9091
previousFieldReadonly = currentFieldReadonly;
@@ -93,7 +94,7 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
9394
}
9495

9596
bool compareReadonly = true;
96-
for (int j = 0; j < readonlyIndex; j++)
97+
for (int j = 0; compareReadonly && j < readonlyIndex; j++)
9798
{
9899
switch (elementOrder[j])
99100
{
@@ -110,7 +111,7 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
110111
continue;
111112

112113
case OrderingTrait.Constant:
113-
if (previousFieldConst || currentFieldConst)
114+
if (previousFieldConst != currentFieldConst)
114115
{
115116
compareReadonly = false;
116117
}
@@ -133,13 +134,13 @@ private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context, Sty
133134

134135
if (compareReadonly)
135136
{
136-
if ((currentFieldReadonly || currentFieldConst) && !previousFieldReadonly && !previousFieldConst)
137+
if (currentFieldReadonly && !previousFieldReadonly)
137138
{
138139
context.ReportDiagnostic(Diagnostic.Create(Descriptor, NamedTypeHelpers.GetNameOrIdentifierLocation(member)));
139140
}
140141
}
141142

142-
previousMember = field;
143+
previousField = field;
143144
previousFieldConst = currentFieldConst;
144145
previousFieldStatic = currentFieldStatic;
145146
previousFieldReadonly = currentFieldReadonly;

0 commit comments

Comments
 (0)