NUnit2044 - improvement proposal - consider variables assigned outside of Assert call

[matode]

If you have code like this:

var actualValue = Foo.GetValue();
Assert.That(actualValue, Is.EqualTo(expectedValue).After(Timeout, PollInterval), "Error message");

Changing to

var actualValue = Foo.GetValue();
Assert.That(() => actualValue, Is.EqualTo(expectedValue).After(Timeout, PollInterval), "Error message");

seems to be useless. The variable actualValue will be set only once before the Assert.That call.

It should be e.g.

Assert.That(() => Foo.GetValue(), Is.EqualTo(expectedValue).After(Timeout, PollInterval), "Error message");

Can the analyzer be enhanced, so that it does not accept variables as actualValue param in the Assert call? And can the fixer be adapted accordingly?

[manfred-brands]

I agree that creating a delegate for a variable doesn't make sense. It would make more sense to remove the After modifier if we can't move the assignment inside the Assert.

[matode]

In most cases After is used for some reason. So removing it is not an option.

If moving the assignment inside Assert is too complicated, it can be proposed to move the assignment into some (local) function.

[manfred-brands]

Actually, I have seen use cases where it does make sense:

bool wasCalled = false;
void Handler() => wasCalled = true;
instance.Event += Handler;
Assert.That(() => wasCalled, Is.EqualTo(expectedValue).After(Timeout, PollInterval), "Event wasn't called");

Finding the scope of what could modify the local variable is outside the scope of the analyzer.

I'm inclined to close this issue.