`PropertyConstraint` Causes Unexpected Behavior Because the Property's Value Is `object` Instead of the Property’s Actual Type

[LeQuackers]

Overview:

As the title says, the PropertyConstraint can cause unexpected behavior in some cases because Actual<T>(T) is called with an object type argument instead of the property’s actual type:

nunit/src/NUnitFramework/framework/Constraints/PropertyConstraint.cs

Lines 70 to 72 in 8a94a49

	_propValue = property.GetValue(actual, null);
	var baseResult = BaseConstraint.ApplyTo(_propValue);
	return new PropertyConstraintResult(this, baseResult);

Example:

One example of unexpected behavior is an interaction with DefaultConstraint, in which the assert fails even though the property is default:

[Test]
public void Example()
{
    var actual = new Foo
    {
        Bar = 0
    };

    Assert.Multiple(() =>
    {
        // passes
        Assert.That(actual.Bar, Is.Default);

        // fails
        Assert.That(actual, Has.Property(nameof(Foo.Bar)).Default);
    });
}

Assert.That(actual, Has.Property(nameof(Foo.Bar)).Default)
Expected: property Bar default
But was:  0

Proposed Solution:

Because IConstraint.Actual<T>(T) is a generic method, it's a bit awkward; we'd have to set the generic parameter and call it with reflection at runtime.

I'll link a pull request for it in a bit.

[LeQuackers]

The associated pull request: #4999

[manfred-brands]

@LeQuackers Thanks for reporting, I can confirm the issue.
It is however more an issue with the DefaultConstraint than the PropertyConstraint.

What do we expect to happen here:

object instance = 0;
Assert.That(instance. Is.Default);

We could say that instance is declared object and only null should match default.

What about:

int? instance = 0;
Assert.That(instance, Is.Default);

The default value for int? would be null, the default value for int is 0.

On the other hand we could check the actual underlying type and check if the value match what ever is default for that type.

The latter comes into play when augmenting the condition:

var instance = 0;
Assert.That(instance, Is.InstanceOf<int>().And.Default);

As object is used a lot in code that is not 100% generic, including NUnit, I think we should ignore that for TActual.

[LeQuackers]

@manfred-brands That's fair. Originally I was planning on modifying DefaultConstraint, but when I did it caused an existing unit test fail (a boxed value type test I think?). I wasn't sure on the stance for altering existing behavior, so I figured patching PropertyConstraint was the safe bet.

Anyway, glad it's getting some attention; thanks for looking into it.