proposal: Add a new matcher which return true if it matches one of given value

[MakDon]

Requested feature

A In([]T) matcher which returns true if it matches one of the given value.

For Example:

m.
    EXPECT().
    CheckIsFruit(gomock.In([]string{"Apple", "Banana", "Cherry"})).
    Return(True)

Why the feature is needed

With this matcher, we could cover the scenes where the args could be one of the options and make the EXPECT() assertion more reusable.

Proposed solution

When Matches() is called, it generates Eq Matchers from the given slice, and check eqMatcher.Matches() one by one.
I've tried to make a PR. See #582

[codyoss]

Thanks for the feature request. I think this sounds pretty similar to the feature implemented here: 0cdccf5

I am not sure it this is needed as well. What do you think?

[MakDon]

Thanks for the feature request. I think this sounds pretty similar to the feature implemented here: 0cdccf5

I am not sure it this is needed as well. What do you think?

InAnyOrder matches slice with given slice, but in matches object with given slice.
These are two different matcher designed for different purpose.

[MakDon]

Suppose we have an interface:

type foo interface{
IsFruit(string)bool
AreFavorite([]string)bool
}

And here's part of the test code:

favoriteFruit := []string{"Apple", "Banana", "Cherry"}
m.EXPECT().
    IsFruit(gomock.In(favoriteFruit)).
    Return(True)

m.EXPECT().
    AreFavorite(gomock.InAnyOrder(favoriteFruit)).
    Return(True)

[codyoss]

Thank you for the clarification. I will leave this issue open for a bit to see what others think. In the meantime you can always implement this matcher for yourself.

[MakDon]

Thank you for the clarification. I will leave this issue open for a bit to see what others think. In the meantime you can always implement this matcher for yourself.

Thanks codyoss. I've implemented this in my private lib. Maybe it would help others so I created a PR for this feature.

[MakDon]

hi @codyoss , it has been a month since the last update. Is it time to push it forward, or just leave this issue open, or close the issue cause no further update?

[codyoss]

I will leave this issue open a little longer, but generally before adding a new matcher I like to see others in the community also asking for the feature. As this has not gotten much traction as of right now I am leaning towards declining this feature request. Especially since this can always be implemented in user code with the public interface.

[favonia]

I found a use of this feature in my project. Having it built-in sounds nice.

I have two suggestions to make In potentially more useful: make it (1) compositional and (1) variadic. It means

In(Len(3),Len(4),Eq(1),"hi")

can match something of length 3 or 4, or the number 1 or the string "hi". This requires In to treat Matcher specially and only use Eq for wrapping other types of arguments, as what EXPECT() is doing right now. Using a variadic interface instead of taking a slice directly could save some syntax. However, the interface proposed by @MakDon is already good enough for me.

[MakDon]

I found a use of this feature in my project. Having it built-in sounds nice.

I have two suggestions to make In potentially more useful: make it (1) compositional and (1) variadic. It means

In(Len(3),Len(4),Eq(1),"hi")

can match something of length 3 or 4, or the number 1 or the string "hi". This requires In to treat Matcher specially and only use Eq for wrapping other types of arguments, as what EXPECT() is doing right now. Using a variadic interface instead of taking a slice directly could save some syntax. However, the interface proposed by @MakDon is already good enough for me.

It seems a little bit too complicated for common users. How about keep it simple?

[favonia]

It seems a little bit too complicated for common users. How about keep it simple?

@MakDon Common users can also benefit from the variadic syntax, as you will see below. What I showed in the previous comment was the full power of a compositional design, not the common usage. Besides, there's already gomock.All in the current API that creates a conjunctive matcher, and the version I proposed is just the flip of it---the disjunctive matcher. I do not understand the maintainer @codyoss's hesitation on your proposal, but I suppose following the style of the current API could increase the likelihood of acceptance. In my opinion, this is a natural extension to the current interface---surely one would consider logical disjunction after having logical conjunction (gomock.All), logical negation (gomock.Not), and logical truth (gomock.Any)?

In any case, here are common usages of the new Some I proposed: (I changed In to Some to better match gomock.All, but I am all for a better name as I am not a native English speaker.)

gomock.Some("Apple", "Banana", "Cherry") // matching "Apple", "Banana", and "Cherry"

If you want to pass a slice of items, then it could be

gomock.Some(favoriteFruits...) // matching any item in favoriteFruits

Being compositional only means that, in addition to the above simple usages, you can also write

gomock.Some(Len(1), Len(3)) // matching something if Len(1) or Len(3) matches it

to match things of length 1 or 3.

---

In terms of implementation, it is not more complicated than #582, if not actually simpler. The main difference is that, instead of keeping a list of items, I keep a list of matchers. Here's the full code from one of my fun projects: Again, this is just the flip of the current implementation of gomock.All but augmented with the auto-conversion from an item x to the matcher Eq(x). (As a bonus, I don't need to use the reflection API due to the variadic design.)

type someMatcher struct {
	matchers []Matcher
}

func (sm someMatcher) Matches(x interface{}) bool {
	for _, m := range sm.matchers {
		if m.Matches(x) {
			return true
		}
	}
	return false
}

func (sm someMatcher) String() string {
	ss := make([]string, 0, len(sm.matchers))
	for _, matcher := range sm.matchers {
		ss = append(ss, matcher.String())
	}
	return strings.Join(ss, " | ")
}

func Some(xs ...interface{}) Matcher {
	ms := make([]Matcher, 0, len(xs))
	for _, x := range xs {
		if m, ok := x.(Matcher); ok {
			ms = append(ms, m)
		} else {
			ms = append(ms, Eq(x))
		}
	}
	return someMatcher{ms}
}

I can create a PR once @codyoss green-lights it.

[MakDon]

@favonia I get your point now and it looks good and worth discussing.

[favonia]

The PR #673 implements this mechanism in a different way. Personally I prefer having a new Some matcher that will return false on empty lists. In any case, such a PR shows that others are asking for this feature.