Skip to content

Commit

Permalink
Update
Browse files Browse the repository at this point in the history
  • Loading branch information
@pshao25
pshao25 committed Jul 17, 2023
1 parent 73d1830 commit 9f9b90e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 33 deletions.
68 changes: 37 additions & 31 deletions src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ public class ClientMethodsAnalyzer : ClientAnalyzerBase
{
private const string AsyncSuffix = "Async";

private const string PageableTypeName = "Pageable";
private const string AsyncPageableTypeName = "AsyncPageable";
private const string BinaryDataTypeName = "BinaryData";
private const string ResponseTypeName = "Response";
private const string NullableResponseTypeName = "NullableResponse";
private const string OperationTypeName = "Operation";
private const string TaskTypeName = "Task";

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(new[]
{
Descriptors.AZC0002,
Expand Down Expand Up @@ -140,8 +148,7 @@ static bool IsCancellationOrRequestContext(IParameterSymbol parameterSymbol)
else if (IsCancellationToken(lastArgument))
{
// A convenience method should not have RequestContent as parameter
var requestContent = member.Parameters.FirstOrDefault(parameter => parameter.Type.Name == "RequestContent");
if (requestContent != null)
if (member.Parameters.FirstOrDefault(IsRequestContent) != null)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0017, member.Locations.FirstOrDefault()), member);
}
Expand Down Expand Up @@ -186,7 +193,7 @@ private static void CheckProtocolMethodParameters(ISymbolAnalysisContext context
return;
}

var requestContent = method.Parameters.FirstOrDefault(p => p.Type.Name == "RequestContent");
var requestContent = method.Parameters.FirstOrDefault(IsRequestContent);
if (requestContent == null && method.Parameters.Last().IsOptional)
{
INamedTypeSymbol type = (INamedTypeSymbol)context.Symbol;
Expand All @@ -205,7 +212,7 @@ private static void CheckProtocolMethodReturnType(ISymbolAnalysisContext context
{
bool IsValidPageable(ITypeSymbol typeSymbol)
{
if (typeSymbol.Name != "Pageable" && typeSymbol.Name != "AsyncPageable")
if ((!IsOrImplements(typeSymbol, PageableTypeName)) && (!IsOrImplements(typeSymbol, AsyncPageableTypeName)))
{
return false;
}
Expand All @@ -217,7 +224,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
}

var pageableReturn = pageableTypeSymbol.TypeArguments.Single();
if (pageableReturn.Name != "BinaryData")
if (!IsOrImplements(pageableReturn, BinaryDataTypeName))
{
return false;
}
Expand All @@ -230,25 +237,25 @@ bool IsValidPageable(ITypeSymbol typeSymbol)

if (method.ReturnType is INamedTypeSymbol namedTypeSymbol &&
namedTypeSymbol.IsGenericType &&
namedTypeSymbol.Name == "Task")
namedTypeSymbol.Name == TaskTypeName)
{
unwrappedType = namedTypeSymbol.TypeArguments.Single();
}

if (unwrappedType.Name == "Response")
if (IsOrImplements(unwrappedType, ResponseTypeName))
{
if (unwrappedType is INamedTypeSymbol responseTypeSymbol && responseTypeSymbol.IsGenericType)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
return;
}
else if (unwrappedType.Name == "Operation")
else if (IsOrImplements(unwrappedType, OperationTypeName))
{
if (unwrappedType is INamedTypeSymbol operationTypeSymbol && operationTypeSymbol.IsGenericType)
{
var operationReturn = operationTypeSymbol.TypeArguments.Single();
if (operationReturn.Name == "Pageable" || operationReturn.Name == "AsyncPageable")
if (IsOrImplements(operationReturn, PageableTypeName) || IsOrImplements(operationReturn, AsyncPageableTypeName))
{
if (!IsValidPageable(operationReturn))
{
Expand All @@ -257,14 +264,14 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
return;
}

if (operationReturn.Name != "BinaryData")
if (!IsOrImplements(operationReturn, BinaryDataTypeName))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
}
return;
}
else if (originalType.Name == "Pageable" || originalType.Name == "AsyncPageable")
else if (IsOrImplements(originalType, PageableTypeName) || IsOrImplements(originalType, AsyncPageableTypeName))
{
if (!IsValidPageable(originalType))
{
Expand All @@ -276,45 +283,44 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}

private static void CheckClientMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method)
private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName)
{
bool IsOrImplements(ITypeSymbol typeSymbol, string typeName)
if (typeSymbol.Name == typeName)
{
if (typeSymbol.Name == typeName)
{
return true;
}

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

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

return false;
}

private static void CheckClientMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method)
{
ITypeSymbol originalType = method.ReturnType;
ITypeSymbol unwrappedType = method.ReturnType;

if (method.ReturnType is INamedTypeSymbol namedTypeSymbol &&
namedTypeSymbol.IsGenericType &&
namedTypeSymbol.Name == "Task")
namedTypeSymbol.Name == TaskTypeName)
{
unwrappedType = namedTypeSymbol.TypeArguments.Single();
}

if (IsOrImplements(unwrappedType, "Response") ||
IsOrImplements(unwrappedType, "NullableResponse") ||
IsOrImplements(unwrappedType, "Operation") ||
IsOrImplements(originalType, "Pageable") ||
IsOrImplements(originalType, "AsyncPageable") ||
if (IsOrImplements(unwrappedType, ResponseTypeName) ||
IsOrImplements(unwrappedType, NullableResponseTypeName) ||
IsOrImplements(unwrappedType, OperationTypeName) ||
IsOrImplements(originalType, PageableTypeName) ||
IsOrImplements(originalType, AsyncPageableTypeName) ||
originalType.Name.EndsWith(ClientSuffix))
{
return;
}

context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0015, method.Locations.FirstOrDefault(), originalType.ToDisplayString()), method);

}

private bool IsCheckExempt(IMethodSymbol method)
Expand All @@ -323,7 +329,7 @@ private bool IsCheckExempt(IMethodSymbol method)
method.DeclaredAccessibility != Accessibility.Public ||
method.OverriddenMethod != null ||
method.IsImplicitlyDeclared ||
(method.Name.StartsWith("Get") && method.Name.EndsWith("Client"));
(method.Name.StartsWith("Get") && method.Name.EndsWith(ClientSuffix));
}

public override void AnalyzeCore(ISymbolAnalysisContext context)
Expand Down
4 changes: 2 additions & 2 deletions src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ internal class Descriptors

public static DiagnosticDescriptor AZC0018 = new DiagnosticDescriptor(
nameof(AZC0018),
"Do ensure protocol method take a RequestContext parameter called context and not take models as parameter type or return type. Do ensure protocol method will not cause ambiguous call with convenience method.",
"Protocol method should have requestContext as the last parameter and don't have model as parameter type or return type. Protocol method should not have optional parameters if ambiguity exists between protocol method and convenience method.",
"Do ensure protocol methods take a RequestContext parameter called `context` and do not take models as parameter or return types. Do ensure protocol methods will not cause an ambiguous overload resolution with convenience methods.",
"Protocol method should have `requestContext` as the last parameter and not use a model as parameter or return types. Protocol methods should not have optional parameters if ambiguity exists between the protocol method and convenience methods.",
"Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null);
#endregion

Expand Down

0 comments on commit 9f9b90e

Please sign in to comment.