Export mock.argumentMatcher

[connor4312]

This allows the creation of predicate-generating functions, which can make code a lot cleaner. To give a very simple example, this is not possible with the current non-exported argumentMatcher:

type Counter struct{ x int }

func counterArg(expectedX int) mock.ArgumentMatcher {
    return mock.MatchedBy(func(c *Counter) bool {
        return c.x == expectedX
    })
}

mock.On("Method", counterArg(1))
mock.On("Method", counterArg(2))
mock.On("Method", counterArg(3))

Also fixes the outstanding lint issues from #230

[ernesto-jimenez]

@connor4312 sorry for the late reply. It has been a crazy summer :)

I would rather avoid exporting an struct that cannot be properly initialised.

Instead, it would be better to define an ArgumentMatcher interface.

The tricky part is that we must ensure the interface is never satisfied by a struct from some other package, otherwise we might have weird collisions and we would not be able to add anything to the interface without breaking backwards compatibility.

The idea would be to export

type ArgumentMatcher interface {
    Matches(interface{}) bool
    String()
    // This method ensures only `mock` package can implement ArgmentMatchers
    private()
}

Then, we include:

func (argumentMatcher) private() { }

MatchedBy would return ArgumentMatcher, and line 570 will assert for ArgumentMatcher instead of argumentMatcher.

With these changes, you could define functions like yours and the package would be much safer to use.

What do you think?

Do you want to make those changes?

[connor4312]

Hm, I see the issue, but to me it seems like that might be a bit of an abuse of the type system. Perhaps a struct or function type which encapsulates a predicate, rather than using an interface.

// An ArgumentMatcher takes a function argument and returns whether it
// matches the Call's desired signature
type ArgumentMatcher func(v interface{}) bool

To be a bit more ambitious, have On convert its args ...interface{} into ArgumentMatchers (for those which are not already matchers) when the expectation is initially created. This isolates the loosely-typed code into a smaller surface area, which is always a win in my book.

Additionally, some existing code like AnythingOfTypeArgument and the magic Anything string could be expressed via these generic matchers.

[ernesto-jimenez]

My point is that I would rather lock-in this feature so only mock can create custom argument matchers for the time being.

Adding a private method to an exported interface is not an abuse of the type system, it is a safeguard to be able to export an interface only your package can satisfy so you are free to modify it without breaking backwards compatibility. testing.TB is an example of this pattern from the standard library.

Thinking more about this, I would probably do:

type ArgumentMatcher interface {
    ArgumentMatches(interface{}) error
    // This method ensures only `mock` package can implement ArgmentMatchers
    private()
}

And refactor the current argumentMatcher to fit the new interface. Which we can do without breaking backwards compatibility due the small surface.

With that change, not only we can definitely AnythingOfTypeArgument and Anything to return private structs that implement ArgumentMatcher.

[dolmen]

The argumentMatcher type should never have been exposed in the signature function. Instead it should have been hidden behind an interface (that could be called ArgumentMatcher, but would have all its methods private).

For now, the right way to rewrite the use case of the description of this issue is to use the any interface:

type Counter struct{ x int }

func counterArg(expectedX int) any {
    return mock.MatchedBy(func(c *Counter) bool {
        return c.x == expectedX
    })
}

mock.On("Method", counterArg(1))
mock.On("Method", counterArg(2))
mock.On("Method", counterArg(3))