Skip to content

new Analyzer+CodeFix: recommend against wrong assertion method (e.g. Assert.IsTrue(x != null)) #3315

@LukasGelke

Description

@LukasGelke

Summary

As some assertions may be written in different ways, some of them provide a better experience: better/richer message, clearer intent, better readability/maintainability.

Background and Motivation

  • default Exception-Messages are better
string text = "Mouse";
Assert.IsTrue("House" == text);
//  Assert.IsTrue failed.
Assert.AreEqual("House", text);
// Assert.AreEqual failed. Expected:<House>. Actual:<Mouse>. 

This is also useful when a test is failing in a pipeline, or you get the stack trace only. This way you may not have to dig too deep and "waste" time debugging, just to get to that point to know what text actually is. Or when you get a notification from the pipeline a la test method "VeryImportantTest" failed with message "Assert.IsTrue failed."

Proposed Feature

essentially all permutations of IsFalse/IsTrue with is/is not/==/!=

  • Assert.IsNotNull, e.g.

    • Assert.IsTrue(x != null) => Assert.IsNotNull(x)
    • Assert.IsTrue(null != x) => Assert.IsNotNull(x)
    • Assert.IsFalse(x != null) => Assert.IsNull(x)
    • Assert.IsTrue(x is not null) => Assert.IsNotNull(x)
    • ...
    • Assert.AreNotEqual(null, x) => Assert.IsNotNull(x)
    • ...
    • either side may be null literal
  • Assert.Are[Not]Equal, e.g.

    • Assert.IsTrue(x == y) => Assert.AreEqual(x, y)
    • Assert.IsTrue(x != y) => Assert.AreNotEqual(x, y)
    • Assert.IsFalse(x == y) => Assert.AreNotEqual(x, y)
    • Assert.IsFalse(x != y) => Assert.AreEqual(x, y)
    • ...
    • maybe don't be overly smart, and just let the 'const in expected' be handled by the other analyzer
  • Assert.IsTrue/IsFalse, e.g.

    • Assert.AreEqual(true, b) => Assert.IsTrue(b)
    • only when there is a bool const as one param
    • may be a cascaded result result from Assert.IsTrue(true == b), stupid, but possible; again, just let it do one step at a time, for simplicity?

Edit 1:
maybe also consider:

  • Assert.AreEqual(typeof(string), o.GetType()) => Assert.IsInstanceOfType<string>(o)

also wouldn't matter to me if they are all the same DiagnosticID, since those cases are all "wrong similarly"

Alternative Designs

As mentioned, our analyzer grew over time and is quite complex. And I'm not sure if we have a bug or a missing case. Having such an Analyzer+CodeFix in MsTest directly would also provide a wider audience for finding such "bugs".

AB#2200926

Metadata

Metadata

Assignees

Labels

area/analyzersMSTest.Analyzers Roslyn analyzers and code fixes.state/in-prA PR is open for this issue.
No fields configured for Feature.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions