Skip to content

Commit 88fd95e

Browse files
committed
Update SA1129 to validate symbol and full namespace path
1 parent 3e2e37f commit 88fd95e

2 files changed

Lines changed: 86 additions & 9 deletions

File tree

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

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
67
using System.Collections.Immutable;
78
using System.Composition;
89
using System.Linq;
@@ -49,22 +50,22 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
4950
private static async Task<Document> GetTransformedDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
5051
{
5152
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
53+
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
5254

5355
var newExpression = syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
54-
var newSyntaxRoot = syntaxRoot.ReplaceNode(newExpression, GetReplacementNode(newExpression));
56+
var newSyntaxRoot = syntaxRoot.ReplaceNode(newExpression, GetReplacementNode(newExpression, semanticModel, cancellationToken));
57+
5558
return document.WithSyntaxRoot(newSyntaxRoot);
5659
}
5760

58-
private static SyntaxNode GetReplacementNode(SyntaxNode node)
61+
private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel semanticModel, CancellationToken cancellationToken)
5962
{
6063
var newExpression = (ObjectCreationExpressionSyntax)node;
61-
var nameSyntax = newExpression.Type as NameSyntax;
62-
var identifierName = NameSyntaxHelpers.ToNormalizedString(nameSyntax);
6364

6465
SyntaxNode replacement = null;
65-
if (identifierName.EndsWith(nameof(CancellationToken), System.StringComparison.OrdinalIgnoreCase))
66+
if (IsType<CancellationToken>(newExpression.Type, semanticModel, cancellationToken))
6667
{
67-
replacement = GetCancellationTokenNoneSyntax(nameSyntax);
68+
replacement = GetCancellationTokenNoneSyntax(newExpression.Type);
6869
}
6970
else
7071
{
@@ -76,11 +77,40 @@ private static SyntaxNode GetReplacementNode(SyntaxNode node)
7677
.WithTrailingTrivia(newExpression.GetTrailingTrivia());
7778
}
7879

79-
private static SyntaxNode GetCancellationTokenNoneSyntax(NameSyntax nameSyntax)
80+
private static bool IsType<T>(TypeSyntax typeSyntax, SemanticModel semanticModel, CancellationToken cancellationToken)
81+
{
82+
var expectedType = typeof(T);
83+
var symbolInfo = semanticModel.GetSymbolInfo(typeSyntax, cancellationToken);
84+
var namedTypeSymbol = symbolInfo.Symbol as INamedTypeSymbol;
85+
86+
if (namedTypeSymbol == null)
87+
{
88+
return false;
89+
}
90+
91+
if (!string.Equals(expectedType.Name, namedTypeSymbol.Name, StringComparison.Ordinal))
92+
{
93+
return false;
94+
}
95+
96+
if (!string.Equals(expectedType.Namespace, namedTypeSymbol.ContainingNamespace?.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)), StringComparison.Ordinal))
97+
{
98+
return false;
99+
}
100+
101+
return true;
102+
}
103+
104+
/// <summary>
105+
/// Gets a qualified member access expression for <c>CancellationToken.None</c>.
106+
/// </summary>
107+
/// <param name="typeSyntax">The type syntax from the original constructor.</param>
108+
/// <returns>A new member access expression.</returns>
109+
private static SyntaxNode GetCancellationTokenNoneSyntax(TypeSyntax typeSyntax)
80110
{
81111
return SyntaxFactory.MemberAccessExpression(
82112
SyntaxKind.SimpleMemberAccessExpression,
83-
nameSyntax,
113+
typeSyntax,
84114
SyntaxFactory.IdentifierName(nameof(CancellationToken.None)));
85115
}
86116

@@ -101,10 +131,11 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
101131
}
102132

103133
var syntaxRoot = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
134+
var semanticModel = await document.GetSemanticModelAsync().ConfigureAwait(false);
104135

105136
var nodes = diagnostics.Select(diagnostic => syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true));
106137

107-
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => GetReplacementNode(rewrittenNode));
138+
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => GetReplacementNode(rewrittenNode, semanticModel, CancellationToken.None));
108139
}
109140
}
110141
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,52 @@ public void TestMethod()
357357
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
358358
}
359359

360+
/// <summary>
361+
/// Verifies that <c>new CancellationToken()</c> is <b>not</b> replaced by <c>CancellationToken.None</c>
362+
/// if the qualified name is not exactly <c>System.Threading.CancellationToken</c>.
363+
/// </summary>
364+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
365+
[Fact]
366+
public async Task VerifyCustomCancellationTokenClassIsNotReplacedAsync()
367+
{
368+
var testCode = @"public class TestClass
369+
{
370+
public void TestMethod()
371+
{
372+
var ct = new CancellationToken();
373+
}
374+
375+
private struct CancellationToken
376+
{
377+
public int TestProperty { get; set; }
378+
}
379+
}
380+
";
381+
382+
var fixedTestCode = @"public class TestClass
383+
{
384+
public void TestMethod()
385+
{
386+
var ct = default(CancellationToken);
387+
}
388+
389+
private struct CancellationToken
390+
{
391+
public int TestProperty { get; set; }
392+
}
393+
}
394+
";
395+
396+
DiagnosticResult[] expected =
397+
{
398+
this.CSharpDiagnostic().WithLocation(5, 18),
399+
};
400+
401+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
402+
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
403+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode).ConfigureAwait(false);
404+
}
405+
360406
/// <inheritdoc/>
361407
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
362408
{

0 commit comments

Comments
 (0)