Skip to content

Commit

Permalink
Merge pull request #2882 from sharwell/fix-nested-generic
Browse files Browse the repository at this point in the history 
Fix incorrect qualification of types nested within generic types
  • Loading branch information
@sharwell
sharwell authored Feb 15, 2019
2 parents b0aed87 + 1b6e1c0 commit dbf5704
Show file tree
Hide file tree
Showing 7 changed files with 348 additions and 77 deletions.
2 changes: 1 addition & 1 deletion StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1135CodeFixProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
private static SyntaxNode GetReplacementNode(SemanticModel semanticModel, UsingDirectiveSyntax node, CancellationToken cancellationToken)
{
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(node.Name, cancellationToken);
var symbolNameSyntax = SyntaxFactory.ParseName(symbolInfo.Symbol.ToQualifiedString());
var symbolNameSyntax = SyntaxFactory.ParseName(symbolInfo.Symbol.ToQualifiedString(node.Name));

var newName = GetReplacementName(symbolNameSyntax, node.Name);
return node.WithName((NameSyntax)newName);
Expand Down
32 changes: 32 additions & 0 deletions ...eCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1135CSharp7UnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,41 @@

namespace StyleCop.Analyzers.Test.CSharp7.ReadabilityRules
{
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.Test.ReadabilityRules;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.ReadabilityRules.SA1135UsingDirectivesMustBeQualified,
StyleCop.Analyzers.ReadabilityRules.SA1135CodeFixProvider>;

public class SA1135CSharp7UnitTests : SA1135UnitTests
{
[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestTupleTypeInUsingAliasAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Generic.List<(int, int)>;
}
";
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestTupleTypeWithNamedElementsInUsingAliasAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Generic.List<(int x, int y)>;
}
";
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}
}
}
96 changes: 96 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1135UnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules
{
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.ReadabilityRules;
using Xunit;
Expand Down Expand Up @@ -319,5 +320,100 @@ namespace MyNamespace {

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestOmittedTypeInGenericAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Generic.List<>;
}
";

var expected = new DiagnosticResult("CS7003", DiagnosticSeverity.Error).WithLocation(4, 48).WithMessage("Unexpected use of an unbound generic name");
await VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestNullableTypeInGenericAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Generic.List<int?>;
}
";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestGlobalQualifiedTypeInGenericAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Generic.List<global::System.Int32>;
}
";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestTypeInGlobalNamespaceAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = MyClass;
}
class MyClass
{
}
";

await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestAliasTypeNestedInGenericAsync()
{
var testCode = @"
namespace TestNamespace
{
using Example = System.Collections.Immutable.ImmutableDictionary<int, int>.Builder;
}
";
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
}

[Fact]
[WorkItem(2879, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/2879")]
public async Task TestValueTupleInUsingAliasAsync()
{
var testCode = @"
namespace System
{
using Example = System.Collections.Generic.List<ValueTuple<int, int>>;
}
";
var fixedCode = @"
namespace System
{
using Example = System.Collections.Generic.List<System.ValueTuple<int, int>>;
}
";

var expected = Diagnostic(SA1135UsingDirectivesMustBeQualified.DescriptorType).WithLocation(4, 5).WithArguments("System.Collections.Generic.List<System.ValueTuple<int, int>>");
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
}
}
}
150 changes: 103 additions & 47 deletions StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SymbolNameHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace StyleCop.Analyzers.Helpers
{
using System.Text;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using StyleCop.Analyzers.Lightup;

Expand All @@ -17,83 +18,138 @@ internal static class SymbolNameHelpers
private const string GenericTypeParametersClose = ">";
private const string GenericSeparator = ", ";

private const string TupleTypeOpen = "(";
private const string TupleTypeClose = ")";
private const string TupleElementSeparator = ", ";

/// <summary>
/// Generates the qualified name for the given symbol.
/// </summary>
/// <param name="symbol">The symbol to use.</param>
/// <param name="name">The syntax node which resolves to the symbol.</param>
/// <returns>The generated qualified name.</returns>
public static string ToQualifiedString(this ISymbol symbol)
public static string ToQualifiedString(this ISymbol symbol, NameSyntax name)
{
var builder = ObjectPools.StringBuilderPool.Allocate();
AppendQualifiedSymbolName(builder, symbol, name);
return ObjectPools.StringBuilderPool.ReturnAndFree(builder);
}

if (symbol is INamedTypeSymbol namedTypeSymbol)
private static bool AppendQualifiedSymbolName(StringBuilder builder, ISymbol symbol, TypeSyntax type)
{
switch (symbol.Kind)
{
if (SpecialTypeHelper.TryGetPredefinedType(namedTypeSymbol.SpecialType, out PredefinedTypeSyntax specialTypeSyntax))
case SymbolKind.ArrayType:
var arraySymbol = (IArrayTypeSymbol)symbol;
AppendQualifiedSymbolName(builder, arraySymbol.ElementType, (type as ArrayTypeSyntax)?.ElementType);
builder
.Append("[")
.Append(',', arraySymbol.Rank - 1)
.Append("]");
return true;

case SymbolKind.Namespace:
var namespaceSymbol = (INamespaceSymbol)symbol;
if (namespaceSymbol.IsGlobalNamespace)
{
return specialTypeSyntax.ToFullString();
return false;
}

if (namedTypeSymbol.IsTupleType())
builder.Append(namespaceSymbol.ToDisplayString());
return true;

case SymbolKind.NamedType:
var namedTypeSymbol = (INamedTypeSymbol)symbol;
if (SpecialTypeHelper.TryGetPredefinedType(namedTypeSymbol.SpecialType, out var specialTypeSyntax))
{
namedTypeSymbol = namedTypeSymbol.TupleUnderlyingType();
builder.Append(specialTypeSyntax.ToFullString());
return true;
}
else if (namedTypeSymbol.IsTupleType())
{
if (TupleTypeSyntaxWrapper.IsInstance(type))
{
var tupleType = (TupleTypeSyntaxWrapper)type;

builder.Append(TupleTypeOpen);
var elements = namedTypeSymbol.TupleElements();
for (int i = 0; i < elements.Length; i++)
{
var field = elements[i];
var fieldType = tupleType.Elements.Count > i ? tupleType.Elements[i] : default;

AppendQualifiedSymbolName(builder, namedTypeSymbol);
if (i > 0)
{
builder.Append(TupleElementSeparator);
}

AppendQualifiedSymbolName(builder, field.Type, fieldType.Type);
if (field != field.CorrespondingTupleField())
{
builder.Append(" ").Append(field.Name);
}
}

if (namedTypeSymbol.IsGenericType)
builder.Append(TupleTypeClose);
return true;
}
else
{
return AppendQualifiedSymbolName(builder, namedTypeSymbol.TupleUnderlyingType(), type);
}
}
else if (namedTypeSymbol.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T)
{
AppendQualifiedSymbolName(builder, namedTypeSymbol.TypeArguments[0], (type as NullableTypeSyntax)?.ElementType);
builder.Append("?");
return true;
}
else
{
builder.Append(GenericTypeParametersOpen);
if (AppendQualifiedSymbolName(builder, symbol.ContainingSymbol, (type as QualifiedNameSyntax)?.Left))
{
builder.Append(".");
}

foreach (var typeArgument in namedTypeSymbol.TypeArguments)
builder.Append(symbol.Name);
if (namedTypeSymbol.IsGenericType && !namedTypeSymbol.TypeArguments.IsEmpty)
{
if (typeArgument is INamedTypeSymbol namedTypeArgument && typeArgument.IsTupleType())
{
builder.Append(namedTypeArgument.TupleUnderlyingType().ToQualifiedString());
}
else
builder.Append(GenericTypeParametersOpen);
var arguments = namedTypeSymbol.TypeArguments;
var argumentTypes = type is QualifiedNameSyntax qualifiedName
? (qualifiedName.Right as GenericNameSyntax)?.TypeArgumentList
: (type as GenericNameSyntax)?.TypeArgumentList;

for (int i = 0; i < arguments.Length; i++)
{
builder.Append(typeArgument.ToQualifiedString());
var argument = arguments[i];
var argumentType = argumentTypes != null && argumentTypes.Arguments.Count > i ? argumentTypes.Arguments[i] : null;

if (i > 0)
{
builder.Append(GenericSeparator);
}

if (!argumentType.IsKind(SyntaxKind.OmittedTypeArgument))
{
AppendQualifiedSymbolName(builder, argument, argumentType);
}
}

builder.Append(GenericSeparator);
builder.Append(GenericTypeParametersClose);
}

builder.Remove(builder.Length - GenericSeparator.Length, GenericSeparator.Length);
builder.Append(GenericTypeParametersClose);
return true;
}
}
else
{
AppendQualifiedSymbolName(builder, symbol);
}

return ObjectPools.StringBuilderPool.ReturnAndFree(builder);
}

private static void AppendQualifiedSymbolName(StringBuilder builder, ISymbol symbol)
{
switch (symbol)
{
case IArrayTypeSymbol arraySymbol:
builder
.Append(arraySymbol.ElementType.ContainingNamespace.ToDisplayString())
.Append(".")
.Append(arraySymbol.ElementType.Name)
.Append("[")
.Append(',', arraySymbol.Rank - 1)
.Append("]");
break;

default:
if (!symbol.ContainingNamespace.IsGlobalNamespace)
if (symbol != null)
{
builder
.Append(symbol.ContainingNamespace.ToDisplayString())
.Append(".");
builder.Append(symbol.Name);
return true;
}

builder.Append(symbol.Name);
break;
return false;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion StyleCop.Analyzers/StyleCop.Analyzers/Lightup/CSharp7.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ See [dotnet/roslyn@c2af711](https://github.com/dotnet/roslyn/commit/c2af71127234
* [ ] `Microsoft.CodeAnalysis.IArrayTypeSymbol.Sizes.get -> System.Collections.Immutable.ImmutableArray<int>`
* [ ] `Microsoft.CodeAnalysis.IDiscardSymbol`
* [ ] `Microsoft.CodeAnalysis.IDiscardSymbol.Type.get -> Microsoft.CodeAnalysis.ITypeSymbol`
* [ ] `Microsoft.CodeAnalysis.IFieldSymbol.CorrespondingTupleField.get -> Microsoft.CodeAnalysis.IFieldSymbol`
* [x] `Microsoft.CodeAnalysis.IFieldSymbol.CorrespondingTupleField.get -> Microsoft.CodeAnalysis.IFieldSymbol`
* [ ] `Microsoft.CodeAnalysis.ILocalSymbol.IsRef.get -> bool`
* [ ] `Microsoft.CodeAnalysis.IMethodSymbol.RefCustomModifiers.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.CustomModifier>`
* [ ] `Microsoft.CodeAnalysis.IMethodSymbol.ReturnsByRef.get -> bool`
Expand Down
23 changes: 23 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers/Lightup/IFieldSymbolExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.

namespace StyleCop.Analyzers.Lightup
{
using System;
using Microsoft.CodeAnalysis;

internal static class IFieldSymbolExtensions
{
private static readonly Func<IFieldSymbol, IFieldSymbol> CorrespondingTupleFieldAccessor;

static IFieldSymbolExtensions()
{
CorrespondingTupleFieldAccessor = LightupHelpers.CreateSyntaxPropertyAccessor<IFieldSymbol, IFieldSymbol>(typeof(IFieldSymbol), nameof(CorrespondingTupleField));
}

public static IFieldSymbol CorrespondingTupleField(this IFieldSymbol symbol)
{
return CorrespondingTupleFieldAccessor(symbol);
}
}
}
Loading

0 comments on commit dbf5704

Please sign in to comment.