Add rules for convenience method and protocol method (#6934)

* Add rules for convenience method and protocol method

* update

* updadte
This commit is contained in:
Pan Shao 2023-09-28 11:51:19 +08:00 коммит произвёл GitHub
Родитель 02476be3e8
Коммит f05a61d141
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
6 изменённых файлов: 688 добавлений и 12 удалений

Просмотреть файл

@ -364,5 +364,37 @@ namespace RandomNamespace
.WithDisabledDiagnostics("AZC0015") .WithDisabledDiagnostics("AZC0015")
.RunAsync(); .RunAsync();
} }
[Fact]
public async Task AZC0004NotProducedForMethodsWithOverloadAlternative()
{
const string code = @"
using Azure;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(CancellationToken cancellationToken = default)
{
return null;
}
public virtual Response Get()
{
return null;
}
public virtual Response Get(CancellationToken cancellationToken)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
} }
} }

Просмотреть файл

@ -0,0 +1,64 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
using System.Threading.Tasks;
using Xunit;
using Verifier = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<Azure.ClientSdk.Analyzers.ClientMethodsAnalyzer>;
namespace Azure.ClientSdk.Analyzers.Tests
{
public class AZC0017Tests
{
[Fact]
public async Task AZC0017ProducedForMethodsWithRequestContentParameter()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> {|AZC0017:GetAsync|}(RequestContent content, CancellationToken cancellationToken = default)
{
return null;
}
public virtual Response {|AZC0017:Get|}(RequestContent content, CancellationToken cancellationToken = default)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0017NotProducedForMethodsWithCancellationToken()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(string s, CancellationToken cancellationToken = default)
{
return null;
}
public virtual Response Get(string s, CancellationToken cancellationToken = default)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
}
}

Просмотреть файл

@ -0,0 +1,367 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
using System.Threading.Tasks;
using Xunit;
using Verifier = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<Azure.ClientSdk.Analyzers.ClientMethodsAnalyzer>;
namespace Azure.ClientSdk.Analyzers.Tests
{
public class AZC0018Tests
{
[Fact]
public async Task AZC0018NotProducedForCorrectReturnType()
{
const string code = @"
using Azure;
using Azure.Core;
using System;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response<bool>> GetHeadAsBooleanAsync(string s, RequestContext context)
{
return null;
}
public virtual Response<bool> GetHeadAsBoolean(string s, RequestContext context)
{
return null;
}
public virtual Task<Response> GetResponseAsync(string s, RequestContext context)
{
return null;
}
public virtual Response GetResponse(string s, RequestContext context)
{
return null;
}
public virtual AsyncPageable<BinaryData> GetPageableAsync(string s, RequestContext context)
{
return null;
}
public virtual Pageable<BinaryData> GetPageable(string s, RequestContext context)
{
return null;
}
public virtual Task<Operation> GetOperationAsync(string s, RequestContext context)
{
return null;
}
public virtual Operation GetOperation(string s, RequestContext context)
{
return null;
}
public virtual Task<Operation<BinaryData>> GetOperationOfTAsync(string s, RequestContext context)
{
return null;
}
public virtual Operation<BinaryData> GetOperationOfT(string s, RequestContext context)
{
return null;
}
public virtual Task<Operation<AsyncPageable<BinaryData>>> GetOperationOfPageableAsync(string s, RequestContext context)
{
return null;
}
public virtual Operation<Pageable<BinaryData>> GetOperationOfPageable(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithGenericResponseOfPrimitive()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response<string>> {|AZC0018:GetAsync|}(string s, RequestContext context)
{
return null;
}
public virtual Response<string> {|AZC0018:Get|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithGenericResponseOfModel()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class Model
{
string a;
}
public class SomeClient
{
public virtual Task<Response<Model>> {|AZC0018:GetAsync|}(string s, RequestContext context)
{
return null;
}
public virtual Response<Model> {|AZC0018:Get|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithPageableOfModel()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class Model
{
string a;
}
public class SomeClient
{
public virtual AsyncPageable<Model> {|AZC0018:GetAsync|}(string s, RequestContext context)
{
return null;
}
public virtual Pageable<Model> {|AZC0018:Get|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithOperationOfModel()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class Model
{
string a;
}
public class SomeClient
{
public virtual Task<Operation<Model>> {|AZC0018:GetAsync|}(string s, RequestContext context)
{
return null;
}
public virtual Operation<Model> {|AZC0018:Get|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithOperationOfPageableModel()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
namespace RandomNamespace
{
public class Model
{
string a;
}
public class SomeClient
{
public virtual Task<Operation<AsyncPageable<Model>>> {|AZC0018:GetAsync|}(string s, RequestContext context)
{
return null;
}
public virtual Operation<Pageable<Model>> {|AZC0018:Get|}(string s, RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018ProducedForMethodsWithParameterModel()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
namespace RandomNamespace
{
public struct Model
{
string a;
}
public class SomeClient
{
public virtual Task<Response> {|AZC0018:GetAsync|}(Model model, Azure.RequestContext context)
{
return null;
}
public virtual Response {|AZC0018:Get|}(Model model, Azure.RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018NotProducedForMethodsWithNoRequestContentAndRequiredContext()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(string a, Azure.RequestContext context)
{
return null;
}
public virtual Response Get(string a, Azure.RequestContext context)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018NotProducedForMethodsWithNoRequestContentButOnlyProtocol()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(string a, Azure.RequestContext context = null)
{
return null;
}
public virtual Response Get(string a, Azure.RequestContext context = null)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
[Fact]
public async Task AZC0018NotProducedForMethodsWithRequestContentAndOptionalRequestContext()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(RequestContent content, RequestContext context = null)
{
return null;
}
public virtual Response Get(RequestContent content, RequestContext context = null)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
}
}

Просмотреть файл

@ -0,0 +1,51 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
using System.Threading.Tasks;
using Xunit;
using Verifier = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<Azure.ClientSdk.Analyzers.ClientMethodsAnalyzer>;
namespace Azure.ClientSdk.Analyzers.Tests
{
public class AZC0019Tests
{
[Fact]
public async Task AZC0019ProducedForMethodsWithNoRequestContentButProtocolAndConvenience()
{
const string code = @"
using Azure;
using Azure.Core;
using System.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;
namespace RandomNamespace
{
public class SomeClient
{
public virtual Task<Response> GetAsync(string a, CancellationToken cancellationToken = default)
{
return null;
}
public virtual Response Get(string a, CancellationToken cancellationToken = default)
{
return null;
}
public virtual Task<Response> {|AZC0019:GetAsync|}(string a, Azure.RequestContext context = null)
{
return null;
}
public virtual Response {|AZC0019:Get|}(string a, Azure.RequestContext context = null)
{
return null;
}
}
}";
await Verifier.CreateAnalyzer(code)
.RunAsync();
}
}
}

Просмотреть файл

@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. // Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. // Licensed under the MIT License.
using System.Collections.Generic;
using System.Collections.Immutable; using System.Collections.Immutable;
using System.Linq; using System.Linq;
using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis;
@ -15,19 +16,29 @@ namespace Azure.ClientSdk.Analyzers
private const string PageableTypeName = "Pageable"; private const string PageableTypeName = "Pageable";
private const string AsyncPageableTypeName = "AsyncPageable"; private const string AsyncPageableTypeName = "AsyncPageable";
private const string BinaryDataTypeName = "BinaryData";
private const string ResponseTypeName = "Response"; private const string ResponseTypeName = "Response";
private const string NullableResponseTypeName = "NullableResponse"; private const string NullableResponseTypeName = "NullableResponse";
private const string OperationTypeName = "Operation"; private const string OperationTypeName = "Operation";
private const string TaskTypeName = "Task"; private const string TaskTypeName = "Task";
private const string BooleanTypeName = "Boolean";
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(new[] public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(new[]
{ {
Descriptors.AZC0002, Descriptors.AZC0002,
Descriptors.AZC0003, Descriptors.AZC0003,
Descriptors.AZC0004, Descriptors.AZC0004,
Descriptors.AZC0015 Descriptors.AZC0015,
Descriptors.AZC0017,
Descriptors.AZC0018,
Descriptors.AZC0019,
}); });
private static bool IsRequestContent(IParameterSymbol parameterSymbol)
{
return parameterSymbol.Type.Name == "RequestContent";
}
private static bool IsRequestContext(IParameterSymbol parameterSymbol) private static bool IsRequestContext(IParameterSymbol parameterSymbol)
{ {
return parameterSymbol.Name == "context" && parameterSymbol.Type.Name == "RequestContext"; return parameterSymbol.Name == "context" && parameterSymbol.Type.Name == "RequestContext";
@ -43,7 +54,7 @@ namespace Azure.ClientSdk.Analyzers
return IsCancellationToken(parameterSymbol) || IsRequestContext(parameterSymbol); return IsCancellationToken(parameterSymbol) || IsRequestContext(parameterSymbol);
} }
private static void CheckIsLastArgumentCancellationTokenOrRequestContext(ISymbolAnalysisContext context, IMethodSymbol member) private static void CheckServiceMethod(ISymbolAnalysisContext context, IMethodSymbol member)
{ {
var lastArgument = member.Parameters.LastOrDefault(); var lastArgument = member.Parameters.LastOrDefault();
var isLastArgumentCancellationOrRequestContext = lastArgument != null && IsCancellationOrRequestContext(lastArgument); var isLastArgumentCancellationOrRequestContext = lastArgument != null && IsCancellationOrRequestContext(lastArgument);
@ -61,7 +72,9 @@ namespace Azure.ClientSdk.Analyzers
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member); context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member);
} }
} }
else if (IsCancellationToken(lastArgument) && !lastArgument.IsOptional) else if (IsCancellationToken(lastArgument))
{
if (!lastArgument.IsOptional)
{ {
var overloadWithCancellationToken = FindMethod( var overloadWithCancellationToken = FindMethod(
member.ContainingType.GetMembers(member.Name).OfType<IMethodSymbol>(), member.ContainingType.GetMembers(member.Name).OfType<IMethodSymbol>(),
@ -73,6 +86,137 @@ namespace Azure.ClientSdk.Analyzers
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member); context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member);
} }
} }
if (member.Parameters.FirstOrDefault(IsRequestContent) != null)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0017, member.Locations.FirstOrDefault()), member);
}
}
else if (IsRequestContext(lastArgument))
{
CheckProtocolMethodReturnType(context, member);
CheckProtocolMethodParameters(context, member);
}
}
private static string GetFullNamespaceName(IParameterSymbol parameter)
{
var currentNamespace = parameter.Type.ContainingNamespace;
string currentName = currentNamespace.Name;
string fullNamespace = "";
while (!string.IsNullOrEmpty(currentName))
{
fullNamespace = fullNamespace == "" ? currentName : $"{currentName}.{fullNamespace}";
currentNamespace = currentNamespace.ContainingNamespace;
currentName = currentNamespace.Name;
}
return fullNamespace;
}
// A protocol method should not have model as parameter. If it has ambiguity with convenience method, it should have required RequestContext.
// Ambiguity: doesn't have a RequestContent, but there is a method ending with CancellationToken has same type of parameters
// No ambiguity: has RequestContent.
private static void CheckProtocolMethodParameters(ISymbolAnalysisContext context, IMethodSymbol method)
{
var containsModel = method.Parameters.Any(p =>
{
var fullNamespace = GetFullNamespaceName(p);
return !fullNamespace.StartsWith("System") && !fullNamespace.StartsWith("Azure");
});
if (containsModel)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
return;
}
var requestContent = method.Parameters.FirstOrDefault(IsRequestContent);
if (requestContent == null && method.Parameters.Last().IsOptional)
{
INamedTypeSymbol type = (INamedTypeSymbol)context.Symbol;
IEnumerable<IMethodSymbol> methodList = type.GetMembers(method.Name).OfType<IMethodSymbol>().Where(member => !SymbolEqualityComparer.Default.Equals(member, method));
ImmutableArray<IParameterSymbol> parametersWithoutLast = method.Parameters.RemoveAt(method.Parameters.Length - 1);
IMethodSymbol convenienceMethod = FindMethod(methodList, method.TypeParameters, parametersWithoutLast, symbol => IsCancellationToken(symbol));
if (convenienceMethod != null)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0019, method.Locations.FirstOrDefault()), method);
}
}
}
// A protocol method should not have model as type. Accepted return type: Response, Task<Response>, Response<bool>, Task<Response<bool>>, Pageable<BinaryData>, AsyncPageable<BinaryData>, Operation<BinaryData>, Task<Operation<BinaryData>>, Operation, Task<Operation>, Operation<Pageable<BinaryData>>, Task<Operation<AsyncPageable<BinaryData>>>
private static void CheckProtocolMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method)
{
bool IsValidPageable(ITypeSymbol typeSymbol)
{
var pageableTypeSymbol = typeSymbol as INamedTypeSymbol;
if (!pageableTypeSymbol.IsGenericType)
{
return false;
}
var pageableReturn = pageableTypeSymbol.TypeArguments.Single();
if (!IsOrImplements(pageableReturn, BinaryDataTypeName))
{
return false;
}
return true;
}
ITypeSymbol originalType = method.ReturnType;
ITypeSymbol unwrappedType = method.ReturnType;
if (method.ReturnType is INamedTypeSymbol namedTypeSymbol &&
namedTypeSymbol.IsGenericType &&
namedTypeSymbol.Name == TaskTypeName)
{
unwrappedType = namedTypeSymbol.TypeArguments.Single();
}
if (IsOrImplements(unwrappedType, ResponseTypeName))
{
if (unwrappedType is INamedTypeSymbol responseTypeSymbol && responseTypeSymbol.IsGenericType)
{
var responseReturn = responseTypeSymbol.TypeArguments.Single();
if (responseReturn.Name != BooleanTypeName)
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
}
return;
}
else if (IsOrImplements(unwrappedType, OperationTypeName))
{
if (unwrappedType is INamedTypeSymbol operationTypeSymbol && operationTypeSymbol.IsGenericType)
{
var operationReturn = operationTypeSymbol.TypeArguments.Single();
if (IsOrImplements(operationReturn, PageableTypeName) || IsOrImplements(operationReturn, AsyncPageableTypeName))
{
if (!IsValidPageable(operationReturn))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
return;
}
if (!IsOrImplements(operationReturn, BinaryDataTypeName))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
}
return;
}
else if (IsOrImplements(originalType, PageableTypeName) || IsOrImplements(originalType, AsyncPageableTypeName))
{
if (!IsValidPageable(originalType))
{
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
}
return;
}
context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
} }
private static void CheckClientMethod(ISymbolAnalysisContext context, IMethodSymbol member) private static void CheckClientMethod(ISymbolAnalysisContext context, IMethodSymbol member)
@ -164,7 +308,7 @@ namespace Azure.ClientSdk.Analyzers
if (IsClientMethodReturnType(context, methodSymbol, false)) if (IsClientMethodReturnType(context, methodSymbol, false))
{ {
CheckIsLastArgumentCancellationTokenOrRequestContext(context, methodSymbol); CheckServiceMethod(context, methodSymbol);
} }
} }
} }

Просмотреть файл

@ -112,6 +112,24 @@ namespace Azure.ClientSdk.Analyzers
"Usage", "Usage",
DiagnosticSeverity.Warning, true); DiagnosticSeverity.Warning, true);
public static DiagnosticDescriptor AZC0017 = new DiagnosticDescriptor(
nameof(AZC0017),
"Invalid convenience method signature.",
"Convenience methods shouldn't have parameters with the RequestContent type.",
"Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null);
public static DiagnosticDescriptor AZC0018 = new DiagnosticDescriptor(
nameof(AZC0018),
"Invalid protocol method signature.",
"Protocol methods should take a RequestContext parameter called `context` and not use a model type in a parameter or return type.",
"Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null);
public static DiagnosticDescriptor AZC0019 = new DiagnosticDescriptor(
nameof(AZC0019),
"Potential ambiguous call exists.",
"There will be an ambiguous call error when the user calls with only the required parameters. All parameters of the protocol method should be required.",
"Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null);
public static DiagnosticDescriptor AZC0020 = new DiagnosticDescriptor( public static DiagnosticDescriptor AZC0020 = new DiagnosticDescriptor(
nameof(AZC0020), nameof(AZC0020),
"Avoid using banned types in public APIs", "Avoid using banned types in public APIs",