Reorganize analyzer properties

[tjoubert]

• Got rid of the generic AnalyzerProperties class
• Implemented a custom properties class for each specific analyzer type

[DiscoPYF]

Good idea to refactor into separate classes for analyzer properties 👍

I left a few minor comments. One thing I want to discuss before merging:

With this new approach, analyzer properties will not be available from Analyzer objects returned by API methods (e.g. PostAnalyzerAsync, GetAnalyzerAsync or GetAllAnalyzersAsync). Only an empty AnalyzerPropertiesBase object will be available. The serializer has no way at the moment to know what derived type to create when deserializing.

Is it expected?

A workaround is to instruct the serializer to construct a derived type based on the Analyzer.Type property, e.g. using a converter. However any serializer specific logic should be carefully reviewed because the library allows overriding the serializer, so someone may use System.Text.Json or any other.

Perhaps we should think a bit more about this approach? For now, I propose to:

• Close this pull request
• Open a new pull request to fix the bug by simply adding the properties on the existing AnalyzerProperties object.
• Open a new issue to refactor the properties like you did, which will give some more time to think about. We can link this pull request on the issue.

What do you think?

Also, isn't it a breaking change as we are changing the way analyzer properties are provided? Probably all the more reasons to hold off until the next major release. I've added the "breaking change" label.

[tjoubert]

Good idea to refactor into separate classes for analyzer properties 👍

I left a few minor comments. One thing I want to discuss before merging:

With this new approach, analyzer properties will not be available from Analyzer objects returned by API methods (e.g. PostAnalyzerAsync, GetAnalyzerAsync or GetAllAnalyzersAsync). Only an empty AnalyzerPropertiesBase object will be available. The serializer has no way at the moment to know what derived type to create when deserializing.

Is it expected?

A workaround is to instruct the serializer to construct a derived type based on the Analyzer.Type property, e.g. using a converter. However any serializer specific logic should be carefully reviewed because the library allows overriding the serializer, so someone may use System.Text.Json or any other.

Perhaps we should think a bit more about this approach? For now, I propose to:

* Close this pull request

* Open a new pull request to fix the bug by simply adding the properties on the existing `AnalyzerProperties` object.

* Open a new issue to refactor the properties like you did, which will give some more time to think about. We can link this pull request on the issue.

What do you think?

Also, isn't it a breaking change as we are changing the way analyzer properties are provided? Probably all the more reasons to hold off until the next major release. I've added the "breaking change" label.

@DiscoPYF , you are right. In order to merge this PR correctly, we would need serializer-specific logic to deserialize the Analyzer.Properties based on what is defined in Analyzer.Type. This is also a breaking change (thanks for the tag). We will leave this PR open and put it on hold. I am going to open a new PR that will leave AnalyzerProperties.cs as it was and simply add all the new properties to it.

[tjoubert]

@DiscoPYF, I have revived this PR for your review (our discussions). Here's the update:

• I've updated the PR from the master branch.
• I've brought AnalyzerProperties back in as it currently stands in the master branch.
• I've defined a new class AnalyzerDefinition that will be used to POST analyzers with its Properties property set to one of the custom analyzer properties objects.
• The Analyzer class now inherits from AnalyzerDefinition with its Properties property set to a AnalyzerProperties object hiding the same property in the base class.
  Please share your thoughts and suggestions. Thanks.