From 9f9b90ed63d02376f3e466a0f65bd8f815b0e0c6 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:54:19 +0800 Subject: [PATCH] Update --- .../ClientMethodsAnalyzer.cs | 68 ++++++++++--------- .../Azure.ClientSdk.Analyzers/Descriptors.cs | 4 +- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs index 4dc8ea120d9..d500588f70e 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs @@ -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 SupportedDiagnostics { get; } = ImmutableArray.Create(new[] { Descriptors.AZC0002, @@ -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); } @@ -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; @@ -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; } @@ -217,7 +224,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol) } var pageableReturn = pageableTypeSymbol.TypeArguments.Single(); - if (pageableReturn.Name != "BinaryData") + if (!IsOrImplements(pageableReturn, BinaryDataTypeName)) { return false; } @@ -230,12 +237,12 @@ 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) { @@ -243,12 +250,12 @@ bool IsValidPageable(ITypeSymbol typeSymbol) } 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)) { @@ -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)) { @@ -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) @@ -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) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs index 51331f93a7d..3fd4dda7bd4 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs @@ -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