Skip to content

Commit b2f4e48

Browse files
sharwellsharwell
authored
Merge pull request #2835 from vweijsters/fix-2820
Fixed issues in SA1135 for wrapped using statements
2 parents 744e377 + e8c1361 commit b2f4e48

5 files changed

Lines changed: 252 additions & 8 deletions

File tree

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

Lines changed: 72 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,76 @@ 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((NameSyntax)newName);
72+
}
73+
74+
private static TypeSyntax GetReplacementName(TypeSyntax symbolNameSyntax, TypeSyntax 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(TypeSyntax 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<TypeSyntax, TypeSyntax>();
108+
for (var i = 0; i < genericNameSyntax.TypeArgumentList.Arguments.Count; i++)
109+
{
110+
var argument = genericNameSyntax.TypeArgumentList.Arguments[i];
111+
112+
if (!argument.IsKind(SyntaxKind.PredefinedType))
113+
{
114+
var symbolArgument = symbolGenericNameSyntax.TypeArgumentList.Arguments[i];
115+
116+
var replacementArgument = GetReplacementName(symbolArgument, argument)
117+
.WithLeadingTrivia(argument.GetLeadingTrivia())
118+
.WithTrailingTrivia(argument.GetTrailingTrivia());
119+
120+
replacements.Add(argument, replacementArgument);
121+
}
122+
}
123+
124+
var newTypeArgumentList = genericNameSyntax.TypeArgumentList.ReplaceNodes(replacements.Keys, (original, maybeRewritten) => replacements[original]);
125+
return newTypeArgumentList;
126+
}
127+
128+
private static NameSyntax GetReplacementQualifiedName(QualifiedNameSyntax symbolNameSyntax, QualifiedNameSyntax nameSyntax)
129+
{
130+
if (nameSyntax.Right.IsKind(SyntaxKind.GenericName))
131+
{
132+
return GetReplacementGenericName(symbolNameSyntax, (GenericNameSyntax)nameSyntax.Right);
133+
}
134+
else
135+
{
136+
return symbolNameSyntax;
137+
}
67138
}
68139

69140
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.

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SymbolNameHelpers.cs‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
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.Text;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CSharp.Syntax;
9+
using StyleCop.Analyzers.Lightup;
10+
11+
/// <summary>
12+
/// Contains helper methods to work with symbol names consistently over different C# versions.
13+
/// </summary>
14+
internal static class SymbolNameHelpers
15+
{
16+
private const string GenericTypeParametersOpen = "<";
17+
private const string GenericTypeParametersClose = ">";
18+
private const string GenericSeparator = ", ";
19+
20+
/// <summary>
21+
/// Generates the qualified name for the given symbol.
22+
/// </summary>
23+
/// <param name="symbol">The symbol to use.</param>
24+
/// <returns>The generated qualified name.</returns>
25+
public static string ToQualifiedString(this ISymbol symbol)
26+
{
27+
var builder = ObjectPools.StringBuilderPool.Allocate();
28+
29+
if (symbol is INamedTypeSymbol namedTypeSymbol)
30+
{
31+
if (SpecialTypeHelper.TryGetPredefinedType(namedTypeSymbol.SpecialType, out PredefinedTypeSyntax specialTypeSyntax))
32+
{
33+
return specialTypeSyntax.ToFullString();
34+
}
35+
36+
if (namedTypeSymbol.IsTupleType())
37+
{
38+
namedTypeSymbol = namedTypeSymbol.TupleUnderlyingType();
39+
}
40+
41+
AppendQualifiedSymbolName(builder, namedTypeSymbol);
42+
43+
if (namedTypeSymbol.IsGenericType)
44+
{
45+
builder.Append(GenericTypeParametersOpen);
46+
47+
foreach (var typeArgument in namedTypeSymbol.TypeArguments)
48+
{
49+
if (typeArgument is INamedTypeSymbol namedTypeArgument && typeArgument.IsTupleType())
50+
{
51+
builder.Append(namedTypeArgument.TupleUnderlyingType().ToQualifiedString());
52+
}
53+
else
54+
{
55+
builder.Append(typeArgument.ToQualifiedString());
56+
}
57+
58+
builder.Append(GenericSeparator);
59+
}
60+
61+
builder.Remove(builder.Length - GenericSeparator.Length, GenericSeparator.Length);
62+
builder.Append(GenericTypeParametersClose);
63+
}
64+
}
65+
else
66+
{
67+
AppendQualifiedSymbolName(builder, symbol);
68+
}
69+
70+
return ObjectPools.StringBuilderPool.ReturnAndFree(builder);
71+
}
72+
73+
private static void AppendQualifiedSymbolName(StringBuilder builder, ISymbol symbol)
74+
{
75+
switch (symbol)
76+
{
77+
case IArrayTypeSymbol arraySymbol:
78+
builder
79+
.Append(arraySymbol.ElementType.ContainingNamespace.ToDisplayString())
80+
.Append(".")
81+
.Append(arraySymbol.ElementType.Name)
82+
.Append("[")
83+
.Append(',', arraySymbol.Rank - 1)
84+
.Append("]");
85+
break;
86+
87+
default:
88+
builder
89+
.Append(symbol.ContainingNamespace.ToDisplayString())
90+
.Append(".")
91+
.Append(symbol.Name);
92+
break;
93+
}
94+
}
95+
}
96+
}

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

Lines changed: 41 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,41 @@ 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+
var insideArrayDeclaration = false;
128+
129+
// NOTE: this does not cover embedded comments. It is very unlikely that comments are present
130+
// within a multiline using statement and handling them requires a lot more effort (and keeping of state).
131+
foreach (var c in usingDirective.Name.ToString())
132+
{
133+
switch (c)
134+
{
135+
case ' ':
136+
case '\t':
137+
case '\r':
138+
case '\n':
139+
break;
140+
case '[':
141+
insideArrayDeclaration = true;
142+
builder.Append(c);
143+
break;
144+
case ']':
145+
insideArrayDeclaration = false;
146+
builder.Append(c);
147+
break;
148+
case ',':
149+
builder.Append(insideArrayDeclaration ? "," : ", ");
150+
break;
151+
default:
152+
builder.Append(c);
153+
break;
154+
}
155+
}
156+
157+
return StringBuilderPool.ReturnAndFree(builder);
158+
}
120159
}
121160
}

0 commit comments

Comments
 (0)