Skip to content

Commit 292d769

Browse files
committed
Merge branch 'fix-1972' into 'fix-1973'
2 parents 6665d36 + 0d70904 commit 292d769

2 files changed

Lines changed: 144 additions & 14 deletions

File tree

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

Lines changed: 60 additions & 14 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))
113+
{
114+
return false;
115+
}
116+
117+
return true;
118+
}
119+
120+
/// <summary>
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)
97131
{
98132
return false;
99133
}
100134

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;
101148
return true;
102149
}
103150

104151
/// <summary>
105-
/// Gets a qualified member access expression for <c>CancellationToken.None</c>.
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
@@ -439,6 +439,90 @@ public class TestClass
439439
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
440440
}
441441

442+
/// <summary>
443+
/// Verifies that <c>new MyEnum()</c> is replaced by <c>MyEnum.Member</c>
444+
/// iff there is one member in <c>MyEnum</c> with a value of <c>0</c>.
445+
/// </summary>
446+
/// <param name="declarationBody">The injected <c>enum</c> declaration body.</param>
447+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
448+
[Theory]
449+
[InlineData("Member")]
450+
[InlineData("Member = 0")]
451+
[InlineData("Member = 0, Another = 1")]
452+
[InlineData("Another = 1, Member = 0")]
453+
public async Task VerifyEnumMemberReplacementBehaviorAsync(string declarationBody)
454+
{
455+
var testCode = $@"public class TestClass
456+
{{
457+
public void TestMethod()
458+
{{
459+
var v1 = new MyEnum();
460+
}}
461+
462+
private enum MyEnum {{ {declarationBody} }}
463+
}}";
464+
465+
var fixedTestCode = $@"public class TestClass
466+
{{
467+
public void TestMethod()
468+
{{
469+
var v1 = MyEnum.Member;
470+
}}
471+
472+
private enum MyEnum {{ {declarationBody} }}
473+
}}";
474+
475+
DiagnosticResult[] expected =
476+
{
477+
this.CSharpDiagnostic().WithLocation(5, 18),
478+
};
479+
480+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
481+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
482+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
483+
}
484+
485+
/// <summary>
486+
/// Verifies that <c>new MyEnum()</c> is <b>not</b> replaced by <c>MyEnum.Member</c>
487+
/// if there is no member with a value of <c>0</c>, but instead uses the default replacement behavior.
488+
/// </summary>
489+
/// <param name="declarationBody">The injected <c>enum</c> declaration body.</param>
490+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
491+
[Theory]
492+
[InlineData("Member = 1")]
493+
[InlineData("Member = 1, Another = 2")]
494+
public async Task VerifyEnumMemberDefaultBehaviorAsync(string declarationBody)
495+
{
496+
var testCode = $@"public class TestClass
497+
{{
498+
public void TestMethod()
499+
{{
500+
var v1 = new MyEnum();
501+
}}
502+
503+
private enum MyEnum {{ {declarationBody} }}
504+
}}";
505+
506+
var fixedTestCode = $@"public class TestClass
507+
{{
508+
public void TestMethod()
509+
{{
510+
var v1 = default(MyEnum);
511+
}}
512+
513+
private enum MyEnum {{ {declarationBody} }}
514+
}}";
515+
516+
DiagnosticResult[] expected =
517+
{
518+
this.CSharpDiagnostic().WithLocation(5, 18),
519+
};
520+
521+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
522+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
523+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
524+
}
525+
442526
/// <inheritdoc/>
443527
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
444528
{

0 commit comments

Comments
 (0)