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

[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."

• we have a company-internal analyzer that does this.
• xUnit has a similar analyzers:
  • https://xunit.net/xunit.analyzers/rules/xUnit2003
  • https://xunit.net/xunit.analyzers/rules/xUnit2004
• (though for some tests, it is explicitly wanted due to explicit testing of operator overload; but for those case the warning is just disabled via #pragma or Attribute)

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

[tomhughes]

Yes, the map data is loaded asynchronously so may not appear immediately.

[mmd-osm]

These days the example is taking 2 seconds to load rather than 1 minute or so.