From ba2df729c83dd38e0b0d0c8753b82b7450c7d67d Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 13 Nov 2024 10:18:46 +0800 Subject: [PATCH] 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 #9335 --- .../ModelName/AZC0030OptionsTest.cs | 83 ++++++++++++------- .../ModelName/GeneralSuffixAnalyzer.cs | 20 ++++- 2 files changed, 70 insertions(+), 33 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs index 0e622c9fb33..49a47fd3627 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs +++ b/src/dotnet/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/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs index 15b0addda97..6ba7d304e4d 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs +++ b/src/dotnet/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(); + 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"