Skip to content

Commit 3e23b10

Browse files
committed
Merge pull request #1853 from sharwell/validname-accuracy
Improve IsValidNewMemberName accuracy
2 parents 9c207c4 + b8251f9 commit 3e23b10

4 files changed

Lines changed: 75 additions & 21 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/RenameHelper.cs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace StyleCop.Analyzers.Helpers
55
{
6+
using System.Collections.Immutable;
67
using System.Threading;
78
using System.Threading.Tasks;
89
using Microsoft.CodeAnalysis;
@@ -31,10 +32,19 @@ public static async Task<Solution> RenameSymbolAsync(Document document, SyntaxNo
3132

3233
public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol symbol, string name)
3334
{
34-
var members = (symbol as INamedTypeSymbol)?.GetMembers(name);
35-
if (members.HasValue && !members.Value.IsDefaultOrEmpty)
35+
if (symbol.Kind == SymbolKind.NamedType)
3636
{
37-
return false;
37+
TypeKind typeKind = ((INamedTypeSymbol)symbol).TypeKind;
38+
39+
// If the symbol is a class or struct, the name can't be the same as any of its members.
40+
if (typeKind == TypeKind.Class || typeKind == TypeKind.Struct)
41+
{
42+
var members = (symbol as INamedTypeSymbol)?.GetMembers(name);
43+
if (members.HasValue && !members.Value.IsDefaultOrEmpty)
44+
{
45+
return false;
46+
}
47+
}
3848
}
3949

4050
var containingSymbol = symbol.ContainingSymbol as INamespaceOrTypeSymbol;
@@ -50,16 +60,21 @@ public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol sym
5060
}
5161
else if (containingSymbol.Kind == SymbolKind.NamedType)
5262
{
53-
// The name can't be the same as the name of the containing type
54-
if (containingSymbol.Name == name)
63+
TypeKind typeKind = ((INamedTypeSymbol)containingSymbol).TypeKind;
64+
65+
// If the containing type is a class or struct, the name can't be the same as the name of the containing
66+
// type.
67+
if ((typeKind == TypeKind.Class || typeKind == TypeKind.Struct)
68+
&& containingSymbol.Name == name)
5569
{
5670
return false;
5771
}
5872
}
5973

60-
// The name can't be the same as the name of an other member of the same type
61-
members = containingSymbol.GetMembers(name);
62-
if (!members.Value.IsDefaultOrEmpty)
74+
// The name can't be the same as the name of an other member of the same type. At this point no special
75+
// consideration is given to overloaded methods.
76+
ImmutableArray<ISymbol> siblings = containingSymbol.GetMembers(name);
77+
if (!siblings.IsDefaultOrEmpty)
6378
{
6479
return false;
6580
}

StyleCop.Analyzers/StyleCop.Analyzers.Test/NamingRules/SA1300UnitTests.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,25 @@ public class Test { }";
252252
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
253253
}
254254

255+
[Fact]
256+
public async Task TestLowerCaseEnumWithMemberMatchingTargetNameAsync()
257+
{
258+
var testCode = @"public enum test
259+
{
260+
Test
261+
}";
262+
var fixedCode = @"public enum Test
263+
{
264+
Test
265+
}";
266+
267+
DiagnosticResult expected = this.CSharpDiagnostic().WithArguments("test").WithLocation(1, 13);
268+
269+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
270+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
271+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
272+
}
273+
255274
[Fact]
256275
public async Task TestUpperCaseDelegateAsync()
257276
{
@@ -807,15 +826,17 @@ public event System.EventHandler fooBar
807826
808827
public void foo()
809828
{
810-
811829
}
830+
831+
public string iInterface { get; }
812832
}
813833
814834
public interface IInterface
815835
{
816836
void foo();
817837
int bar { get; }
818838
event System.EventHandler fooBar;
839+
string iInterface { get; }
819840
}";
820841
var fixedCode = @"public class TestClass : IInterface
821842
{
@@ -835,22 +856,25 @@ public event System.EventHandler FooBar
835856
836857
public void Foo()
837858
{
838-
839859
}
860+
861+
public string IInterface { get; }
840862
}
841863
842864
public interface IInterface
843865
{
844866
void Foo();
845867
int Bar { get; }
846868
event System.EventHandler FooBar;
869+
string IInterface { get; }
847870
}";
848871

849872
var expected = new[]
850873
{
851-
this.CSharpDiagnostic().WithLocation(25, 10).WithArguments("foo"),
852-
this.CSharpDiagnostic().WithLocation(26, 9).WithArguments("bar"),
853-
this.CSharpDiagnostic().WithLocation(27, 31).WithArguments("fooBar"),
874+
this.CSharpDiagnostic().WithLocation(26, 10).WithArguments("foo"),
875+
this.CSharpDiagnostic().WithLocation(27, 9).WithArguments("bar"),
876+
this.CSharpDiagnostic().WithLocation(28, 31).WithArguments("fooBar"),
877+
this.CSharpDiagnostic().WithLocation(29, 12).WithArguments("iInterface"),
854878
};
855879

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

StyleCop.Analyzers/StyleCop.Analyzers.Test/NamingRules/SA1302UnitTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,15 @@ public interface IFoo { }";
209209
}
210210

211211
[Fact]
212-
public async Task TestInterfaceDeclarationDoesNotStartWithIWithMemberConflictAsync()
212+
public async Task TestInterfaceDeclarationDoesNotStartWithIWithMemberMatchingTargetNameAsync()
213213
{
214214
string testCode = @"
215215
public interface Foo
216216
{
217217
int IFoo { get; }
218218
}";
219219
string fixedCode = @"
220-
public interface IFoo1
220+
public interface IFoo
221221
{
222222
int IFoo { get; }
223223
}";

documentation/SA1300.md

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,30 @@ The name of a C# element does not begin with an upper-case letter.
2121

2222
## Rule description
2323

24-
A violation of this rule occurs when the names of certain types of elements do not begin with an upper-case letter. The following types of elements should use an upper-case letter as the first letter of the element name: *namespaces, classes, enums, structs, delegates, events, methods,* and *properties*.
25-
26-
In addition, any field which is public, internal, or marked with the const attribute should begin with an upper-case letter. Non-private readonly fields must also be named using an upper-case letter.
27-
28-
If the field or variable name is intended to match the name of an item associated with Win32 or COM, and thus needs to begin with a lower-case letter, place the field or variable within a special *NativeMethods* class. A NativeMethods class is any class which contains a name ending in NativeMethods, and is intended as a placeholder for Win32 or COM wrappers. StyleCop will ignore this violation if the item is placed within a NativeMethods class.
24+
A violation of this rule occurs when the names of certain types of elements do not begin with an upper-case letter. The
25+
following types of elements should use an upper-case letter as the first letter of the element name:
26+
27+
* Namespaces
28+
* Classes
29+
* Enums
30+
* Structs
31+
* Delegates
32+
* Events
33+
* Methods
34+
* Properties
35+
36+
In addition, any field which is public, internal, or marked with the const attribute should begin with an upper-case
37+
letter. Non-private readonly fields must also be named using an upper-case letter.
38+
39+
If the field or variable name is intended to match the name of an item associated with Win32 or COM, and thus needs to
40+
begin with a lower-case letter, place the field or variable within a special `NativeMethods` class. A `NativeMethods`
41+
class is any class which contains a name ending in `NativeMethods`, and is intended as a placeholder for Win32 or COM
42+
wrappers. StyleCop will ignore this violation if the item is placed within a `NativeMethods` class.
2943

3044
## How to fix violations
3145

32-
To fix a violation of this rule, change the name of the element so that it begins with an upper-case letter, or place the item within a NativeMethods class if appropriate.
46+
To fix a violation of this rule, change the name of the element so that it begins with an upper-case letter, or place
47+
the item within a `NativeMethods` class if appropriate.
3348

3449
## How to suppress violations
3550

0 commit comments

Comments
 (0)