Skip to content

Commit 38ab7d3

Browse files
sharwellsharwell
committed
Add edge cases and fix bugs
* Attribute lists are now evaluated along with siblings of their parents
1 parent f5f7d2d commit 38ab7d3

2 files changed

Lines changed: 143 additions & 85 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1137UnitTests.cs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,29 @@ namespace Namespace3
5353
{baseTypeKind} TypeName {{ }}
5454
}}
5555
56+
namespace Namespace4
57+
{{
58+
{baseTypeKind} TypeName1 {{ }}
59+
60+
[My] {baseTypeKind} TypeName2 {{ }}
61+
}}
62+
63+
namespace Namespace5
64+
{{
65+
{baseTypeKind} TypeName1 {{ }}
66+
67+
[My]
68+
[My] {baseTypeKind} TypeName2 {{ }}
69+
}}
70+
71+
namespace Namespace6
72+
{{
73+
{baseTypeKind} TypeName1 {{ }}
74+
75+
[My]
76+
[My] {baseTypeKind} TypeName2 {{ }}
77+
}}
78+
5679
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
5780
class MyAttribute : Attribute {{ }}
5881
";
@@ -84,6 +107,29 @@ namespace Namespace3
84107
{baseTypeKind} TypeName {{ }}
85108
}}
86109
110+
namespace Namespace4
111+
{{
112+
{baseTypeKind} TypeName1 {{ }}
113+
114+
[My] {baseTypeKind} TypeName2 {{ }}
115+
}}
116+
117+
namespace Namespace5
118+
{{
119+
{baseTypeKind} TypeName1 {{ }}
120+
121+
[My]
122+
[My] {baseTypeKind} TypeName2 {{ }}
123+
}}
124+
125+
namespace Namespace6
126+
{{
127+
{baseTypeKind} TypeName1 {{ }}
128+
129+
[My]
130+
[My] {baseTypeKind} TypeName2 {{ }}
131+
}}
132+
87133
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
88134
class MyAttribute : Attribute {{ }}
89135
";
@@ -94,6 +140,9 @@ class MyAttribute : Attribute {{ }}
94140
this.CSharpDiagnostic().WithLocation(18, 1),
95141
this.CSharpDiagnostic().WithLocation(24, 1),
96142
this.CSharpDiagnostic().WithLocation(25, 1),
143+
this.CSharpDiagnostic().WithLocation(33, 1),
144+
this.CSharpDiagnostic().WithLocation(41, 1),
145+
this.CSharpDiagnostic().WithLocation(48, 1),
97146
};
98147

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

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1137ElementsShouldHaveTheSameIndentation.cs‎

Lines changed: 94 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,13 @@ internal class SA1137ElementsShouldHaveTheSameIndentation : DiagnosticAnalyzer
3232
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
3333
private static readonly Action<SyntaxNodeAnalysisContext> CompilationUnitAction = HandleCompilationUnit;
3434
private static readonly Action<SyntaxNodeAnalysisContext> NamespaceDeclarationAction = HandleNamespaceDeclaration;
35-
private static readonly Action<SyntaxNodeAnalysisContext> BaseTypeDeclarationAction = HandleBaseTypeDeclaration;
3635
private static readonly Action<SyntaxNodeAnalysisContext> TypeDeclarationAction = HandleTypeDeclaration;
3736
private static readonly Action<SyntaxNodeAnalysisContext> EnumDeclarationAction = HandleEnumDeclaration;
38-
private static readonly Action<SyntaxNodeAnalysisContext> EnumMemberDeclarationAction = HandleEnumMemberDeclaration;
39-
private static readonly Action<SyntaxNodeAnalysisContext> BaseFieldDeclarationAction = HandleBaseFieldDeclaration;
40-
private static readonly Action<SyntaxNodeAnalysisContext> BaseMethodDeclarationAction = HandleBaseMethodDeclaration;
4137
private static readonly Action<SyntaxNodeAnalysisContext> MethodDeclarationAction = HandleMethodDeclaration;
42-
private static readonly Action<SyntaxNodeAnalysisContext> BasePropertyDeclarationAction = HandleBasePropertyDeclaration;
4338
private static readonly Action<SyntaxNodeAnalysisContext> AccessorListAction = HandleAccessorList;
44-
private static readonly Action<SyntaxNodeAnalysisContext> AccessorDeclarationAction = HandleAccessorDeclaration;
4539
private static readonly Action<SyntaxNodeAnalysisContext> VariableDeclarationAction = HandleVariableDeclaration;
4640
private static readonly Action<SyntaxNodeAnalysisContext> TypeParameterListAction = HandleTypeParameterList;
47-
private static readonly Action<SyntaxNodeAnalysisContext> TypeParameterAction = HandleTypeParameter;
4841
private static readonly Action<SyntaxNodeAnalysisContext> BaseParameterListAction = HandleBaseParameterList;
49-
private static readonly Action<SyntaxNodeAnalysisContext> ParameterAction = HandleParameter;
5042
private static readonly Action<SyntaxNodeAnalysisContext> BaseArgumentListAction = HandleBaseArgumentList;
5143
private static readonly Action<SyntaxNodeAnalysisContext> AttributeListAction = HandleAttributeList;
5244
private static readonly Action<SyntaxNodeAnalysisContext> AttributeArgumentListAction = HandleAttributeArgumentList;
@@ -68,21 +60,13 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte
6860
{
6961
context.RegisterSyntaxNodeActionHonorExclusions(CompilationUnitAction, SyntaxKind.CompilationUnit);
7062
context.RegisterSyntaxNodeActionHonorExclusions(NamespaceDeclarationAction, SyntaxKind.NamespaceDeclaration);
71-
context.RegisterSyntaxNodeActionHonorExclusions(BaseTypeDeclarationAction, SyntaxKinds.BaseTypeDeclaration);
7263
context.RegisterSyntaxNodeActionHonorExclusions(TypeDeclarationAction, SyntaxKinds.TypeDeclaration);
7364
context.RegisterSyntaxNodeActionHonorExclusions(EnumDeclarationAction, SyntaxKind.EnumDeclaration);
74-
context.RegisterSyntaxNodeActionHonorExclusions(EnumMemberDeclarationAction, SyntaxKind.EnumMemberDeclaration);
75-
context.RegisterSyntaxNodeActionHonorExclusions(BaseFieldDeclarationAction, SyntaxKinds.BaseFieldDeclaration);
76-
context.RegisterSyntaxNodeActionHonorExclusions(BaseMethodDeclarationAction, SyntaxKinds.BaseMethodDeclaration);
7765
context.RegisterSyntaxNodeActionHonorExclusions(MethodDeclarationAction, SyntaxKind.MethodDeclaration);
78-
context.RegisterSyntaxNodeActionHonorExclusions(BasePropertyDeclarationAction, SyntaxKinds.BasePropertyDeclaration);
7966
context.RegisterSyntaxNodeActionHonorExclusions(AccessorListAction, SyntaxKind.AccessorList);
80-
context.RegisterSyntaxNodeActionHonorExclusions(AccessorDeclarationAction, SyntaxKinds.AccessorDeclaration);
8167
context.RegisterSyntaxNodeActionHonorExclusions(VariableDeclarationAction, SyntaxKind.VariableDeclaration);
8268
context.RegisterSyntaxNodeActionHonorExclusions(TypeParameterListAction, SyntaxKind.TypeParameterList);
83-
context.RegisterSyntaxNodeActionHonorExclusions(TypeParameterAction, SyntaxKind.TypeParameter);
8469
context.RegisterSyntaxNodeActionHonorExclusions(BaseParameterListAction, SyntaxKinds.BaseParameterList);
85-
context.RegisterSyntaxNodeActionHonorExclusions(ParameterAction, SyntaxKind.Parameter);
8670
context.RegisterSyntaxNodeActionHonorExclusions(BaseArgumentListAction, SyntaxKinds.BaseArgumentList);
8771
context.RegisterSyntaxNodeActionHonorExclusions(AttributeListAction, SyntaxKind.AttributeList);
8872
context.RegisterSyntaxNodeActionHonorExclusions(AttributeArgumentListAction, SyntaxKind.AttributeArgumentList);
@@ -100,7 +84,7 @@ private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context)
10084
elements.AddRange(compilationUnit.Externs);
10185
elements.AddRange(compilationUnit.Usings);
10286
elements.AddRange(compilationUnit.AttributeLists);
103-
elements.AddRange(compilationUnit.Members);
87+
AddMembersAndAttributes(elements, compilationUnit.Members);
10488

10589
CheckElements(context, elements.ToImmutable());
10690
}
@@ -113,51 +97,34 @@ private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context
11397

11498
elements.AddRange(namespaceDeclaration.Externs);
11599
elements.AddRange(namespaceDeclaration.Usings);
116-
elements.AddRange(namespaceDeclaration.Members);
100+
AddMembersAndAttributes(elements, namespaceDeclaration.Members);
117101

118102
CheckElements(context, elements.ToImmutable());
119103
}
120104

121-
private static void HandleBaseTypeDeclaration(SyntaxNodeAnalysisContext context)
122-
{
123-
var baseTypeDeclaration = (BaseTypeDeclarationSyntax)context.Node;
124-
CheckAttributeLists(context, baseTypeDeclaration.AttributeLists, baseTypeDeclaration);
125-
}
126-
127105
private static void HandleTypeDeclaration(SyntaxNodeAnalysisContext context)
128106
{
129107
var typeDeclaration = (TypeDeclarationSyntax)context.Node;
130108

131109
CheckElements(context, typeDeclaration.ConstraintClauses);
132-
CheckElements(context, typeDeclaration.Members);
110+
111+
var elements = ImmutableList.CreateBuilder<SyntaxNode>();
112+
AddMembersAndAttributes(elements, typeDeclaration.Members);
113+
CheckElements(context, elements.ToImmutable());
133114
}
134115

135116
private static void HandleEnumDeclaration(SyntaxNodeAnalysisContext context)
136117
{
137118
var enumDeclaration = (EnumDeclarationSyntax)context.Node;
138119

139-
CheckElements(context, enumDeclaration.Members);
140-
}
141-
142-
private static void HandleEnumMemberDeclaration(SyntaxNodeAnalysisContext context)
143-
{
144-
var enumMemberDeclaration = (EnumMemberDeclarationSyntax)context.Node;
145-
146-
CheckAttributeLists(context, enumMemberDeclaration.AttributeLists, enumMemberDeclaration);
147-
}
148-
149-
private static void HandleBaseFieldDeclaration(SyntaxNodeAnalysisContext context)
150-
{
151-
var baseFieldDeclaration = (BaseFieldDeclarationSyntax)context.Node;
152-
153-
CheckAttributeLists(context, baseFieldDeclaration.AttributeLists, baseFieldDeclaration);
154-
}
155-
156-
private static void HandleBaseMethodDeclaration(SyntaxNodeAnalysisContext context)
157-
{
158-
var baseMethodDeclaration = (BaseMethodDeclarationSyntax)context.Node;
120+
var elements = ImmutableList.CreateBuilder<SyntaxNode>();
121+
foreach (EnumMemberDeclarationSyntax enumMemberDeclaration in enumDeclaration.Members)
122+
{
123+
elements.AddRange(enumMemberDeclaration.AttributeLists);
124+
elements.Add(enumMemberDeclaration);
125+
}
159126

160-
CheckAttributeLists(context, baseMethodDeclaration.AttributeLists, baseMethodDeclaration);
127+
CheckElements(context, elements.ToImmutable());
161128
}
162129

163130
private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)
@@ -167,25 +134,13 @@ private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context)
167134
CheckElements(context, methodDeclaration.ConstraintClauses);
168135
}
169136

170-
private static void HandleBasePropertyDeclaration(SyntaxNodeAnalysisContext context)
171-
{
172-
var basePropertyDeclaration = (BasePropertyDeclarationSyntax)context.Node;
173-
174-
CheckAttributeLists(context, basePropertyDeclaration.AttributeLists, basePropertyDeclaration);
175-
}
176-
177137
private static void HandleAccessorList(SyntaxNodeAnalysisContext context)
178138
{
179139
var accessorList = (AccessorListSyntax)context.Node;
180140

181-
CheckElements(context, accessorList.Accessors);
182-
}
183-
184-
private static void HandleAccessorDeclaration(SyntaxNodeAnalysisContext context)
185-
{
186-
var accessorDeclaration = (AccessorDeclarationSyntax)context.Node;
187-
188-
CheckAttributeLists(context, accessorDeclaration.AttributeLists, accessorDeclaration);
141+
var elements = ImmutableList.CreateBuilder<SyntaxNode>();
142+
AddMembersAndAttributes(elements, accessorList.Accessors);
143+
CheckElements(context, elements.ToImmutable());
189144
}
190145

191146
private static void HandleVariableDeclaration(SyntaxNodeAnalysisContext context)
@@ -198,27 +153,18 @@ private static void HandleTypeParameterList(SyntaxNodeAnalysisContext context)
198153
{
199154
var typeParameterList = (TypeParameterListSyntax)context.Node;
200155

201-
CheckElements(context, typeParameterList.Parameters);
202-
}
203-
204-
private static void HandleTypeParameter(SyntaxNodeAnalysisContext context)
205-
{
206-
var typeParameter = (TypeParameterSyntax)context.Node;
207-
208-
CheckAttributeLists(context, typeParameter.AttributeLists, typeParameter);
156+
var elements = ImmutableList.CreateBuilder<SyntaxNode>();
157+
AddMembersAndAttributes(elements, typeParameterList.Parameters);
158+
CheckElements(context, elements.ToImmutable());
209159
}
210160

211161
private static void HandleBaseParameterList(SyntaxNodeAnalysisContext context)
212162
{
213163
var baseParameterList = (BaseParameterListSyntax)context.Node;
214164

215-
CheckElements(context, baseParameterList.Parameters);
216-
}
217-
218-
private static void HandleParameter(SyntaxNodeAnalysisContext context)
219-
{
220-
var parameter = (ParameterSyntax)context.Node;
221-
CheckAttributeLists(context, parameter.AttributeLists, parameter);
165+
var elements = ImmutableList.CreateBuilder<SyntaxNode>();
166+
AddMembersAndAttributes(elements, baseParameterList.Parameters);
167+
CheckElements(context, elements.ToImmutable());
222168
}
223169

224170
private static void HandleBaseArgumentList(SyntaxNodeAnalysisContext context)
@@ -300,21 +246,75 @@ private static void HandleInitializerExpression(SyntaxNodeAnalysisContext contex
300246
CheckElements(context, initializerExpression.Expressions);
301247
}
302248

303-
private static void CheckAttributeLists(SyntaxNodeAnalysisContext context, SyntaxList<AttributeListSyntax> attributeLists, SyntaxNode parent)
249+
private static void AddMembersAndAttributes<T>(ImmutableList<SyntaxNode>.Builder elements, SeparatedSyntaxList<T> members)
250+
where T : SyntaxNode
304251
{
305-
if (attributeLists.Count == 0)
252+
foreach (SyntaxNode member in members)
306253
{
307-
return;
254+
AddMemberAndAttributes(elements, member);
308255
}
256+
}
309257

310-
var nodes = ImmutableList.CreateBuilder<SyntaxNode>();
258+
private static void AddMembersAndAttributes<T>(ImmutableList<SyntaxNode>.Builder elements, SyntaxList<T> members)
259+
where T : SyntaxNode
260+
{
261+
foreach (SyntaxNode member in members)
262+
{
263+
AddMemberAndAttributes(elements, member);
264+
}
265+
}
311266

312-
// Placing the parent first in this list causes the analysis to prefer to align attribute lists with their
313-
// parent syntax, assuming the parent also appears on its own line.
314-
nodes.Add(parent);
315-
nodes.AddRange(attributeLists);
267+
private static void AddMemberAndAttributes(ImmutableList<SyntaxNode>.Builder elements, SyntaxNode member)
268+
{
269+
switch (member.Kind())
270+
{
271+
case SyntaxKind.ClassDeclaration:
272+
case SyntaxKind.StructDeclaration:
273+
case SyntaxKind.InterfaceDeclaration:
274+
case SyntaxKind.EnumDeclaration:
275+
elements.AddRange(((BaseTypeDeclarationSyntax)member).AttributeLists);
276+
break;
277+
278+
case SyntaxKind.FieldDeclaration:
279+
case SyntaxKind.EventFieldDeclaration:
280+
elements.AddRange(((BaseFieldDeclarationSyntax)member).AttributeLists);
281+
break;
282+
283+
case SyntaxKind.PropertyDeclaration:
284+
case SyntaxKind.EventDeclaration:
285+
case SyntaxKind.IndexerDeclaration:
286+
elements.AddRange(((BasePropertyDeclarationSyntax)member).AttributeLists);
287+
break;
288+
289+
case SyntaxKind.MethodDeclaration:
290+
case SyntaxKind.ConstructorDeclaration:
291+
case SyntaxKind.DestructorDeclaration:
292+
case SyntaxKind.OperatorDeclaration:
293+
case SyntaxKind.ConversionOperatorDeclaration:
294+
elements.AddRange(((BaseMethodDeclarationSyntax)member).AttributeLists);
295+
break;
296+
297+
case SyntaxKind.GetAccessorDeclaration:
298+
case SyntaxKind.SetAccessorDeclaration:
299+
case SyntaxKind.AddAccessorDeclaration:
300+
case SyntaxKind.RemoveAccessorDeclaration:
301+
case SyntaxKind.UnknownAccessorDeclaration:
302+
elements.AddRange(((AccessorDeclarationSyntax)member).AttributeLists);
303+
break;
304+
305+
case SyntaxKind.TypeParameter:
306+
elements.AddRange(((TypeParameterSyntax)member).AttributeLists);
307+
break;
308+
309+
case SyntaxKind.Parameter:
310+
elements.AddRange(((ParameterSyntax)member).AttributeLists);
311+
break;
312+
313+
default:
314+
break;
315+
}
316316

317-
CheckElements(context, nodes.ToImmutable());
317+
elements.Add(member);
318318
}
319319

320320
private static void CheckElements<T>(SyntaxNodeAnalysisContext context, SyntaxList<T> elements)
@@ -359,6 +359,15 @@ private static void CheckElements<T>(SyntaxNodeAnalysisContext context, Immutabl
359359
return;
360360
}
361361

362+
// Try to reorder the list so the first item is not an attribute list. This element will establish the
363+
// expected indentation for the entire collection.
364+
int desiredFirst = elements.FindIndex(x => !x.IsKind(SyntaxKind.AttributeList));
365+
if (desiredFirst >= 0)
366+
{
367+
T newFirstElement = elements[desiredFirst];
368+
elements = elements.RemoveAt(desiredFirst).Insert(0, newFirstElement);
369+
}
370+
362371
bool first = true;
363372
string expectedIndentation = null;
364373
foreach (T element in elements)

0 commit comments

Comments
 (0)