Skip to content

Commit 15bbb60

Browse files
sharwellsharwell
committed
Improve IsValidNewMemberName handling of parameters and locals
Fixes #1315
1 parent 3e23b10 commit 15bbb60

6 files changed

Lines changed: 240 additions & 67 deletions

File tree

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

Lines changed: 188 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
namespace StyleCop.Analyzers.Helpers
55
{
66
using System.Collections.Immutable;
7+
using System.Linq;
78
using System.Threading;
89
using System.Threading.Tasks;
910
using Microsoft.CodeAnalysis;
1011
using Microsoft.CodeAnalysis.CodeActions;
12+
using Microsoft.CodeAnalysis.CSharp;
13+
using Microsoft.CodeAnalysis.CSharp.Syntax;
1114
using Microsoft.CodeAnalysis.Rename;
1215

1316
internal static class RenameHelper
@@ -30,7 +33,7 @@ public static async Task<Solution> RenameSymbolAsync(Document document, SyntaxNo
3033
return newSolution;
3134
}
3235

33-
public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol symbol, string name)
36+
public static async Task<bool> IsValidNewMemberNameAsync(SemanticModel semanticModel, ISymbol symbol, string name, CancellationToken cancellationToken)
3437
{
3538
if (symbol.Kind == SymbolKind.NamedType)
3639
{
@@ -47,39 +50,204 @@ public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol sym
4750
}
4851
}
4952

50-
var containingSymbol = symbol.ContainingSymbol as INamespaceOrTypeSymbol;
51-
if (containingSymbol == null)
53+
var containingSymbol = symbol.ContainingSymbol;
54+
55+
var containingNamespaceOrTypeSymbol = containingSymbol as INamespaceOrTypeSymbol;
56+
if (containingNamespaceOrTypeSymbol != null)
5257
{
58+
if (containingNamespaceOrTypeSymbol.Kind == SymbolKind.Namespace)
59+
{
60+
// Make sure to use the compilation namespace so interfaces in referenced assemblies are considered
61+
containingNamespaceOrTypeSymbol = semanticModel.Compilation.GetCompilationNamespace((INamespaceSymbol)containingNamespaceOrTypeSymbol);
62+
}
63+
else if (containingNamespaceOrTypeSymbol.Kind == SymbolKind.NamedType)
64+
{
65+
TypeKind typeKind = ((INamedTypeSymbol)containingNamespaceOrTypeSymbol).TypeKind;
66+
67+
// If the containing type is a class or struct, the name can't be the same as the name of the containing
68+
// type.
69+
if ((typeKind == TypeKind.Class || typeKind == TypeKind.Struct)
70+
&& containingNamespaceOrTypeSymbol.Name == name)
71+
{
72+
return false;
73+
}
74+
}
75+
76+
// The name can't be the same as the name of an other member of the same type. At this point no special
77+
// consideration is given to overloaded methods.
78+
ImmutableArray<ISymbol> siblings = containingNamespaceOrTypeSymbol.GetMembers(name);
79+
if (!siblings.IsDefaultOrEmpty)
80+
{
81+
return false;
82+
}
83+
5384
return true;
5485
}
55-
56-
if (containingSymbol.Kind == SymbolKind.Namespace)
86+
else if (containingSymbol.Kind == SymbolKind.Method)
5787
{
58-
// Make sure to use the compilation namespace so interfaces in referenced assemblies are considered
59-
containingSymbol = semanticModel.Compilation.GetCompilationNamespace((INamespaceSymbol)containingSymbol);
88+
IMethodSymbol methodSymbol = (IMethodSymbol)containingSymbol;
89+
if (methodSymbol.Parameters.Any(i => i.Name == name)
90+
|| methodSymbol.TypeParameters.Any(i => i.Name == name))
91+
{
92+
return false;
93+
}
94+
95+
IMethodSymbol outermostMethod = methodSymbol;
96+
while (outermostMethod.ContainingSymbol.Kind == SymbolKind.Method)
97+
{
98+
outermostMethod = (IMethodSymbol)outermostMethod.ContainingSymbol;
99+
if (outermostMethod.Parameters.Any(i => i.Name == name)
100+
|| outermostMethod.TypeParameters.Any(i => i.Name == name))
101+
{
102+
return false;
103+
}
104+
}
105+
106+
foreach (var syntaxReference in outermostMethod.DeclaringSyntaxReferences)
107+
{
108+
SyntaxNode syntaxNode = await syntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false);
109+
LocalNameFinder localNameFinder = new LocalNameFinder(name);
110+
localNameFinder.Visit(syntaxNode);
111+
if (localNameFinder.Found)
112+
{
113+
return false;
114+
}
115+
}
116+
117+
return true;
60118
}
61-
else if (containingSymbol.Kind == SymbolKind.NamedType)
119+
else
62120
{
63-
TypeKind typeKind = ((INamedTypeSymbol)containingSymbol).TypeKind;
121+
return true;
122+
}
123+
}
64124

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)
125+
public static SyntaxNode GetParentDeclaration(SyntaxToken token)
126+
{
127+
SyntaxNode parent = token.Parent;
128+
129+
while (parent != null)
130+
{
131+
switch (parent.Kind())
69132
{
70-
return false;
133+
case SyntaxKind.VariableDeclarator:
134+
case SyntaxKind.Parameter:
135+
case SyntaxKind.TypeParameter:
136+
case SyntaxKind.CatchDeclaration:
137+
case SyntaxKind.ExternAliasDirective:
138+
case SyntaxKind.QueryContinuation:
139+
case SyntaxKind.FromClause:
140+
case SyntaxKind.LetClause:
141+
case SyntaxKind.JoinClause:
142+
case SyntaxKind.JoinIntoClause:
143+
case SyntaxKind.ForEachStatement:
144+
case SyntaxKind.UsingDirective:
145+
case SyntaxKind.LabeledStatement:
146+
case SyntaxKind.AnonymousObjectMemberDeclarator:
147+
return parent;
148+
149+
default:
150+
var declarationParent = parent as MemberDeclarationSyntax;
151+
if (declarationParent != null)
152+
{
153+
return declarationParent;
154+
}
155+
156+
break;
71157
}
158+
159+
parent = parent.Parent;
160+
}
161+
162+
return null;
163+
}
164+
165+
private class LocalNameFinder : CSharpSyntaxWalker
166+
{
167+
private readonly string name;
168+
169+
public LocalNameFinder(string name)
170+
{
171+
this.name = name;
172+
}
173+
174+
public bool Found
175+
{
176+
get;
177+
private set;
72178
}
73179

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)
180+
public override void VisitVariableDeclarator(VariableDeclaratorSyntax node)
78181
{
79-
return false;
182+
this.Found |= node.Identifier.ValueText == this.name;
183+
base.VisitVariableDeclarator(node);
80184
}
81185

82-
return true;
186+
public override void VisitParameter(ParameterSyntax node)
187+
{
188+
this.Found |= node.Identifier.ValueText == this.name;
189+
base.VisitParameter(node);
190+
}
191+
192+
public override void VisitTypeParameter(TypeParameterSyntax node)
193+
{
194+
this.Found |= node.Identifier.ValueText == this.name;
195+
base.VisitTypeParameter(node);
196+
}
197+
198+
public override void VisitCatchDeclaration(CatchDeclarationSyntax node)
199+
{
200+
this.Found |= node.Identifier.ValueText == this.name;
201+
base.VisitCatchDeclaration(node);
202+
}
203+
204+
public override void VisitQueryContinuation(QueryContinuationSyntax node)
205+
{
206+
this.Found |= node.Identifier.ValueText == this.name;
207+
base.VisitQueryContinuation(node);
208+
}
209+
210+
public override void VisitFromClause(FromClauseSyntax node)
211+
{
212+
this.Found |= node.Identifier.ValueText == this.name;
213+
base.VisitFromClause(node);
214+
}
215+
216+
public override void VisitLetClause(LetClauseSyntax node)
217+
{
218+
this.Found |= node.Identifier.ValueText == this.name;
219+
base.VisitLetClause(node);
220+
}
221+
222+
public override void VisitJoinClause(JoinClauseSyntax node)
223+
{
224+
this.Found |= node.Identifier.ValueText == this.name;
225+
base.VisitJoinClause(node);
226+
}
227+
228+
public override void VisitJoinIntoClause(JoinIntoClauseSyntax node)
229+
{
230+
this.Found |= node.Identifier.ValueText == this.name;
231+
base.VisitJoinIntoClause(node);
232+
}
233+
234+
public override void VisitForEachStatement(ForEachStatementSyntax node)
235+
{
236+
this.Found |= node.Identifier.ValueText == this.name;
237+
base.VisitForEachStatement(node);
238+
}
239+
240+
public override void VisitLabeledStatement(LabeledStatementSyntax node)
241+
{
242+
this.Found |= node.Identifier.ValueText == this.name;
243+
base.VisitLabeledStatement(node);
244+
}
245+
246+
public override void VisitAnonymousObjectMemberDeclarator(AnonymousObjectMemberDeclaratorSyntax node)
247+
{
248+
this.Found |= node.NameEquals?.Name?.Identifier.ValueText == this.name;
249+
base.VisitAnonymousObjectMemberDeclarator(node);
250+
}
83251
}
84252
}
85253
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/RenameToLowerCaseCodeFixProvider.cs‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace StyleCop.Analyzers.NamingRules
1010
using Microsoft.CodeAnalysis;
1111
using Microsoft.CodeAnalysis.CodeActions;
1212
using Microsoft.CodeAnalysis.CodeFixes;
13+
using Microsoft.CodeAnalysis.CSharp;
1314

1415
/// <summary>
1516
/// Implements a code fix for diagnostics which are fixed by renaming a symbol to start with a lower case letter.
@@ -44,22 +45,45 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
4445
foreach (var diagnostic in context.Diagnostics)
4546
{
4647
var token = root.FindToken(diagnostic.Location.SourceSpan.Start);
47-
if (!string.IsNullOrEmpty(token.ValueText))
48+
if (string.IsNullOrEmpty(token.ValueText))
4849
{
49-
var newName = token.ValueText.TrimStart('_');
50+
continue;
51+
}
5052

51-
// only offer a codefix if the name does not consist of only underscores.
52-
if (!string.IsNullOrEmpty(newName))
53-
{
54-
newName = char.ToLower(newName[0]) + newName.Substring(1);
55-
context.RegisterCodeFix(
56-
CodeAction.Create(
57-
string.Format(NamingResources.RenameToCodeFix, newName),
58-
cancellationToken => RenameHelper.RenameSymbolAsync(document, root, token, newName, cancellationToken),
59-
nameof(RenameToLowerCaseCodeFixProvider)),
60-
diagnostic);
61-
}
53+
var originalName = token.ValueText;
54+
var baseName = originalName.TrimStart('_');
55+
if (baseName.Length == 0)
56+
{
57+
// only offer a code fix if the name does not consist of only underscores.
58+
continue;
6259
}
60+
61+
baseName = char.ToLower(baseName[0]) + baseName.Substring(1);
62+
int underscoreCount = originalName.Length - baseName.Length;
63+
var newName = baseName;
64+
var memberSyntax = RenameHelper.GetParentDeclaration(token);
65+
66+
SemanticModel semanticModel = await document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
67+
68+
var declaredSymbol = semanticModel.GetDeclaredSymbol(memberSyntax);
69+
if (declaredSymbol == null)
70+
{
71+
continue;
72+
}
73+
74+
int index = 0;
75+
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
76+
{
77+
index++;
78+
newName = baseName + index;
79+
}
80+
81+
context.RegisterCodeFix(
82+
CodeAction.Create(
83+
string.Format(NamingResources.RenameToCodeFix, newName),
84+
cancellationToken => RenameHelper.RenameSymbolAsync(document, root, token, newName, cancellationToken),
85+
nameof(RenameToLowerCaseCodeFixProvider) + "_" + underscoreCount + "_" + index),
86+
diagnostic);
6387
}
6488
}
6589
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/RenameToUpperCaseCodeFixProvider.cs‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
5757
var token = root.FindToken(diagnostic.Location.SourceSpan.Start);
5858
var baseName = char.ToUpper(token.ValueText[0]) + token.ValueText.Substring(1);
5959
var newName = baseName;
60-
var memberSyntax = GetParentTypeDeclaration(token);
60+
var memberSyntax = RenameHelper.GetParentDeclaration(token);
6161

6262
if (memberSyntax is NamespaceDeclarationSyntax)
6363
{
@@ -90,14 +90,14 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
9090
}
9191

9292
bool usedSuffix = false;
93-
if (declaredSymbol.Kind == SymbolKind.Field && !RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
93+
if (declaredSymbol.Kind == SymbolKind.Field && !await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
9494
{
9595
usedSuffix = true;
9696
newName = newName + Suffix;
9797
}
9898

9999
int index = 0;
100-
while (!RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
100+
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
101101
{
102102
usedSuffix = false;
103103
index++;
@@ -113,24 +113,5 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
113113
}
114114
}
115115
}
116-
117-
private static SyntaxNode GetParentTypeDeclaration(SyntaxToken token)
118-
{
119-
SyntaxNode parent = token.Parent;
120-
121-
while (parent != null)
122-
{
123-
var declarationParent = parent as MemberDeclarationSyntax
124-
?? (SyntaxNode)(parent as VariableDeclaratorSyntax);
125-
if (declarationParent != null)
126-
{
127-
return declarationParent;
128-
}
129-
130-
parent = parent.Parent;
131-
}
132-
133-
return null;
134-
}
135116
}
136117
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/SA1302CodeFixProvider.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ private static async Task<Solution> CreateChangedSolutionAsync(Document document
5454

5555
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
5656
var declaredSymbol = semanticModel.GetDeclaredSymbol(token.Parent, cancellationToken);
57-
while (!RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
57+
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, cancellationToken).ConfigureAwait(false))
5858
{
5959
index++;
6060
newName = baseName + index;

0 commit comments

Comments
 (0)