Skip to content

Commit

Permalink
Improve IsValidNewMemberName handling of parameters and locals
Browse files Browse the repository at this point in the history 
Fixes #1315
  • Loading branch information
@sharwell
sharwell committed Nov 28, 2015
1 parent 3e23b10 commit 15bbb60
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 67 deletions.
208 changes: 188 additions & 20 deletions StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/RenameHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
namespace StyleCop.Analyzers.Helpers
{
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Rename;

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

public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol symbol, string name)
public static async Task<bool> IsValidNewMemberNameAsync(SemanticModel semanticModel, ISymbol symbol, string name, CancellationToken cancellationToken)
{
if (symbol.Kind == SymbolKind.NamedType)
{
Expand All @@ -47,39 +50,204 @@ public static bool IsValidNewMemberName(SemanticModel semanticModel, ISymbol sym
}
}

var containingSymbol = symbol.ContainingSymbol as INamespaceOrTypeSymbol;
if (containingSymbol == null)
var containingSymbol = symbol.ContainingSymbol;

var containingNamespaceOrTypeSymbol = containingSymbol as INamespaceOrTypeSymbol;
if (containingNamespaceOrTypeSymbol != null)
{
if (containingNamespaceOrTypeSymbol.Kind == SymbolKind.Namespace)
{
// Make sure to use the compilation namespace so interfaces in referenced assemblies are considered
containingNamespaceOrTypeSymbol = semanticModel.Compilation.GetCompilationNamespace((INamespaceSymbol)containingNamespaceOrTypeSymbol);
}
else if (containingNamespaceOrTypeSymbol.Kind == SymbolKind.NamedType)
{
TypeKind typeKind = ((INamedTypeSymbol)containingNamespaceOrTypeSymbol).TypeKind;

// If the containing type is a class or struct, the name can't be the same as the name of the containing
// type.
if ((typeKind == TypeKind.Class || typeKind == TypeKind.Struct)
&& containingNamespaceOrTypeSymbol.Name == name)
{
return false;
}
}

// The name can't be the same as the name of an other member of the same type. At this point no special
// consideration is given to overloaded methods.
ImmutableArray<ISymbol> siblings = containingNamespaceOrTypeSymbol.GetMembers(name);
if (!siblings.IsDefaultOrEmpty)
{
return false;
}

return true;
}

if (containingSymbol.Kind == SymbolKind.Namespace)
else if (containingSymbol.Kind == SymbolKind.Method)
{
// Make sure to use the compilation namespace so interfaces in referenced assemblies are considered
containingSymbol = semanticModel.Compilation.GetCompilationNamespace((INamespaceSymbol)containingSymbol);
IMethodSymbol methodSymbol = (IMethodSymbol)containingSymbol;
if (methodSymbol.Parameters.Any(i => i.Name == name)
|| methodSymbol.TypeParameters.Any(i => i.Name == name))
{
return false;
}

IMethodSymbol outermostMethod = methodSymbol;
while (outermostMethod.ContainingSymbol.Kind == SymbolKind.Method)
{
outermostMethod = (IMethodSymbol)outermostMethod.ContainingSymbol;
if (outermostMethod.Parameters.Any(i => i.Name == name)
|| outermostMethod.TypeParameters.Any(i => i.Name == name))
{
return false;
}
}

foreach (var syntaxReference in outermostMethod.DeclaringSyntaxReferences)
{
SyntaxNode syntaxNode = await syntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false);
LocalNameFinder localNameFinder = new LocalNameFinder(name);
localNameFinder.Visit(syntaxNode);
if (localNameFinder.Found)
{
return false;
}
}

return true;
}
else if (containingSymbol.Kind == SymbolKind.NamedType)
else
{
TypeKind typeKind = ((INamedTypeSymbol)containingSymbol).TypeKind;
return true;
}
}

// If the containing type is a class or struct, the name can't be the same as the name of the containing
// type.
if ((typeKind == TypeKind.Class || typeKind == TypeKind.Struct)
&& containingSymbol.Name == name)
public static SyntaxNode GetParentDeclaration(SyntaxToken token)
{
SyntaxNode parent = token.Parent;

while (parent != null)
{
switch (parent.Kind())
{
return false;
case SyntaxKind.VariableDeclarator:
case SyntaxKind.Parameter:
case SyntaxKind.TypeParameter:
case SyntaxKind.CatchDeclaration:
case SyntaxKind.ExternAliasDirective:
case SyntaxKind.QueryContinuation:
case SyntaxKind.FromClause:
case SyntaxKind.LetClause:
case SyntaxKind.JoinClause:
case SyntaxKind.JoinIntoClause:
case SyntaxKind.ForEachStatement:
case SyntaxKind.UsingDirective:
case SyntaxKind.LabeledStatement:
case SyntaxKind.AnonymousObjectMemberDeclarator:
return parent;

default:
var declarationParent = parent as MemberDeclarationSyntax;
if (declarationParent != null)
{
return declarationParent;
}

break;
}

parent = parent.Parent;
}

return null;
}

private class LocalNameFinder : CSharpSyntaxWalker
{
private readonly string name;

public LocalNameFinder(string name)
{
this.name = name;
}

public bool Found
{
get;
private set;
}

// The name can't be the same as the name of an other member of the same type. At this point no special
// consideration is given to overloaded methods.
ImmutableArray<ISymbol> siblings = containingSymbol.GetMembers(name);
if (!siblings.IsDefaultOrEmpty)
public override void VisitVariableDeclarator(VariableDeclaratorSyntax node)
{
return false;
this.Found |= node.Identifier.ValueText == this.name;
base.VisitVariableDeclarator(node);
}

return true;
public override void VisitParameter(ParameterSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitParameter(node);
}

public override void VisitTypeParameter(TypeParameterSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitTypeParameter(node);
}

public override void VisitCatchDeclaration(CatchDeclarationSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitCatchDeclaration(node);
}

public override void VisitQueryContinuation(QueryContinuationSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitQueryContinuation(node);
}

public override void VisitFromClause(FromClauseSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitFromClause(node);
}

public override void VisitLetClause(LetClauseSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitLetClause(node);
}

public override void VisitJoinClause(JoinClauseSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitJoinClause(node);
}

public override void VisitJoinIntoClause(JoinIntoClauseSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitJoinIntoClause(node);
}

public override void VisitForEachStatement(ForEachStatementSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitForEachStatement(node);
}

public override void VisitLabeledStatement(LabeledStatementSyntax node)
{
this.Found |= node.Identifier.ValueText == this.name;
base.VisitLabeledStatement(node);
}

public override void VisitAnonymousObjectMemberDeclarator(AnonymousObjectMemberDeclaratorSyntax node)
{
this.Found |= node.NameEquals?.Name?.Identifier.ValueText == this.name;
base.VisitAnonymousObjectMemberDeclarator(node);
}
}
}
}
50 changes: 37 additions & 13 deletions ...op.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/RenameToLowerCaseCodeFixProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace StyleCop.Analyzers.NamingRules
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;

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

// only offer a codefix if the name does not consist of only underscores.
if (!string.IsNullOrEmpty(newName))
{
newName = char.ToLower(newName[0]) + newName.Substring(1);
context.RegisterCodeFix(
CodeAction.Create(
string.Format(NamingResources.RenameToCodeFix, newName),
cancellationToken => RenameHelper.RenameSymbolAsync(document, root, token, newName, cancellationToken),
nameof(RenameToLowerCaseCodeFixProvider)),
diagnostic);
}
var originalName = token.ValueText;
var baseName = originalName.TrimStart('_');
if (baseName.Length == 0)
{
// only offer a code fix if the name does not consist of only underscores.
continue;
}

baseName = char.ToLower(baseName[0]) + baseName.Substring(1);
int underscoreCount = originalName.Length - baseName.Length;
var newName = baseName;
var memberSyntax = RenameHelper.GetParentDeclaration(token);

SemanticModel semanticModel = await document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);

var declaredSymbol = semanticModel.GetDeclaredSymbol(memberSyntax);
if (declaredSymbol == null)
{
continue;
}

int index = 0;
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
{
index++;
newName = baseName + index;
}

context.RegisterCodeFix(
CodeAction.Create(
string.Format(NamingResources.RenameToCodeFix, newName),
cancellationToken => RenameHelper.RenameSymbolAsync(document, root, token, newName, cancellationToken),
nameof(RenameToLowerCaseCodeFixProvider) + "_" + underscoreCount + "_" + index),
diagnostic);
}
}
}
Expand Down
25 changes: 3 additions & 22 deletions ...op.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/RenameToUpperCaseCodeFixProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var token = root.FindToken(diagnostic.Location.SourceSpan.Start);
var baseName = char.ToUpper(token.ValueText[0]) + token.ValueText.Substring(1);
var newName = baseName;
var memberSyntax = GetParentTypeDeclaration(token);
var memberSyntax = RenameHelper.GetParentDeclaration(token);

if (memberSyntax is NamespaceDeclarationSyntax)
{
Expand Down Expand Up @@ -90,14 +90,14 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
}

bool usedSuffix = false;
if (declaredSymbol.Kind == SymbolKind.Field && !RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
if (declaredSymbol.Kind == SymbolKind.Field && !await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
{
usedSuffix = true;
newName = newName + Suffix;
}

int index = 0;
while (!RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, context.CancellationToken).ConfigureAwait(false))
{
usedSuffix = false;
index++;
Expand All @@ -113,24 +113,5 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
}
}
}

private static SyntaxNode GetParentTypeDeclaration(SyntaxToken token)
{
SyntaxNode parent = token.Parent;

while (parent != null)
{
var declarationParent = parent as MemberDeclarationSyntax
?? (SyntaxNode)(parent as VariableDeclaratorSyntax);
if (declarationParent != null)
{
return declarationParent;
}

parent = parent.Parent;
}

return null;
}
}
}
2 changes: 1 addition & 1 deletion StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/NamingRules/SA1302CodeFixProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private static async Task<Solution> CreateChangedSolutionAsync(Document document

var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var declaredSymbol = semanticModel.GetDeclaredSymbol(token.Parent, cancellationToken);
while (!RenameHelper.IsValidNewMemberName(semanticModel, declaredSymbol, newName))
while (!await RenameHelper.IsValidNewMemberNameAsync(semanticModel, declaredSymbol, newName, cancellationToken).ConfigureAwait(false))
{
index++;
newName = baseName + index;
Expand Down
Loading

0 comments on commit 15bbb60

Please sign in to comment.