Skip to content

Commit

Permalink
temp
Browse files Browse the repository at this point in the history
  • Loading branch information
@jorgerangel-msft
jorgerangel-msft committed Mar 5, 2025
1 parent 17a743b commit a9926ed
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 42 deletions.
129 changes: 125 additions & 4 deletions ...Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030OptionsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
using Xunit;

using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<
Azure.ClientSdk.Analyzers.ModelName.GeneralSuffixAnalyzer>;
Azure.ClientSdk.Analyzers.ModelName.OptionsSuffixAnalyzer>;

// Test cases for `Options` suffix. We allow `Options` suffix for property bags.
// So need to skip property bags which do not have any serialization codes.
namespace Azure.ClientSdk.Analyzers.Tests.ModelName
{
public class AZC0030OptionsSuffixTests
{
private const string diagnosticId = "AZC0030";
private const string DiagnosticId = "AZC0030";

[Fact]
public async Task WithoutAnySerialization()
Expand Down Expand Up @@ -38,7 +38,8 @@ public static ResponseOptions DeserializeResponseOptions(JsonElement element)
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 33).WithArguments("ResponseOptions", "Options", "'ResponseConfig'");
var expectedMessage = GetDiagnosticMessage("ResponseOptions");
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(4, 18, 4, 33).WithArguments("ResponseOptions", "Options", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand All @@ -58,8 +59,128 @@ public class DiskOptions: IUtf8JsonSerializable
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", "'DiskConfig'");
var expectedMessage = GetDiagnosticMessage("DiskOptions");
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

[Fact]
public async Task WithNestedNamespaceAndSerialization()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager.Models
{
namespace SubTest
{
internal interface IUtf8JsonSerializable
{
void Write(Utf8JsonWriter writer);
};
public class DiskOptions: IUtf8JsonSerializable
{
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
}
}
}";
var expectedMessage = GetDiagnosticMessage("DiskOptions");
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(11, 22, 11, 33).WithArguments("DiskOptions", "Options", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

// This test validates that the analyzer is triggered when the model is directly in the Azure.ResourceManager namespace
// and has serialization methods.
[Fact]
public async Task WithExactNamespaceAndSerialization()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager
{
internal interface IUtf8JsonSerializable
{
void Write(Utf8JsonWriter writer);
};
public class DiskOptions: IUtf8JsonSerializable
{
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
}
}";
var expectedMessage = GetDiagnosticMessage("DiskOptions");
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

// This test validates that the analyzer is not triggered when the model is directly in the Azure.ResourceManager namespace
// and has no serialization methods.
[Fact]
public async Task WithExactNamespaceAndNoSerialization()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager
{
public class DiskOptions
{
}
}";
await VerifyCS.VerifyAnalyzerAsync(test);
}

// This test validates that the analyzer does not trigger on a class that is not in the Azure.ResourceManager.Models namespace
// despite it having serialization.
[Fact]
public async Task NonManagementModelWithSerializationMethod()
{
var test = @"using System.Text.Json;
namespace Azure.Foo.Models
{
internal interface IUtf8JsonSerializable
{
void Write(Utf8JsonWriter writer);
};
public class DiskOptions: IUtf8JsonSerializable
{
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
}
}";
await VerifyCS.VerifyAnalyzerAsync(test);
}

[Fact]
public async Task NonManagementModelWithDeserializationMethod()
{
var test = @"using System.Text.Json;
namespace Azure.Foo.Models
{
public class ResponseOptions
{
public static ResponseOptions DeserializeResponseOptions(JsonElement element)
{
return null;
}
}
}";
await VerifyCS.VerifyAnalyzerAsync(test);
}

[Fact]
public async Task NonManagementModelWithNoMethods()
{
var test = @"using System.Text.Json;
namespace Azure.Foo.Models
{
public class DiskOptions
{
}
}";
await VerifyCS.VerifyAnalyzerAsync(test);
}

private static string GetDiagnosticMessage(string typeName)
{
return $"The `Options` suffix is reserved for input models described by https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-parameters. " +
$"Please rename `{typeName}` according to our guidelines at https://azure.github.io/azure-sdk/general_design.html#model-types for output or roundtrip models.";
}
}
}
15 changes: 9 additions & 6 deletions ...otnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/AZC0030Tests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Threading.Tasks;
using Azure.ClientSdk.Analyzers.ModelName;
using Xunit;

using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<
Expand All @@ -9,7 +8,7 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName
{
public class AZC0030Tests
{
private const string diagnosticId = "AZC0030";
private const string DiagnosticId = "AZC0030";

[Fact]
public async Task GoodSuffix()
Expand Down Expand Up @@ -37,7 +36,8 @@ public static ResponseParameters DeserializeResponseParameters(JsonElement eleme
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 36).WithArguments("ResponseParameters", "Parameters", "'ResponseContent' or 'ResponsePatch'");
var expectedMessage = $"Suggest to rename it to 'ResponseContent' or 'ResponsePatch' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(4, 18, 4, 36).WithArguments("ResponseParameters", "Parameters", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand All @@ -55,7 +55,8 @@ public static DiskOption DeserializeDiskOption(JsonElement element)
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 28).WithArguments("DiskOption", "Option", "'DiskConfig'");
var expectedMessage = $"Suggest to rename it to 'DiskConfig' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(4, 18, 4, 28).WithArguments("DiskOption", "Option", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand All @@ -72,7 +73,8 @@ public class DiskOption
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(6, 22, 6, 32).WithArguments("DiskOption", "Option", "'DiskConfig'");
var expectedMessage = $"Suggest to rename it to 'DiskConfig' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(6, 22, 6, 32).WithArguments("DiskOption", "Option", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand All @@ -93,7 +95,8 @@ public static CreationResponses DeserializeCreationResponses(JsonElement element
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(6, 22, 6, 39).WithArguments("CreationResponses", "Responses", "'CreationResults'");
var expectedMessage = $"Suggest to rename it to 'CreationResults' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(6, 22, 6, 39).WithArguments("CreationResponses", "Responses", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}
}
Expand Down
16 changes: 9 additions & 7 deletions ....ClientSdk.Analyzers/Azure.ClientSdk.Analyzers.Tests/ModelName/SuffixAnalyzerBaseTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Threading.Tasks;
using Azure.ClientSdk.Analyzers.ModelName;
using Xunit;

using VerifyCS = Azure.ClientSdk.Analyzers.Tests.AzureAnalyzerVerifier<
Expand All @@ -9,7 +8,7 @@ namespace Azure.ClientSdk.Analyzers.Tests.ModelName
{
public class SuffixAnalyzerBaseTests
{
private const string diagnosticId = "AZC0030";
private const string DiagnosticId = "AZC0030";

[Fact]
public async Task NonPublicClassIsNotChecked()
Expand Down Expand Up @@ -38,9 +37,10 @@ public class MonitorParameter
public class MonitorParameter
{
}")]
public async Task ClassWithoutSerliaizationMethodsButInModelsNamespaceIsChecked(string test)
public async Task ClassWithoutSerializationMethodsButInModelsNamespaceIsChecked(string test)
{
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(3, 14, 3, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'");
var expectedMessage = $"Suggest to rename it to 'MonitorContent' or 'MonitorPatch' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(3, 14, 3, 30).WithArguments("MonitorParameter", "Parameter", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand All @@ -57,12 +57,13 @@ public static MonitorParameter DeserializeMonitorParameter(JsonElement element)
return null;
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 14, 4, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'");
var expectedMessage = $"Suggest to rename it to 'MonitorContent' or 'MonitorPatch' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(4, 14, 4, 30).WithArguments("MonitorParameter", "Parameter", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

[Fact]
public async Task ClassWithDeserliazationMethodIsChecked()
public async Task ClassWithDeserializationMethodIsChecked()
{
var test = @"using System.Text.Json;
namespace Azure.NotModels;
Expand All @@ -79,7 +80,8 @@ void IUtf8JsonSerializable.Write(Utf8JsonWriter writer)
return;
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 14, 9, 30).WithArguments("MonitorParameter", "Parameter", "'MonitorContent' or 'MonitorPatch'");
var expectedMessage = $"Suggest to rename it to 'MonitorContent' or 'MonitorPatch' or any other appropriate name.";
var expected = VerifyCS.Diagnostic(DiagnosticId).WithSpan(9, 14, 9, 30).WithArguments("MonitorParameter", "Parameter", expectedMessage);
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

Expand Down
2 changes: 1 addition & 1 deletion src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/Descriptors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ internal class Descriptors
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.",
"Model name '{0}' ends with '{1}'. {2}",
DiagnosticCategory.Naming,
DiagnosticSeverity.Warning,
true,
Expand Down
29 changes: 5 additions & 24 deletions ...et/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/GeneralSuffixAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

Expand All @@ -17,8 +16,8 @@ public class GeneralSuffixAnalyzer : SuffixAnalyzerBase
{
private static readonly ImmutableHashSet<string> reservedNames = ImmutableHashSet.Create("ErrorResponse");

// 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" };
// Avoid to use suffixes "Request(s)", "Parameter(s)", "Option", "Response(s)", "Collection"
private static readonly string[] generalSuffixes = new string[] { "Request", "Requests", "Response", "Responses", "Parameter", "Parameters", "Option", "Collection" };

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Descriptors.AZC0030); } }

Expand All @@ -27,25 +26,6 @@ protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContex
if (reservedNames.Contains(symbol.Name))
return true;

// skip property bag classes which have `Options` suffix and don't have serialization
if (symbol.Name.EndsWith("Options") && !SupportSerialization(symbol))
return true;

return false;
}

private bool SupportSerialization(INamedTypeSymbol symbol)
{
// if it has serialization method: `IUtf8JsonSerializable.Write`, e.g. ": IUtf8JsonSerializable"
if (symbol.Interfaces.Any(i => i.Name is "IUtf8JsonSerializable"))
return true;

// if it has deserialization method: static <T> Deserialize<T>(JsonElement element)
if (symbol.GetMembers($"Deserialize{symbol.Name}").Any(m => m is IMethodSymbol methodSymbol &&
methodSymbol is { IsStatic: true, ReturnType: INamedTypeSymbol symbol, Parameters.Length: 1 } &&
methodSymbol.Parameters[0].Type.Name is "JsonElement"))
return true;

return false;
}

Expand All @@ -54,8 +34,9 @@ protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string
{
var name = typeSymbol.Name;
var suggestedName = GetSuggestedName(name, suffix);
var additionalMessage = $"Suggest to rename it to {suggestedName} or any other appropriate name.";
return Diagnostic.Create(Descriptors.AZC0030, context.Symbol.Locations[0],
new Dictionary<string, string> { { "SuggestedName", suggestedName } }.ToImmutableDictionary(), name, suffix, suggestedName);
new Dictionary<string, string> { { "SuggestedName", suggestedName } }.ToImmutableDictionary(), name, suffix, additionalMessage);
}

private string GetSuggestedName(string originalName, string suffix)
Expand All @@ -65,7 +46,7 @@ private string GetSuggestedName(string originalName, string suffix)
{
"Request" or "Requests" => $"'{nameWithoutSuffix}Content'",
"Parameter" or "Parameters" => $"'{nameWithoutSuffix}Content' or '{nameWithoutSuffix}Patch'",
"Option" or "Options" => $"'{nameWithoutSuffix}Config'",
"Option" => $"'{nameWithoutSuffix}Config'",
"Response" => $"'{nameWithoutSuffix}Result'",
"Responses" => $"'{nameWithoutSuffix}Results'",
"Collection" => $"'{nameWithoutSuffix}Group' or '{nameWithoutSuffix}List'",
Expand Down
Loading

0 comments on commit a9926ed

Please sign in to comment.