Skip to content

Commit 1c63d28

Browse files
sharwellsharwell
committed
Merge pull request #2165 from bjornhellander/Issue1928_UpdateSA1305ToHandleParameters
Updated SA1305 to also handle parameters
2 parents 7919bb6 + e2b0cc8 commit 1c63d28

3 files changed

Lines changed: 310 additions & 5 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/NamingRules/SA1305UnitTests.cs‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,278 @@ public async Task TestExcludedPrefixesAreNotReportedAsync()
191191
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
192192
}
193193

194+
[Fact]
195+
public async Task TestParameterInInterfaceMethodParameterDeclarationAsync()
196+
{
197+
var testCode = @"
198+
public interface TypeName
199+
{
200+
void MethodName(bool abX);
201+
}";
202+
203+
DiagnosticResult[] expected =
204+
{
205+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(4, 26),
206+
};
207+
208+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
209+
}
210+
211+
[Fact]
212+
public async Task TestParameterInClassMethodAsync()
213+
{
214+
var testCode = @"
215+
public class TypeName
216+
{
217+
public void MethodName(bool abX)
218+
{
219+
}
220+
}";
221+
222+
DiagnosticResult[] expected =
223+
{
224+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(4, 33),
225+
};
226+
227+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
228+
}
229+
230+
[Fact]
231+
public async Task TestParameterInNativeClassMethodAsync()
232+
{
233+
var testCode = @"
234+
public class TypeNameNativeMethods
235+
{
236+
public void MethodName(bool abX)
237+
{
238+
}
239+
}";
240+
241+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
242+
}
243+
244+
[Fact]
245+
public async Task TestParameterInImplementedInterfaceMethodDeclarationAsync()
246+
{
247+
var testCode = @"
248+
public interface Interface
249+
{
250+
void MethodName(bool x);
251+
}
252+
253+
public class Class : Interface
254+
{
255+
public void MethodName(bool abX)
256+
{
257+
}
258+
}";
259+
260+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
261+
}
262+
263+
[Fact]
264+
public async Task TestParameterInIndirectlyImplementedInterfaceMethodDeclarationAsync()
265+
{
266+
var testCode = @"
267+
public interface Interface1
268+
{
269+
void MethodName(bool x);
270+
}
271+
272+
public interface Interface2 : Interface1
273+
{
274+
}
275+
276+
public class Class : Interface2
277+
{
278+
public void MethodName(bool abX)
279+
{
280+
}
281+
}";
282+
283+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
284+
}
285+
286+
[Fact]
287+
public async Task TestParameterInOverriddenMethodDeclarationAsync()
288+
{
289+
var testCode = @"
290+
public class BaseClass
291+
{
292+
public virtual void MethodName(bool x)
293+
{
294+
}
295+
}
296+
297+
public class SubClass : BaseClass
298+
{
299+
public override void MethodName(bool abX)
300+
{
301+
}
302+
}";
303+
304+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
305+
}
306+
307+
[Fact]
308+
public async Task TestParameterInConstructorDeclarationAsync()
309+
{
310+
var testCode = @"
311+
public class TypeName
312+
{
313+
public TypeName(string abX)
314+
{
315+
}
316+
}";
317+
318+
DiagnosticResult[] expected =
319+
{
320+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(4, 28),
321+
};
322+
323+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
324+
}
325+
326+
[Fact]
327+
public async Task TestParameterInIndexerDeclarationAsync()
328+
{
329+
var testCode = @"
330+
public class TypeName
331+
{
332+
public int this[int abX]
333+
{
334+
get { return 0; }
335+
}
336+
}";
337+
338+
DiagnosticResult[] expected =
339+
{
340+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(4, 25),
341+
};
342+
343+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
344+
}
345+
346+
[Fact]
347+
public async Task TestParameterInImplementedInterfaceIndexerMethodDeclarationAsync()
348+
{
349+
var testCode = @"
350+
public interface Interface
351+
{
352+
int this[int x] { get; }
353+
}
354+
355+
public class Class : Interface
356+
{
357+
public int this[int abX]
358+
{
359+
get { return 0; }
360+
}
361+
}";
362+
363+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
364+
}
365+
366+
[Fact]
367+
public async Task TestParameterInIndirectlyImplementedInterfaceIndexerDeclarationAsync()
368+
{
369+
var testCode = @"
370+
public interface Interface1
371+
{
372+
int this[int x] { get; }
373+
}
374+
375+
public interface Interface2 : Interface1
376+
{
377+
}
378+
379+
public class Class : Interface2
380+
{
381+
public int this[int abX]
382+
{
383+
get { return 0; }
384+
}
385+
}";
386+
387+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
388+
}
389+
390+
[Fact]
391+
public async Task TestParameterInOverriddenIndexerDeclarationAsync()
392+
{
393+
var testCode = @"
394+
public class BaseClass
395+
{
396+
public virtual int this[int x]
397+
{
398+
get { return 0; }
399+
}
400+
}
401+
402+
public class SubClass : BaseClass
403+
{
404+
public override int this[int abX]
405+
{
406+
get { return 0; }
407+
}
408+
}";
409+
410+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
411+
}
412+
413+
[Fact]
414+
public async Task TestParameterInLambdaDeclarationAsync()
415+
{
416+
var testCode = @"
417+
using System;
418+
public class TypeName
419+
{
420+
public void Method()
421+
{
422+
Func<float, float> y = (float abX) => abX;
423+
}
424+
}";
425+
426+
DiagnosticResult[] expected =
427+
{
428+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(7, 39),
429+
};
430+
431+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
432+
}
433+
434+
[Fact]
435+
public async Task TestParameterInGlobalDelegateDeclarationAsync()
436+
{
437+
var testCode = @"
438+
public delegate void Delegate(double abX);
439+
";
440+
441+
DiagnosticResult[] expected =
442+
{
443+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(2, 38),
444+
};
445+
446+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
447+
}
448+
449+
[Fact]
450+
public async Task TestParameterInInnerDelegateDeclarationAsync()
451+
{
452+
var testCode = @"
453+
public class TypeName
454+
{
455+
public delegate void Delegate(double abX);
456+
}";
457+
458+
DiagnosticResult[] expected =
459+
{
460+
this.CSharpDiagnostic().WithArguments("parameter", "abX").WithLocation(4, 42),
461+
};
462+
463+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
464+
}
465+
194466
[Fact]
195467
public async Task TestVariableInCatchDeclarationAsync()
196468
{

‎StyleCop.Analyzers/StyleCop.Analyzers/NamingRules/SA1305FieldNamesMustNotUseHungarianNotation.cs‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.NamingRules
55
{
66
using System;
77
using System.Collections.Immutable;
8+
using System.Linq;
89
using System.Text.RegularExpressions;
910
using Helpers;
1011
using Microsoft.CodeAnalysis;
@@ -67,6 +68,7 @@ internal class SA1305FieldNamesMustNotUseHungarianNotation : DiagnosticAnalyzer
6768
private static readonly Regex HungarianRegex = new Regex(@"^(?<notation>[a-z]{1,2})[A-Z]");
6869

6970
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> VariableDeclarationAction = Analyzer.HandleVariableDeclaration;
71+
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> ParameterDeclarationAction = Analyzer.HandleParameterDeclaration;
7072
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> CatchDeclarationAction = Analyzer.HandleCatchDeclaration;
7173
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> QueryContinuationAction = Analyzer.HandleQueryContinuation;
7274
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> FromClauseAction = Analyzer.HandleFromClause;
@@ -86,6 +88,7 @@ public override void Initialize(AnalysisContext context)
8688
context.EnableConcurrentExecution();
8789

8890
context.RegisterSyntaxNodeAction(VariableDeclarationAction, SyntaxKind.VariableDeclaration);
91+
context.RegisterSyntaxNodeAction(ParameterDeclarationAction, SyntaxKind.Parameter);
8992
context.RegisterSyntaxNodeAction(CatchDeclarationAction, SyntaxKind.CatchDeclaration);
9093
context.RegisterSyntaxNodeAction(QueryContinuationAction, SyntaxKind.QueryContinuation);
9194
context.RegisterSyntaxNodeAction(FromClauseAction, SyntaxKind.FromClause);
@@ -111,7 +114,7 @@ public static void HandleVariableDeclaration(SyntaxNodeAnalysisContext context,
111114
return;
112115
}
113116

114-
var fieldDeclaration = syntax.Parent.IsKind(SyntaxKind.FieldDeclaration);
117+
var declarationType = syntax.Parent.IsKind(SyntaxKind.FieldDeclaration) ? "field" : "variable";
115118
foreach (var variableDeclarator in syntax.Variables)
116119
{
117120
if (variableDeclarator == null)
@@ -120,10 +123,37 @@ public static void HandleVariableDeclaration(SyntaxNodeAnalysisContext context,
120123
}
121124

122125
var identifier = variableDeclarator.Identifier;
123-
CheckIdentifier(context, identifier, settings, fieldDeclaration);
126+
CheckIdentifier(context, identifier, settings, declarationType);
124127
}
125128
}
126129

130+
public static void HandleParameterDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
131+
{
132+
var parameter = (ParameterSyntax)context.Node;
133+
134+
if (NamedTypeHelpers.IsContainedInNativeMethodsClass(parameter))
135+
{
136+
return;
137+
}
138+
139+
// Only parameters from method declarations can be exempt from this rule
140+
var memberDeclaration = parameter?.Parent?.Parent as MemberDeclarationSyntax;
141+
if (memberDeclaration != null)
142+
{
143+
var semanticModel = context.SemanticModel;
144+
var symbol = semanticModel.GetDeclaredSymbol(memberDeclaration);
145+
if (symbol != null)
146+
{
147+
if (symbol.IsOverride || NamedTypeHelpers.IsImplementingAnInterfaceMember(symbol))
148+
{
149+
return;
150+
}
151+
}
152+
}
153+
154+
CheckIdentifier(context, parameter.Identifier, settings, "parameter");
155+
}
156+
127157
public static void HandleCatchDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
128158
{
129159
CheckIdentifier(context, ((CatchDeclarationSyntax)context.Node).Identifier, settings);
@@ -159,7 +189,7 @@ public static void HandleForEachStatement(SyntaxNodeAnalysisContext context, Sty
159189
CheckIdentifier(context, ((ForEachStatementSyntax)context.Node).Identifier, settings);
160190
}
161191

162-
private static void CheckIdentifier(SyntaxNodeAnalysisContext context, SyntaxToken identifier, StyleCopSettings settings, bool fieldDeclaration = false)
192+
private static void CheckIdentifier(SyntaxNodeAnalysisContext context, SyntaxToken identifier, StyleCopSettings settings, string declarationType = "variable")
163193
{
164194
if (identifier.IsMissing)
165195
{
@@ -190,7 +220,7 @@ private static void CheckIdentifier(SyntaxNodeAnalysisContext context, SyntaxTok
190220
}
191221

192222
// Variable names must begin with lower-case letter
193-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, identifier.GetLocation(), fieldDeclaration ? "field" : "variable", name));
223+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, identifier.GetLocation(), declarationType, name));
194224
}
195225
}
196226
}

‎documentation/KnownChanges.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ var a = new[] { 1, 2, 3 }.ToArray();
154154
### SA1208
155155

156156
StyleCop Analyzers only considers using directives to be "System" using directives if they are not alias-qualified,
157-
while StyleCop Classic ignored the alias. For example, `using global::System;` would be not be considered a System using
157+
while StyleCop Classic ignored the alias. For example, `using global::System;` would not be considered a System using
158158
directive by StyleCop Analyzers, but it would be considered a System using directive by StyleCop Classic.
159159

160160
### SA1210
@@ -196,6 +196,9 @@ upper-case letter.
196196

197197
This rule is disabled by default in StyleCop Analyzers, but can be enabled by users via a rule set file.
198198

199+
:warning: StyleCop Analyzers does not report SA1305 for parameters in overriding methods and methods which implement an
200+
interface. StyleCop Classic reported SA1305 for all methods.
201+
199202
## Maintainability Rules
200203

201204
There are no known changes at this time.

0 commit comments

Comments
 (0)