Skip to content

Commit 0d70904

Browse files
committed
Add special handling of single-member enums to SA1129
1 parent 88fd95e commit 0d70904

2 files changed

Lines changed: 145 additions & 15 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1129CodeFixProvider.cs

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,19 @@ private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel sema
6262
{
6363
var newExpression = (ObjectCreationExpressionSyntax)node;
6464

65+
var symbolInfo = semanticModel.GetSymbolInfo(newExpression.Type, cancellationToken);
66+
var namedTypeSymbol = symbolInfo.Symbol as INamedTypeSymbol;
67+
6568
SyntaxNode replacement = null;
66-
if (IsType<CancellationToken>(newExpression.Type, semanticModel, cancellationToken))
69+
string memberName = null;
70+
71+
if (IsType<CancellationToken>(namedTypeSymbol))
72+
{
73+
replacement = ConstructMemberAccessSyntax(newExpression.Type, nameof(CancellationToken.None));
74+
}
75+
else if (IsEnumWithDefaultMember(namedTypeSymbol, out memberName))
6776
{
68-
replacement = GetCancellationTokenNoneSyntax(newExpression.Type);
77+
replacement = ConstructMemberAccessSyntax(newExpression.Type, memberName);
6978
}
7079
else
7180
{
@@ -77,42 +86,79 @@ private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel sema
7786
.WithTrailingTrivia(newExpression.GetTrailingTrivia());
7887
}
7988

80-
private static bool IsType<T>(TypeSyntax typeSyntax, SemanticModel semanticModel, CancellationToken cancellationToken)
89+
/// <summary>
90+
/// Determines whether a symbol is an instance of a given <see cref="Type"/>.
91+
/// </summary>
92+
/// <typeparam name="T">The type to match.</typeparam>
93+
/// <param name="namedTypeSymbol">The symbol.</param>
94+
/// <returns><see langword="true"/> if the syntax matches the type; <see langword="false"/> otherwise.</returns>
95+
private static bool IsType<T>(INamedTypeSymbol namedTypeSymbol)
8196
{
82-
var expectedType = typeof(T);
83-
var symbolInfo = semanticModel.GetSymbolInfo(typeSyntax, cancellationToken);
84-
var namedTypeSymbol = symbolInfo.Symbol as INamedTypeSymbol;
85-
8697
if (namedTypeSymbol == null)
8798
{
8899
return false;
89100
}
90101

102+
var expectedType = typeof(T);
103+
91104
if (!string.Equals(expectedType.Name, namedTypeSymbol.Name, StringComparison.Ordinal))
92105
{
93106
return false;
94107
}
95108

96-
if (!string.Equals(expectedType.Namespace, namedTypeSymbol.ContainingNamespace?.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)), StringComparison.Ordinal))
109+
if (!string.Equals(
110+
expectedType.Namespace,
111+
namedTypeSymbol.ContainingNamespace?.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)),
112+
StringComparison.Ordinal))
97113
{
98114
return false;
99115
}
100-
116+
101117
return true;
102118
}
103119

104120
/// <summary>
105-
/// Gets a qualified member access expression for <c>CancellationToken.None</c>.
121+
/// Determines whether a given enumeration symbol contains a member with value <c>0</c>.
122+
/// </summary>
123+
/// <param name="namedTypeSymbol">The symbol.</param>
124+
/// <param name="foundMemberName">Will be set to the string name of the member, if one is found.</param>
125+
/// <returns><see langword="true"/> if the syntax is an enumeration with a value of <c>0</c>; <see langword="false"/> otherwise.</returns>
126+
private static bool IsEnumWithDefaultMember(INamedTypeSymbol namedTypeSymbol, out string foundMemberName)
127+
{
128+
foundMemberName = null;
129+
130+
if (namedTypeSymbol == null || namedTypeSymbol.TypeKind != TypeKind.Enum)
131+
{
132+
return false;
133+
}
134+
135+
var foundMembers = namedTypeSymbol
136+
.GetMembers()
137+
.Where(m => m.Kind == SymbolKind.Field)
138+
.OfType<IFieldSymbol>()
139+
.Where(fs => fs.ConstantValue.Equals(0))
140+
.ToList();
141+
142+
if (foundMembers.Count != 1)
143+
{
144+
return false;
145+
}
146+
147+
foundMemberName = foundMembers.Single().Name;
148+
return true;
149+
}
150+
151+
/// <summary>
152+
/// Gets a qualified member access expression for the given <paramref name="typeSyntax"/>.
106153
/// </summary>
107154
/// <param name="typeSyntax">The type syntax from the original constructor.</param>
155+
/// <param name="memberName">The member name.</param>
108156
/// <returns>A new member access expression.</returns>
109-
private static SyntaxNode GetCancellationTokenNoneSyntax(TypeSyntax typeSyntax)
110-
{
111-
return SyntaxFactory.MemberAccessExpression(
157+
private static SyntaxNode ConstructMemberAccessSyntax(TypeSyntax typeSyntax, string memberName)
158+
=> SyntaxFactory.MemberAccessExpression(
112159
SyntaxKind.SimpleMemberAccessExpression,
113160
typeSyntax,
114-
SyntaxFactory.IdentifierName(nameof(CancellationToken.None)));
115-
}
161+
SyntaxFactory.IdentifierName(memberName));
116162

117163
private class FixAll : DocumentBasedFixAllProvider
118164
{

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1129UnitTests.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,90 @@ private struct CancellationToken
403403
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
404404
}
405405

406+
/// <summary>
407+
/// Verifies that <c>new MyEnum()</c> is replaced by <c>MyEnum.Member</c>
408+
/// iff there is one member in <c>MyEnum</c> with a value of <c>0</c>.
409+
/// </summary>
410+
/// <param name="declarationBody">The injected <c>enum</c> declaration body.</param>
411+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
412+
[Theory]
413+
[InlineData("Member")]
414+
[InlineData("Member = 0")]
415+
[InlineData("Member = 0, Another = 1")]
416+
[InlineData("Another = 1, Member = 0")]
417+
public async Task VerifyEnumMemberReplacementBehaviorAsync(string declarationBody)
418+
{
419+
var testCode = $@"public class TestClass
420+
{{
421+
public void TestMethod()
422+
{{
423+
var v1 = new MyEnum();
424+
}}
425+
426+
private enum MyEnum {{ {declarationBody} }}
427+
}}";
428+
429+
var fixedTestCode = $@"public class TestClass
430+
{{
431+
public void TestMethod()
432+
{{
433+
var v1 = MyEnum.Member;
434+
}}
435+
436+
private enum MyEnum {{ {declarationBody} }}
437+
}}";
438+
439+
DiagnosticResult[] expected =
440+
{
441+
this.CSharpDiagnostic().WithLocation(5, 18),
442+
};
443+
444+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
445+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
446+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
447+
}
448+
449+
/// <summary>
450+
/// Verifies that <c>new MyEnum()</c> is <b>not</b> replaced by <c>MyEnum.Member</c>
451+
/// if there is no member with a value of <c>0</c>, but instead uses the default replacement behavior.
452+
/// </summary>
453+
/// <param name="declarationBody">The injected <c>enum</c> declaration body.</param>
454+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
455+
[Theory]
456+
[InlineData("Member = 1")]
457+
[InlineData("Member = 1, Another = 2")]
458+
public async Task VerifyEnumMemberDefaultBehaviorAsync(string declarationBody)
459+
{
460+
var testCode = $@"public class TestClass
461+
{{
462+
public void TestMethod()
463+
{{
464+
var v1 = new MyEnum();
465+
}}
466+
467+
private enum MyEnum {{ {declarationBody} }}
468+
}}";
469+
470+
var fixedTestCode = $@"public class TestClass
471+
{{
472+
public void TestMethod()
473+
{{
474+
var v1 = default(MyEnum);
475+
}}
476+
477+
private enum MyEnum {{ {declarationBody} }}
478+
}}";
479+
480+
DiagnosticResult[] expected =
481+
{
482+
this.CSharpDiagnostic().WithLocation(5, 18),
483+
};
484+
485+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
486+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
487+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
488+
}
489+
406490
/// <inheritdoc/>
407491
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
408492
{

0 commit comments

Comments
 (0)