From 988cef0154d298072b73ad17da23e257656669a3 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 6 Sep 2023 12:12:26 +0800 Subject: [PATCH 01/20] feat(analzyer): add rules regarding model name suffixes - add four rules checking the suffixes of model names - upgrade Roslyn to latest version since new rules require some API - minor refactor existing codes - add test cases for the new rules resolve #6905 --- .../Azure.ClientSdk.Analyzers.Tests.csproj | 8 +- .../ModelName/AZC0030Tests.cs | 114 ++++++++++++++++++ .../ModelName/AZC0031Tests.cs | 64 ++++++++++ .../ModelName/AZC0032Tests.cs | 78 ++++++++++++ .../ModelName/AZC0033Tests.cs | 76 ++++++++++++ .../AnalyzerUtils.cs | 80 ++++++++++++ .../Azure.ClientSdk.Analyzers.csproj | 4 +- .../Azure.ClientSdk.Analyzers/Descriptors.cs | 64 +++++----- .../DiagnosticCategory.cs | 14 +++ .../INameSpaceSymbolExtensions.cs | 20 +++ .../ModelName/DataSuffixAnalyzer.cs | 42 +++++++ .../ModelName/DefinitionSuffixAnalyzer.cs | 49 ++++++++ .../ModelName/GeneralSuffixAnalyzer.cs | 59 +++++++++ .../ModelName/OperationSuffixAnalyzer.cs | 41 +++++++ .../ModelName/SuffixAnalyzerBase.cs | 79 ++++++++++++ 15 files changed, 754 insertions(+), 38 deletions(-) create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/DiagnosticCategory.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj index 4e207cd1a60..da16d640721 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj @@ -14,17 +14,17 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive - + - - + + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs new file mode 100644 index 00000000000..bc4aacb52c4 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs @@ -0,0 +1,114 @@ +using System.Threading.Tasks; +using Azure.ClientSdk.Analyzers.ModelName; +using Xunit; + +using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< + Azure.ClientSdk.Analyzers.ModelName.GeneralSuffixAnalyzer>; + +namespace Azure.ClientSdk.Analyzers.Test.ModelName +{ + public class AZC0030Tests + { + [Fact] + public async Task ClassNotUnderModelsNamespaceIsNotChecked() + { + var test = @"namespace Azure.ResourceManager; + +class MonitorResult +{ +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task OnlyModelsNamespaceIsChecked() + { + var test = @"namespace Azure.ResourceManager.AModels; + +class MonitorResult +{ +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task EnumIsNotChecked() + { + var test = @"namespace Azure.ResourceManager.Models; + +enum MonitorResult +{ +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task GoodSuffix() + { + var test = @"namespace Azure.ResourceManager.Models; + +class MonitorContent +{ +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ParametersSuffix() + { + var test = @"namespace Azure.ResourceManager.Models +{ + public class ResponseParameters + { + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 18, 3, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task ResultSuffix() + { + var test = @"namespace Azure.ResourceManager.Models +{ + public class NetworkRequest + { + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 18, 3, 32).WithArguments("NetworkRequest", "Request", "'NetworkContent'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task OptionSuffixWithNestedNameSpace() + { + var test = @"namespace Azure.ResourceManager.Models +{ + namespace SubTest + { + public class DiskOption + { + } + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(5, 22, 5, 32).WithArguments("DiskOption", "Option", "'DiskConfig'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task ResponsesSuffix() + { + var test = @"namespace Azure.ResourceManager.Models +{ + namespace SubTest + { + public class CreationResponses + { + } + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(5, 22, 5, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs new file mode 100644 index 00000000000..bd9838bc45a --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs @@ -0,0 +1,64 @@ +using System.Threading.Tasks; +using Azure.ClientSdk.Analyzers.ModelName; +using Xunit; + +using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< + Azure.ClientSdk.Analyzers.ModelName.DefinitionSuffixAnalyzer>; + +namespace Azure.ClientSdk.Analyzers.Tests.ModelName +{ + public class AZC0031Tests + { + [Fact] + public async Task ModelWithDefinitionSuffix() + { + var test = @" +namespace Azure.ResourceManager.Network.Models +{ + public partial class AadAuthenticationDefinition + { + } +}"; + var expected = VerifyCS.Diagnostic(DefinitionSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 53).WithArguments("AadAuthenticationDefinition", "Definition"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task ArmResourceIsNotChecked() + { + var test = @" +using Azure.ResourceManager; +namespace Azure.ResourceManager +{ + public class ArmResource { + } +} +namespace Azure.ResourceManager.Network.Models +{ + public partial class AadAuthenticationDefinition: ArmResource + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task NotCheckIfRemovingSuffixIsAnotherType() + { + var test = @" +using Azure.ResourceManager; +namespace Azure.ResourceManager.Network.Models +{ + public class AadAuthentication { + } + + public partial class AadAuthenticationDefinition + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + } +} + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs new file mode 100644 index 00000000000..a492205afa6 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs @@ -0,0 +1,78 @@ +using System.Threading.Tasks; +using Xunit; + +using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< + Azure.ClientSdk.Analyzers.ModelName.DataSuffixAnalyzer>; +using Azure.ClientSdk.Analyzers.ModelName; + +namespace Azure.ClientSdk.Analyzers.Tests.ModelName +{ + public class AZC0032Tests + { + [Fact] + public async Task ClassUnderNonModelsNamespaceIsNotChecked() + { + var test = @" +namespace Azure.ResourceManager.Network.Temp +{ + public partial class AadAuthenticationData + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ModelClassWithDataSuffix() + { + var test = @" +namespace Azure.ResourceManager.Network.Models +{ + public partial class AadAuthenticationData + { + } +}"; + var expected = VerifyCS.Diagnostic(DataSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 47).WithArguments("AadAuthenticationData", "Data"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task ResourceDataClassesAreNotChecked() + { + var test = @" +using Azure.ResourceManager.Models; +namespace Azure.ResourceManager.Models +{ + public class ResourceData { + } +} +namespace Azure.ResourceManager.Network.Models +{ + public partial class AadAuthenticationData: ResourceData + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task TrackedResourceDataClassesAreNotChecked() + { + var test = @" +using Azure.ResourceManager.Models; +namespace Azure.ResourceManager.Models +{ + public class TrackedResourceData { + } +} +namespace Azure.ResourceManager.Network.Models +{ + public partial class AadAuthenticationData: TrackedResourceData + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + } +} + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs new file mode 100644 index 00000000000..8565631c8fd --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs @@ -0,0 +1,76 @@ +using System.Threading.Tasks; +using Azure.ClientSdk.Analyzers.ModelName; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< + Azure.ClientSdk.Analyzers.ModelName.OperationSuffixAnalyzer>; + +namespace Azure.ClientSdk.Analyzers.Test.ModelName +{ + public class AZC0033Tests + { + [Fact] + public async Task OperationClassIsNotChecked() + { + var test = @" +using Azure; +using Azure.ResourceManager; +namespace Azure +{ + public class Operation + { + } + public class Operation + { + } +} +namespace Azure.ResourceManager +{ + public class ArmOperation : Operation + { + } + public class ArmOperation : Operation + { + } +} +namespace Azure.ResourceManager.Network.Models +{ + internal class DnsOperation : Operation + { + } + internal class DnsArmOperation : ArmOperation + { + } + internal class DnsOperation : Operation + { + } + internal class DnsArmOperation : ArmOperation + { + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task OperationSuffix() + { + var test = @" +namespace Azure.ResourceManager.Network.Models +{ + public class DnsOperation + { + } + public class DnsArmOperation + { + } +}"; + DiagnosticResult[] expected = { + VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 30).WithArguments("DnsOperation", "Operation", "DnsData", "DnsInfo"), + VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(7, 18, 7, 33).WithArguments("DnsArmOperation", "Operation", "DnsArmData", "DnsArmInfo") + }; + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + } +} + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs new file mode 100644 index 00000000000..e112f03f95a --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using System; + +namespace Azure.ClientSdk.Analyzers +{ + internal class AnalyzerUtils + { + internal static bool IsNotSdkCode(ISymbol symbol) => !IsSdkCode(symbol); + + internal static bool IsSdkCode(ISymbol symbol) + { + var ns = symbol.ContainingNamespace.GetFullNamespaceName(); + + return IsSdkNamespace(ns); + } + + internal static bool IsNotSdkCode(SyntaxNode node, SemanticModel model) => !IsSdkCode(node, model); + + internal static bool IsSdkCode(SyntaxNode node, SemanticModel model) + { + var symbol = model.GetDeclaredSymbol(node); + if (symbol != null) + { + return IsSdkCode(symbol); + } + + var ns = GetNamespace(node); + return IsSdkNamespace(ns); + } + + private static bool IsSdkNamespace(string ns) + { + var namespaces = ns.Split('.'); + // skip Azure or Azure.Core + if (namespaces.Length >= 2) + { + return namespaces[0] == "Azure" && + namespaces[1] != "Core"; + } + + return false; + } + + private static string GetNamespace(SyntaxNode node) + { + var ns = string.Empty; + + var parent = node.Parent; + + while (parent != null && + parent is not NamespaceDeclarationSyntax + && parent is not FileScopedNamespaceDeclarationSyntax) + { + parent = parent.Parent; + } + + if (parent is BaseNamespaceDeclarationSyntax namespaceParent) + { + ns = namespaceParent.Name.ToString(); + + while (true) + { + if (namespaceParent.Parent is not NamespaceDeclarationSyntax parentNamespace) + { + break; + } + + ns = $"{namespaceParent.Name}.{ns}"; + namespaceParent = parentNamespace; + } + } + + return ns; + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj index 5d1f12494fa..d4a8ea01b68 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj @@ -16,8 +16,8 @@ - - + + 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 0b292863d5b..c6423ff9554 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs @@ -12,13 +12,13 @@ internal class Descriptors #region Guidelines public static DiagnosticDescriptor AZC0001 = new DiagnosticDescriptor( nameof(AZC0001), AZC0001Title, - "Namespace '{0}' shouldn't contain public types. " + AZC0001Title, "Usage", DiagnosticSeverity.Warning, true); + "Namespace '{0}' shouldn't contain public types. " + AZC0001Title, DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); 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 called cancellationToken (both name and it being optional matters) or a RequestContext called context as the last parameter.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-service-methods-cancellation" ); @@ -26,7 +26,7 @@ internal class Descriptors nameof(AZC0003), "DO make service methods virtual.", "DO make service methods virtual.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-service-methods-virtual" ); @@ -34,7 +34,7 @@ internal class Descriptors nameof(AZC0004), "DO provide both asynchronous and synchronous variants for all service methods.", "DO provide both asynchronous and synchronous variants for all service methods.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-service-methods-sync-and-async" ); @@ -42,7 +42,7 @@ internal class Descriptors nameof(AZC0005), "DO provide protected parameterless constructor for mocking.", "DO provide protected parameterless constructor for mocking.", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-client-constructor-for-mocking" ); @@ -50,7 +50,7 @@ internal class Descriptors nameof(AZC0006), "DO provide constructor overloads that allow specifying additional options.", "A client type should have a public constructor with equivalent parameters that takes a Azure.Core.ClientOptions-derived type as the last argument", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-client-constructor-overloads" ); @@ -58,151 +58,151 @@ internal class Descriptors nameof(AZC0007), "DO provide a minimal constructor that takes only the parameters required to connect to the service.", "A client type should have a public constructor with equivalent parameters that doesn't take a Azure.Core.ClientOptions-derived type as the last argument", - "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: null, "https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-client-constructor-minimal" ); public static DiagnosticDescriptor AZC0008 = new DiagnosticDescriptor( nameof(AZC0008), "ClientOptions should have a nested enum called ServiceVersion", - "Client type should have a nested enum called ServiceVersion", "Usage", DiagnosticSeverity.Warning, true); + "Client type should have a nested enum called ServiceVersion", DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0009 = new DiagnosticDescriptor( nameof(AZC0009), "ClientOptions constructors should take a ServiceVersion as their first parameter", - "ClientOptions constructors should take a ServiceVersion as their first parameter. Default constructor should be overloaded to provide ServiceVersion.", "Usage", DiagnosticSeverity.Warning, true); + "ClientOptions constructors should take a ServiceVersion as their first parameter. Default constructor should be overloaded to provide ServiceVersion.", DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0010 = new DiagnosticDescriptor( nameof(AZC0010), "ClientOptions constructors should default ServiceVersion to latest supported service version", - "ClientOptions constructors should default ServiceVersion to latest supported service version", "Usage", DiagnosticSeverity.Warning, true); + "ClientOptions constructors should default ServiceVersion to latest supported service version", DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0011 = new DiagnosticDescriptor( nameof(AZC0011), "Avoid InternalsVisibleTo to non-test assemblies", - "Internal visible to product libraries effectively become public API and have to be versioned appropriately", "Usage", DiagnosticSeverity.Warning, true); + "Internal visible to product libraries effectively become public API and have to be versioned appropriately", DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0012 = new DiagnosticDescriptor( nameof(AZC0012), "Avoid single word type names", "Single word class names are too generic and have high chance of collision with BCL types or types from other libraries", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0013 = new DiagnosticDescriptor( nameof(AZC0013), "Use TaskCreationOptions.RunContinuationsAsynchronously when instantiating TaskCompletionSource", "All the task’s continuations are executed synchronously unless TaskCreationOptions.RunContinuationsAsynchronously option is specified. This may cause deadlocks and other threading issues if all \"async\" continuations have to run in the thread that sets the result of a task.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0014 = new DiagnosticDescriptor( nameof(AZC0014), "Avoid using banned types in public API", "Types from {0} assemblies should not be exposed as part of public API surface.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0015 = new DiagnosticDescriptor( nameof(AZC0015), "Unexpected client method return type.", "Client methods should return Pageable/AsyncPageable/Operation/Task>/Response/Response/Task/Task> or other client class found {0} instead.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0016 = new DiagnosticDescriptor( nameof(AZC0016), "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); - + DiagnosticCategory.Usage, + DiagnosticSeverity.Warning, true); + public static DiagnosticDescriptor AZC0020 = new DiagnosticDescriptor( nameof(AZC0020), "Avoid using banned types in public APIs", "The Azure.Core internal shared source types {0} should not be used outside of the Azure.Core library.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); #endregion - + #region General public static DiagnosticDescriptor AZC0100 = new DiagnosticDescriptor( nameof(AZC0100), "ConfigureAwait(false) must be used.", "ConfigureAwait(false) must be used.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0101 = new DiagnosticDescriptor( nameof(AZC0101), "Use ConfigureAwait(false) instead of ConfigureAwait(true).", "Use ConfigureAwait(false) instead of ConfigureAwait(true).", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0102 = new DiagnosticDescriptor( nameof(AZC0102), "Do not use GetAwaiter().GetResult().", "Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0103 = new DiagnosticDescriptor( nameof(AZC0103), "Do not wait synchronously in asynchronous scope.", "Do not use {0} in asynchronous scope. Use await keyword instead.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0104 = new DiagnosticDescriptor( nameof(AZC0104), "Use EnsureCompleted() directly on asynchronous method return value.", "Don't use {0}. Call EnsureCompleted() extension method directly on the return value of the asynchronous method that has 'bool async' parameter.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0105 = new DiagnosticDescriptor( nameof(AZC0105), "DO NOT add 'async' parameter to public methods.", "DO provide both asynchronous and synchronous variants for all service methods instead of one variant with 'async' parameter.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0106 = new DiagnosticDescriptor( nameof(AZC0106), "Non-public asynchronous method needs 'async' parameter.", "Non-public asynchronous method that is called in synchronous scope should have a boolean 'async' parameter.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0107 = new DiagnosticDescriptor( nameof(AZC0107), "DO NOT call public asynchronous method in synchronous scope.", "Public asynchronous method shouldn't be called in synchronous scope. Use synchronous version of the method if it is available.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0108 = new DiagnosticDescriptor( nameof(AZC0108), "Incorrect 'async' parameter value.", "In {0} scope 'async' parameter for the '{1}' method call should {2}.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0109 = new DiagnosticDescriptor( nameof(AZC0109), "Misuse of 'async' parameter.", "'async' parameter in asynchronous method can't be changed and can only be used as an exclusive condition in '?:' operator or conditional statement.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0110 = new DiagnosticDescriptor( nameof(AZC0110), "DO NOT use await keyword in possibly synchronous scope.", "Asynchronous method with `async` parameter can be called from both synchronous and asynchronous scopes. 'await' keyword can be safely used either in guaranteed asynchronous scope (i.e. `if (async) {...}`) or if `async` parameter is passed into awaited method. Awaiting on variables, fields, properties, conditional operators or async methods that don't use `async` parameter isn't allowed outside of the guaranteed asynchronous scope.", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); public static DiagnosticDescriptor AZC0111 = new DiagnosticDescriptor( nameof(AZC0111), "DO NOT use EnsureCompleted in possibly asynchronous scope.", "Asynchronous method with `async` parameter can be called from both synchronous and asynchronous scopes. 'EnsureCompleted' extension method can be safely used on in guaranteed synchronous scope (i.e. `if (!async) {...}`).", - "Usage", + DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); #endregion } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/DiagnosticCategory.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/DiagnosticCategory.cs new file mode 100644 index 00000000000..736373861e1 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/DiagnosticCategory.cs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace Azure.ClientSdk.Analyzers +{ + /// + /// Diagnostic category. + /// + public static class DiagnosticCategory + { + public static readonly string Naming = "Naming"; + public static readonly string Usage = "Usage"; + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs new file mode 100644 index 00000000000..e4f2a7dbc4c --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; + +namespace Azure.ClientSdk.Analyzers +{ + public static class INamespaceSymbolExtensions + { + public static string GetFullNamespaceName(this INamespaceSymbol namespaceSymbo) + { + if (namespaceSymbo is { ContainingNamespace: not null and { IsGlobalNamespace: false} }) + { + return $"{namespaceSymbo.ContainingNamespace.GetFullNamespaceName()}.{namespaceSymbo.Name}"; + } + + return namespaceSymbo.Name; + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs new file mode 100644 index 00000000000..d2f73d7f70b --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Text.RegularExpressions; + +namespace Azure.ClientSdk.Analyzers.ModelName +{ + // + /// + /// Analyzer to check the model names ending with "Data". Avoid using "Data" as model suffix unless the model derives from ResourceData/TrackedResourceData. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class DataSuffixAnalyzer : SuffixAnalyzerBase + { + public const string DiagnosticId = nameof(AZC0032); + + private static readonly DiagnosticDescriptor AZC0032 = new DiagnosticDescriptor(DiagnosticId, Title, + GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: Description); + + private static readonly Regex DataSuffixRegex = new Regex(".+(?(Data))$"); + + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0032); } } + + // unless the model derives from ResourceData/TrackedResourceData + protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure.ResourceManager.Models", "ResourceData") || + IsTypeOf(symbol, "Azure.ResourceManager.Models", "TrackedResourceData"); + + protected override Regex SuffixRegex => DataSuffixRegex; + + protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) + { + var name = typeSymbol.Name; + return Diagnostic.Create(AZC0032, context.Symbol.Locations[0], + new Dictionary { { "SuggestedName", name.Substring(0, name.Length - suffix.Length) } }.ToImmutableDictionary(), name, suffix); + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs new file mode 100644 index 00000000000..d97ee3d68f1 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Azure.ClientSdk.Analyzers.ModelName +{ + /// + /// Analyzer to check model names ending with "Definition". Avoid using "Definition" as model suffix unless it's the name of a Resource or + /// after removing the suffix it's another type. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class DefinitionSuffixAnalyzer : SuffixAnalyzerBase + { + public const string DiagnosticId = nameof(AZC0031); + + private static readonly DiagnosticDescriptor AZC0031 = new DiagnosticDescriptor(DiagnosticId, Title, + GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: Description); + + private static readonly Regex conditionSuffixRegex = new Regex(".+(?(Definition?))$"); + + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0031); } } + + // unless the type is a Resource or after removing the suffix it's another type + protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) + { + if (IsTypeOf(symbol, "Azure.ResourceManager", "ArmResource")) + return true; + + var name = symbol.Name; + var suggestedName = name.Substring(0, name.Length - "Definition".Length); + return context.Compilation.GetTypeByMetadataName($"{symbol.ContainingNamespace.GetFullNamespaceName()}.{suggestedName}") is not null; + } + + protected override Regex SuffixRegex => conditionSuffixRegex; + + protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) + { + var name = typeSymbol.Name; + return Diagnostic.Create(AZC0031, context.Symbol.Locations[0], + new Dictionary { { "SuggestedName", name.Substring(0, name.Length - suffix.Length) } }.ToImmutableDictionary(), name, suffix); + } + } +} 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 new file mode 100644 index 00000000000..da9055b310b --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Azure.ClientSdk.Analyzers.ModelName +{ + /// + /// Analyzer to check general model name suffix issues. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class GeneralSuffixAnalyzer : SuffixAnalyzerBase + { + public const string DiagnosticId = nameof(AZC0030); + + private static readonly string messageFormat = "Model name '{0}' ends with '{1}'. Suggest to rename it to {2} or any other appropriate name."; + + private static readonly ImmutableHashSet reservedNames = ImmutableHashSet.Create("ErrorResponse"); + + private static readonly DiagnosticDescriptor AZC0030 = new DiagnosticDescriptor(DiagnosticId, Title, + messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: Description); + + // Avoid to use suffixes "Request", "Parameter", "Otpion", "Response", "Collection" + private static readonly Regex generalSuffixRegex = new Regex(".+(?(Requests?)|(Responses?)|(Parameters?)|(Options?)|(Collection))$"); + + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0030); } } + + protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => reservedNames.Contains(symbol.Name); + + protected override Regex SuffixRegex => generalSuffixRegex; + protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) + { + var name = typeSymbol.Name; + var suggestedName = GetSuggestedName(name, suffix); + return Diagnostic.Create(AZC0030, context.Symbol.Locations[0], + new Dictionary { { "SuggestedName", suggestedName } }.ToImmutableDictionary(), name, suffix, suggestedName); + } + + private string GetSuggestedName(string originalName, string suffix) + { + var nameWithoutSuffix = originalName.Substring(0, originalName.Length - suffix.Length); + return suffix switch + { + "Request" or "Requests" => $"'{nameWithoutSuffix}Content'", + "Parameter" or "Parameters" => $"'{nameWithoutSuffix}Content' or '{nameWithoutSuffix}Patch'", + "Option" or "Options" => $"'{nameWithoutSuffix}Config'", + "Response" => $"'{nameWithoutSuffix}Result'", + "Responses" => $"'{nameWithoutSuffix}Results'", + "Collection" => $"'{nameWithoutSuffix}Group' or '{nameWithoutSuffix}List'", + _ => nameWithoutSuffix, + }; + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs new file mode 100644 index 00000000000..348b89edc1c --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Azure.ClientSdk.Analyzers.ModelName +{ + /// + /// Analyzer to check model names ending with "Operation". Avoid using Operation as model suffix unless the model derives from Operation + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class OperationSuffixAnalyzer : SuffixAnalyzerBase + { + public const string DiagnosticId = nameof(AZC0033); + + private static readonly string messageFormat = "Model name '{0}' ends with '{1}'. Suggest to rename it to '{2}' or '{3}', if an appropriate name could not be found."; + + private static readonly DiagnosticDescriptor AZC0033 = new DiagnosticDescriptor(DiagnosticId, Title, + messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: Description); + + private static readonly Regex operationSuffixRegex = new Regex(".+(?(Operation))$"); + + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0033); } } + + // Unless the model derivew from Operation + protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure", "Operation"); + + protected override Regex SuffixRegex => operationSuffixRegex; + protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) + { + var name = typeSymbol.Name; + var nameWithoutSuffix = name.Substring(0, name.Length - suffix.Length); + return Diagnostic.Create(AZC0033, context.Symbol.Locations[0], + name, suffix, $"{nameWithoutSuffix}Data", $"{nameWithoutSuffix}Info"); + } + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs new file mode 100644 index 00000000000..2c73b6ee709 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -0,0 +1,79 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Immutable; +using System.Linq; +using System.Text.RegularExpressions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Azure.ClientSdk.Analyzers.ModelName +{ + /// + /// Base analyzer to check model name suffixes. + /// + public abstract class SuffixAnalyzerBase : DiagnosticAnalyzer + { + protected static readonly string Title = "Improper model name suffix"; + protected static readonly string Description = "Suffix is not recommended. Consider to remove or modify it."; + protected static readonly string GeneralRenamingMessageFormat = "Model name '{0}' ends with '{1}'. Suggest to rename it to an appropriate name."; + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Empty; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics); + context.EnableConcurrentExecution(); + context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType); + } + + private void AnalyzeSymbol(SymbolAnalysisContext context) + { + var typeSymbol = (INamedTypeSymbol)context.Symbol; + if (!IsClassUnderModelsNamespace(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol)) + return; + + var match = SuffixRegex.Match(typeSymbol.Name); + if (match.Success && !ShouldSkip(typeSymbol, context)) + { + var suffix = match.Groups["Suffix"].Value; + context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); + } + } + + protected virtual bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => false; + protected virtual Regex SuffixRegex => throw new NotImplementedException(); + protected virtual Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) => throw new NotImplementedException(); + + // check if a given symbol is the sub-type of the given type + protected bool IsTypeOf(INamedTypeSymbol typeSymbol, string namespaceName, string typeName) + { + if (typeSymbol is null) + return false; + + // check class hierachy + for (var classType = typeSymbol; classType is not null; classType = classType.BaseType) + { + if (classType.Name == typeName && classType.ContainingNamespace.GetFullNamespaceName() == namespaceName) + return true; + }; + + // check interfaces + return typeSymbol.AllInterfaces.Any(i => i.Name == typeName && i.ContainingNamespace.Name == namespaceName); + } + + private bool IsClassUnderModelsNamespace(ITypeSymbol symbol) => symbol is { TypeKind: TypeKind.Class } && HasModelsNamespace(symbol); + + private bool HasModelsNamespace(ITypeSymbol typeSymbol) + { + for (var namespaceSymbol = typeSymbol.ContainingNamespace; namespaceSymbol != null; namespaceSymbol = namespaceSymbol.ContainingNamespace) + { + if (namespaceSymbol.Name.Split('.').Any(name => name == "Models")) + return true; + } + return false; + } + } +} + From 2e313f401eeb8adc5f3d4c7c1ec18eaace8af0be Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Thu, 7 Sep 2023 16:11:11 +0800 Subject: [PATCH 02/20] fix and suppress warnings due to upgrade of Roslyn --- .../Azure.ClientSdk.Analyzers/.editorconfig | 23 +++++++++++++++++++ .../Azure.ClientSdk.Analyzers.csproj | 1 + 2 files changed, 24 insertions(+) create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/.editorconfig diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/.editorconfig b/src/dotnet/Azure.ClientSdk.Analyzers/.editorconfig new file mode 100644 index 00000000000..6ff62a0f2a9 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/.editorconfig @@ -0,0 +1,23 @@ +# The warnings below are found after upgrading to Roslyn 4.7.0 +# It should be able to fix them all. Suppressing them is just a temporary work-around. + +[*.cs] + +# CS0618: 'IOperation.Children' is obsolete: 'This API has performance penalties, please use ChildOperations instead. +dotnet_diagnostic.CS0618.severity = none + +# RS1031: The diagnostic title should not contain a period, nor any line return character, nor any leading or trailing whitespaces +dotnet_diagnostic.RS1031.severity = none + +# RS1032: The diagnostic message should not contain any line return character nor any leading or trailing whitespaces and should either be a single sentence without a trailing period or a multi-sentences with a trailing period +dotnet_diagnostic.RS1032.severity = none + +# RS2008: Enable analyzer release tracking for the analyzer project containing rule 'XXX' +dotnet_diagnostic.RS2008.severity = none + +# RS1024: Use 'SymbolEqualityComparer' when comparing symbols +dotnet_diagnostic.RS1024.severity = none + +# RS1037: Add "CompilationEnd" custom tag to the diagnostic descriptor used to initialize field 'AZC0011' as it is used to report a compilation end diagnostic +dotnet_diagnostic.RS1037.severity = none + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj index d4a8ea01b68..a82552210c5 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj @@ -13,6 +13,7 @@ analyzers/dotnet/cs/ 0.1.1 true + true From 78e74d24a9f732b97efb1014bc93237f058b4f1a Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Mon, 11 Sep 2023 14:21:43 +0800 Subject: [PATCH 03/20] downgrade to Roslyn 4.4.0.0 to align with SDK repo --- .../Azure.ClientSdk.Analyzers.Tests.csproj | 4 ++-- .../Azure.ClientSdk.Analyzers.csproj | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj index da16d640721..aa76426ffc9 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/Azure.ClientSdk.Analyzers.Tests.csproj @@ -23,8 +23,8 @@ - - + + diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj index a82552210c5..52c27551bd8 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj @@ -17,8 +17,8 @@ - - + + From 5dd23ddd5ce74e5d62b691602fd5edbfc079a71a Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Mon, 11 Sep 2023 14:34:32 +0800 Subject: [PATCH 04/20] temporarily suprress AD0001 --- .../Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj index 52c27551bd8..61603e27c88 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.csproj @@ -14,6 +14,7 @@ 0.1.1 true true + AD0001 From 60822544cd848ce0687889c8db72951c691e2c9d Mon Sep 17 00:00:00 2001 From: Mingzhe Huang Date: Tue, 12 Sep 2023 15:42:06 +0800 Subject: [PATCH 05/20] Update comments Co-authored-by: Christopher Scott --- .../ModelName/GeneralSuffixAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 da9055b310b..99b7c6127d8 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 @@ -25,7 +25,7 @@ public class GeneralSuffixAnalyzer : SuffixAnalyzerBase messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); - // Avoid to use suffixes "Request", "Parameter", "Otpion", "Response", "Collection" + // Avoid to use suffixes "Request(s)", "Parameter(s)", "Option(s)", "Response(s)", "Collection" private static readonly Regex generalSuffixRegex = new Regex(".+(?(Requests?)|(Responses?)|(Parameters?)|(Options?)|(Collection))$"); public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0030); } } From e0b561e42426e81f534edf67a9e18be2c5e60fd9 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 15:41:40 +0800 Subject: [PATCH 06/20] change virtual methods in `SuffixAnalyzerBase` into abstract methods --- .../ModelName/SuffixAnalyzerBase.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index 2c73b6ee709..4cda40fef3f 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -42,9 +42,9 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) } } - protected virtual bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => false; - protected virtual Regex SuffixRegex => throw new NotImplementedException(); - protected virtual Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) => throw new NotImplementedException(); + protected abstract bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context); + protected abstract Regex SuffixRegex { get; } + protected abstract Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context); // check if a given symbol is the sub-type of the given type protected bool IsTypeOf(INamedTypeSymbol typeSymbol, string namespaceName, string typeName) From c94ea1c536b8852659a24336f07b1d4e22163272 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 16:18:45 +0800 Subject: [PATCH 07/20] repalce regular expression with raw string comparision --- .../ModelName/DataSuffixAnalyzer.cs | 4 ++-- .../ModelName/DefinitionSuffixAnalyzer.cs | 4 ++-- .../ModelName/GeneralSuffixAnalyzer.cs | 4 ++-- .../ModelName/OperationSuffixAnalyzer.cs | 4 ++-- .../ModelName/SuffixAnalyzerBase.cs | 14 ++++++++------ 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs index d2f73d7f70b..86755a9c0b4 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs @@ -22,7 +22,7 @@ public class DataSuffixAnalyzer : SuffixAnalyzerBase GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); - private static readonly Regex DataSuffixRegex = new Regex(".+(?(Data))$"); + private static readonly string[] dataSuffix = new string[] { "Data" }; public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0032); } } @@ -30,7 +30,7 @@ public class DataSuffixAnalyzer : SuffixAnalyzerBase protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure.ResourceManager.Models", "ResourceData") || IsTypeOf(symbol, "Azure.ResourceManager.Models", "TrackedResourceData"); - protected override Regex SuffixRegex => DataSuffixRegex; + protected override string[] SuffixesToCatch => dataSuffix; protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs index d97ee3d68f1..d1e6885b2ee 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs @@ -22,7 +22,7 @@ public class DefinitionSuffixAnalyzer : SuffixAnalyzerBase GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); - private static readonly Regex conditionSuffixRegex = new Regex(".+(?(Definition?))$"); + private static readonly string[] definitionSuffix = new string[] { "Definition" }; public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0031); } } @@ -37,7 +37,7 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex return context.Compilation.GetTypeByMetadataName($"{symbol.ContainingNamespace.GetFullNamespaceName()}.{suggestedName}") is not null; } - protected override Regex SuffixRegex => conditionSuffixRegex; + protected override string[] SuffixesToCatch => definitionSuffix; protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { 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 99b7c6127d8..bda36c4c7f9 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 @@ -26,13 +26,13 @@ public class GeneralSuffixAnalyzer : SuffixAnalyzerBase description: Description); // Avoid to use suffixes "Request(s)", "Parameter(s)", "Option(s)", "Response(s)", "Collection" - private static readonly Regex generalSuffixRegex = new Regex(".+(?(Requests?)|(Responses?)|(Parameters?)|(Options?)|(Collection))$"); + private static readonly string[] generalSuffixes = new string[] { "Request", "Requests", "Response", "Responses", "Parameter", "Parameters", "Option", "Options", "Collection"}; public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0030); } } protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => reservedNames.Contains(symbol.Name); - protected override Regex SuffixRegex => generalSuffixRegex; + protected override string[] SuffixesToCatch => generalSuffixes; protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { var name = typeSymbol.Name; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs index 348b89edc1c..b230d969d4d 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs @@ -22,14 +22,14 @@ public class OperationSuffixAnalyzer : SuffixAnalyzerBase messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description); - private static readonly Regex operationSuffixRegex = new Regex(".+(?(Operation))$"); + private static readonly string[] operationSuffix = new string[] { "Operation" }; public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0033); } } // Unless the model derivew from Operation protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure", "Operation"); - protected override Regex SuffixRegex => operationSuffixRegex; + protected override string[] SuffixesToCatch => operationSuffix; protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { var name = typeSymbol.Name; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index 4cda40fef3f..69112e61e98 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -31,19 +31,21 @@ public override void Initialize(AnalysisContext context) private void AnalyzeSymbol(SymbolAnalysisContext context) { var typeSymbol = (INamedTypeSymbol)context.Symbol; - if (!IsClassUnderModelsNamespace(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol)) + if (!IsClassUnderModelsNamespace(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol) || ShouldSkip(typeSymbol, context)) return; - var match = SuffixRegex.Match(typeSymbol.Name); - if (match.Success && !ShouldSkip(typeSymbol, context)) + foreach (var suffix in SuffixesToCatch) { - var suffix = match.Groups["Suffix"].Value; - context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); + if (typeSymbol.Name.EndsWith(suffix)) + { + context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); + return; + } } } protected abstract bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context); - protected abstract Regex SuffixRegex { get; } + protected abstract string[] SuffixesToCatch { get; } protected abstract Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context); // check if a given symbol is the sub-type of the given type From d0de622b3731224dedfdb3dc81d9adbf43ff241c Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 17:07:31 +0800 Subject: [PATCH 08/20] use span to check if a namespace belong to SDK --- .../AnalyzerUtils.cs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs index e112f03f95a..8bbe762c5c1 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs @@ -34,15 +34,22 @@ internal static bool IsSdkCode(SyntaxNode node, SemanticModel model) private static bool IsSdkNamespace(string ns) { - var namespaces = ns.Split('.'); - // skip Azure or Azure.Core - if (namespaces.Length >= 2) - { - return namespaces[0] == "Azure" && - namespaces[1] != "Core"; - } - - return false; + var namespaces = ns.AsSpan(); + // if the namespace contains only one level, it's not SDK namespace + var indexOfFirstDot = namespaces.IndexOf('.'); + if (indexOfFirstDot == -1) + return false; + + // first namespace must be `Azure` + var firstNamespace = namespaces.Slice(0, indexOfFirstDot); + if (!firstNamespace.Equals("Azure".AsSpan(), StringComparison.Ordinal)) + return false; + + // second namespace must not be `Core` + var remainingNamespace = namespaces.Slice(indexOfFirstDot + 1); + var indexOfSecondDot = remainingNamespace.IndexOf('.'); + var seondNamespace = (indexOfSecondDot == -1 ? remainingNamespace : remainingNamespace.Slice(0, indexOfSecondDot)); + return !seondNamespace.Equals("Core".AsSpan(), StringComparison.Ordinal); } private static string GetNamespace(SyntaxNode node) From d36cc186a00cb310b745f44542e85d1faf8c4ee8 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 22:40:18 +0800 Subject: [PATCH 09/20] avoid unnecessary string allocation when getting full namespace name --- .../INameSpaceSymbolExtensions.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs index e4f2a7dbc4c..c61a1bacd65 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs @@ -1,20 +1,29 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; using Microsoft.CodeAnalysis; namespace Azure.ClientSdk.Analyzers { public static class INamespaceSymbolExtensions { - public static string GetFullNamespaceName(this INamespaceSymbol namespaceSymbo) + public static string GetFullNamespaceName(this INamespaceSymbol namespaceSymbol) { - if (namespaceSymbo is { ContainingNamespace: not null and { IsGlobalNamespace: false} }) + return string.Join(".", GetAllNamespaces(namespaceSymbol)); + } + + private static IList GetAllNamespaces(INamespaceSymbol namespaceSymbol) + { + if (namespaceSymbol is { ContainingNamespace: not null and { IsGlobalNamespace: false } }) { - return $"{namespaceSymbo.ContainingNamespace.GetFullNamespaceName()}.{namespaceSymbo.Name}"; + var namespaces = GetAllNamespaces(namespaceSymbol.ContainingNamespace); + namespaces.Add(namespaceSymbol.Name); + return namespaces; } - return namespaceSymbo.Name; + return new List { namespaceSymbol.Name }; + } } } From 65ff2c985b3e08bf66aaf5a3312dd4d643e330aa Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 22:52:41 +0800 Subject: [PATCH 10/20] avoid unnecessary string allocation when getting full namespace name from syntax node --- .../Azure.ClientSdk.Analyzers/AnalyzerUtils.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs index 8bbe762c5c1..2428a2c098d 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs @@ -4,6 +4,8 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using System; +using System.Collections.Generic; +using System.Linq; namespace Azure.ClientSdk.Analyzers { @@ -54,7 +56,7 @@ private static bool IsSdkNamespace(string ns) private static string GetNamespace(SyntaxNode node) { - var ns = string.Empty; + var namespaces = new List(); var parent = node.Parent; @@ -67,7 +69,7 @@ parent is not NamespaceDeclarationSyntax if (parent is BaseNamespaceDeclarationSyntax namespaceParent) { - ns = namespaceParent.Name.ToString(); + namespaces.Add(namespaceParent.Name.ToString()); while (true) { @@ -76,12 +78,13 @@ parent is not NamespaceDeclarationSyntax break; } - ns = $"{namespaceParent.Name}.{ns}"; + namespaces.Add(namespaceParent.Name.ToString()); namespaceParent = parentNamespace; } } - return ns; + + return string.Join(".", namespaces.Reverse()); } } } From 2f9ca124530c0c9ae21d2e54913b0c945d4cda47 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Tue, 12 Sep 2023 23:17:57 +0800 Subject: [PATCH 11/20] avoid unnecessary split of namespace name --- .../Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index 69112e61e98..b6db04428fc 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -71,7 +71,7 @@ private bool HasModelsNamespace(ITypeSymbol typeSymbol) { for (var namespaceSymbol = typeSymbol.ContainingNamespace; namespaceSymbol != null; namespaceSymbol = namespaceSymbol.ContainingNamespace) { - if (namespaceSymbol.Name.Split('.').Any(name => name == "Models")) + if (namespaceSymbol.Name == "Models") return true; } return false; From 5e5b5ec0cad00871f67a1724ebcb492ea16cd925 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 13 Sep 2023 10:19:58 +0800 Subject: [PATCH 12/20] use span to compare suffix --- .../Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index b6db04428fc..413e8d18099 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -36,7 +36,7 @@ private void AnalyzeSymbol(SymbolAnalysisContext context) foreach (var suffix in SuffixesToCatch) { - if (typeSymbol.Name.EndsWith(suffix)) + if (MemoryExtensions.EndsWith(typeSymbol.Name.AsSpan(), suffix.AsSpan())) { context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); return; From 30c0799b713a389905f1aeeaebe4d08318872c65 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 13 Sep 2023 11:05:26 +0800 Subject: [PATCH 13/20] optimize performance of definition suffix analyzer --- .../ModelName/DefinitionSuffixAnalyzer.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs index d1e6885b2ee..c2ad145d6bc 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Text.RegularExpressions; +using System.Text; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -33,8 +34,12 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex return true; var name = symbol.Name; - var suggestedName = name.Substring(0, name.Length - "Definition".Length); - return context.Compilation.GetTypeByMetadataName($"{symbol.ContainingNamespace.GetFullNamespaceName()}.{suggestedName}") is not null; + var suggestedName = name.AsSpan().Slice(0, name.Length - "Definition".Length); + + var strBuilder = new StringBuilder(symbol.ContainingNamespace.GetFullNamespaceName()); + strBuilder.Append('.').Append(suggestedName.ToArray()); + + return context.Compilation.GetTypeByMetadataName(strBuilder.ToString()) is not null; } protected override string[] SuffixesToCatch => definitionSuffix; From a7f5b6972b9f7cebb0f45a1ac4a49c2b41fbb82c Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 13 Sep 2023 16:51:09 +0800 Subject: [PATCH 14/20] change model determination logic to check serialization methods --- .../ModelName/AZC0030Tests.cs | 84 ++++++++--------- .../ModelName/AZC0031Tests.cs | 18 +++- .../ModelName/AZC0032Tests.cs | 31 ++++--- .../ModelName/AZC0033Tests.cs | 41 +++++++-- .../ModelName/SuffixAnalyzerBaseTests.cs | 90 +++++++++++++++++++ .../ModelName/SuffixAnalyzerBase.cs | 25 +++--- 6 files changed, 206 insertions(+), 83 deletions(-) create mode 100644 src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs index bc4aacb52c4..27234f04542 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs @@ -5,50 +5,22 @@ using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< Azure.ClientSdk.Analyzers.ModelName.GeneralSuffixAnalyzer>; -namespace Azure.ClientSdk.Analyzers.Test.ModelName +namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0030Tests { - [Fact] - public async Task ClassNotUnderModelsNamespaceIsNotChecked() - { - var test = @"namespace Azure.ResourceManager; - -class MonitorResult -{ -}"; - await VerifyCS.VerifyAnalyzerAsync(test); - } - - [Fact] - public async Task OnlyModelsNamespaceIsChecked() - { - var test = @"namespace Azure.ResourceManager.AModels; - -class MonitorResult -{ -}"; - await VerifyCS.VerifyAnalyzerAsync(test); - } - - [Fact] - public async Task EnumIsNotChecked() - { - var test = @"namespace Azure.ResourceManager.Models; - -enum MonitorResult -{ -}"; - await VerifyCS.VerifyAnalyzerAsync(test); - } - [Fact] public async Task GoodSuffix() { - var test = @"namespace Azure.ResourceManager.Models; + var test = @"using System.Text.Json; +namespace Azure.ResourceManager.Models; -class MonitorContent +public class MonitorContent { + public static MonitorContent DeserializeMonitorContent(JsonElement element) + { + return null; + } }"; await VerifyCS.VerifyAnalyzerAsync(test); } @@ -56,58 +28,78 @@ class MonitorContent [Fact] public async Task ParametersSuffix() { - var test = @"namespace Azure.ResourceManager.Models + var test = @"using System.Text.Json; +namespace Azure.ResourceManager { public class ResponseParameters { + public static ResponseParameters DeserializeResponseParameters(JsonElement element) + { + return null; + } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 18, 3, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'"); + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } [Fact] - public async Task ResultSuffix() + public async Task RequestSuffix() { - var test = @"namespace Azure.ResourceManager.Models + var test = @"using System.Text.Json; +namespace Azure.ResourceManager.Models { - public class NetworkRequest + public class DiskOption { + public static DiskOption DeserializeDiskOption(JsonElement element) + { + return null; + } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 18, 3, 32).WithArguments("NetworkRequest", "Request", "'NetworkContent'"); + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 28).WithArguments("DiskOption", "Option", "'DiskConfig'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } [Fact] public async Task OptionSuffixWithNestedNameSpace() { - var test = @"namespace Azure.ResourceManager.Models + var test = @"using System.Text.Json; +namespace Azure.ResourceManager.Models { namespace SubTest { public class DiskOption { + public static DiskOption DeserializeDiskOption(JsonElement element) + { + return null; + } } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(5, 22, 5, 32).WithArguments("DiskOption", "Option", "'DiskConfig'"); + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(6, 22, 6, 32).WithArguments("DiskOption", "Option", "'DiskConfig'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } [Fact] public async Task ResponsesSuffix() { - var test = @"namespace Azure.ResourceManager.Models + var test = @"using System.Text.Json; +namespace Azure.ResourceManager.Models { namespace SubTest { public class CreationResponses { + public static CreationResponses DeserializeCreationResponses(JsonElement element) + { + return null; + } } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(5, 22, 5, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'"); + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(6, 22, 6, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs index bd9838bc45a..de6c203f7cd 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs @@ -12,11 +12,15 @@ public class AZC0031Tests [Fact] public async Task ModelWithDefinitionSuffix() { - var test = @" + var test = @"using System.Text.Json; namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationDefinition { + public static AadAuthenticationDefinition DeserializeAadAuthenticationDefinition(JsonElement element) + { + return null; + } } }"; var expected = VerifyCS.Diagnostic(DefinitionSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 53).WithArguments("AadAuthenticationDefinition", "Definition"); @@ -26,7 +30,7 @@ public partial class AadAuthenticationDefinition [Fact] public async Task ArmResourceIsNotChecked() { - var test = @" + var test = @"using System.Text.Json; using Azure.ResourceManager; namespace Azure.ResourceManager { @@ -37,6 +41,10 @@ namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationDefinition: ArmResource { + public static AadAuthenticationDefinition DeserializeAadAuthenticationDefinition(JsonElement element) + { + return null; + } } }"; await VerifyCS.VerifyAnalyzerAsync(test); @@ -45,7 +53,7 @@ public partial class AadAuthenticationDefinition: ArmResource [Fact] public async Task NotCheckIfRemovingSuffixIsAnotherType() { - var test = @" + var test = @"using System.Text.Json; using Azure.ResourceManager; namespace Azure.ResourceManager.Network.Models { @@ -54,6 +62,10 @@ public class AadAuthentication { public partial class AadAuthenticationDefinition { + public static AadAuthenticationDefinition DeserializeAadAuthenticationDefinition(JsonElement element) + { + return null; + } } }"; await VerifyCS.VerifyAnalyzerAsync(test); diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs index a492205afa6..ffcbe6e93c5 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs @@ -9,27 +9,18 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0032Tests { - [Fact] - public async Task ClassUnderNonModelsNamespaceIsNotChecked() - { - var test = @" -namespace Azure.ResourceManager.Network.Temp -{ - public partial class AadAuthenticationData - { - } -}"; - await VerifyCS.VerifyAnalyzerAsync(test); - } - [Fact] public async Task ModelClassWithDataSuffix() { - var test = @" + var test = @"using System.Text.Json; namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationData { + public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement element) + { + return null; + } } }"; var expected = VerifyCS.Diagnostic(DataSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 47).WithArguments("AadAuthenticationData", "Data"); @@ -39,7 +30,7 @@ public partial class AadAuthenticationData [Fact] public async Task ResourceDataClassesAreNotChecked() { - var test = @" + var test = @"using System.Text.Json; using Azure.ResourceManager.Models; namespace Azure.ResourceManager.Models { @@ -50,6 +41,10 @@ namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationData: ResourceData { + public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement element) + { + return null; + } } }"; await VerifyCS.VerifyAnalyzerAsync(test); @@ -58,7 +53,7 @@ public partial class AadAuthenticationData: ResourceData [Fact] public async Task TrackedResourceDataClassesAreNotChecked() { - var test = @" + var test = @"using System.Text.Json; using Azure.ResourceManager.Models; namespace Azure.ResourceManager.Models { @@ -69,6 +64,10 @@ namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationData: TrackedResourceData { + public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement element) + { + return null; + } } }"; await VerifyCS.VerifyAnalyzerAsync(test); diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs index 8565631c8fd..0fa74677616 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs @@ -6,14 +6,15 @@ using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< Azure.ClientSdk.Analyzers.ModelName.OperationSuffixAnalyzer>; -namespace Azure.ClientSdk.Analyzers.Test.ModelName +namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0033Tests { [Fact] public async Task OperationClassIsNotChecked() { - var test = @" + var test = @"using System.Text.Json; + using Azure; using Azure.ResourceManager; namespace Azure @@ -36,17 +37,33 @@ public class ArmOperation : Operation } namespace Azure.ResourceManager.Network.Models { - internal class DnsOperation : Operation + public class DnsOperation : Operation { + public static DnsOperation DeserializeDnsOperation(JsonElement element) + { + return null; + } } - internal class DnsArmOperation : ArmOperation + public class DnsArmOperation : ArmOperation { + public static DnsArmOperation DeserializeDnsArmOperation(JsonElement element) + { + return null; + } } - internal class DnsOperation : Operation + public class DnsOperation : Operation { + public static DnsOperation DeserializeDnsOperation(JsonElement element) + { + return null; + } } - internal class DnsArmOperation : ArmOperation + public class DnsArmOperation : ArmOperation { + public static DnsArmOperation DeserializeDnsOperation(JsonElement element) + { + return null; + } } }"; await VerifyCS.VerifyAnalyzerAsync(test); @@ -55,19 +72,27 @@ internal class DnsArmOperation : ArmOperation [Fact] public async Task OperationSuffix() { - var test = @" + var test = @"using System.Text.Json; namespace Azure.ResourceManager.Network.Models { public class DnsOperation { + public static DnsOperation DeserializeDnsOperation(JsonElement element) + { + return null; + } } public class DnsArmOperation { + public static DnsArmOperation DeserializeDnsArmOperation(JsonElement element) + { + return null; + } } }"; DiagnosticResult[] expected = { VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 30).WithArguments("DnsOperation", "Operation", "DnsData", "DnsInfo"), - VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(7, 18, 7, 33).WithArguments("DnsArmOperation", "Operation", "DnsArmData", "DnsArmInfo") + VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(11, 18, 11, 33).WithArguments("DnsArmOperation", "Operation", "DnsArmData", "DnsArmInfo") }; await VerifyCS.VerifyAnalyzerAsync(test, expected); } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs new file mode 100644 index 00000000000..e7c39654ca0 --- /dev/null +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs @@ -0,0 +1,90 @@ +using System.Threading.Tasks; +using Azure.ClientSdk.Analyzers.ModelName; +using Xunit; + +using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier< + Azure.ClientSdk.Analyzers.ModelName.GeneralSuffixAnalyzer>; + +namespace Azure.ClientSdk.Analyzers.Tests.ModelName +{ + public class SuffixAnalyzerBaseTests + { + [Fact] + public async Task NonPublicClassIsNotChecked() + { + var test = @"using System.Text.Json; +namespace Azure.Test.Models; + +class MonitorParameter +{ + public static MonitorParameter DeserializeMonitorParameter(JsonElement element) + { + return null; + } +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + [Fact] + public async Task ClassWithoutSerliaizationMethodsIsNotChecked() + { + var test = @"namespace Azure.Test.Models; + +public class MonitorParameter +{ +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ClassWithSerliazationMethodIsChecked() + { + var test = @"using System.Text.Json; +namespace Azure.NotModels; + +public class MonitorParameter +{ + public static MonitorParameter DeserializeMonitorParameter(JsonElement element) + { + return null; + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 14, 4, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task ClassWithDeserliazationMethodIsChecked() + { + var test = @"using System.Text.Json; +namespace Azure.NotModels; + +// workaround since IUtf8JsonSerializable is internally shared +internal interface IUtf8JsonSerializable +{ + void Write(Utf8JsonWriter writer); +} +public class MonitorParameter : IUtf8JsonSerializable +{ + void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) + { + return; + } +}"; + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(9, 14, 9, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); + } + + [Fact] + public async Task EnumIsNotChecked() + { + var test = @"namespace Azure.ResourceManager.Models; + +public enum MonitorParameter +{ + One +}"; + await VerifyCS.VerifyAnalyzerAsync(test); + } + + } +} diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index 413e8d18099..e485283cdc3 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -31,12 +31,13 @@ public override void Initialize(AnalysisContext context) private void AnalyzeSymbol(SymbolAnalysisContext context) { var typeSymbol = (INamedTypeSymbol)context.Symbol; - if (!IsClassUnderModelsNamespace(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol) || ShouldSkip(typeSymbol, context)) + if (typeSymbol.DeclaredAccessibility != Accessibility.Public || !IsModelClass(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol) || ShouldSkip(typeSymbol, context)) return; + var nameSpan = typeSymbol.Name.AsSpan(); foreach (var suffix in SuffixesToCatch) { - if (MemoryExtensions.EndsWith(typeSymbol.Name.AsSpan(), suffix.AsSpan())) + if (MemoryExtensions.EndsWith(nameSpan, suffix.AsSpan())) { context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); return; @@ -65,15 +66,19 @@ protected bool IsTypeOf(INamedTypeSymbol typeSymbol, string namespaceName, strin return typeSymbol.AllInterfaces.Any(i => i.Name == typeName && i.ContainingNamespace.Name == namespaceName); } - private bool IsClassUnderModelsNamespace(ITypeSymbol symbol) => symbol is { TypeKind: TypeKind.Class } && HasModelsNamespace(symbol); + private bool IsModelClass(ITypeSymbol symbol){ + if (symbol is not ({ TypeKind: TypeKind.Class })) + return false; + + // check serialize interface, TODO: include public serializer interface + if (symbol.Interfaces.Any(i => i.Name == "IUtf8JsonSerializable")) + return true; + + // check if deserialize method exists, e.g. internal static Foo DeserializeFoo(JsonElement element) + if (symbol.GetMembers($"Deserialize{symbol.Name}").Any(m => m is IMethodSymbol method && method is { IsStatic: true } && + method.ReturnType == symbol && method.Parameters.Length == 1 && method.Parameters[0] is { Type.Name: "JsonElement" })) + return true; - private bool HasModelsNamespace(ITypeSymbol typeSymbol) - { - for (var namespaceSymbol = typeSymbol.ContainingNamespace; namespaceSymbol != null; namespaceSymbol = namespaceSymbol.ContainingNamespace) - { - if (namespaceSymbol.Name == "Models") - return true; - } return false; } } From 8c78b84ab17d5d452e77a72a5e1be5667ea1ef61 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Thu, 14 Sep 2023 11:52:41 +0800 Subject: [PATCH 15/20] add back check of `Models` namespace --- .../ModelName/AZC0030Tests.cs | 8 -------- .../ModelName/AZC0031Tests.cs | 4 ---- .../ModelName/AZC0032Tests.cs | 10 +--------- .../ModelName/AZC0033Tests.cs | 18 +----------------- .../ModelName/SuffixAnalyzerBaseTests.cs | 19 +++++++++++++------ .../ModelName/SuffixAnalyzerBase.cs | 15 +++++++++++++++ 6 files changed, 30 insertions(+), 44 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs index 27234f04542..d58964a283c 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs @@ -17,10 +17,6 @@ namespace Azure.ResourceManager.Models; public class MonitorContent { - public static MonitorContent DeserializeMonitorContent(JsonElement element) - { - return null; - } }"; await VerifyCS.VerifyAnalyzerAsync(test); } @@ -71,10 +67,6 @@ namespace SubTest { public class DiskOption { - public static DiskOption DeserializeDiskOption(JsonElement element) - { - return null; - } } } }"; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs index de6c203f7cd..1c55ae0982f 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs @@ -17,10 +17,6 @@ namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationDefinition { - public static AadAuthenticationDefinition DeserializeAadAuthenticationDefinition(JsonElement element) - { - return null; - } } }"; var expected = VerifyCS.Diagnostic(DefinitionSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 53).WithArguments("AadAuthenticationDefinition", "Definition"); diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs index ffcbe6e93c5..299e9acce32 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs @@ -37,14 +37,10 @@ namespace Azure.ResourceManager.Models public class ResourceData { } } -namespace Azure.ResourceManager.Network.Models +namespace Azure.ResourceManager.Models.Network { public partial class AadAuthenticationData: ResourceData { - public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement element) - { - return null; - } } }"; await VerifyCS.VerifyAnalyzerAsync(test); @@ -64,10 +60,6 @@ namespace Azure.ResourceManager.Network.Models { public partial class AadAuthenticationData: TrackedResourceData { - public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement element) - { - return null; - } } }"; await VerifyCS.VerifyAnalyzerAsync(test); diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs index 0fa74677616..d177712e462 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs @@ -39,31 +39,15 @@ namespace Azure.ResourceManager.Network.Models { public class DnsOperation : Operation { - public static DnsOperation DeserializeDnsOperation(JsonElement element) - { - return null; - } } public class DnsArmOperation : ArmOperation { - public static DnsArmOperation DeserializeDnsArmOperation(JsonElement element) - { - return null; - } } public class DnsOperation : Operation { - public static DnsOperation DeserializeDnsOperation(JsonElement element) - { - return null; - } } public class DnsArmOperation : ArmOperation { - public static DnsArmOperation DeserializeDnsOperation(JsonElement element) - { - return null; - } } }"; await VerifyCS.VerifyAnalyzerAsync(test); @@ -73,7 +57,7 @@ public static DnsArmOperation DeserializeDnsOperation(JsonElement element) public async Task OperationSuffix() { var test = @"using System.Text.Json; -namespace Azure.ResourceManager.Network.Models +namespace Azure.ResourceManager.Network { public class DnsOperation { diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs index e7c39654ca0..eeb46226264 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs @@ -24,15 +24,22 @@ public static MonitorParameter DeserializeMonitorParameter(JsonElement element) }"; await VerifyCS.VerifyAnalyzerAsync(test); } - [Fact] - public async Task ClassWithoutSerliaizationMethodsIsNotChecked() - { - var test = @"namespace Azure.Test.Models; + + [Theory] + [InlineData(@"namespace Azure.Test.Models; public class MonitorParameter { -}"; - await VerifyCS.VerifyAnalyzerAsync(test); +}")] + [InlineData(@"namespace Azure.Models.Test; + +public class MonitorParameter +{ +}")] + public async Task ClassWithoutSerliaizationMethodsButInModelsNamespaceIsChecked(string test) + { + var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 14, 3, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + await VerifyCS.VerifyAnalyzerAsync(test, expected); } [Fact] diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index e485283cdc3..f8dcb2db39e 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -70,6 +70,11 @@ private bool IsModelClass(ITypeSymbol symbol){ if (symbol is not ({ TypeKind: TypeKind.Class })) return false; + // some SDKs could have models without any serialization method but under `Models` namespace + // like alertsmanagement\Azure.ResourceManager.AlertsManagement\src\Generated\Models\SmartGroupCollectionGetAllOptions.cs + if (HasModelsNamespace(symbol)) + return true; + // check serialize interface, TODO: include public serializer interface if (symbol.Interfaces.Any(i => i.Name == "IUtf8JsonSerializable")) return true; @@ -81,6 +86,16 @@ private bool IsModelClass(ITypeSymbol symbol){ return false; } + + private bool HasModelsNamespace(ITypeSymbol typeSymbol) + { + for (var namespaceSymbol = typeSymbol.ContainingNamespace; namespaceSymbol != null; namespaceSymbol = namespaceSymbol.ContainingNamespace) + { + if (namespaceSymbol.Name == "Models") + return true; + } + return false; + } } } From 5d28699ee360232a8870331ee5e157cb9b9b7ddc Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Thu, 14 Sep 2023 11:54:11 +0800 Subject: [PATCH 16/20] improve the performacne of checking sdk namespace --- .../AnalyzerUtils.cs | 37 ++++--------------- .../INameSpaceSymbolExtensions.cs | 15 ++++---- 2 files changed, 15 insertions(+), 37 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs index 2428a2c098d..528399fe894 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs @@ -1,11 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; -using System; -using System.Collections.Generic; -using System.Linq; namespace Azure.ClientSdk.Analyzers { @@ -15,9 +13,9 @@ internal class AnalyzerUtils internal static bool IsSdkCode(ISymbol symbol) { - var ns = symbol.ContainingNamespace.GetFullNamespaceName(); + var namespaces = symbol.ContainingNamespace.GetAllNamespaces(); - return IsSdkNamespace(ns); + return IsSdkNamespace(namespaces); } internal static bool IsNotSdkCode(SyntaxNode node, SemanticModel model) => !IsSdkCode(node, model); @@ -30,31 +28,13 @@ internal static bool IsSdkCode(SyntaxNode node, SemanticModel model) return IsSdkCode(symbol); } - var ns = GetNamespace(node); - return IsSdkNamespace(ns); + var namespaces = GetNamespace(node); + return IsSdkNamespace(namespaces); } - private static bool IsSdkNamespace(string ns) - { - var namespaces = ns.AsSpan(); - // if the namespace contains only one level, it's not SDK namespace - var indexOfFirstDot = namespaces.IndexOf('.'); - if (indexOfFirstDot == -1) - return false; - - // first namespace must be `Azure` - var firstNamespace = namespaces.Slice(0, indexOfFirstDot); - if (!firstNamespace.Equals("Azure".AsSpan(), StringComparison.Ordinal)) - return false; + private static bool IsSdkNamespace(IReadOnlyList namespaces) => namespaces.Count >= 2 && namespaces[0] == "Azure" && namespaces[1] != "Core"; - // second namespace must not be `Core` - var remainingNamespace = namespaces.Slice(indexOfFirstDot + 1); - var indexOfSecondDot = remainingNamespace.IndexOf('.'); - var seondNamespace = (indexOfSecondDot == -1 ? remainingNamespace : remainingNamespace.Slice(0, indexOfSecondDot)); - return !seondNamespace.Equals("Core".AsSpan(), StringComparison.Ordinal); - } - - private static string GetNamespace(SyntaxNode node) + private static IReadOnlyList GetNamespace(SyntaxNode node) { var namespaces = new List(); @@ -83,8 +63,7 @@ parent is not NamespaceDeclarationSyntax } } - - return string.Join(".", namespaces.Reverse()); + return namespaces; } } } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs index c61a1bacd65..b40e621eb27 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs @@ -10,20 +10,19 @@ public static class INamespaceSymbolExtensions { public static string GetFullNamespaceName(this INamespaceSymbol namespaceSymbol) { - return string.Join(".", GetAllNamespaces(namespaceSymbol)); + return string.Join(".", namespaceSymbol.GetAllNamespaces()); } - private static IList GetAllNamespaces(INamespaceSymbol namespaceSymbol) + internal static IReadOnlyList GetAllNamespaces(this INamespaceSymbol namespaceSymbol) { - if (namespaceSymbol is { ContainingNamespace: not null and { IsGlobalNamespace: false } }) + var namespaces = new List(); + while (namespaceSymbol is { IsGlobalNamespace: false }) { - var namespaces = GetAllNamespaces(namespaceSymbol.ContainingNamespace); namespaces.Add(namespaceSymbol.Name); - return namespaces; + namespaceSymbol = namespaceSymbol.ContainingNamespace; } - - return new List { namespaceSymbol.Name }; - + namespaces.Reverse(); + return namespaces; } } } From 9abf0865f130aee50f3827567e7b5e1845a962e4 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Thu, 14 Sep 2023 14:52:22 +0800 Subject: [PATCH 17/20] fix an index out of range exception --- .../ModelName/SuffixAnalyzerBase.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index f8dcb2db39e..7264f2d6964 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Immutable; using System.Linq; -using System.Text.RegularExpressions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; @@ -31,13 +30,13 @@ public override void Initialize(AnalysisContext context) private void AnalyzeSymbol(SymbolAnalysisContext context) { var typeSymbol = (INamedTypeSymbol)context.Symbol; - if (typeSymbol.DeclaredAccessibility != Accessibility.Public || !IsModelClass(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol) || ShouldSkip(typeSymbol, context)) + if (typeSymbol.DeclaredAccessibility != Accessibility.Public || !IsModelClass(typeSymbol) || AnalyzerUtils.IsNotSdkCode(typeSymbol)) return; var nameSpan = typeSymbol.Name.AsSpan(); foreach (var suffix in SuffixesToCatch) { - if (MemoryExtensions.EndsWith(nameSpan, suffix.AsSpan())) + if (MemoryExtensions.EndsWith(nameSpan, suffix.AsSpan()) && !ShouldSkip(typeSymbol, context)) { context.ReportDiagnostic(GetDiagnostic(typeSymbol, suffix, context)); return; From 3dbe78d1d0e90e7c185a0037533398335f3e1c43 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 25 Oct 2023 12:01:01 +0800 Subject: [PATCH 18/20] replace List and Join with StringBuilder --- .../INameSpaceSymbolExtensions.cs | 15 +++++++++++++-- .../ModelName/DefinitionSuffixAnalyzer.cs | 2 +- .../ModelName/SuffixAnalyzerBase.cs | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs index b40e621eb27..f3305e5f4d6 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs @@ -2,15 +2,26 @@ // Licensed under the MIT License. using System.Collections.Generic; +using System.Text; using Microsoft.CodeAnalysis; namespace Azure.ClientSdk.Analyzers { public static class INamespaceSymbolExtensions { - public static string GetFullNamespaceName(this INamespaceSymbol namespaceSymbol) + public static StringBuilder GetFullNamespaceName(this INamespaceSymbol namespaceSymbol) { - return string.Join(".", namespaceSymbol.GetAllNamespaces()); + var namespaceName = new StringBuilder(); + while (namespaceSymbol is { IsGlobalNamespace: false }) + { + if (namespaceName.Length > 0) + { + namespaceName.Insert(0, '.'); + } + namespaceName.Insert(0, namespaceSymbol.Name); + namespaceSymbol = namespaceSymbol.ContainingNamespace; + } + return namespaceName; } internal static IReadOnlyList GetAllNamespaces(this INamespaceSymbol namespaceSymbol) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs index c2ad145d6bc..f93a5cc2b22 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs @@ -36,7 +36,7 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex var name = symbol.Name; var suggestedName = name.AsSpan().Slice(0, name.Length - "Definition".Length); - var strBuilder = new StringBuilder(symbol.ContainingNamespace.GetFullNamespaceName()); + var strBuilder = symbol.ContainingNamespace.GetFullNamespaceName(); strBuilder.Append('.').Append(suggestedName.ToArray()); return context.Compilation.GetTypeByMetadataName(strBuilder.ToString()) is not null; diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs index 7264f2d6964..8438cf657fa 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/SuffixAnalyzerBase.cs @@ -57,7 +57,7 @@ protected bool IsTypeOf(INamedTypeSymbol typeSymbol, string namespaceName, strin // check class hierachy for (var classType = typeSymbol; classType is not null; classType = classType.BaseType) { - if (classType.Name == typeName && classType.ContainingNamespace.GetFullNamespaceName() == namespaceName) + if (classType.Name == typeName && classType.ContainingNamespace.GetFullNamespaceName().ToString() == namespaceName) return true; }; From 15faf32ac7157bcef81738212913b745206f30a5 Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 25 Oct 2023 16:53:17 +0800 Subject: [PATCH 19/20] Using ArrayPool to avoid array allocation --- .../AnalyzerUtils.cs | 44 ++++++++++++++++--- .../INameSpaceSymbolExtensions.cs | 6 +-- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs index 528399fe894..8ac140ccf43 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/AnalyzerUtils.cs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections.Generic; +using System; +using System.Buffers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -13,8 +14,7 @@ internal class AnalyzerUtils internal static bool IsSdkCode(ISymbol symbol) { - var namespaces = symbol.ContainingNamespace.GetAllNamespaces(); - + using var namespaces = symbol.ContainingNamespace.GetAllNamespaces(); return IsSdkNamespace(namespaces); } @@ -28,15 +28,15 @@ internal static bool IsSdkCode(SyntaxNode node, SemanticModel model) return IsSdkCode(symbol); } - var namespaces = GetNamespace(node); + using var namespaces = GetNamespace(node); return IsSdkNamespace(namespaces); } - private static bool IsSdkNamespace(IReadOnlyList namespaces) => namespaces.Count >= 2 && namespaces[0] == "Azure" && namespaces[1] != "Core"; + private static bool IsSdkNamespace(Namespaces namespaces) => namespaces.Count >= 2 && namespaces[0] == "Azure" && namespaces[1] != "Core"; - private static IReadOnlyList GetNamespace(SyntaxNode node) + private static Namespaces GetNamespace(SyntaxNode node) { - var namespaces = new List(); + var namespaces = new Namespaces(); var parent = node.Parent; @@ -63,7 +63,37 @@ parent is not NamespaceDeclarationSyntax } } + namespaces.Reverse(); + return namespaces; } + + internal class Namespaces : IDisposable + { + private int count; + private readonly string[] namespaces = ArrayPool.Shared.Rent(10); + + public int Count => this.count; + + public void Add(string name) + { + this.namespaces[this.count++] = name; + } + + public string this[int i] + { + get => this.namespaces[i]; + } + + public void Reverse() + { + Array.Reverse(this.namespaces, 0, this.count); + } + + public void Dispose() + { + ArrayPool.Shared.Return(this.namespaces); + } + } } } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs index f3305e5f4d6..dd2378a1c84 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/INameSpaceSymbolExtensions.cs @@ -1,9 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections.Generic; using System.Text; using Microsoft.CodeAnalysis; +using static Azure.ClientSdk.Analyzers.AnalyzerUtils; namespace Azure.ClientSdk.Analyzers { @@ -24,9 +24,9 @@ public static StringBuilder GetFullNamespaceName(this INamespaceSymbol namespace return namespaceName; } - internal static IReadOnlyList GetAllNamespaces(this INamespaceSymbol namespaceSymbol) + internal static Namespaces GetAllNamespaces(this INamespaceSymbol namespaceSymbol) { - var namespaces = new List(); + var namespaces = new Namespaces(); while (namespaceSymbol is { IsGlobalNamespace: false }) { namespaces.Add(namespaceSymbol.Name); From 99dd5e0b428e4478b690d1176e30c61ad148aecb Mon Sep 17 00:00:00 2001 From: "Mingzhe Huang (from Dev Box)" Date: Wed, 25 Oct 2023 17:21:23 +0800 Subject: [PATCH 20/20] extract analyzer descriptors --- .../ModelName/AZC0030Tests.cs | 10 +++--- .../ModelName/AZC0031Tests.cs | 4 ++- .../ModelName/AZC0032Tests.cs | 4 ++- .../ModelName/AZC0033Tests.cs | 6 ++-- .../ModelName/SuffixAnalyzerBaseTests.cs | 8 +++-- .../Azure.ClientSdk.Analyzers/Descriptors.cs | 36 +++++++++++++++++++ .../ModelName/DataSuffixAnalyzer.cs | 10 ++---- .../ModelName/DefinitionSuffixAnalyzer.cs | 10 ++---- .../ModelName/GeneralSuffixAnalyzer.cs | 12 ++----- .../ModelName/OperationSuffixAnalyzer.cs | 12 ++----- 10 files changed, 65 insertions(+), 47 deletions(-) diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs index d58964a283c..4a3e8b1b8ad 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs @@ -9,6 +9,8 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0030Tests { + private const string diagnosticId = "AZC0030"; + [Fact] public async Task GoodSuffix() { @@ -35,7 +37,7 @@ public static ResponseParameters DeserializeResponseParameters(JsonElement eleme } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } @@ -53,7 +55,7 @@ public static DiskOption DeserializeDiskOption(JsonElement element) } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 28).WithArguments("DiskOption", "Option", "'DiskConfig'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 28).WithArguments("DiskOption", "Option", "'DiskConfig'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } @@ -70,7 +72,7 @@ public class DiskOption } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(6, 22, 6, 32).WithArguments("DiskOption", "Option", "'DiskConfig'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(6, 22, 6, 32).WithArguments("DiskOption", "Option", "'DiskConfig'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } @@ -91,7 +93,7 @@ public static CreationResponses DeserializeCreationResponses(JsonElement element } } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(6, 22, 6, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(6, 22, 6, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs index 1c55ae0982f..1e3391672a8 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0031Tests.cs @@ -9,6 +9,8 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0031Tests { + private const string diagnosticId = "AZC0031"; + [Fact] public async Task ModelWithDefinitionSuffix() { @@ -19,7 +21,7 @@ public partial class AadAuthenticationDefinition { } }"; - var expected = VerifyCS.Diagnostic(DefinitionSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 53).WithArguments("AadAuthenticationDefinition", "Definition"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 26, 4, 53).WithArguments("AadAuthenticationDefinition", "Definition"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs index 299e9acce32..1731ab1975f 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0032Tests.cs @@ -9,6 +9,8 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0032Tests { + private const string diagnosticId = "AZC0032"; + [Fact] public async Task ModelClassWithDataSuffix() { @@ -23,7 +25,7 @@ public static AadAuthenticationData DeserializeAadAuthenticationData(JsonElement } } }"; - var expected = VerifyCS.Diagnostic(DataSuffixAnalyzer.DiagnosticId).WithSpan(4, 26, 4, 47).WithArguments("AadAuthenticationData", "Data"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 26, 4, 47).WithArguments("AadAuthenticationData", "Data"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs index d177712e462..ed6aadc96cf 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0033Tests.cs @@ -10,6 +10,8 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class AZC0033Tests { + private const string diagnosticId = "AZC0033"; + [Fact] public async Task OperationClassIsNotChecked() { @@ -75,8 +77,8 @@ public static DnsArmOperation DeserializeDnsArmOperation(JsonElement element) } }"; DiagnosticResult[] expected = { - VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(4, 18, 4, 30).WithArguments("DnsOperation", "Operation", "DnsData", "DnsInfo"), - VerifyCS.Diagnostic(OperationSuffixAnalyzer.DiagnosticId).WithSpan(11, 18, 11, 33).WithArguments("DnsArmOperation", "Operation", "DnsArmData", "DnsArmInfo") + VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 30).WithArguments("DnsOperation", "Operation", "DnsData", "DnsInfo"), + VerifyCS.Diagnostic(diagnosticId).WithSpan(11, 18, 11, 33).WithArguments("DnsArmOperation", "Operation", "DnsArmData", "DnsArmInfo") }; await VerifyCS.VerifyAnalyzerAsync(test, expected); } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs index eeb46226264..32be2243ac3 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs @@ -9,6 +9,8 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName { public class SuffixAnalyzerBaseTests { + private const string diagnosticId = "AZC0030"; + [Fact] public async Task NonPublicClassIsNotChecked() { @@ -38,7 +40,7 @@ public class MonitorParameter }")] public async Task ClassWithoutSerliaizationMethodsButInModelsNamespaceIsChecked(string test) { - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(3, 14, 3, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(3, 14, 3, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } @@ -55,7 +57,7 @@ public static MonitorParameter DeserializeMonitorParameter(JsonElement element) return null; } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(4, 14, 4, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 14, 4, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } @@ -77,7 +79,7 @@ void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) return; } }"; - var expected = VerifyCS.Diagnostic(GeneralSuffixAnalyzer.DiagnosticId).WithSpan(9, 14, 9, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); + var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 14, 9, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'"); await VerifyCS.VerifyAnalyzerAsync(test, expected); } 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 3871e5fee57..25016015a76 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs @@ -136,6 +136,42 @@ internal class Descriptors "The Azure.Core internal shared source types {0} should not be used outside of the Azure.Core library.", DiagnosticCategory.Usage, DiagnosticSeverity.Warning, true); + + public static readonly DiagnosticDescriptor AZC0030 = new DiagnosticDescriptor( + nameof(AZC0030), + "Improper model name suffix", + "Model name '{0}' ends with '{1}'. Suggest to rename it to {2} or any other appropriate name.", + DiagnosticCategory.Naming, + DiagnosticSeverity.Warning, + true, + "Suffix is not recommended. Consider to remove or modify it."); + + public static readonly DiagnosticDescriptor AZC0031 = new DiagnosticDescriptor( + nameof(AZC0031), + "Improper model name suffix", + "Model name '{0}' ends with '{1}'. Suggest to rename it to an appropriate name.", + DiagnosticCategory.Naming, + DiagnosticSeverity.Warning, + true, + "Suffix is not recommended. Consider to remove or modify it."); + + public static readonly DiagnosticDescriptor AZC0032 = new DiagnosticDescriptor( + nameof(AZC0032), + "Improper model name suffix", + "Model name '{0}' ends with '{1}'. Suggest to rename it to an appropriate name.", + DiagnosticCategory.Naming, + DiagnosticSeverity.Warning, + true, + "Suffix is not recommended. Consider to remove or modify it."); + + public static readonly DiagnosticDescriptor AZC0033 = new DiagnosticDescriptor( + nameof(AZC0033), + "Improper model name suffix", + "Model name '{0}' ends with '{1}'. Suggest to rename it to '{2}' or '{3}', if an appropriate name could not be found.", + DiagnosticCategory.Naming, + DiagnosticSeverity.Warning, + true, + "Suffix is not recommended. Consider to remove or modify it."); #endregion #region General diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs index 86755a9c0b4..339fb0d9f91 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DataSuffixAnalyzer.cs @@ -16,15 +16,9 @@ namespace Azure.ClientSdk.Analyzers.ModelName [DiagnosticAnalyzer(LanguageNames.CSharp)] public class DataSuffixAnalyzer : SuffixAnalyzerBase { - public const string DiagnosticId = nameof(AZC0032); - - private static readonly DiagnosticDescriptor AZC0032 = new DiagnosticDescriptor(DiagnosticId, Title, - GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: Description); - private static readonly string[] dataSuffix = new string[] { "Data" }; - public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0032); } } + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(Descriptors.AZC0032); } } // unless the model derives from ResourceData/TrackedResourceData protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure.ResourceManager.Models", "ResourceData") || @@ -35,7 +29,7 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { var name = typeSymbol.Name; - return Diagnostic.Create(AZC0032, context.Symbol.Locations[0], + return Diagnostic.Create(Descriptors.AZC0032, context.Symbol.Locations[0], new Dictionary { { "SuggestedName", name.Substring(0, name.Length - suffix.Length) } }.ToImmutableDictionary(), name, suffix); } } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs index f93a5cc2b22..45062ab471f 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs @@ -17,15 +17,9 @@ namespace Azure.ClientSdk.Analyzers.ModelName [DiagnosticAnalyzer(LanguageNames.CSharp)] public class DefinitionSuffixAnalyzer : SuffixAnalyzerBase { - public const string DiagnosticId = nameof(AZC0031); - - private static readonly DiagnosticDescriptor AZC0031 = new DiagnosticDescriptor(DiagnosticId, Title, - GeneralRenamingMessageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: Description); - private static readonly string[] definitionSuffix = new string[] { "Definition" }; - public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0031); } } + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(Descriptors.AZC0031); } } // unless the type is a Resource or after removing the suffix it's another type protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) @@ -47,7 +41,7 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context) { var name = typeSymbol.Name; - return Diagnostic.Create(AZC0031, context.Symbol.Locations[0], + return Diagnostic.Create(Descriptors.AZC0031, context.Symbol.Locations[0], new Dictionary { { "SuggestedName", name.Substring(0, name.Length - suffix.Length) } }.ToImmutableDictionary(), name, suffix); } } 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 bda36c4c7f9..ade8cc58d01 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 @@ -15,20 +15,12 @@ namespace Azure.ClientSdk.Analyzers.ModelName [DiagnosticAnalyzer(LanguageNames.CSharp)] public class GeneralSuffixAnalyzer : SuffixAnalyzerBase { - public const string DiagnosticId = nameof(AZC0030); - - private static readonly string messageFormat = "Model name '{0}' ends with '{1}'. Suggest to rename it to {2} or any other appropriate name."; - private static readonly ImmutableHashSet reservedNames = ImmutableHashSet.Create("ErrorResponse"); - private static readonly DiagnosticDescriptor AZC0030 = new DiagnosticDescriptor(DiagnosticId, Title, - messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: Description); - // Avoid to use suffixes "Request(s)", "Parameter(s)", "Option(s)", "Response(s)", "Collection" private static readonly string[] generalSuffixes = new string[] { "Request", "Requests", "Response", "Responses", "Parameter", "Parameters", "Option", "Options", "Collection"}; - public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0030); } } + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(Descriptors.AZC0030); } } protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => reservedNames.Contains(symbol.Name); @@ -37,7 +29,7 @@ protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string { var name = typeSymbol.Name; var suggestedName = GetSuggestedName(name, suffix); - return Diagnostic.Create(AZC0030, context.Symbol.Locations[0], + return Diagnostic.Create(Descriptors.AZC0030, context.Symbol.Locations[0], new Dictionary { { "SuggestedName", suggestedName } }.ToImmutableDictionary(), name, suffix, suggestedName); } diff --git a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs index b230d969d4d..fecdd64be68 100644 --- a/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs +++ b/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/OperationSuffixAnalyzer.cs @@ -14,17 +14,9 @@ namespace Azure.ClientSdk.Analyzers.ModelName [DiagnosticAnalyzer(LanguageNames.CSharp)] public class OperationSuffixAnalyzer : SuffixAnalyzerBase { - public const string DiagnosticId = nameof(AZC0033); - - private static readonly string messageFormat = "Model name '{0}' ends with '{1}'. Suggest to rename it to '{2}' or '{3}', if an appropriate name could not be found."; - - private static readonly DiagnosticDescriptor AZC0033 = new DiagnosticDescriptor(DiagnosticId, Title, - messageFormat, DiagnosticCategory.Naming, DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: Description); - private static readonly string[] operationSuffix = new string[] { "Operation" }; - public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(AZC0033); } } + public override ImmutableArray SupportedDiagnostics { get { return ImmutableArray.Create(Descriptors.AZC0033); } } // Unless the model derivew from Operation protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => IsTypeOf(symbol, "Azure", "Operation"); @@ -34,7 +26,7 @@ protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string { var name = typeSymbol.Name; var nameWithoutSuffix = name.Substring(0, name.Length - suffix.Length); - return Diagnostic.Create(AZC0033, context.Symbol.Locations[0], + return Diagnostic.Create(Descriptors.AZC0033, context.Symbol.Locations[0], name, suffix, $"{nameWithoutSuffix}Data", $"{nameWithoutSuffix}Info"); } }