NUnit2044 false positive with `Has`

[chm-tm]

The following code works as intended, but is banned with NUnit2044.

var list = new List<int>();
Task.Run(async () => {
    await Task.Delay(500);
    list.Add(0);
});
Assert.That(list, Has.Count.EqualTo(1).After(1000, 1));

Maybe there are other use cases where a delegate condition isn't necessary.

To stay with the analyzers, simply use

Assert.That(() => list, Has.Count.EqualTo(1).After(1000, 1));

for now.

[manfred-brands]

The NUnit2044 is there to detect errors where the left-hand-side is a constant and won't change over time.

Assert.That(list.Count, Is.EqualTo(1).After(1000, 1));

Even though in the example the list instance is constant, its contents can change. I'm just wondering what the best way to fix this is.

Besides Has.Count, I would say most other Has members qualify, except for Has.Attribute which wouldn't change ever. But it wouldn't change when using a delegate either. Other allowed constraints would be is Is.Empty, Is.Unique, Does.Contain

An alternative is to only trigger the rule if the left-hand-side is a value type. The problem with that is that we will not trigger enough, if there are properties that return different classes (aka strings) when called every time:

var messageSupplier = new MessageSupplier();
Assert.That(messageSupplier.Message, Does.Contain("9").After(100, 10));

[manfred-brands]

One suggestion is to not fix this at all.

Yes, NUnit works with the first version, but the suggested version with the delegate makes it clearer that the first argument is re-evaluated. Isn't it the job of the analyzer to make better/more-understandable code?

[chm-tm]

I think that NUnit2044 happens most of the time since .After clearly expresses that the expected value has to be evaluated, but one forgets to supply the first argument in a technical manner that allows NUnit to do so.

That said, I can imagine that some fellows preferred to always use a delegate notation for enhanced readability. They do this in the same mood as they wouldn't omit an Is.True argument. In my opinion, the analyzer shouldn't get in the way here. At most, it could be configurable for teams which want it that way.

[manfred-brands]

From #433

Given that I've found just one single occurrence triggering the false positive, I prefer adding the () => there and continue to have reference type values covered by NUnit2044.