fix(.net analyzer): only check Options suffix for mgmt sdk

- update analyzer to only check `Options` suffix if the class is under a mgmt SDK namespace - update test cases accordingly fix Azure#9335
archerzz · Nov 13, 2024 · ba2df72 · ba2df72
1 parent 59d869d
commit ba2df72
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 33 deletions.
diff --git a/...Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs b/...Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs
@@ -12,54 +12,75 @@ public class AZC0030OptionsSuffixTests
     {
         private const string diagnosticId = "AZC0030";
 
-        [Fact]
-        public async Task WithoutAnySerialization()
+        [Theory]
+        [InlineData("Azure.DataPlane.Models")]
+        [InlineData("Azure.ResourceManager.Models")]
+        public async Task WithoutAnySerialization(string ns)
         {
-            var test = @"using System.Text.Json;
-namespace Azure.ResourceManager.Models;
+            var test = $@"using System.Text.Json;
+namespace {ns};
 
 public class DiskOptions
-{
-}";
+{{
+}}";
             await VerifyCS.VerifyAnalyzerAsync(test);
         }
 
-        [Fact]
-        public async Task WithDeserializationMethod()
+
+        [Theory]
+        [InlineData("Azure.DataPlane.Models", false)]
+        [InlineData("Azure.ResourceManager.Network.Models", true)]
+        public async Task WithDeserializationMethod(string ns, bool hasError)
         {
-            var test = @"using System.Text.Json;
-namespace Azure.ResourceManager
-{
+            var test = $@"using System.Text.Json;
+namespace {ns}
+{{
     public class ResponseOptions
-    {
+    {{
         public static ResponseOptions DeserializeResponseOptions(JsonElement element)
-        {
+        {{
             return null;
-        }
-    }
-}";
-            var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 33).WithArguments("ResponseOptions", "Options", "'ResponseConfig'");
-            await VerifyCS.VerifyAnalyzerAsync(test, expected);
+        }}
+    }}
+}}";
+            if (hasError)
+            {
+                var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 33).WithArguments("ResponseOptions", "Options", "'ResponseConfig'");
+                await VerifyCS.VerifyAnalyzerAsync(test, expected);
+            }
+            else
+            {
+                await VerifyCS.VerifyAnalyzerAsync(test);
+            }
         }
 
-        [Fact]
-        public async Task WithSerializationMethod()
+        [Theory]
+        [InlineData("Azure.DataPlane.Models", false)]
+        [InlineData("Azure.ResourceManager.Batch.Models", true)]
+        public async Task WithSerializationMethod(string ns, bool hasError)
         {
-            var test = @"using System.Text.Json;
-namespace Azure.ResourceManager.Models
-{
+            var test = $@"using System.Text.Json;
+namespace {ns}
+{{
     internal interface IUtf8JsonSerializable
-    {
+    {{
         void Write(Utf8JsonWriter writer);
-    };
+    }};
 
     public class DiskOptions: IUtf8JsonSerializable
-    {
-        void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
-    }
-}";
-            var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", "'DiskConfig'");
-            await VerifyCS.VerifyAnalyzerAsync(test, expected);
+    {{
+        void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {{}}
+    }}
+}}";
+            if (hasError)
+            {
+                var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", "'DiskConfig'");
+                await VerifyCS.VerifyAnalyzerAsync(test, expected);
+            }
+            else
+            {
+                await VerifyCS.VerifyAnalyzerAsync(test);
+            }
         }
     }
 }
diff --git a/...et/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs b/...et/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs
@@ -27,13 +27,29 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex
             if (reservedNames.Contains(symbol.Name))
                 return true;
 
-            // skip property bag classes which have `Options` suffix and don't have serialization
-            if (symbol.Name.EndsWith("Options") && !SupportSerialization(symbol))
+            // skip classes which have `Options` suffix and:
+            //   - not in Azure.ResourceManager namespace
+            //   - or does not have serialization
+            if (symbol.Name.EndsWith("Options") && (!HasMgmtNamespace(symbol) || !SupportSerialization(symbol)))
                 return true;
 
             return false;
         }
 
+        private bool HasMgmtNamespace(INamedTypeSymbol symbol)
+        {
+            var namespaces = new Stack<string>();
+            var namespaceSymbol = symbol.ContainingNamespace;
+            while (namespaceSymbol is not null && namespaceSymbol.IsGlobalNamespace is false)
+            {
+                namespaces.Push(namespaceSymbol.Name);
+                namespaceSymbol = namespaceSymbol.ContainingNamespace;
+            }
+
+            var namespaceArray = namespaces.ToArray();
+            return namespaceArray.Length >= 2 && namespaceArray[0] == "Azure" && namespaceArray[1] == "ResourceManager";
+        }
+
         private bool SupportSerialization(INamedTypeSymbol symbol)
         {
             // if it has serialization method: `IUtf8JsonSerializable.Write`, e.g. ": IUtf8JsonSerializable"