Skip to content

Commit 84f14c1

Browse files
committed
Fixed issues in SA1135 for wrapped using statements
1 parent 3d6cf93 commit 84f14c1

5 files changed

Lines changed: 227 additions & 8 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1135CodeFixProvider.cs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
7+
using System.Collections.Generic;
68
using System.Collections.Immutable;
79
using System.Composition;
810
using System.Linq;
@@ -63,7 +65,75 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
6365
private static SyntaxNode GetReplacementNode(SemanticModel semanticModel, UsingDirectiveSyntax node, CancellationToken cancellationToken)
6466
{
6567
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(node.Name, cancellationToken);
66-
return node.WithName(SyntaxFactory.ParseName(symbolInfo.Symbol.ToString()));
68+
var symbolNameSyntax = SyntaxFactory.ParseName(symbolInfo.Symbol.ToQualifiedString());
69+
70+
var newName = GetReplacementName(symbolNameSyntax, node.Name);
71+
return node.WithName(newName);
72+
}
73+
74+
private static NameSyntax GetReplacementName(NameSyntax symbolNameSyntax, NameSyntax nameSyntax)
75+
{
76+
switch (nameSyntax.Kind())
77+
{
78+
case SyntaxKind.GenericName:
79+
return GetReplacementGenericName(symbolNameSyntax, (GenericNameSyntax)nameSyntax);
80+
81+
case SyntaxKind.QualifiedName:
82+
return GetReplacementQualifiedName((QualifiedNameSyntax)symbolNameSyntax, (QualifiedNameSyntax)nameSyntax);
83+
84+
default:
85+
return symbolNameSyntax;
86+
}
87+
}
88+
89+
private static NameSyntax GetReplacementGenericName(NameSyntax symbolNameSyntax, GenericNameSyntax genericNameSyntax)
90+
{
91+
var symbolQualifiedNameSyntax = symbolNameSyntax as QualifiedNameSyntax;
92+
var symbolGenericNameSyntax = (GenericNameSyntax)(symbolQualifiedNameSyntax?.Right ?? symbolNameSyntax);
93+
94+
TypeArgumentListSyntax newTypeArgumentList = GetReplacementTypeArgumentList(symbolGenericNameSyntax, genericNameSyntax);
95+
96+
if (symbolQualifiedNameSyntax != null)
97+
{
98+
var newRightPart = ((GenericNameSyntax)symbolQualifiedNameSyntax.Right).WithTypeArgumentList(newTypeArgumentList);
99+
return symbolQualifiedNameSyntax.WithRight(newRightPart);
100+
}
101+
102+
return genericNameSyntax.WithTypeArgumentList(newTypeArgumentList);
103+
}
104+
105+
private static TypeArgumentListSyntax GetReplacementTypeArgumentList(GenericNameSyntax symbolGenericNameSyntax, GenericNameSyntax genericNameSyntax)
106+
{
107+
var replacements = new Dictionary<NameSyntax, NameSyntax>();
108+
for (var i = 0; i < genericNameSyntax.TypeArgumentList.Arguments.Count; i++)
109+
{
110+
if (!genericNameSyntax.TypeArgumentList.Arguments[i].IsKind(SyntaxKind.PredefinedType))
111+
{
112+
var argument = (NameSyntax)genericNameSyntax.TypeArgumentList.Arguments[i];
113+
var symbolArgument = (NameSyntax)symbolGenericNameSyntax.TypeArgumentList.Arguments[i];
114+
115+
var replacementArgument = GetReplacementName(symbolArgument, argument)
116+
.WithLeadingTrivia(argument.GetLeadingTrivia())
117+
.WithTrailingTrivia(argument.GetTrailingTrivia());
118+
119+
replacements.Add(argument, replacementArgument);
120+
}
121+
}
122+
123+
var newTypeArgumentList = genericNameSyntax.TypeArgumentList.ReplaceNodes(replacements.Keys, (original, maybeRewritten) => replacements[original]);
124+
return newTypeArgumentList;
125+
}
126+
127+
private static NameSyntax GetReplacementQualifiedName(QualifiedNameSyntax symbolNameSyntax, QualifiedNameSyntax nameSyntax)
128+
{
129+
if (nameSyntax.Right.IsKind(SyntaxKind.GenericName))
130+
{
131+
return GetReplacementGenericName(symbolNameSyntax, (GenericNameSyntax)nameSyntax.Right);
132+
}
133+
else
134+
{
135+
return symbolNameSyntax;
136+
}
67137
}
68138

69139
private class FixAll : DocumentBasedFixAllProvider

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1135UnitTests.cs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules
77
using System.Threading.Tasks;
88
using Microsoft.CodeAnalysis.Testing;
99
using StyleCop.Analyzers.ReadabilityRules;
10-
using TestHelper;
1110
using Xunit;
1211
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
1312
StyleCop.Analyzers.ReadabilityRules.SA1135UsingDirectivesMustBeQualified,
@@ -243,30 +242,69 @@ public async Task TestFullyQualifiedAliasWithWrappedTypeArgumentsAsync()
243242
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
244243
}
245244

246-
[Fact(Skip = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2820")]
245+
[Fact]
247246
[WorkItem(2820, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2820")]
248247
public async Task TestAliasWithWrappedTypeArgumentsInsideNamespaceAsync()
249248
{
250249
var testCode = @"
251250
using System;
251+
using MyAlias = System.Exception;
252252
253-
namespace Test {
253+
namespace Test
254+
{
254255
using Example = System.ValueTuple<
255256
Exception,
256257
Exception>;
258+
259+
using Example2 = ValueTuple<
260+
Exception,
261+
Exception>;
262+
263+
using Example3 = ValueTuple<
264+
ValueTuple<
265+
Exception,
266+
Exception>,
267+
Exception>;
268+
269+
using Example4 = ValueTuple<
270+
MyAlias,
271+
Exception>;
257272
}
258273
";
259274
var fixedCode = @"
260275
using System;
276+
using MyAlias = System.Exception;
261277
262-
namespace Test {
278+
namespace Test
279+
{
263280
using Example = System.ValueTuple<
264281
System.Exception,
265282
System.Exception>;
283+
284+
using Example2 = System.ValueTuple<
285+
System.Exception,
286+
System.Exception>;
287+
288+
using Example3 = System.ValueTuple<
289+
System.ValueTuple<
290+
System.Exception,
291+
System.Exception>,
292+
System.Exception>;
293+
294+
using Example4 = System.ValueTuple<
295+
System.Exception,
296+
System.Exception>;
266297
}
267298
";
268299

269-
var expected = Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(5, 5).WithArguments("System.ValueTuple<System.Exception, System.Exception>");
300+
DiagnosticResult[] expected =
301+
{
302+
Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(7, 5).WithArguments("System.ValueTuple<System.Exception, System.Exception>"),
303+
Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(11, 5).WithArguments("System.ValueTuple<System.Exception, System.Exception>"),
304+
Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(15, 5).WithArguments("System.ValueTuple<System.ValueTuple<System.Exception, System.Exception>, System.Exception>"),
305+
Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(21, 5).WithArguments("System.ValueTuple<System.Exception, System.Exception>"),
306+
};
307+
270308
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
271309
}
272310
}

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/SpecialTypeHelper.cs renamed to StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SpecialTypeHelper.cs

File renamed without changes.
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Helpers
5+
{
6+
using System;
7+
using System.Text;
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.CSharp.Syntax;
10+
using StyleCop.Analyzers.Lightup;
11+
12+
/// <summary>
13+
/// Contains helper methods to work with symbol names consistently over different C# versions.
14+
/// </summary>
15+
internal static class SymbolNameHelpers
16+
{
17+
private const string GenericTypeParametersOpen = "<";
18+
private const string GenericTypeParametersClose = ">";
19+
private const string GenericSeparator = ", ";
20+
21+
/// <summary>
22+
/// Generates the qualified name for the given symbol.
23+
/// </summary>
24+
/// <param name="symbol">The symbol to use.</param>
25+
/// <returns>The generated qualified name.</returns>
26+
public static string ToQualifiedString(this ISymbol symbol)
27+
{
28+
var builder = ObjectPools.StringBuilderPool.Allocate();
29+
30+
if (symbol is INamedTypeSymbol namedTypeSymbol)
31+
{
32+
if (SpecialTypeHelper.TryGetPredefinedType(namedTypeSymbol.SpecialType, out PredefinedTypeSyntax specialTypeSyntax))
33+
{
34+
return specialTypeSyntax.ToFullString();
35+
}
36+
37+
if (namedTypeSymbol.IsTupleType())
38+
{
39+
namedTypeSymbol = namedTypeSymbol.TupleUnderlyingType();
40+
}
41+
42+
AppendQualifiedSymbolName(builder, namedTypeSymbol);
43+
44+
if (namedTypeSymbol.IsGenericType)
45+
{
46+
builder.Append(GenericTypeParametersOpen);
47+
48+
foreach (var typeArgument in namedTypeSymbol.TypeArguments)
49+
{
50+
if (typeArgument is INamedTypeSymbol namedTypeArgument && typeArgument.IsTupleType())
51+
{
52+
builder.Append(namedTypeArgument.TupleUnderlyingType().ToQualifiedString());
53+
}
54+
else
55+
{
56+
builder.Append(typeArgument.ToQualifiedString());
57+
}
58+
59+
builder.Append(GenericSeparator);
60+
}
61+
62+
builder.Remove(builder.Length - GenericSeparator.Length, GenericSeparator.Length);
63+
builder.Append(GenericTypeParametersClose);
64+
}
65+
}
66+
else
67+
{
68+
AppendQualifiedSymbolName(builder, symbol);
69+
}
70+
71+
return ObjectPools.StringBuilderPool.ReturnAndFree(builder);
72+
}
73+
74+
private static void AppendQualifiedSymbolName(StringBuilder builder, ISymbol symbol)
75+
{
76+
builder.Append(symbol.ContainingNamespace.ToDisplayString());
77+
builder.Append(".");
78+
builder.Append(symbol.Name);
79+
}
80+
}
81+
}

StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1135UsingDirectivesMustBeQualified.cs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
67
using System.Collections.Immutable;
78
using Microsoft.CodeAnalysis;
89
using Microsoft.CodeAnalysis.CSharp;
910
using Microsoft.CodeAnalysis.CSharp.Syntax;
1011
using Microsoft.CodeAnalysis.Diagnostics;
1112

1213
using StyleCop.Analyzers.Helpers;
14+
using StyleCop.Analyzers.Helpers.ObjectPools;
1315
using StyleCop.Analyzers.Lightup;
1416

1517
/// <summary>
@@ -96,8 +98,9 @@ private static void CheckUsingDeclaration(SyntaxNodeAnalysisContext context, Usi
9698
symbol = typeSymbol.TupleUnderlyingType();
9799
}
98100

99-
string symbolString = symbol.ToString();
100-
string usingString = usingDirective.Name.ToString();
101+
string symbolString = symbol.ToQualifiedString();
102+
103+
string usingString = UsingDirectiveSyntaxToCanonicalString(usingDirective);
101104
if ((symbolString != usingString) && !usingDirective.StartsWithAlias(context.SemanticModel, context.CancellationToken))
102105
{
103106
switch (symbol.Kind)
@@ -117,5 +120,32 @@ private static void CheckUsingDeclaration(SyntaxNodeAnalysisContext context, Usi
117120
}
118121
}
119122
}
123+
124+
private static string UsingDirectiveSyntaxToCanonicalString(UsingDirectiveSyntax usingDirective)
125+
{
126+
var builder = StringBuilderPool.Allocate();
127+
128+
// NOTE: this does not cover embedded comments. It is very unlikely that comments are present
129+
// within a multiline using statement and handling them requires a lot more effort (and keeping of state).
130+
foreach (var c in usingDirective.Name.ToString())
131+
{
132+
switch (c)
133+
{
134+
case ' ':
135+
case '\t':
136+
case '\r':
137+
case '\n':
138+
break;
139+
case ',':
140+
builder.Append(", ");
141+
break;
142+
default:
143+
builder.Append(c);
144+
break;
145+
}
146+
}
147+
148+
return StringBuilderPool.ReturnAndFree(builder);
149+
}
120150
}
121151
}

0 commit comments

Comments
 (0)