nunit/nunit.analyzers

New diagnostic: Either Assert.Throws or Assert.ThrowsAsync is swallowing the exception thrown

Bartleby2718 opened this issue · 1 comments

Consider the following example:

List<int> numbers = [ 1, 2 ];
foreach (var number in numbers)
{
	// exception message: Collection was modified; enumeration operation may not execute.
	Assert.Throws<InvalidOperationException>(() => numbers.Add(number));
}

Assert.Throws<TException> returns a non-null TException if the exception was thrown and null otherwise.

To those who are not familiar with NUnit, the above code looks alright, but as a matter of fact, the correct way to assert is:

	var exception = Assert.Throws<InvalidOperationException>(() => numbers.Add(number));
	Assert.That(exception, Is.Not.Null);

or in one line:

	Assert.That(Assert.Throws<InvalidOperationException>(() => numbers.Add(number)), Is.Not.Null);

Therefore, I think we could add a new diagnostic that warns the user if Assert.Throws<TException> is not assigned to a variable or used as an argument. What do you think?

Welp, I had misunderstood how Assert.Throws<TException> worked. Closing!