Consider testing all `Diagnostic`s that may be returned by certain diagnostic_id on CodeFix testing
ledjon-behluli opened this issue · 0 comments
Issue
Right now the protected
methods CodeFixTestFixture.TestCodeFix
accept: markupCode
, expected
, and the diagnosticId
to locate the Diagnostic
s occurring in markupCode
based on diagnosticId
(see: GetDiagnostic(Document document, string diagnosticId, IDiagnosticLocator locator)
.
The underlying method GetReportedDiagnostics
returns correctly all the Diagnostic
s for a given diagnosticId
& IDiagnosticLocator
but only the first available Diagnostic
is being tested against, this results in failing tests
Example
Suppose we write an analyzer that checks on the ctor
arguments of MyClass
. We check that any types of System.Type
passed as an argument is NOT a valid arg. It means the analyzer will report a Diagnostic
foreach of the unsupported args.
Case 1: One offending argument type (works)
[Fact]
public void A()
{
string markupCode = @"
class MyClass { }
MyClass c = new MyClass(1, [|typeof(int)|], 'a');";
string expected = @"
class MyClass { }
MyClass c = new MyClass(1, 'a');";
TestCodeFix(markupCode, expected, "ID");
}
Case 2: Two or more offending argument types - Using one "locator" (doesnt work)
[Fact]
public void A()
{
string markupCode = @"
class MyClass { }
MyClass c = new MyClass(1, [|typeof(int), 'a', typeof(string)|]);"; // See how [||] wraps both parts of expected diagnostics
string expected = @"
class MyClass { }
MyClass c = new MyClass(1, 'a');
";
TestCodeFix(markupCode, expected, "ID"); // Result is: MyClass c = new MyClass(1, 'a', [|typeof(string)|]);
}
Case 3: Two or more offending argument types - Using two or more "locator"'s [||] (doesnt work)
[Fact]
public void A()
{
string markupCode = @"
class MyClass { }
MyClass c = new MyClass(1, [|typeof(int)|], 'a', [|typeof(string)|]); // See how two [||] wrap, each their expected diagnostic
";
string expected = @"
class MyClass { }
MyClass c = new MyClass(1, 'a');
";
TestCodeFix(markupCode, expected, "ID"); // Result is: MyClass c = new MyClass(1, 'a', [|typeof(string)|]);
}
Proposal
As I see it, this can be fixed in two ways:
- Keep one
IDiagnosticLocator
but test all the reported diagnostics - Need to update documentation to tell that if you expect multiple occurrence of aDiagnostic
one should wrap the whole code (including parts you don't expect, if they are in between parts you do expect diagnostic's). - Support multiple
IDiagnosticLocator
a.k.a multiple [||] - Need to update documentation to tell that if you expect multiple occurrence of aDiagnostic
one should wrap each offending part within [||]
I think both would be fine, but if asked I think the 2nd solution would be cleaner and removes potential ambiguity as a user of the library.