Skip to content

What to do with IConstraint.Apply<TActual>(ref TActual) #4930

Description

@manfred-brands

The IConstraint interface defines an overload where one can pass a actual value by reference.
Nothing in NUnit tests seems to call this as far as I can tell.

There is one overload in DelayedConstraint but that drops the ref prefix and would pass actual by value to the base constraint.

The only place where this might have been intended for is a ref struct that modifies itself, like in:

private struct PretendList
{
    public int PollCount { get; private set; }

    public int Count
    {
        get
        {
            PollCount++;
            return 0;
        }
    }
}

Mutable structs are a bad concept unless one can guarantee it is always passed by ref.

The DelayedConstraint calls BaseConstraint.Apply(actual) without ref, it therefore uses copy semantics and the PollCount will always be 0.
Even changing the code to pass ref actual doesn't fix it as the PropertyConstraint called by Has.Count looses the ref.
I wouldn't even know of passing ref to PropertyInfo.GetValue even works.

I suggest to remove the Apply<TActual>(ref TActual) overloads in the next major version as it is technically breaking.
But it is currently not working for any of our constraints as they all behave like Apply<TActual>(TActual) and any mutations are lost.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Breaking ChangeIndicates resolving the issue will be a breaking change and will require it to be in a major versionis:refactor

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions