CodeFixProvider that instantiates collections

Another piece of Roslyn-based code, this time a CodeFixProvider.

I'm looking for feedback on Roslyn-specific code:

• Am I using the SemanticModel correctly?

• Are there any codeflows I have overlooked that might cause issues?

• Am I violating any best practices I'm unaware of?

• etc

Analyzer

```
namespace DiagnosticTools.Collections.NotInitializedException
{
[DiagnosticAnalyzer]
[ExportDiagnosticAnalyzer(DiagnosticId, LanguageNames.CSharp)]
internal class NotInstantiatedCollectionAnalyzer : ISyntaxNodeAnalyzer
{
internal const string DiagnosticId = "CollectionNotInstantiated";
private static string Description = Resources.NotInstantiatedCollection_Description;
private static string MessageFormat = Resources.NotInstantiatedCollection_MessageFormat;
private static string Category = Resources.Category_Collections;
private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, Description, MessageFormat, Category, DiagnosticSeverity.Warning);

public ImmutableArray SupportedDiagnostics
{
get
{
return ImmutableArray.Create(Rule);
}
}

public ImmutableArray SyntaxKindsOfInterest
{
get
{
return ImmutableArray.Create(SyntaxKind.VariableDeclaration);
}
}

public void AnalyzeNode(SyntaxNode node, SemanticModel semanticModel, Action addDiagnostic, CancellationToken cancellationToken)
{
var declaration = node as VariableDeclarationSyntax;
var typeOfVariable = semanticModel.GetTypeInfo(declaration.Type).Type;
var isCollection = typeOfVariable.IsCollection();
var isInitialized = declaration.Variables[0].Initializer != null;

if (isCollection && !isInitialized)
{
addDiagnostic(Diagnostic.Create(Rule, declaration.GetLocation(), declara

I know nothing about rosyln, so there would be someone better to check that, however...
Analyzer

• You create Description, MessageFormat & Category as static strings but only use them in the constructor of Rule. I don't think that the extra fields provide anything other than a minor increase in readability.

• Inconsistent use of readonly. If you keep the fields I suggested removing above, make them all readonly. This will give clear intention that they should not be used (I guess that you want them as const's ideally)

• The result of SyntaxKindsOfInterest is deterministic, therefore, it can be made a readonly static (A Micro-Optimisation I know). I can't remember the language semantics off hand, but the same could probably happen for SupportedDiagnostics

• There is no parameter checking inside of AnalyzeNode

• Inside AnalyzeNode, you don't check for a null reference after the type cast of declaration before attempting to use it

• You assume that the Variables collection is going to always have 1 or more elements in the collection

CodeFixProvider

• The result of GetFixableDiagnosticIds is deterministic and can be kept as a static readonly field

• No parameter checking inside of GetFixesAsync

• No checking for a null reference after the type case of statement

• You assume that the Variables collection is going to always have 1 or more elements in the collection

• You could have something similar to EventArgs.Empty for the result of if (newType.Type.IsAbstract)

Misc

• RosylnUtils performs no parameter checking

• GetTypedArguments should use return String.Empty not ""

I relise that this is probably just test code, hope this helps in either case. These are just the things that I would do.

StackExchange Code Review Q#48170, answer score: 3