[C# analyzers] Update AZC0030 so that it doesn't prevent clients from having serializable models with the Options suffix

[annelo-msft]

It is a common pattern in data plane clients to have model types with the Options suffix.

This issue tracks the work to understand and document the intended scope of analyzer warning AZC0030, so it can be scoped down to address only intended cases.

This warning was introduced in #6916 and modified in #7250. @jsquire has suggested that it was intended to prevent subtypes of ClientOptions from being serializable. We need to confirm this is the intended scope so we don't change an analyzer that is needed for other cases.

If this is the correct intended scope, the analyzer should be updated to validate specifically that subtypes of ClientOptions do not implement the IPersistableModel<T> interface, and not prevent any type with the Options suffix from being serializable.

@archerzz, @m-nash, @christothes, would it be possible to help us understand the intended scope of this warning?

[archerzz]

@annelo-msft This rule was from .NET Mgmt SDK Review Guide:

The rule regarding Options seems have been changed since I implemented the analyzer. The latest version is opposite to what Jesse suggested.

Anyway, the key point is that when I wrote those rules, they were for mgmt SDK only. See the original repo: https://github.com/archerzz/Azure.MgmtSdk.Analyzers

Then when we added them into current analyzer package, we thought those are generic and we should them enable for all SDKs.

To resolve this issue, I think we could:

• remove Options suffix check from AZC0030
• create a Azure.MgmtSdk.Analzyers package, put that rule and any further mgmt specific rules into that package
• in our build script, enable Azure.MgmtSdk.Analzyers only for mgmt SDKs

cc @ArthurMa1978

[ArthurMa1978]

This convention rule has been confirmed by @KrzysztofCwalina. If we need alternative suggestions, we'll require input from KC.

[annelo-msft]

Super helpful context, many thanks for sharing it, @archerzz and @ArthurMa1978!

I worked with @m-nash and @KrzysztofCwalina to clarify the guidance around the options parameter pattern in our Azure SDK C# guidelines. Existing guidance is here:

• https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-params-complex
• https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-params-options

I will update the guidelines and I expect that to be completed in a few days -- I can CC you on the PR if you are interested.

We decided that with regard to the analyzer, if it is possible to scope the rule to management-plane only, that would be ideal. @m-nash has shared that the analyzer has been very helpful in the development of management plane libraries, and we of course don't want to remove something that is bringing value here! On the data-plane side, the rule is creating some friction for TypeSpec-first partners, and we are seeking to make that as streamlined a process as possible.

@archerzz, is having a separate package the easiest way to scope this analyzer to management-plane only? Are there other approaches that could keep the analyzer in a single package, for example, by checking whether the Options-suffixed type is in a namespace that begins with Azure.ResourceManager before raising the warning? If the latter would catch all management plane cases, it might be faster to implement and deploy ... but let me know if you can think of more efficient approaches here.

[KrzysztofCwalina]

This convention rule has been confirmed by @KrzysztofCwalina. If we need alternative suggestions, we'll require input from KC.

I don't remember this, though my memory might be failing me :-). But, in .NET, the term "config" is associated with configuration systems, i.e. the ability to change apps without recompiling them.

[archerzz]

Super helpful context, many thanks for sharing it, @archerzz and @ArthurMa1978!

I worked with @m-nash and @KrzysztofCwalina to clarify the guidance around the options parameter pattern in our Azure SDK C# guidelines. Existing guidance is here:

• azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-params-complex
• azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-params-options

I will update the guidelines and I expect that to be completed in a few days -- I can CC you on the PR if you are interested.

We decided that with regard to the analyzer, if it is possible to scope the rule to management-plane only, that would be ideal. @m-nash has shared that the analyzer has been very helpful in the development of management plane libraries, and we of course don't want to remove something that is bringing value here! On the data-plane side, the rule is creating some friction for TypeSpec-first partners, and we are seeking to make that as streamlined a process as possible.

@archerzz, is having a separate package the easiest way to scope this analyzer to management-plane only? Are there other approaches that could keep the analyzer in a single package, for example, by checking whether the Options-suffixed type is in a namespace that begins with Azure.ResourceManager before raising the warning? If the latter would catch all management plane cases, it might be faster to implement and deploy ... but let me know if you can think of more efficient approaches here.

@annelo-msft Of course. We've done similar thing before, like:

azure-sdk-tools/src/dotnet/Azure.ClientSdk.Analyzers/Azure.ClientSdk.Analyzers/ModelName/DefinitionSuffixAnalyzer.cs

Lines 27 to 28 in 59d869d

	if (IsTypeOf(symbol, "Azure.ResourceManager", "ArmResource"))
	return true;