AnalyzerAssert.Valid: One analyzer with three diagnostics with the same id

[mikkelbu]

I was trying to make usage of this library in connection with nunit/nunit.analyzers#70, and I ran into some problems with an analyzer that has 3 diagnostics with the same id (see snippet below). I don't know if this is a "major no no" with Roslyn analyzers, but as far as I can tell most of the other analyzers have distinct ids for each diagnostic (and I think that I'm going to change the class to follow that approach). So feedback about this is much appreciated.

TestCaseUsageAnalyzer.cs

public static DiagnosticDescriptor CreateDescriptor(string message) =>
    new DiagnosticDescriptor(AnalyzerIdentifiers.TestCaseUsage, TestCaseUsageAnalyzerConstants.Title,
        message, Categories.Usage, DiagnosticSeverity.Error, true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
    TestCaseUsageAnalyzer.CreateDescriptor(TestCaseUsageAnalyzerConstants.NotEnoughArgumentsMessage),
    TestCaseUsageAnalyzer.CreateDescriptor(TestCaseUsageAnalyzerConstants.ParameterTypeMismatchMessage),
    TestCaseUsageAnalyzer.CreateDescriptor(TestCaseUsageAnalyzerConstants.TooManyArgumentsMessage));

When the code fails from AnalyzerAssert.Valid(Analyzer, testCode) it fails with a general unspecific error message that requires one to examine the code of this project to figure out what the problem is (that the id is used as a key in a dictionary). And I think that this could be improved.

1) Error : NUnit.Analyzers.Tests.TestCaseUsage.TestCaseUsageAnalyzerTests.TestGuRoslynAsserts
System.ArgumentException : An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at Gu.Roslyn.Asserts.CodeFactory.CreateSpecificDiagnosticOptions(IEnumerable`1 enabled, IEnumerable`1 suppressed)
   at Gu.Roslyn.Asserts.CodeFactory.DefaultCompilationOptions(IReadOnlyList`1 analyzers, IEnumerable`1 suppressed)
   at Gu.Roslyn.Asserts.AnalyzerAssert.Valid(DiagnosticAnalyzer analyzer, String code, CSharpCompilationOptions compilationOptions, IEnumerable`1 metadataReferences)
   at NUnit.Analyzers.Tests.TestCaseUsage.TestCaseUsageAnalyzerTests.TestGuRoslynAsserts()

If I also pass in a DiagnosticDescriptor, like AnalyzerAssert.Valid(Analyzer, Analyzer.SupportedDiagnostics.First(), testCode), the I get the following more useful error message.

1) Error : NUnit.Analyzers.Tests.TestCaseUsage.TestCaseUsageAnalyzerTests.TestGuRoslynAsserts
Gu.Roslyn.Asserts.AssertException : Analyzer NUnit.Analyzers.TestCaseUsage.TestCaseUsageAnalyzer has more than one diagnostic with ID NUNIT_7.
   at Gu.Roslyn.Asserts.AnalyzerAssert.VerifyAnalyzerSupportsDiagnostic(DiagnosticAnalyzer analyzer, String expectedId)
   at Gu.Roslyn.Asserts.AnalyzerAssert.Valid(DiagnosticAnalyzer analyzer, DiagnosticDescriptor descriptor, String[] code)
   at NUnit.Analyzers.Tests.TestCaseUsage.TestCaseUsageAnalyzerTests.TestGuRoslynAsserts()

[JohanLarsson]

Agreed the exception message should be improved.

About multiple diagnostics with the same ID:
It is a bit weird, I think, where should the help link point for example?
Reason I added this check is that it finds copy-paste bugs but we can remove it.

[mikkelbu]

I think it is fine to keep the check. I was just hoping that there were some guidelines - or similar - on the Net about issues such as these as I find Roslyn lacking some proper documentation.

I will make all the diagnostics in NUnit.Analyzers unique (and perhaps create a PR for the exception message 😄).