Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Bartleby2718 opened this issue Jun 2, 2024 · 1 comment

Comments

@Bartleby2718
Copy link
Contributor

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
Copy link
Member

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants