Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix analyzer issues #7222

Merged
merged 4 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,25 @@ public virtual Response Get(string s, CancellationToken cancellationToken)
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}

[Fact]
public async Task AZC0002NotProducedForGetSubClientMethods()
{
const string code = @"
namespace RandomNamespace
{
public class SomeClient
{
public class Operation {}
public virtual Operation GetOperationClient(string apiVersion = ""1.0.0"")
{
return new Operation();
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,26 @@ public class SomeClient
protected SomeClient() {}
public SomeClient(string connectionString, SomeClientsOptions options = null) {}
}
}";
await Verifier.VerifyAnalyzerAsync(code);
}

[Fact]
public async Task AZC0006NotProducedForClientsWithStaticProperties()
{
const string code = @"
namespace RandomNamespace
{
public class SomeClientOptions : Azure.Core.ClientOptions { }

public class SomeClient
{
public static int a = 1;
public SomeClient() {}
public SomeClient(SomeClientOptions options) {}
}
}";
await Verifier.VerifyAnalyzerAsync(code);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,80 @@ await Verifier.CreateAnalyzer(code)
.RunAsync();
}

[Fact]
public async Task AZC0018NotProducedForExtendedReturnType()
{
const string code = @"
using Azure;
using Azure.Core;
using System;
using System.Threading;
using System.Threading.Tasks;

namespace Arbitrary
{
public class ExtendedBinaryData : BinaryData
{
public ExtendedBinaryData(string data): base(data){}
}
}

namespace RandomNamespace
{
public class SomeClient
{
public virtual AsyncPageable<Arbitrary.ExtendedBinaryData> GetPageableAsync(string s, RequestContext context)
{
return null;
}

public virtual Pageable<Arbitrary.ExtendedBinaryData> GetPageable(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}

[Fact]
public async Task AZC0018ProducedForCustomTypeWithSystemName()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;

namespace Custom
{
namespace System
{
public class BinaryData {}
}
}

namespace RandomNamespace
{
using Custom.System;
public class SomeClient
{
public virtual AsyncPageable<BinaryData> {|AZC0018:GetPageableAsync|}(string s, RequestContext context)
{
return null;
}

public virtual Pageable<BinaryData> {|AZC0018:GetPageable|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}

[Fact]
public async Task AZC0018ProducedForMethodsWithGenericResponseOfPrimitive()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ public int GetHashCode(ITypeParameterSymbol obj)
}
}

protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbols, ImmutableArray<ITypeParameterSymbol> genericParameters, ImmutableArray<IParameterSymbol> parameters)
protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbols, ImmutableArray<ITypeParameterSymbol> genericParameters, ImmutableArray<IParameterSymbol> parameters, bool ignoreStatic = false)
{
return methodSymbols.SingleOrDefault(symbol =>
(!ignoreStatic || !symbol.IsStatic) &&
genericParameters.SequenceEqual(symbol.TypeParameters, ParameterEquivalenceComparer.Default) &&
parameters.SequenceEqual(symbol.Parameters, ParameterEquivalenceComparer.Default));
}
Expand All @@ -105,4 +106,4 @@ protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbo

public abstract void AnalyzeCore(ISymbolAnalysisContext context);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ public override void AnalyzeCore(ISymbolAnalysisContext context)
// Allow optional options parameters
if (lastParameter.IsOptional) continue;

// When there are static properties in client, there would be static constructor implicitly added
var nonOptionsMethod = FindMethod(
type.Constructors, constructor.TypeParameters, constructor.Parameters.RemoveAt(constructor.Parameters.Length - 1));
type.Constructors, constructor.TypeParameters, constructor.Parameters.RemoveAt(constructor.Parameters.Length - 1), true);

if (nonOptionsMethod == null || nonOptionsMethod.DeclaredAccessibility != Accessibility.Public)
{
Expand All @@ -62,4 +63,4 @@ public override void AnalyzeCore(ISymbolAnalysisContext context)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public class ClientMethodsAnalyzer : ClientAnalyzerBase
{
private const string AsyncSuffix = "Async";

private const string AzureNamespace = "Azure";
private const string SystemNamespace = "System";
private const string PageableTypeName = "Pageable";
private const string AsyncPageableTypeName = "AsyncPageable";
private const string BinaryDataTypeName = "BinaryData";
Expand Down Expand Up @@ -156,7 +158,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
}

var pageableReturn = pageableTypeSymbol.TypeArguments.Single();
if (!IsOrImplements(pageableReturn, BinaryDataTypeName))
if (!IsOrImplements(pageableReturn, BinaryDataTypeName, SystemNamespace))
pshao25 marked this conversation as resolved.
Show resolved Hide resolved
{
return false;
}
Expand All @@ -174,7 +176,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
unwrappedType = namedTypeSymbol.TypeArguments.Single();
}

if (IsOrImplements(unwrappedType, ResponseTypeName))
if (IsOrImplements(unwrappedType, ResponseTypeName, AzureNamespace))
{
if (unwrappedType is INamedTypeSymbol responseTypeSymbol && responseTypeSymbol.IsGenericType)
{
Expand All @@ -186,12 +188,12 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
}
return;
}
else if (IsOrImplements(unwrappedType, OperationTypeName))
else if (IsOrImplements(unwrappedType, OperationTypeName, AzureNamespace))
{
if (unwrappedType is INamedTypeSymbol operationTypeSymbol && operationTypeSymbol.IsGenericType)
{
var operationReturn = operationTypeSymbol.TypeArguments.Single();
if (IsOrImplements(operationReturn, PageableTypeName) || IsOrImplements(operationReturn, AsyncPageableTypeName))
if (IsOrImplements(operationReturn, PageableTypeName, AzureNamespace) || IsOrImplements(operationReturn, AsyncPageableTypeName, AzureNamespace))
{
if (!IsValidPageable(operationReturn))
{
Expand All @@ -200,14 +202,14 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
return;
}

if (!IsOrImplements(operationReturn, BinaryDataTypeName))
if (!IsOrImplements(operationReturn, BinaryDataTypeName, SystemNamespace))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
}
return;
}
else if (IsOrImplements(originalType, PageableTypeName) || IsOrImplements(originalType, AsyncPageableTypeName))
else if (IsOrImplements(originalType, PageableTypeName, AzureNamespace) || IsOrImplements(originalType, AsyncPageableTypeName, AzureNamespace))
{
if (!IsValidPageable(originalType))
{
Expand All @@ -229,16 +231,16 @@ private static void CheckClientMethod(ISymbolAnalysisContext context, IMethodSym
}
}

private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName)
private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName, string namespaceName)
{
if (typeSymbol.Name == typeName)
if (typeSymbol.Name == typeName && typeSymbol.ContainingNamespace.Name == namespaceName && typeSymbol.ContainingNamespace.ContainingNamespace.Name == "")
{
return true;
}

if (typeSymbol.BaseType != null)
{
return IsOrImplements(typeSymbol.BaseType, typeName);
return IsOrImplements(typeSymbol.BaseType, typeName, namespaceName);
}

return false;
Expand All @@ -261,11 +263,11 @@ private static bool IsClientMethodReturnType(ISymbolAnalysisContext context, IMe
unwrappedType = namedTypeSymbol.TypeArguments.Single();
}

if (IsOrImplements(unwrappedType, ResponseTypeName) ||
IsOrImplements(unwrappedType, NullableResponseTypeName) ||
IsOrImplements(unwrappedType, OperationTypeName) ||
IsOrImplements(originalType, PageableTypeName) ||
IsOrImplements(originalType, AsyncPageableTypeName))
if (IsOrImplements(unwrappedType, ResponseTypeName, AzureNamespace) ||
IsOrImplements(unwrappedType, NullableResponseTypeName, AzureNamespace) ||
IsOrImplements(unwrappedType, OperationTypeName, AzureNamespace) ||
IsOrImplements(originalType, PageableTypeName, AzureNamespace) ||
IsOrImplements(originalType, AsyncPageableTypeName, AzureNamespace))
{
return true;
}
Expand Down