nunit / nunit.analyzers

Roslyn analyzers for writing unit tests with NUnit
MIT License
89 stars 32 forks source link

[suggestion] New diagnostic: Split up ternaries for better assertion messages in case of failure #747

Open Bartleby2718 opened 5 months ago

Bartleby2718 commented 5 months ago
I've noticed the following diagnotics: ID description
NUnit2010 Use EqualConstraint for better assertion messages in case of failure
NUnit2011 Use ContainsConstraint for better assertion messages in case of failure
NUnit2012 Use StartsWithConstraint for better assertion messages in case of failure
NUnit2013 Use EndsWithConstraint for better assertion messages in case of failure
NUnit2014 Use SomeItemsConstraint for better assertion messages in case of failure
NUnit2043 Use ComparisonConstraint for better assertion messages in case of failure
NUnit2046 Use CollectionConstraint for better assertion messages in case of failure

We can take a step further and add a new diagnostic: "Split up ternaries for better assertion messages in case of failure"

Consider the following setup (context):

private record Japanese(bool IsRoyal, string FirstName, string? LastName);

Now, suppose there's an assertion like this:

var japanese = new Japanese(IsRoyal: false, FirstName: "Hayao", LastName: "Miyazaki");
Assert.That(japanese.IsRoyal ? japanese.LastName is null : japanese.LastName is not null);

The new diagnostic can convert this to

if (japanese.IsRoyal)
{
    Assert.That(japanese.LastName is null);
}
else
{
    Assert.That(japanese.LastName is not null);
}

Once #746 is complete, NUnit2010 will again suggest

if (japanese.IsRoyal)
{
    Assert.That(japanese.LastName, Is.Null);
}
else
{
    Assert.That(japanese.LastName, Is.Not.Null);
}

which will eventually give you a good error message.

manfred-brands commented 4 months ago

Not sure if we want to go that far rewriting code. The user might like the ternary operator. It could also be written as:

var japanese = new Japanese(IsRoyal: false, FirstName: "Hayao", LastName: "Miyazaki");
Assert.That(japanese.LastName, japanese.IsRoyal ? Is.Null : Is.Not.Null);