Get rid of MSTEST0026 code suggestion (Avoid conditional access in assertions)

[Evangelink]

Part of the point of adding conditional access operators is to help make the code more readable through brevity. But suggestion MSTEST0026 encourages code bloat by adding unnecessary Assert.IsNotNull() statements. Assert.AreEqual already perfectly handles null values and using a conditional access operator here can combine a !null assertion and an equality assertion together in a readable, compact way.

Here is the example from the MSTEST0026 docs:

Assert.AreEqual("Contoso", company?.Name); // MSTEST0026

// Fixed code
Assert.IsNotNull(company);
Assert.AreEqual("Contoso", company.Name);

But the original statement is both technically correct and readable and the ‘fixed’ code is overly verbose.

Originally reported in https://developercommunity.visualstudio.com/t/Get-rid-of-MSTEST0026-code-suggestion-A/10819575

[nohwnd]

Will there still be warning when the expected side uses ? or produces null?

This would be important in situation where we don't hardcode the data, but instead get them from some api, which can produce null. Then we get pushed by nullability analyzers into doing:

Assert.AreEqual(expected?.Name, actual?.Name);

So null == null if the api that gives us data breaks.

[UniSnake]

If you want to assert that your expected value is not null then yes you should do so explicitly. But a warning isn't warranted because in some cases your tests are unit tests and have a hard-coded range of expected values that will not change based on externalities, in which case a warning would not make sense. And in other cases a null expected value may itself be expected.

[Evangelink]

Dumping a couple of notes/info:

• currently the analyzer is enabled by default as Info only.
• we have had a couple of feedback that this is a nice analyzer and that customers like the fact that it was "enforcing" explicit checks.

Given the mixed feedback, I would be ok with disabling it by default on the default profile but keeping it on on the "recommended" profile. Note the profiles don't yet exist but this is a feature we are discussing internally.