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

Commit b78f11d

Browse files
committed
Responded to PR feedback
1 parent 122ba74 commit b78f11d

File tree

4 files changed

+108
-26
lines changed

4 files changed

+108
-26
lines changed

src/CodeFormatter.sln

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".nuget", ".nuget", "{34034F
1414
.nuget\packages.config = .nuget\packages.config
1515
EndProjectSection
1616
EndProject
17-
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{297488BF-08A5-4D06-A0C3-B1623F49162A}"
18-
EndProject
1917
Global
2018
GlobalSection(SolutionConfigurationPlatforms) = preSolution
2119
Debug|Any CPU = Debug|Any CPU

src/Microsoft.DotNet.CodeFormatting.Tests/Rules/PrivateFieldNamingRuleTests.cs

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ namespace Microsoft.DotNet.CodeFormatting.Tests
77
{
88
public class PrivateFieldNamingRuleTests : CodeFormattingTestBase
99
{
10+
internal override IFormattingRule GetFormattingRule()
11+
{
12+
return new Rules.PrivateFieldNamingRule();
13+
}
14+
1015
[Fact]
1116
public void TestUnderScoreInPrivateFields()
1217
{
@@ -43,9 +48,70 @@ class T
4348
Verify(text, expected);
4449
}
4550

46-
internal override IFormattingRule GetFormattingRule()
51+
[Fact]
52+
public void CornerCaseNames()
4753
{
48-
return new Rules.PrivateFieldNamingRule();
54+
var text = @"
55+
class C
56+
{
57+
private int x_;
58+
private int _;
59+
private int __;
60+
private int m_field1;
61+
private int field2_;
62+
";
63+
64+
var expected = @"
65+
class C
66+
{
67+
private int _x;
68+
private int _;
69+
private int __;
70+
private int _field1;
71+
private int _field2;
72+
";
73+
74+
Verify(text, expected, runFormatter: false);
75+
}
76+
77+
[Fact]
78+
public void MultipleDeclarators()
79+
{
80+
var text = @"
81+
class C1
82+
{
83+
private int field1, field2, field3;
84+
}
85+
86+
class C2
87+
{
88+
private static int field1, field2, field3;
89+
}
90+
91+
class C3
92+
{
93+
internal int field1, field2, field3;
94+
}
95+
";
96+
97+
var expected = @"
98+
class C1
99+
{
100+
private int _field1, _field2, _field3;
101+
}
102+
103+
class C2
104+
{
105+
private static int s_field1, s_field2, s_field3;
106+
}
107+
108+
class C3
109+
{
110+
internal int field1, field2, field3;
111+
}
112+
";
113+
114+
Verify(text, expected, runFormatter: true);
49115
}
50116
}
51117
}

src/Microsoft.DotNet.CodeFormatting/Rules/ExplicitThisRule.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ private sealed class ExplicitThisRewriter : CSharpSyntaxRewriter
2121
internal ExplicitThisRewriter(SemanticModel semanticModel, CancellationToken cancellationToken)
2222
{
2323
_semanticModel = semanticModel;
24+
_cancellationToken = cancellationToken;
2425
}
2526

2627
public override SyntaxNode VisitMemberAccessExpression(MemberAccessExpressionSyntax node)

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

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ internal sealed class PrivateFieldNamingRule : IFormattingRule
2323
/// <summary>
2424
/// This will add an annotation to any private field that needs to be renamed.
2525
/// </summary>
26-
private sealed class PrivateFieldAnnotationsRewriter : CSharpSyntaxRewriter
26+
internal sealed class PrivateFieldAnnotationsRewriter : CSharpSyntaxRewriter
2727
{
2828
internal readonly static SyntaxAnnotation Marker = new SyntaxAnnotation("PrivateFieldToRename");
2929

@@ -47,19 +47,20 @@ internal static SyntaxNode AddAnnotations(SyntaxNode node, out int count)
4747

4848
public override SyntaxNode VisitFieldDeclaration(FieldDeclarationSyntax node)
4949
{
50-
if (NeedsRewrite(node))
50+
bool isInstance;
51+
if (NeedsRewrite(node, out isInstance))
5152
{
5253
var list = new List<VariableDeclaratorSyntax>(node.Declaration.Variables.Count);
5354
foreach (var v in node.Declaration.Variables)
5455
{
55-
if (IsBadName(v))
56+
if (IsGoodName(v, isInstance))
5657
{
57-
list.Add(v.WithAdditionalAnnotations(Marker));
58-
_count++;
58+
list.Add(v);
5959
}
6060
else
6161
{
62-
list.Add(v);
62+
list.Add(v.WithAdditionalAnnotations(Marker));
63+
_count++;
6364
}
6465
}
6566

@@ -72,16 +73,16 @@ public override SyntaxNode VisitFieldDeclaration(FieldDeclarationSyntax node)
7273
return node;
7374
}
7475

75-
private static bool NeedsRewrite(FieldDeclarationSyntax fieldSyntax)
76+
private static bool NeedsRewrite(FieldDeclarationSyntax fieldSyntax, out bool isInstance)
7677
{
77-
if (!IsPrivateField(fieldSyntax))
78+
if (!IsPrivateField(fieldSyntax, out isInstance))
7879
{
7980
return false;
8081
}
8182

8283
foreach (var v in fieldSyntax.Declaration.Variables)
8384
{
84-
if (IsBadName(v))
85+
if (!IsGoodName(v, isInstance))
8586
{
8687
return true;
8788
}
@@ -90,14 +91,22 @@ private static bool NeedsRewrite(FieldDeclarationSyntax fieldSyntax)
9091
return false;
9192
}
9293

93-
private static bool IsBadName(VariableDeclaratorSyntax node)
94+
private static bool IsGoodName(VariableDeclaratorSyntax node, bool isInstance)
9495
{
9596
var name = node.Identifier.ValueText;
96-
return name.Length > 0 && name[0] != '_';
97+
if (isInstance)
98+
{
99+
return name.Length > 0 && name[0] == '_';
100+
}
101+
else
102+
{
103+
return name.Length > 1 && (name[0] == 's' || name[0] == 't') && name[1] == '_';
104+
}
97105
}
98106

99-
private static bool IsPrivateField(FieldDeclarationSyntax fieldSyntax)
107+
private static bool IsPrivateField(FieldDeclarationSyntax fieldSyntax, out bool isInstance)
100108
{
109+
isInstance = true;
101110
foreach (var modifier in fieldSyntax.Modifiers)
102111
{
103112
switch (modifier.CSharpKind())
@@ -107,6 +116,9 @@ private static bool IsPrivateField(FieldDeclarationSyntax fieldSyntax)
107116
case SyntaxKind.InternalKeyword:
108117
case SyntaxKind.ProtectedKeyword:
109118
return false;
119+
case SyntaxKind.StaticKeyword:
120+
isInstance = false;
121+
break;
110122
}
111123
}
112124

@@ -189,6 +201,13 @@ private static async Task<Solution> RenameFields(Solution solution, DocumentId d
189201
var declaration = root.GetAnnotatedNodes(PrivateFieldAnnotationsRewriter.Marker).ElementAt(i);
190202
var fieldSymbol = (IFieldSymbol)semanticModel.GetDeclaredSymbol(declaration, cancellationToken);
191203
var newName = GetNewFieldName(fieldSymbol);
204+
205+
// Can happen with pathologically bad field names like _
206+
if (newName == fieldSymbol.Name)
207+
{
208+
continue;
209+
}
210+
192211
solution = await Renamer.RenameSymbolAsync(solution, fieldSymbol, newName, solution.Workspace.Options, cancellationToken).ConfigureAwait(false);
193212
}
194213

@@ -197,17 +216,15 @@ private static async Task<Solution> RenameFields(Solution solution, DocumentId d
197216

198217
private static string GetNewFieldName(IFieldSymbol fieldSymbol)
199218
{
200-
var name = fieldSymbol.Name;
201-
if (name.Length > 1)
219+
var name = fieldSymbol.Name.Trim('_');
220+
if (name.Length > 2 && char.IsLetter(name[0]) && name[1] == '_')
202221
{
203-
if (name[0] == '_')
204-
{
205-
name = name.TrimStart('_');
206-
}
207-
else if (char.IsLetter(name[0]) && name[1] == '_')
208-
{
209-
name = name.Substring(2);
210-
}
222+
name = name.Substring(2);
223+
}
224+
225+
if (name.Length == 0)
226+
{
227+
return fieldSymbol.Name;
211228
}
212229

213230
if (fieldSymbol.IsStatic)

0 commit comments

Comments
 (0)