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..582a4f00c6c 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 @@ -10,7 +10,7 @@ namespace Azure.ClientSdk.Analyzers.Tests public class AZC0002Tests { [Fact] - public async Task AZC0002ProducedForMethodsWithoutCancellationTokenOrRequestContext() + public async Task AZC0002ProducedForMethodsWithoutCancellationToken() { const string code = @" using System.Threading.Tasks; @@ -35,7 +35,7 @@ await Verifier.CreateAnalyzer(code) } [Fact] - public async Task AZC0002ProducedForMethodsWithWrongNameCancellationToken() + public async Task AZC0002ProducedForMethodsWithNonOptionalCancellationToken() { const string code = @" using System.Threading; @@ -45,12 +45,12 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellation = default) + public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellation = default) + public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken) { } } @@ -61,10 +61,9 @@ await Verifier.CreateAnalyzer(code) } [Fact] - public async Task AZC0002ProducedForMethodsWithWrongNameRequestContext() + public async Task AZC0002ProducedForMethodsWithWrongNameParameter() { const string code = @" -using Azure; using System.Threading; using System.Threading.Tasks; @@ -72,12 +71,12 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(RequestContext cancellation = default) + public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellation = default) { return null; } - public virtual void {|AZC0002:Get|}(RequestContext cancellation = default) + public virtual void {|AZC0002:Get|}(CancellationToken cancellation = default) { } } @@ -86,11 +85,12 @@ await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") .RunAsync(); } - + [Fact] - public async Task AZC0002ProducedForMethodsWithNonOptionalCancellationToken() + public async Task AZC0002ProducedForMethodsWhereRequestContextIsNotLast() { const string code = @" +using Azure; using System.Threading; using System.Threading.Tasks; @@ -98,12 +98,12 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken) + public virtual Task {|AZC0002:GetAsync|}(RequestContext context = default, string text = default) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken) + public virtual void {|AZC0002:Get|}(RequestContext context = default, string text = default) { } } @@ -112,12 +112,11 @@ await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") .RunAsync(); } - + [Fact] - public async Task AZC0002ProducedForMethodsWhereRequestContextIsNotLast() + public async Task AZC0002DoesntFireIfThereIsAnOverloadWithCancellationToken() { const string code = @" -using Azure; using System.Threading; using System.Threading.Tasks; @@ -125,12 +124,21 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(RequestContext context = default, string text = default) + public virtual Task GetAsync(string s) { return null; } - public virtual void {|AZC0002:Get|}(RequestContext context = default, string text = default) + public virtual void Get(string s) + { + } + + public virtual Task GetAsync(string s, CancellationToken cancellationToken) + { + return null; + } + + public virtual void Get(string s, CancellationToken cancellationToken) { } } @@ -138,10 +146,10 @@ public class SomeClient await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") .RunAsync(); - } - + } + [Fact] - public async Task AZC0002ProducedForMethodsWhereCancellationTokenIsNotLast() + public async Task AZC0002DoesntFireIfThereIsAnOverloadWithRequestContext() { const string code = @" using Azure; @@ -152,38 +160,21 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken = default, string text = default) + public virtual Task GetAsync(string s) { return null; } - public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken = default, string text = default) + public virtual void Get(string s) { } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - [Fact] - public async Task AZC0002NotProducedForMethodsWithCancellationToken() - { - const string code = @" -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(CancellationToken cancellationToken = default) + public virtual Task GetAsync(string s, RequestContext context = default) { return null; } - public virtual void Get(CancellationToken cancellationToken = default) + public virtual void Get(string s, RequestContext context = default) { } } @@ -191,13 +182,12 @@ public virtual void Get(CancellationToken cancellationToken = default) await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") .RunAsync(); - } - + } + [Fact] - public async Task AZC0002NotProducedForMethodsWithRequestContextAndCancellationToken() + public async Task AZC0002ProducedWhenCancellationTokenOverloadsDontMatch() { const string code = @" -using Azure; using System.Threading; using System.Threading.Tasks; @@ -205,50 +195,56 @@ namespace RandomNamespace { public class SomeClient { - public virtual Task Get1Async(string s, CancellationToken cancellationToken = default) + public virtual Task {|AZC0002:GetAsync|}(string s) { return null; } - public virtual void Get1(string s, CancellationToken cancellationToken = default) + public virtual void {|AZC0002:Get|}(string s) { } - public virtual Task Get1Async(string s, RequestContext context) + public virtual Task {|AZC0002:GetAsync|}(CancellationToken cancellationToken) { return null; } - public virtual void Get1(string s, RequestContext context) + public virtual void {|AZC0002:Get|}(CancellationToken cancellationToken) { } - - public virtual Task Get2Async(CancellationToken cancellationToken = default) - { - return null; + } +}"; + await Verifier.CreateAnalyzer(code) + .WithDisabledDiagnostics("AZC0015") + .RunAsync(); } - public virtual void Get2(CancellationToken cancellationToken = default) + [Fact] + public async Task AZC0002NotProducedForMethodsWithCancellationToken() { - } + const string code = @" +using System.Threading; +using System.Threading.Tasks; - public virtual Task Get2Async(RequestContext context) +namespace RandomNamespace +{ + public class SomeClient + { + public virtual Task GetAsync(CancellationToken cancellationToken = default) { return null; } - public virtual void Get2(RequestContext context) + public virtual void Get(CancellationToken cancellationToken = default) { } } }"; await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") - .WithDisabledDiagnostics("AZC0018") - .WithDisabledDiagnostics("AD0001") .RunAsync(); - } - + } + [Fact] public async Task AZC0002NotProducedForMethodsWithRequestContext() { @@ -269,20 +265,10 @@ public virtual Task GetAsync(RequestContext context = default) public virtual void Get(RequestContext context = default) { } - - public virtual Task Get2Async(RequestContext context) - { - return null; - } - - public virtual void Get2(RequestContext context) - { - } } }"; await Verifier.CreateAnalyzer(code) .WithDisabledDiagnostics("AZC0015") - .WithDisabledDiagnostics("AZC0018") .RunAsync(); } } 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 3d35c522ea7..ca9cddf34a5 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 @@ -1,6 +1,6 @@ // 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; @@ -32,29 +32,7 @@ await Verifier.CreateAnalyzer(code) } [Fact] - public async Task AZC0004ProducedForMethodsWithoutAsyncAlternative() - { - const string code = @" -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0004:Get|}(CancellationToken cancellationToken = default) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - - [Fact] - public async Task AZC0004NotProducedForMethodsWithCancellationToken() + public async Task AZC0004NotProducedForMethodsWithoutSyncAlternative() { const string code = @" using System.Threading; @@ -79,116 +57,6 @@ await Verifier.CreateAnalyzer(code) .RunAsync(); } - [Fact] - public async Task AZC0004NotProducedForMethodsWithOptionalRequestContext() - { - const string code = @" -using Azure; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(RequestContext context = null) - { - return null; - } - public virtual Task Get(RequestContext context = null) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .WithDisabledDiagnostics("AZC0018") - .RunAsync(); - } - - [Fact] - public async Task AZC0004NotProducedForMethodsWithRequiredRequestContext() - { - const string code = @" -using Azure; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(RequestContext context) - { - return null; - } - public virtual Response Get(RequestContext context) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - - [Fact] - public async Task AZC0004ProducedForMethodsNotMatch() - { - const string code = @" -using Azure; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0004:GetAsync|}(string a, CancellationToken cancellationToken = default) - { - return null; - } - public virtual Task {|AZC0004:Get|}(string a, RequestContext context) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - - [Fact] - public async Task AZC0004ProducedForMethodsWithNotMatchedRequestContext() - { - const string code = @" -using Azure; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0004:GetAsync|}(RequestContext context = null) - { - return null; - } - public virtual Response {|AZC0004:Get|}(RequestContext context) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .WithDisabledDiagnostics("AZC0018") - .RunAsync(); - } - [Fact] public async Task AZC0004ProducedForGenericMethodsWithSyncAlternative() { @@ -339,7 +207,7 @@ public class SomeClient return null; } - public virtual void {|AZC0004:Get|}(string sameNameDifferentType, CancellationToken cancellationToken = default) + public virtual void Get(string sameNameDifferentType, CancellationToken cancellationToken = default) { } } @@ -395,7 +263,7 @@ public class SomeClient return null; } - public virtual void {|AZC0004:Query|}(Expression> filter, CancellationToken cancellationToken = default) + public virtual void Query(Expression> filter, CancellationToken cancellationToken = default) { } } @@ -459,7 +327,7 @@ public class SomeClient } - public virtual void {|AZC0004:Append|}( + public virtual void Append( string[] arr, CancellationToken cancellationToken = default) { @@ -487,7 +355,7 @@ public class SomeClient return null; } - public virtual void {|AZC0004:Get|}(int differentName, CancellationToken cancellationToken = default) + public virtual void Get(int differentName, CancellationToken cancellationToken = default) { } } @@ -497,4 +365,4 @@ await Verifier.CreateAnalyzer(code) .RunAsync(); } } -} +} \ No newline at end of file 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 deleted file mode 100644 index 41edbc10aee..00000000000 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0017Tests.cs +++ /dev/null @@ -1,66 +0,0 @@ -// 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; - -namespace Azure.ClientSdk.Analyzers.Tests -{ - public class AZC0017Tests - { - [Fact] - public async Task AZC0017ProducedForMethodsWithRequestContentParameter() - { - const string code = @" -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0017:GetAsync|}(RequestContent content, CancellationToken cancellationToken = default) - { - return null; - } - - public virtual void {|AZC0017:Get|}(RequestContent content, CancellationToken cancellationToken = default) - { - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - - [Fact] - public async Task AZC0017NotProducedForMethodsWithCancellationToken() - { - const string code = @" -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(string s, CancellationToken cancellationToken = default) - { - return null; - } - - public virtual void Get(string s, CancellationToken cancellationToken = default) - { - } - } -}"; - await Verifier.CreateAnalyzer(code) - .WithDisabledDiagnostics("AZC0015") - .RunAsync(); - } - } -} 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 deleted file mode 100644 index cc55370a9fc..00000000000 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0018Tests.cs +++ /dev/null @@ -1,433 +0,0 @@ -// 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; - -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 GetResponseAsync(string s, RequestContext context) - { - return null; - } - - public virtual Response GetResponse(string s, RequestContext context) - { - return null; - } - - public virtual AsyncPageable GetPageableAsync(string s, RequestContext context) - { - return null; - } - - public virtual Pageable GetPageable(string s, RequestContext context) - { - return null; - } - - public virtual Task GetOperationAsync(string s, RequestContext context) - { - return null; - } - - public virtual Operation GetOperation(string s, RequestContext context) - { - return null; - } - - public virtual Task> GetOperationOfTAsync(string s, RequestContext context) - { - return null; - } - - public virtual Operation GetOperationOfT(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> {|AZC0018:GetAsync|}(string s, RequestContext context) - { - return null; - } - - public virtual Response {|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> {|AZC0018:GetAsync|}(string s, RequestContext context) - { - return null; - } - - public virtual Response {|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 {|AZC0018:GetAsync|}(string s, RequestContext context) - { - return null; - } - - public virtual Pageable {|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> {|AZC0018:GetAsync|}(string s, RequestContext context) - { - return null; - } - - public virtual Operation {|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 {|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 AZC0018ProducedForMethodsWithNoRequestContentAndOptionalRequestContext() - { - 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 {|AZC0018:GetAsync|}(string a, Azure.RequestContext context = null) - { - return null; - } - - public virtual Response {|AZC0018:Get|}(string a, Azure.RequestContext context = null) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .RunAsync(); - } - - [Fact] - public async Task AZC0018ProducedForMethodsWithRequiredRequestContentAndRequiredRequestContext() - { - const string code = @" -using Azure; -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; -using System.Collections.Generic; - -namespace Azure.Core -{ - internal static partial class Argument - { - public static void AssertNotNull(T value, string name) - { - if (value is null) - { - throw new System.ArgumentNullException(name); - } - } - } -} - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0018:GetAsync|}(RequestContent content, RequestContext context) - { - Argument.AssertNotNull(content, nameof(content)); - return null; - } - - public virtual Response {|AZC0018:Get|}(RequestContent content, RequestContext context) - { - Argument.AssertNotNull(content, nameof(content)); - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .RunAsync(); - } - - [Fact] - public async Task AZC0018ProducedForMethodsWithOptionalRequestContentAndOptionalRequestContext() - { - const string code = @" -using Azure; -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; -using System.Collections.Generic; - -namespace Azure.Core -{ - internal static partial class Argument - { - public static void AssertNotNull(T value, string name) - { - if (value is null) - { - throw new System.ArgumentNullException(name); - } - } - } -} - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task {|AZC0018:GetAsync|}(RequestContent content, RequestContext context = null) - { - return null; - } - - public virtual Response {|AZC0018:Get|}(RequestContent content, RequestContext context = null) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .RunAsync(); - } - - [Fact] - public async Task AZC0018NotProducedForMethodsWithOptionalRequestContentAndRequiredRequestContext() - { - const string code = @" -using Azure; -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; -using System.Collections.Generic; - -namespace Azure.Core -{ - internal static partial class Argument - { - public static void AssertNotNull(T value, string name) - { - if (value is null) - { - throw new System.ArgumentNullException(name); - } - } - } -} - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(RequestContent content, RequestContext context) - { - return null; - } - - public virtual Response Get(RequestContent content, RequestContext context) - { - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .RunAsync(); - } - - [Fact] - public async Task AZC0018NotProducedForMethodsWithRequiredRequestContentAndOptionalRequestContext() - { - const string code = @" -using Azure; -using Azure.Core; -using System.Threading; -using System.Threading.Tasks; -using System.Collections.Generic; - -namespace Azure.Core -{ - internal static partial class Argument - { - public static void AssertNotNull(T value, string name) - { - if (value is null) - { - throw new System.ArgumentNullException(name); - } - } - } -} - -namespace RandomNamespace -{ - public class SomeClient - { - public virtual Task GetAsync(RequestContent content, RequestContext context = null) - { - Argument.AssertNotNull(content, nameof(content)); - return null; - } - - public virtual Response Get(RequestContent content, RequestContext context = null) - { - Argument.AssertNotNull(content, nameof(content)); - return null; - } - } -}"; - await Verifier.CreateAnalyzer(code) - .RunAsync(); - } - } -} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs index 2e3480b9910..25b20d345fc 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs @@ -32,7 +32,7 @@ protected class ParameterEquivalenceComparer : IEqualityComparer methodSymbo public abstract void AnalyzeCore(ISymbolAnalysisContext context); } -} +} \ No newline at end of file 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 db3ce3d066f..ab20919b072 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientMethodsAnalyzer.cs @@ -1,11 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; namespace Azure.ClientSdk.Analyzers @@ -20,107 +18,15 @@ public class ClientMethodsAnalyzer : ClientAnalyzerBase Descriptors.AZC0002, Descriptors.AZC0003, Descriptors.AZC0004, - Descriptors.AZC0015, - Descriptors.AZC0017, - Descriptors.AZC0018 + Descriptors.AZC0015 }); - public override void Initialize(AnalysisContext context) - { - context.EnableConcurrentExecution(); - context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); - base.Initialize(context); - context.RegisterCodeBlockAction(c => AnalyzeCodeBlock(c)); - } - - private void AnalyzeCodeBlock(CodeBlockAnalysisContext codeBlock) - { - var symbol = codeBlock.OwningSymbol; - if (symbol is IMethodSymbol methodSymbol) - { - var lastParameter = methodSymbol.Parameters.LastOrDefault(); - if (lastParameter != null && IsRequestContext(lastParameter)) - { - var requestContent = methodSymbol.Parameters.FirstOrDefault(p => IsRequestContent(p)); - if (requestContent != null) - { - bool isRequired = ContainsAssertNotNull(codeBlock, requestContent.Name); - if (isRequired && !lastParameter.IsOptional) - { - codeBlock.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, symbol.Locations.FirstOrDefault())); - } - if (!isRequired && lastParameter.IsOptional) - { - codeBlock.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, symbol.Locations.FirstOrDefault())); - } - } - } - } - } - - private static bool ContainsAssertNotNull(CodeBlockAnalysisContext codeBlock, string variableName) - { - // Check Argument.AssertNotNull(variableName, nameof(variableName)); - foreach (var invocation in codeBlock.CodeBlock.DescendantNodes().OfType()) - { - if (invocation.Expression is MemberAccessExpressionSyntax assertNotNull && assertNotNull.Name.Identifier.Text == "AssertNotNull") - { - if (assertNotNull.Expression is IdentifierNameSyntax identifierName && identifierName.Identifier.Text == "Argument" || - assertNotNull.Expression is MemberAccessExpressionSyntax memberAccessExpression && memberAccessExpression.Name.Identifier.Text == "Argument") - { - var argumentsList = invocation.ArgumentList.Arguments; - if (argumentsList.Count != 2) - { - continue; - } - if (argumentsList.First().Expression is IdentifierNameSyntax first) - { - if (first.Identifier.Text != variableName) - { - continue; - } - if (argumentsList.Last().Expression is InvocationExpressionSyntax second) - { - if (second.Expression is IdentifierNameSyntax nameof && nameof.Identifier.Text == "nameof") - { - if (second.ArgumentList.Arguments.Count != 1) - { - continue; - } - if (second.ArgumentList.Arguments.First().Expression is IdentifierNameSyntax contentName && contentName.Identifier.Text == variableName) - { - return true; - } - } - } - } - } - } - } - - return false; - } - - private static bool IsRequestContent(IParameterSymbol parameterSymbol) - { - return parameterSymbol.Type.Name == "RequestContent"; - } - - private static bool IsRequestContext(IParameterSymbol parameterSymbol) - { - return parameterSymbol.Name == "context" && parameterSymbol.Type.Name == "RequestContext"; - } - private static void CheckClientMethod(ISymbolAnalysisContext context, IMethodSymbol member) { - static bool IsCancellationOrRequestContext(IParameterSymbol parameterSymbol) - { - return IsCancellationToken(parameterSymbol) || IsRequestContext(parameterSymbol); - } - - static bool IsCancellationToken(IParameterSymbol parameterSymbol) + static bool SupportsCancellationsParameter(IParameterSymbol parameterSymbol) { - return parameterSymbol.Name == "cancellationToken" && parameterSymbol.Type.Name == "CancellationToken" && parameterSymbol.IsOptional; + return (parameterSymbol.Name == "cancellationToken" && parameterSymbol.Type.Name == "CancellationToken") || + (parameterSymbol.Name == "context" && parameterSymbol.Type.Name == "RequestContext"); } CheckClientMethodReturnType(context, member); @@ -131,123 +37,39 @@ static bool IsCancellationToken(IParameterSymbol parameterSymbol) } var lastArgument = member.Parameters.LastOrDefault(); - var isCancellationOrRequestContext = lastArgument != null && IsCancellationOrRequestContext(lastArgument); - - if (!isCancellationOrRequestContext) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member); - } - 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) - { - 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: no RequestContent or has optional RequestContent. - // No ambiguity: has required 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"); - }); + var supportsCancellations = lastArgument != null && SupportsCancellationsParameter(lastArgument); - if (containsModel) + if (!supportsCancellations) { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); - return; - } + var overloadSupportsCancellations = FindMethod( + member.ContainingType.GetMembers(member.Name).OfType(), + member.TypeParameters, + member.Parameters, + p => SupportsCancellationsParameter(p)); - var requestContent = method.Parameters.FirstOrDefault(p => p.Type.Name == "RequestContent"); - if (requestContent == null) - { - if (method.Parameters.LastOrDefault().IsOptional) + if (overloadSupportsCancellations != null) { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); + // Skip methods that have overloads with cancellation tokens + return; } - } - // Optional RequestContent or required RequestContent is checked in AnalyzeCodeBlock. - } - // A protocol method should not have model as type. Accepted return type: Response, Task, Pageable, AsyncPageable, Operation, Task>, Operation, Task - private static void CheckProtocolMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method) - { - ITypeSymbol originalType = method.ReturnType; - ITypeSymbol unwrappedType = method.ReturnType; - - if (method.ReturnType is INamedTypeSymbol namedTypeSymbol && - namedTypeSymbol.IsGenericType && - namedTypeSymbol.Name == "Task") - { - unwrappedType = namedTypeSymbol.TypeArguments.Single(); - } - - if (unwrappedType.Name == "Response") - { - if (unwrappedType is INamedTypeSymbol responseTypeSymbol && responseTypeSymbol.IsGenericType) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); - } - return; - } - else if (unwrappedType.Name == "Operation") - { - if (unwrappedType is INamedTypeSymbol operationTypeSymbol && operationTypeSymbol.IsGenericType) - { - var operationReturn = operationTypeSymbol.TypeArguments.Single(); - if (operationReturn.Name != "BinaryData") - { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); - } - } - return; + context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member); } - else if (originalType.Name == "Pageable" || originalType.Name == "AsyncPageable") + else if (!lastArgument.IsOptional) { - if (originalType is INamedTypeSymbol pageableTypeSymbol) - { - if (!pageableTypeSymbol.IsGenericType) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); - } + var overloadWithCancellationToken = FindMethod( + member.ContainingType.GetMembers(member.Name).OfType(), + member.TypeParameters, + member.Parameters.RemoveAt(member.Parameters.Length - 1)); - var pageableReturn = pageableTypeSymbol.TypeArguments.Single(); - if (pageableReturn.Name != "BinaryData") - { - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); - } + if (overloadWithCancellationToken != null) + { + // Skip methods that have non-optional cancellation token if overload exists without one + return; } - return; + context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0002, member.Locations.FirstOrDefault()), member); } - - context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method); } private static void CheckClientMethodReturnType(ISymbolAnalysisContext context, IMethodSymbol method) @@ -294,16 +116,15 @@ bool IsOrImplements(ITypeSymbol typeSymbol, string typeName) 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 && asyncMethodSymbol.Name.EndsWith(AsyncSuffix) && member.DeclaredAccessibility == Accessibility.Public) + if (member is IMethodSymbol methodSymbol && methodSymbol.Name.EndsWith(AsyncSuffix) && member.DeclaredAccessibility == Accessibility.Public) { - CheckClientMethod(context, asyncMethodSymbol); + CheckClientMethod(context, methodSymbol); var syncMemberName = member.Name.Substring(0, member.Name.Length - AsyncSuffix.Length); - var syncMember = FindMethod(type.GetMembers(syncMemberName).OfType(), asyncMethodSymbol.TypeParameters, asyncMethodSymbol.Parameters); + var syncMember = FindMethod(type.GetMembers(syncMemberName).OfType(), methodSymbol.TypeParameters, methodSymbol.Parameters); if (syncMember == null) { @@ -311,19 +132,9 @@ public override void AnalyzeCore(ISymbolAnalysisContext context) } else { - visitedSyncMember.Add(syncMember); CheckClientMethod(context, syncMember); } } - else if (member is IMethodSymbol syncMethodSymbol && !member.IsImplicitlyDeclared && !syncMethodSymbol.Name.EndsWith(AsyncSuffix) && member.DeclaredAccessibility == Accessibility.Public && !visitedSyncMember.Contains(syncMethodSymbol)) - { - 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); - } - } } } } 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 42161acffca..ebcd98325ba 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs @@ -16,8 +16,8 @@ internal class Descriptors public static DiagnosticDescriptor AZC0002 = new DiagnosticDescriptor( nameof(AZC0002), - "DO ensure all service methods, both asynchronous and synchronous, take an optional CancellationToken parameter called cancellationToken or a RequestContext parameter called context.", - "Client method should have an optional CancellationToken (both name and it being optional matters) or a RequestContext as the last parameter.", + "DO ensure all service methods, both asynchronous and synchronous, take an optional CancellationToken parameter called cancellationToken.", + "Client method should have cancellationToken as the last optional parameter (both name and it being optional matters)", "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-service-methods-cancellation" ); @@ -110,19 +110,7 @@ internal class Descriptors "Invalid ServiceVersion member name.", "All parts of ServiceVersion members' names must begin with a number or uppercase letter and cannot have consecutive underscores.", "Usage", - DiagnosticSeverity.Warning, true); - - public static DiagnosticDescriptor AZC0017 = new DiagnosticDescriptor( - nameof(AZC0017), - "Do ensure convenience method not take RequestContent as parameter type.", - "Convenience method shouldn't have prameters with type RequestContent.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null); - - 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.", - "Protocol method should have requestContext as the last parameter and don't have model as parameter type or return type.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null); + DiagnosticSeverity.Warning, true); #endregion #region General diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/SymbolAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/SymbolAnalyzerBase.cs index 3e5f7880953..c3030310f61 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/SymbolAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/SymbolAnalyzerBase.cs @@ -16,7 +16,7 @@ public abstract class SymbolAnalyzerBase : DiagnosticAnalyzer protected INamedTypeSymbol ClientOptionsType { get; private set; } - public override void Initialize(AnalysisContext context) + public sealed override void Initialize(AnalysisContext context) { context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); context.EnableConcurrentExecution();