Skip to content

Commit

Permalink
fix(.net analyzer): ignore property bag with Options suffix
Browse files Browse the repository at this point in the history
- change `GenerlSuffixAnalyzer` to ignore model classes with `Options` suffix if there is no serialization
- add test cases

fix Azure#7247
  • Loading branch information
Mingzhe Huang (from Dev Box) committed Nov 7, 2023
1 parent 41c4ad1 commit fbc61ed
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using System.Threading.Tasks;
using Xunit;

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

// 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";

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

[Fact]
public async Task WithDeserializationMethod()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager
{
public class ResponseOptions
{
public static ResponseOptions DeserializeResponseOptions(JsonElement element)
{
return null;
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(4, 18, 4, 33).WithArguments("ResponseOptions", "Options", "'ResponseConfig'");
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

[Fact]
public async Task WithSerializationMethod()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager.Models
{
internal interface IUtf8JsonSerializable
{
void Write(Utf8JsonWriter writer);
};
public class DiskOptions: IUtf8JsonSerializable
{
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) {}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(9, 18, 9, 29).WithArguments("DiskOptions", "Options", "'DiskConfig'");
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}

[Fact]
public async Task WithPublicSerialization()
{
var test = @"using System.Text.Json;
namespace Azure.ResourceManager.Models
{
public class ModelReaderWriterOptions
{
public string Foo;
}
public interface IJsonModel<out T>
{
void Write(Utf8JsonWriter writer, ModelReaderWriterOptions options);
};
namespace SubTest
{
public class DiskOptions: IJsonModel<DiskOptions>
{
void IJsonModel<DiskOptions>.Write(Utf8JsonWriter writer, ModelReaderWriterOptions options) {}
}
}
}";
var expected = VerifyCS.Diagnostic(diagnosticId).WithSpan(16, 22, 16, 33).WithArguments("DiskOptions", "Options", "'DiskConfig'");
await VerifyCS.VerifyAnalyzerAsync(test, expected);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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

Expand All @@ -18,11 +18,41 @@ 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"};
private static readonly string[] generalSuffixes = new string[] { "Request", "Requests", "Response", "Responses", "Parameter", "Parameters", "Option", "Options", "Collection" };

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

protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context) => reservedNames.Contains(symbol.Name);
protected override bool ShouldSkip(INamedTypeSymbol symbol, SymbolAnalysisContext context)
{
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;

// if it has public serialization: IJsonModel<T>
if (symbol.Interfaces.Any(i => i is { Name: "IJsonModel", IsGenericType: true, TypeArguments.Length: 1 } &&
i.TypeArguments[0] == symbol))
return true;

return false;
}

protected override string[] SuffixesToCatch => generalSuffixes;
protected override Diagnostic GetDiagnostic(INamedTypeSymbol typeSymbol, string suffix, SymbolAnalysisContext context)
Expand Down

0 comments on commit fbc61ed

Please sign in to comment.