From 14554b1f9d478b83dedecad63ef2958ff56a5335 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Wed, 2 Aug 2023 14:49:55 +0800 Subject: [PATCH] update --- .../AZC0002Tests.cs | 34 +++++--- .../AZC0003Tests.cs | 8 +- .../AZC0004Tests.cs | 81 +++++++++---------- .../AZC0015Tests.cs | 6 +- .../AZC0017Tests.cs | 6 +- .../AZC0018Tests.cs | 2 + .../ClientMethodsAnalyzer.cs | 38 +++++---- 7 files changed, 94 insertions(+), 81 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0002Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0002Tests.cs index b63e6dbe8f0..3c5b56b99ed 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0002Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0002Tests.cs @@ -13,19 +13,21 @@ public class AZC0002Tests public async Task AZC0002ProducedForMethodsWithoutCancellationTokenOrRequestContext() { const string code = @" +using Azure; using System.Threading.Tasks; namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}() + public virtual Response {|AZC0002:GetAsync|}() { return null; } - public virtual void {|AZC0002:Get|}() + public virtual Response {|AZC0002:Get|}() { + return null; } } }"; @@ -38,6 +40,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0002ProducedForMethodsWithWrongNameCancellationToken() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -45,13 +48,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellation = default) + public virtual Response {|AZC0002:GetAsync|}(CancellationToken cancellation = default) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellation = default) + public virtual Response {|AZC0002:Get|}(CancellationToken cancellation = default) { + return null; } } }"; @@ -65,6 +69,7 @@ public async Task AZC0002ProducedForMethodsWithWrongNameRequestContext() { const string code = @" using Azure; +using Azure.Core; using System.Threading; using System.Threading.Tasks; @@ -72,13 +77,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(RequestContext cancellation = default) + public virtual Response {|AZC0002:GetAsync|}(RequestContext cancellation = default) { return null; } - public virtual void {|AZC0002:Get|}(RequestContext cancellation = default) + public virtual Response {|AZC0002:Get|}(RequestContext cancellation = default) { + return null; } } }"; @@ -91,6 +97,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0002ProducedForMethodsWithNonOptionalCancellationToken() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -98,13 +105,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken) + public virtual Response {|AZC0002:GetAsync|}(CancellationToken cancellationToken) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken) + public virtual Response {|AZC0002:Get|}(CancellationToken cancellationToken) { + return null; } } }"; @@ -125,13 +133,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(RequestContext context = default, string text = default) + public virtual Response {|AZC0002:GetAsync|}(RequestContext context = default, string text = default) { return null; } - public virtual void {|AZC0002:Get|}(RequestContext context = default, string text = default) + public virtual Response {|AZC0002:Get|}(RequestContext context = default, string text = default) { + return null; } } }"; @@ -152,13 +161,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken = default, string text = default) + public virtual Response {|AZC0002:GetAsync|}(CancellationToken cancellationToken = default, string text = default) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken = default, string text = default) + public virtual Response {|AZC0002:Get|}(CancellationToken cancellationToken = default, string text = default) { + return null; } } }"; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0003Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0003Tests.cs index f37e3bef93d..9b7cfe2c483 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0003Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0003Tests.cs @@ -13,6 +13,7 @@ public class AZC0003Tests public async Task AZC0003ProducedForNonVirtualMethods() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -20,13 +21,14 @@ namespace RandomNamespace { public class SomeClient { - public Task {|AZC0003:GetAsync|](CancellationToken cancellationToken = default) + public Response {|AZC0003:GetAsync|](CancellationToken cancellationToken = default) { return null; } - public void {|AZC0003:Get|](CancellationToken cancellationToken = default) + public Response {|AZC0003:Get|](CancellationToken cancellationToken = default) { + return null; } } }"; @@ -95,4 +97,4 @@ await Verifier.CreateAnalyzer(code) .RunAsync(); } } -} \ No newline at end of file +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0004Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0004Tests.cs index 70bc7444df0..efb2d992778 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0004Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0004Tests.cs @@ -13,6 +13,7 @@ public class AZC0004Tests public async Task AZC0004ProducedForMethodsWithoutSyncAlternative() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -20,7 +21,7 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:GetAsync|}(CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:GetAsync|}(CancellationToken cancellationToken = default) { return null; } @@ -35,6 +36,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForMethodsWithoutAsyncAlternative() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -42,7 +44,7 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:Get|}(CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:Get|}(CancellationToken cancellationToken = default) { return null; } @@ -146,11 +148,11 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:GetAsync|}(string a, CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:GetAsync|}(string a, CancellationToken cancellationToken = default) { return null; } - public virtual Task {|AZC0004:Get|}(string a, RequestContext context) + public virtual Response {|AZC0004:Get|}(string a, RequestContext context) { return null; } @@ -193,6 +195,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForGenericMethodsWithSyncAlternative() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -200,16 +203,17 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task GetAsync(CancellationToken cancellationToken = default) + public virtual Response GetAsync(CancellationToken cancellationToken = default) { return null; } - public virtual void Get(CancellationToken cancellationToken = default) + public virtual Response Get(CancellationToken cancellationToken = default) { + return null; } - public virtual Task {|AZC0004:GetAsync|}(CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:GetAsync|}(CancellationToken cancellationToken = default) { return null; } @@ -229,8 +233,15 @@ public async Task AZC0004NotProducedForGenericMethodsWithSyncAlternative() namespace RandomNamespace { +public class CapacityReservationCollection{ +} public class SomeClient { + public virtual CapacityReservationCollection GetCapacityReservations() + { + return null; + } + public virtual Task GetAsync(CancellationToken cancellationToken = default) { return null; @@ -260,6 +271,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForGenericMethodsTakingGenericArgWithoutSyncAlternative() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -267,16 +279,17 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task GetAsync(CancellationToken cancellationToken = default) + public virtual Response GetAsync(CancellationToken cancellationToken = default) { return null; } - public virtual void Get(CancellationToken cancellationToken = default) + public virtual Response Get(CancellationToken cancellationToken = default) { + return null; } - public virtual Task {|AZC0004:GetAsync|}(T item, CancellationToken cancellationToken = default) + public virtual Task {|AZC0004:GetAsync|}(T item, CancellationToken cancellationToken = default) { return null; } @@ -323,32 +336,6 @@ await Verifier.CreateAnalyzer(code) .RunAsync(); } - [Fact] - public async Task AZC0004NProducedForMethodsWithoutArgMatchedSyncAlternative() - { - const string code = @" -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0004:GetAsync|}(int sameNameDifferentType, CancellationToken cancellationToken = default) - { - return null; - } - - public virtual void {|AZC0004:Get|}(string sameNameDifferentType, CancellationToken cancellationToken = default) - { - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - [Fact] public async Task AZC0004NotProducedForGenericMethodsTakingGenericExpressionArgWithSyncAlternative() { @@ -381,6 +368,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForGenericMethodsTakingGenericExpressionArgWithoutSyncAlternative() { const string code = @" +using Azure; using System; using System.Linq.Expressions; using System.Threading; @@ -390,13 +378,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:QueryAsync|}(Expression> filter, CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:QueryAsync|}(Expression> filter, CancellationToken cancellationToken = default) { return null; } - public virtual void {|AZC0004:Query|}(Expression> filter, CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:Query|}(Expression> filter, CancellationToken cancellationToken = default) { + return null; } } }"; @@ -442,6 +431,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForArrayTypesWithoutSyncAlternative() { const string code = @" +using Azure; using System; using System.IO; using System.Threading; @@ -451,7 +441,7 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:AppendAsync|}( + public virtual Response {|AZC0004:AppendAsync|}( byte[] arr, CancellationToken cancellationToken = default) { @@ -459,10 +449,11 @@ public class SomeClient } - public virtual void {|AZC0004:Append|}( + public virtual Response {|AZC0004:Append|}( string[] arr, CancellationToken cancellationToken = default) { + return null; } } }"; @@ -475,6 +466,7 @@ await Verifier.CreateAnalyzer(code) public async Task AZC0004ProducedForMethodsWithoutSyncAlternativeWithMatchingArgNames() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -482,13 +474,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0004:GetAsync|}(int foo, CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:GetAsync|}(int foo, CancellationToken cancellationToken = default) { return null; } - public virtual void {|AZC0004:Get|}(int differentName, CancellationToken cancellationToken = default) + public virtual Response {|AZC0004:Get|}(int differentName, CancellationToken cancellationToken = default) { + return null; } } }"; @@ -532,13 +525,13 @@ namespace RandomNamespace { public class SomeClient { - public SubClient GetSubClient() + public Sub GetSubClient() { return null; } } - public class SubClient + public class Sub { } }"; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0015Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0015Tests.cs index 462a30629f1..05e5aa8994c 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0015Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0015Tests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using System.Threading.Tasks; @@ -9,7 +9,7 @@ namespace Azure.ClientSdk.Analyzers.Tests { public class AZC0015Tests { - [Theory] + [Theory(Skip = "We use return type as an exit criteria to check method, so this test set is not applicable until we find a better criteria.")] [InlineData("public Task> {|AZC0015:ClientMethodAsync|}() { return default; }")] [InlineData("public Task> {|AZC0015:ClientMethodAsync|}() { return default; }")] [InlineData("public int {|AZC0015:ClientMethodAsync|}() { return default; }")] @@ -36,7 +36,7 @@ await Verifier.CreateAnalyzer(code) .RunAsync(); } - [Theory] + [Theory(Skip = "We use return type as an exit criteria to check method, so this test set is not applicable until we find a better criteria.")] [InlineData("public Task> ClientMethodAsync() { return default; }")] [InlineData("public Operation ClientMethodAsync() { return default; }")] [InlineData("public Pageable ClientMethodAsync() { return default; }")] diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0017Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0017Tests.cs index 41edbc10aee..a8f25144260 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0017Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0017Tests.cs @@ -13,6 +13,7 @@ public class AZC0017Tests public async Task AZC0017ProducedForMethodsWithRequestContentParameter() { const string code = @" +using Azure; using Azure.Core; using System.Threading; using System.Threading.Tasks; @@ -21,13 +22,14 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0017:GetAsync|}(RequestContent content, CancellationToken cancellationToken = default) + public virtual Response {|AZC0017:GetAsync|}(RequestContent content, CancellationToken cancellationToken = default) { return null; } - public virtual void {|AZC0017:Get|}(RequestContent content, CancellationToken cancellationToken = default) + public virtual Response {|AZC0017:Get|}(RequestContent content, CancellationToken cancellationToken = default) { + return null; } } }"; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0018Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0018Tests.cs index 7a593bd42dc..956592f0350 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0018Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0018Tests.cs @@ -21,6 +21,8 @@ public async Task AZC0018NotProducedForCorrectReturnType() namespace RandomNamespace { +public class AssetConversion {} + public class SomeClient { public virtual Task GetResponseAsync(string s, RequestContext context) 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 8caf97d2788..d3be35e253c 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs @@ -221,6 +221,11 @@ private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName) } private static void CheckClientMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method) + { + IsClientMethodReturnType(context, method, true); + } + + private static bool IsClientMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method, bool throwError = false) { ITypeSymbol originalType = method.ReturnType; ITypeSymbol unwrappedType = method.ReturnType; @@ -236,56 +241,55 @@ private static void CheckClientMethodReturnType(ISymbolAnalysisContext context, IsOrImplements(unwrappedType, NullableResponseTypeName) || IsOrImplements(unwrappedType, OperationTypeName) || IsOrImplements(originalType, PageableTypeName) || - IsOrImplements(originalType, AsyncPageableTypeName) || - originalType.Name.EndsWith(ClientSuffix)) + IsOrImplements(originalType, AsyncPageableTypeName)) { - return; + return true; } - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0015, method.Locations.FirstOrDefault(), originalType.ToDisplayString()), method); + if (throwError) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0015, method.Locations.FirstOrDefault(), originalType.ToDisplayString()), method); + } + return false; } - private bool IsCheckExempt(IMethodSymbol method) + private bool IsCheckExempt(ISymbolAnalysisContext context, IMethodSymbol method) { return method.MethodKind != MethodKind.Ordinary || method.DeclaredAccessibility != Accessibility.Public || method.OverriddenMethod != null || method.IsImplicitlyDeclared || - (method.Name.StartsWith("Get") && method.Name.EndsWith(ClientSuffix)); + !IsClientMethodReturnType(context, method); } public override void AnalyzeCore(ISymbolAnalysisContext context) { INamedTypeSymbol type = (INamedTypeSymbol)context.Symbol; - List visitedSyncMember = new List(); foreach (var member in type.GetMembers()) { - if (member is IMethodSymbol asyncMethodSymbol && !IsCheckExempt(asyncMethodSymbol) && asyncMethodSymbol.Name.EndsWith(AsyncSuffix)) + if (member is IMethodSymbol asyncMethodSymbol && !IsCheckExempt(context, asyncMethodSymbol) && asyncMethodSymbol.Name.EndsWith(AsyncSuffix)) { - CheckClientMethod(context, asyncMethodSymbol); - var syncMemberName = member.Name.Substring(0, member.Name.Length - AsyncSuffix.Length); - var syncMember = FindMethod(type.GetMembers(syncMemberName).OfType(), asyncMethodSymbol.TypeParameters, asyncMethodSymbol.Parameters); if (syncMember == null) { context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0004, member.Locations.First()), member); } - else - { - visitedSyncMember.Add(syncMember); - CheckClientMethod(context, syncMember); - } + + CheckClientMethod(context, asyncMethodSymbol); } - else if (member is IMethodSymbol syncMethodSymbol && !IsCheckExempt(syncMethodSymbol) && !syncMethodSymbol.Name.EndsWith(AsyncSuffix) && !visitedSyncMember.Contains(syncMethodSymbol)) + else if (member is IMethodSymbol syncMethodSymbol && !IsCheckExempt(context, syncMethodSymbol) && !syncMethodSymbol.Name.EndsWith(AsyncSuffix)) { var asyncMemberName = member.Name + AsyncSuffix; var asyncMember = FindMethod(type.GetMembers(asyncMemberName).OfType(), syncMethodSymbol.TypeParameters, syncMethodSymbol.Parameters); + if (asyncMember == null) { context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0004, member.Locations.First()), member); } + + CheckClientMethod(context, syncMethodSymbol); } } }