Detect more Classic Assert #791
Conversation
There was a problem hiding this comment.
Great work @manfred-brands. I've added some minor comments/questions, but I'm happy to merge this when you are.
| Assert.That(diagnosticIds, Contains.Item(AnalyzerIdentifiers.PositiveUsage), | ||
| $"{AnalyzerIdentifiers.PositiveUsage} is missing."); | ||
| Assert.That(diagnosticIds, Contains.Item(AnalyzerIdentifiers.NegativeUsage), | ||
| $"{AnalyzerIdentifiers.NegativeUsage} is missing."); | ||
| }); |
There was a problem hiding this comment.
I think we should also add
Assert.That(diagnosticIds, Contains.Item(AnalyzerIdentifiers.IsAssignableFromUsage),
$"{AnalyzerIdentifiers.IsAssignableFromUsage} is missing.");
Assert.That(diagnosticIds, Contains.Item(AnalyzerIdentifiers.IsNotAssignableFromUsage),
$"{AnalyzerIdentifiers.IsNotAssignableFromUsage} is missing.");
| public void Test() | ||
| { | ||
| ↓ClassicAssert.Negative(2); | ||
| } | ||
| }"); |
There was a problem hiding this comment.
Should we also add a method for IsAssignableFromUsage and IsNotAssignableFromUsage
| } | ||
|
|
||
| [Test] | ||
| public void CodeFixMaintainsReasonableTriviaWithNewLineClosingParen() |
There was a problem hiding this comment.
I diffed IsAssignableFromClassicModelAssertUsageCodeFixTests.cs and IsNotAssignableFromClassicModelAssertUsageCodeFixTests.cs and in the other file this test is parametrized with [Values] bool hasMessage. Is there a reason that this one different?
| } | ||
|
|
||
| [Test] | ||
| public void CodeFixForGenericMaintainsReasonableTriviaWithNewLineClosingParen([Values] bool hasMessage) |
There was a problem hiding this comment.
A similar test does not exists in IsNotAssignableFromClassicModelAssertUsageCodeFixTests.cs. Is there a reason for this?
| @@ -109,6 +109,10 @@ Rules which improve assertions in the test code. | |||
| | [NUnit2048](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2048.md) | Consider using Assert.That(...) instead of StringAssert(...) | :white_check_mark: | :warning: | :white_check_mark: | | |||
| | [NUnit2049](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2049.md) | Consider using Assert.That(...) instead of CollectionAssert(...) | :white_check_mark: | :warning: | :white_check_mark: | | |||
| | [NUnit2050](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2050.md) | NUnit 4 no longer supports string.Format specification | :white_check_mark: | :exclamation: | :white_check_mark: | | |||
| | [NUnit2051](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2051.md) | Consider using Assert.That(expr, Is.Positive) instead of ClassicAssert.Positive(expr) | :white_check_mark: | :information_source: | :white_check_mark: | | |||
There was a problem hiding this comment.
I think we also add the documentation files to the solution file
Fixes #789 Assert.Positive, Assert.Negative
Fixes #790 Assert.IsAssignableFrom, Assert.IsNotAssignableFrom