Fix analyzer issues (#7222)

* Fix analyzer issues * update * update * update
Azure · Nov 16, 2023 · c4395d1 · c4395d1
1 parent 7441f21
commit c4395d1
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 19 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
@@ -294,6 +294,25 @@ public virtual Response Get(string s, CancellationToken cancellationToken)
             return null;
         }
     }
+}";
+            await Verifier.CreateAnalyzer(code)
+                .RunAsync();
+        }
+
+        [Fact]
+        public async Task AZC0002NotProducedForGetSubClientMethods()
+        {
+            const string code = @"
+namespace RandomNamespace
+{
+    public class SomeClient
+    {
+        public class Operation {}
+        public virtual Operation GetOperationClient(string apiVersion = ""1.0.0"")
+        {
+            return new Operation();
+        }
+    }
 }";
             await Verifier.CreateAnalyzer(code)
                 .RunAsync();

diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0006Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/AZC0006Tests.cs
@@ -90,8 +90,26 @@ public class SomeClient
         protected SomeClient() {}
         public SomeClient(string connectionString, SomeClientsOptions options = null) {}
     }
+}";
+            await Verifier.VerifyAnalyzerAsync(code);
+        }
+
+        [Fact]
+        public async Task AZC0006NotProducedForClientsWithStaticProperties()
+        {
+            const string code = @"
+namespace RandomNamespace
+{
+    public class SomeClientOptions : Azure.Core.ClientOptions { }
+
+    public class SomeClient
+    {
+        public static int a = 1;
+        public SomeClient() {}
+        public SomeClient(SomeClientOptions options) {}
+    }
 }";
             await Verifier.VerifyAnalyzerAsync(code);
         }
     }
-}
+}
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
@@ -88,6 +88,80 @@ await Verifier.CreateAnalyzer(code)
                 .RunAsync();
         }
 
+        [Fact]
+        public async Task AZC0018NotProducedForExtendedReturnType()
+        {
+            const string code = @"
+using Azure;
+using Azure.Core;
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Arbitrary
+{
+    public class ExtendedBinaryData : BinaryData
+    {
+        public ExtendedBinaryData(string data): base(data){}
+    }
+}
+
+namespace RandomNamespace
+{
+    public class SomeClient
+    {
+        public virtual AsyncPageable<Arbitrary.ExtendedBinaryData> GetPageableAsync(string s, RequestContext context)
+        {
+            return null;
+        }
+
+        public virtual Pageable<Arbitrary.ExtendedBinaryData> GetPageable(string s, RequestContext context)
+        {
+            return null;
+        }
+    }
+}";
+            await Verifier.CreateAnalyzer(code)
+                .RunAsync();
+        }
+
+        [Fact]
+        public async Task AZC0018ProducedForCustomTypeWithSystemName()
+        {
+            const string code = @"
+using Azure;
+using Azure.Core;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Custom
+{
+    namespace System
+    {
+        public class BinaryData {}
+    }
+}
+
+namespace RandomNamespace
+{
+    using Custom.System;
+    public class SomeClient
+    {
+        public virtual AsyncPageable<BinaryData> {|AZC0018:GetPageableAsync|}(string s, RequestContext context)
+        {
+            return null;
+        }
+
+        public virtual Pageable<BinaryData> {|AZC0018:GetPageable|}(string s, RequestContext context)
+        {
+            return null;
+        }
+    }
+}";
+            await Verifier.CreateAnalyzer(code)
+                .RunAsync();
+        }
+
         [Fact]
         public async Task AZC0018ProducedForMethodsWithGenericResponseOfPrimitive()
         {

diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientAnalyzerBase.cs
@@ -79,9 +79,10 @@ public int GetHashCode(ITypeParameterSymbol obj)
             }
         }
 
-        protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbols, ImmutableArray<ITypeParameterSymbol> genericParameters, ImmutableArray<IParameterSymbol> parameters)
+        protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbols, ImmutableArray<ITypeParameterSymbol> genericParameters, ImmutableArray<IParameterSymbol> parameters, bool ignoreStatic = false)
         {
             return methodSymbols.SingleOrDefault(symbol =>
+                (!ignoreStatic || !symbol.IsStatic) &&
                 genericParameters.SequenceEqual(symbol.TypeParameters, ParameterEquivalenceComparer.Default) &&
                 parameters.SequenceEqual(symbol.Parameters, ParameterEquivalenceComparer.Default));
         }
@@ -105,4 +106,4 @@ protected static IMethodSymbol FindMethod(IEnumerable<IMethodSymbol> methodSymbo
 
         public abstract void AnalyzeCore(ISymbolAnalysisContext context);
     }
-}
+}
diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientConstructorAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ClientConstructorAnalyzer.cs
@@ -40,8 +40,9 @@ public override void AnalyzeCore(ISymbolAnalysisContext context)
                         // Allow optional options parameters
                         if (lastParameter.IsOptional) continue;
 
+                        // When there are static properties in client, there would be static constructor implicitly added
                         var nonOptionsMethod = FindMethod(
-                            type.Constructors, constructor.TypeParameters, constructor.Parameters.RemoveAt(constructor.Parameters.Length - 1));
+                            type.Constructors, constructor.TypeParameters, constructor.Parameters.RemoveAt(constructor.Parameters.Length - 1), true);
 
                         if (nonOptionsMethod == null || nonOptionsMethod.DeclaredAccessibility != Accessibility.Public)
                         {
@@ -62,4 +63,4 @@ public override void AnalyzeCore(ISymbolAnalysisContext 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
@@ -14,6 +14,8 @@ public class ClientMethodsAnalyzer : ClientAnalyzerBase
     {
         private const string AsyncSuffix = "Async";
 
+        private const string AzureNamespace = "Azure";
+        private const string SystemNamespace = "System";
         private const string PageableTypeName = "Pageable";
         private const string AsyncPageableTypeName = "AsyncPageable";
         private const string BinaryDataTypeName = "BinaryData";
@@ -156,7 +158,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
                 }
 
                 var pageableReturn = pageableTypeSymbol.TypeArguments.Single();
-                if (!IsOrImplements(pageableReturn, BinaryDataTypeName))
+                if (!IsOrImplements(pageableReturn, BinaryDataTypeName, SystemNamespace))
                 {
                     return false;
                 }
@@ -174,7 +176,7 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
                 unwrappedType = namedTypeSymbol.TypeArguments.Single();
             }
 
-            if (IsOrImplements(unwrappedType, ResponseTypeName))
+            if (IsOrImplements(unwrappedType, ResponseTypeName, AzureNamespace))
             {
                 if (unwrappedType is INamedTypeSymbol responseTypeSymbol && responseTypeSymbol.IsGenericType)
                 {
@@ -186,12 +188,12 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
                 }
                 return;
             }
-            else if (IsOrImplements(unwrappedType, OperationTypeName))
+            else if (IsOrImplements(unwrappedType, OperationTypeName, AzureNamespace))
             {
                 if (unwrappedType is INamedTypeSymbol operationTypeSymbol && operationTypeSymbol.IsGenericType)
                 {
                     var operationReturn = operationTypeSymbol.TypeArguments.Single();
-                    if (IsOrImplements(operationReturn, PageableTypeName) || IsOrImplements(operationReturn, AsyncPageableTypeName))
+                    if (IsOrImplements(operationReturn, PageableTypeName, AzureNamespace) || IsOrImplements(operationReturn, AsyncPageableTypeName, AzureNamespace))
                     {
                         if (!IsValidPageable(operationReturn))
                         {
@@ -200,14 +202,14 @@ bool IsValidPageable(ITypeSymbol typeSymbol)
                         return;
                     }
 
-                    if (!IsOrImplements(operationReturn, BinaryDataTypeName))
+                    if (!IsOrImplements(operationReturn, BinaryDataTypeName, SystemNamespace))
                     {
                         context.ReportDiagnostic(Diagnostic.Create(Descriptors.AZC0018, method.Locations.FirstOrDefault()), method);
                     }
                 }
                 return;
             }
-            else if (IsOrImplements(originalType, PageableTypeName) || IsOrImplements(originalType, AsyncPageableTypeName))
+            else if (IsOrImplements(originalType, PageableTypeName, AzureNamespace) || IsOrImplements(originalType, AsyncPageableTypeName, AzureNamespace))
             {
                 if (!IsValidPageable(originalType))
                 {
@@ -229,16 +231,16 @@ private static void CheckClientMethod(ISymbolAnalysisContext context, IMethodSym
             }
         }
 
-        private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName)
+        private static bool IsOrImplements(ITypeSymbol typeSymbol, string typeName, string namespaceName)
         {
-            if (typeSymbol.Name == typeName)
+            if (typeSymbol.Name == typeName && typeSymbol.ContainingNamespace.Name == namespaceName && typeSymbol.ContainingNamespace.ContainingNamespace.Name == "")
             {
                 return true;
             }
 
             if (typeSymbol.BaseType != null)
             {
-                return IsOrImplements(typeSymbol.BaseType, typeName);
+                return IsOrImplements(typeSymbol.BaseType, typeName, namespaceName);
             }
 
             return false;
@@ -261,11 +263,11 @@ private static bool IsClientMethodReturnType(ISymbolAnalysisContext context, IMe
                 unwrappedType = namedTypeSymbol.TypeArguments.Single();
             }
 
-            if (IsOrImplements(unwrappedType, ResponseTypeName) ||
-                IsOrImplements(unwrappedType, NullableResponseTypeName) ||
-                IsOrImplements(unwrappedType, OperationTypeName) ||
-                IsOrImplements(originalType, PageableTypeName) ||
-                IsOrImplements(originalType, AsyncPageableTypeName))
+            if (IsOrImplements(unwrappedType, ResponseTypeName, AzureNamespace) ||
+                IsOrImplements(unwrappedType, NullableResponseTypeName, AzureNamespace) ||
+                IsOrImplements(unwrappedType, OperationTypeName, AzureNamespace) ||
+                IsOrImplements(originalType, PageableTypeName, AzureNamespace) ||
+                IsOrImplements(originalType, AsyncPageableTypeName, AzureNamespace))
             {
                 return true;
             }