rubocop-rspec icon indicating copy to clipboard operation
rubocop-rspec copied to clipboard

RSpec/PredicateMatcher: more careful approach?

Open zverok opened this issue 8 years ago • 7 comments

Imagine the code:

expect(parser.handles?('string')).to be_truthy

RSpec/PredicateMatcher will suggest to change this into:

expect(parser).to be_handles('string')

...which reads pretty unnatural, to say the least. Maybe there could be some check to prevent this suggestion in some of cases? (It is hard to clearly specify them, though)

zverok avatar Sep 21 '17 22:09 zverok

The ExampleWording cop (I think that is the name) already does some checks like this. I have no time to get to this stuff but if you're interested you could start looking at that code

backus avatar Sep 22 '17 04:09 backus

Perhaps an option to allow explicit matchers if there's an argument, and set it to true by default.

# Good
expect(Thing.exists?(id)).to be_falsey

# Bad
expect(Thing).to exist(id)

I've found predicate matchers to be hard to understand, it adds another layer between the test and the code being tested, but once I figured out the pattern the compact nature is worth it. But with an argument it's much more difficult to understand and it defeats the "reads like English" intent.

schwern avatar Jul 16 '20 20:07 schwern

What might make this better is a with or by method to take the arguments.

expect(Thing).to exist.with(id)
expect(obj).to be_handled.by(handler)

I don't know if it's worth the effort, it makes predicate matchers even more divorced from the code, but at least it reads better.

schwern avatar Jul 16 '20 20:07 schwern

There's a long thread in a quite recent #919 with a lot of thoughts on predicate matchers. Unrelated to this question, but linking anyway since this relates to the same cop.

pirj avatar Jul 16 '20 20:07 pirj

I'm not convinced enought that the latter is a proper semantic replacement for the former.

expect(parser.handles?('string')).to be_truthy
expect(parser).to be_handled.by('string')

Not to say it's not supported by RSpec itself, and if it will be, it will only happen in RSpec 4. And we don't have support for different RSpec versions (e.g. skip offence for 3.0, but raise and autocorrect for 4.0). With not many changes coming with RSpec 4.0, more likely a major cleanup, I'm not sure it's even worth it.

pirj avatar Jul 16 '20 21:07 pirj