Skip to content
This repository was archived by the owner on Jul 12, 2022. It is now read-only.

Commit 8ffa8f9

Browse files
jaredparjaredpar
committed
Cleaned up the private field rename
The private field rename rule was spending a good chunk of time in Linq.Any methods. Spoke with the Roslyn team about the pattern and they suggested using rewriters as it was a more established way for doing that kind of edit.
1 parent 9a136fb commit 8ffa8f9

File tree

1 file changed

+182
-81
lines changed

1 file changed

+182
-81
lines changed

‎src/Microsoft.DotNet.CodeFormatting/Rules/PrivateFieldNamingRule.cs

Lines changed: 182 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,102 @@
1818
namespace Microsoft.DotNet.CodeFormatting.Rules
1919
{
2020
[RuleOrder(RuleOrder.PrivateFieldNamingRule)]
21-
// TODO Bug 1086632: Deactivated due to active bug in Roslyn.
22-
// There is a hack to run this rule, but it's slow.
23-
// If needed, enable the rule and enable the hack at the code below in RenameFields.
2421
internal sealed class PrivateFieldNamingRule : IFormattingRule
2522
{
23+
/// <summary>
24+
/// This will add an annotation to any private field that needs to be renamed.
25+
/// </summary>
26+
private sealed class PrivateFieldAnnotationsRewriter : CSharpSyntaxRewriter
27+
{
28+
internal readonly static SyntaxAnnotation Marker = new SyntaxAnnotation("PrivateFieldToRename");
29+
30+
// Used to avoid the array allocation on calls to WithAdditionalAnnotations
31+
private readonly static SyntaxAnnotation[] s_markerArray;
32+
33+
static PrivateFieldAnnotationsRewriter()
34+
{
35+
s_markerArray = new SyntaxAnnotation[] { Marker };
36+
}
37+
38+
private int _count;
39+
40+
internal static SyntaxNode AddAnnotations(SyntaxNode node, out int count)
41+
{
42+
var rewriter = new PrivateFieldAnnotationsRewriter();
43+
var newNode = rewriter.Visit(node);
44+
count = rewriter._count;
45+
return newNode;
46+
}
47+
48+
public override SyntaxNode VisitFieldDeclaration(FieldDeclarationSyntax node)
49+
{
50+
if (NeedsRewrite(node))
51+
{
52+
var list = new List<VariableDeclaratorSyntax>(node.Declaration.Variables.Count);
53+
foreach (var v in node.Declaration.Variables)
54+
{
55+
if (IsBadName(v))
56+
{
57+
list.Add(v.WithAdditionalAnnotations(Marker));
58+
_count++;
59+
}
60+
else
61+
{
62+
list.Add(v);
63+
}
64+
}
65+
66+
var declaration = node.Declaration.WithVariables(SyntaxFactory.SeparatedList(list));
67+
node = node.WithDeclaration(declaration);
68+
69+
return node;
70+
}
71+
72+
return node;
73+
}
74+
75+
private static bool NeedsRewrite(FieldDeclarationSyntax fieldSyntax)
76+
{
77+
if (!IsPrivateField(fieldSyntax))
78+
{
79+
return false;
80+
}
81+
82+
foreach (var v in fieldSyntax.Declaration.Variables)
83+
{
84+
if (IsBadName(v))
85+
{
86+
return true;
87+
}
88+
}
89+
90+
return false;
91+
}
92+
93+
private static bool IsBadName(VariableDeclaratorSyntax node)
94+
{
95+
var name = node.Identifier.ValueText;
96+
return name.Length > 0 && name[0] != '_';
97+
}
98+
99+
private static bool IsPrivateField(FieldDeclarationSyntax fieldSyntax)
100+
{
101+
foreach (var modifier in fieldSyntax.Modifiers)
102+
{
103+
switch (modifier.CSharpKind())
104+
{
105+
case SyntaxKind.PublicKeyword:
106+
case SyntaxKind.ConstKeyword:
107+
case SyntaxKind.InternalKeyword:
108+
case SyntaxKind.ProtectedKeyword:
109+
return false;
110+
}
111+
}
112+
113+
return true;
114+
}
115+
}
116+
26117
/// <summary>
27118
/// This rewriter exists to work around DevDiv 1086632 in Roslyn. The Rename action is
28119
/// leaving a set of annotations in the tree. These annotations slow down further processing
@@ -31,126 +122,136 @@ internal sealed class PrivateFieldNamingRule : IFormattingRule
31122
/// </summary>
32123
private sealed class RemoveRenameAnnotationsRewriter : CSharpSyntaxRewriter
33124
{
125+
private const string RenameAnnotationName = "Rename";
34126

35-
}
127+
public override SyntaxNode Visit(SyntaxNode node)
128+
{
129+
node = base.Visit(node);
130+
if (node != null && node.ContainsAnnotations && node.GetAnnotations(RenameAnnotationName).Any())
131+
{
132+
node = node.WithoutAnnotations(RenameAnnotationName);
133+
}
134+
135+
return node;
136+
}
137+
138+
public override SyntaxToken VisitToken(SyntaxToken token)
139+
{
140+
if (token.ContainsAnnotations && token.GetAnnotations(RenameAnnotationName).Any())
141+
{
142+
token = token.WithoutAnnotations(RenameAnnotationName);
143+
}
36144

37-
private static string[] s_keywordsToIgnore = { "public", "internal", "protected", "const" };
38-
private static readonly SyntaxAnnotation s_annotationMarker = new SyntaxAnnotation();
39-
private static readonly Regex s_regex = new Regex("^._");
145+
return token;
146+
}
147+
}
40148

41149
public async Task<Document> ProcessAsync(Document document, CancellationToken cancellationToken)
42150
{
43151
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken) as CSharpSyntaxNode;
44152
if (syntaxRoot == null)
45-
return document;
46-
47-
var typeNodes = syntaxRoot.DescendantNodes().Where(type => type.CSharpKind() == SyntaxKind.ClassDeclaration || type.CSharpKind() == SyntaxKind.StructDeclaration);
48-
IEnumerable<SyntaxNode> privateFields = GetPrivateFields(typeNodes);
49-
if (privateFields.Any())
50153
{
51-
var solutionWithAnnotation = GetSolutionWithRenameAnnotation(document, syntaxRoot, privateFields, cancellationToken);
52-
var solution = await RenameFields(solutionWithAnnotation, document.Id, privateFields, cancellationToken);
53-
return solution.GetDocument(document.Id);
154+
return document;
54155
}
55156

56-
return document;
57-
}
157+
int count;
158+
var newSyntaxRoot = PrivateFieldAnnotationsRewriter.AddAnnotations(syntaxRoot, out count);
58159

59-
private IEnumerable<SyntaxNode> GetPrivateFields(IEnumerable<SyntaxNode> typeNodes)
60-
{
61-
IEnumerable<SyntaxNode> privateFields = Enumerable.Empty<SyntaxNode>();
62-
foreach (var type in typeNodes)
160+
if (count == 0)
63161
{
64-
privateFields = privateFields.Concat(type.ChildNodes().OfType<FieldDeclarationSyntax>()
65-
.Where(f => !s_keywordsToIgnore.Any(f.Modifiers.ToString().Contains)).SelectMany(f => f.Declaration.Variables))
66-
.Where(v => !(v as VariableDeclaratorSyntax).Identifier.Text.StartsWith("_"));
162+
return document;
67163
}
68164

69-
return privateFields;
165+
var documentId = document.Id;
166+
var solution = document.Project.Solution;
167+
solution = solution.WithDocumentSyntaxRoot(documentId, newSyntaxRoot);
168+
solution = await RenameFields(solution, documentId, count, cancellationToken);
169+
return solution.GetDocument(documentId);
70170
}
71171

72-
private Solution GetSolutionWithRenameAnnotation(Document document, SyntaxNode syntaxRoot, IEnumerable<SyntaxNode> privateFields, CancellationToken cancellationToken)
172+
private static async Task<Solution> RenameFields(Solution solution, DocumentId documentId, int count, CancellationToken cancellationToken)
73173
{
74-
Func<SyntaxNode, SyntaxNode, SyntaxNode> addAnnotation = (variable, dummy) =>
174+
Solution oldSolution = null;
175+
for (int i = 0; i < count; i++)
75176
{
76-
return variable.WithAdditionalAnnotations(s_annotationMarker);
77-
};
177+
// If this is not the first field then clean up the Rename annotations left
178+
// in the tree.
179+
if (i > 0)
180+
{
181+
solution = await CleanSolutionAsync(solution, oldSolution, cancellationToken);
182+
}
78183

79-
return document.WithSyntaxRoot(syntaxRoot.ReplaceNodes(privateFields, addAnnotation)).Project.Solution;
80-
}
184+
oldSolution = solution;
81185

82-
private async Task<Solution> RenameFields(Solution solution, DocumentId documentId, IEnumerable<SyntaxNode> privateFields, CancellationToken cancellationToken)
83-
{
84-
int count = privateFields.Count();
85-
for (int i = 0; i < count; i++)
86-
{
87-
// This is a hack to till the roslyn bug is fixed. Very slow, enable this statement only if the rule is enabled.
88-
solution = await CleanSolutionAsync(solution, cancellationToken);
89-
var model = await solution.GetDocument(documentId).GetSemanticModelAsync(cancellationToken);
90-
var root = await model.SyntaxTree.GetRootAsync(cancellationToken) as CSharpSyntaxNode;
91-
var symbol = model.GetDeclaredSymbol(root.GetAnnotatedNodes(s_annotationMarker).ElementAt(i), cancellationToken);
92-
var newName = GetNewSymbolName(symbol);
93-
solution = await Renamer.RenameSymbolAsync(solution, symbol, newName, solution.Workspace.Options, cancellationToken).ConfigureAwait(false);
186+
var semanticModel = await solution.GetDocument(documentId).GetSemanticModelAsync(cancellationToken);
187+
var root = await semanticModel.SyntaxTree.GetRootAsync(cancellationToken) as CSharpSyntaxNode;
188+
var declaration = root.GetAnnotatedNodes(PrivateFieldAnnotationsRewriter.Marker).ElementAt(i);
189+
var fieldSymbol = (IFieldSymbol)semanticModel.GetDeclaredSymbol(declaration, cancellationToken);
190+
var newName = GetNewFieldName(fieldSymbol);
191+
solution = await Renamer.RenameSymbolAsync(solution, fieldSymbol, newName, solution.Workspace.Options, cancellationToken).ConfigureAwait(false);
94192
}
95193

96194
return solution;
97195
}
98196

99-
private static string GetNewSymbolName(ISymbol symbol)
197+
private static string GetNewFieldName(IFieldSymbol fieldSymbol)
100198
{
101-
var symbolName = symbol.Name.TrimStart('_');
102-
if (s_regex.IsMatch(symbolName)) symbolName = symbolName.Remove(0, 2);
103-
if (symbol.IsStatic)
199+
var name = fieldSymbol.Name;
200+
if (name.Length > 1)
104201
{
105-
// Check for ThreadStatic private fields.
106-
if (symbol.GetAttributes().Any(a => a.AttributeClass.Name.Equals("ThreadStaticAttribute")))
202+
if (name[0] == '_')
203+
{
204+
name = name.TrimStart('_');
205+
}
206+
else if (char.IsLetter(name[0]) && name[1] == '_')
107207
{
108-
if (!symbolName.StartsWith("t_", StringComparison.OrdinalIgnoreCase))
109-
return "t_" + symbolName;
208+
name = name.Substring(2);
110209
}
111-
else if (!symbolName.StartsWith("s_", StringComparison.OrdinalIgnoreCase))
112-
return "s_" + symbolName;
210+
}
113211

114-
return symbolName;
212+
if (fieldSymbol.IsStatic)
213+
{
214+
// Check for ThreadStatic private fields.
215+
if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass.Name.Equals("ThreadStaticAttribute", StringComparison.Ordinal)))
216+
{
217+
return "t_" + name;
218+
}
219+
else
220+
{
221+
return "s_" + name;
222+
}
115223
}
116224

117-
return "_" + symbolName;
225+
return "_" + name;
118226
}
119227

120-
private async Task<Solution> CleanSolutionAsync(Solution solution, CancellationToken cancellationToken)
228+
private static async Task<Solution> CleanSolutionAsync(Solution newSolution, Solution oldSolution, CancellationToken cancellationToken)
121229
{
122-
var documentIdsToProcess = solution.Projects.SelectMany(p => p.DocumentIds).ToList();
123-
const string rename = "Rename";
124-
foreach (var documentId in documentIdsToProcess)
125-
{
126-
var root = await solution.GetDocument(documentId).GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
230+
var rewriter = new RemoveRenameAnnotationsRewriter();
231+
var solution = newSolution;
127232

128-
while (true)
233+
foreach (var projectChange in newSolution.GetChanges(oldSolution).GetProjectChanges())
234+
{
235+
foreach (var documentId in projectChange.GetChangedDocuments())
129236
{
130-
var renameNodes = root.DescendantNodes(descendIntoTrivia: true).Where(s => s.GetAnnotations(rename).Any());
131-
if (!renameNodes.Any())
132-
{
133-
break;
134-
}
135-
136-
root = root.ReplaceNode(renameNodes.First(), renameNodes.First().WithoutAnnotations(rename));
237+
solution = await CleanSolutionDocument(rewriter, solution, documentId, cancellationToken);
137238
}
239+
}
138240

139-
while (true)
140-
{
141-
var renameTokens = root.DescendantTokens(descendIntoTrivia: true).Where(s => s.GetAnnotations(rename).Any());
142-
if (!renameTokens.Any())
143-
{
144-
break;
145-
}
146-
147-
root = root.ReplaceToken(renameTokens.First(), renameTokens.First().WithoutAnnotations(rename));
148-
}
241+
return solution;
242+
}
149243

150-
solution = solution.WithDocumentSyntaxRoot(documentId, root);
244+
private static async Task<Solution> CleanSolutionDocument(RemoveRenameAnnotationsRewriter rewriter, Solution solution, DocumentId documentId, CancellationToken cancellationToken)
245+
{
246+
var document = solution.GetDocument(documentId);
247+
var syntaxNode = await document.GetSyntaxRootAsync(cancellationToken) as CSharpSyntaxNode;
248+
if (syntaxNode == null)
249+
{
250+
return solution;
151251
}
152252

153-
return solution;
253+
var newNode = rewriter.Visit(syntaxNode);
254+
return solution.WithDocumentSyntaxRoot(documentId, newNode);
154255
}
155256
}
156257
}

0 commit comments

Comments
 (0)