Implement ContainExactly and MatchArray cops with autocorrect

[Darhazer]

I have rebased #245 on master, added changelog and cleaned up a bit the code (e.g. applied my own feedback from the original pull request). I'm happy with this version of the cops.

So if anyone else approves, we can release this.

[pirj]

My only concern is that there should be a cop to detect offense in the following code:

eq([single])

and propose to replace with contains_exactly(single). I'm not sure what the name for it should be so that it does not clash with the one in this pull request.

What is stopping you from updating and merging this one?

[Darhazer]

Rebased and updated

[mvz]

First, a question. Given the following code:

is_expected_to match_array %w(tremble in fear foolish mortals)

Will the MatchArray cop suggest the following change? It seems worse to me :slightly_smiling_face:.

is_expected_to contain_exactly('tremble', 'in', 'fear', 'foolish', 'mortals')

I'm also not sure about the split between when to use match_array and when to use contain_exactly. It seems a bit arbitrary and I would prefer to be able to enforce using only one or the other.

Finally, since the function of these two cops is so similar, why not merge them into one? This would allow easy configuration to enforce either one or the other matcher, or the currently proposed mixed style.

[Darhazer]

Thank you for your feedback @mvz I agree that %w() should not be flagged and will update the test cases accordingly.

Regarding merging the cops into one, it makes sense to me. I don't have strong opinion on this though.

[mvz]

@Darhazer any news on this?

[pirj]

@Darhazer the cops seem to be one hair from the finish line. Are you up to finish it?

[ydah]

@Darhazer I'm eagerly awaiting the addition of this cop. Please let me know if there is anything I can do to help.

[Darhazer]

@ydah do you want to update the cops and handle the %w() case? I don't have much time lately, but I promise to review with high priority ;)

[pirj]

@Darhazer Done

[pirj]

@Darhazer Thanks for the review. Please have another look.

[Darhazer]

Oh I can't approve, but thanks to previous approvals I can merge 😄

[pirj]

Thanks everyone involved 🎉

[Darhazer]

Took more than 6 years since the original PR, but finally ... @faucct you can consider yourself contributor to rubocop-rspec 😅

[faucct]

Thanks, though looking at this cop now I think of me being too petty about this code style. :)

[ydah]

@pirj @Darhazer Thanks so much for bringing this forward! I'm sorry I said I would help but I haven't done anything 😅.

[stoivo]

Why is contain_exactly preferred over match_array?

[bquorning]

contain_exactly and match_array are two different ways of accomplishing the same thing: Making an assertion about which elements are in an array, while ignoring the order of the elements.

It is not that one of the methods are better than the other. But when calling match_array with an array that is created on the fly (e.g. expect(foo).to match_array(['x', 'y'])) you might as well call contain_exactly and not create that array: expect(foo).to contain_exactly('x', 'y').

In the same vein, when calling contain_exactly with a splatted array as argument (e.g. expect(foo).to contain_exactly(*bar)) you might as well call match_array and not splat the array: expect(foo).to match_array(bar).

I hope that answers your question.

[stoivo]

Yes, it's a good answer. My take away is that match_array(['x', 'y']))created a little more garbage and some unneeded characters.

You could also argue that we should so contain_exactly(*[1,2,3]) since this is what match_array method does http://rspec.info/documentation/3.12/rspec-expectations/RSpec/Matchers.html#match_array-instance_method