Prefer be_nil over be nil

[nijikon]

I would like to introduce a check like this.

# Bad
expect(foo).to be nil
# Good
expect(foo).to be_nil

My question is where should it be implemented. Should I add it in the Predicate Matcher?

[nijikon]

/cc @bquorning @pocke

[bquorning]

/cc @Darhazer

[bquorning]

I think this is a duplicate of #176 and/or #244 – do you agree?

[nijikon]

I would say yes, I’m open to doing work on fixing those two as well. Just need your guidance where that work should be done. We could also solve the other issue that I saw which is some dead code in predicates matcher.

[nijikon]

Refs #578

[nijikon]

Ping

[bquorning]

Alright, I am back 👋

I would say the be(nil) -> be_nil check does not belong in PredicateMatcher. That cop is for changing e.g. expect(foo.bar?).to be(true) to expect(foo).to be_bar. (And that cop has plenty of code already, so I was hoping to remove/split some of it rather than adding to it 😄)

So I think this would be a brand new cop. The examples in #244 don’t really apply here, but please note this comment (#244 (comment)):

Personally, I like using the strictest matcher that works. Therefore I prefer be(nil), be(true), be(false). I think it would be inconsistent to prefer be_nil instead of be(true) if you are going to enforce identity comparison for some it seems reasonable to enforce it consistently.

So perhaps your cop should be configurable, to let people prefer enforcing either be_nil or be(nil). What do you think?

---

You are welcome to also work on the “unused code in predicate_matcher.rb“ issue #578, but let’s discuss that → over there.

[gardwired]

what about the fact that we have be true and be false. seems more consistent to still be able to say be nil without the underscore no? we no longer have be_true/false so enforcing the now seemingly one-off be_nil feels odd to me. my .0000btc.

[pirj]

we no longer have be_true/false

Can you please elaborate on that?

[bquorning]

In RSpec v3, the old be_true and be_false expectations were renamed to be_truthy and be_falsey to match with how they were actually implemented (e.g. expect(nil).to be_false was a passing spec in RSpec 2).

Maybe we can get correctly implemented be_true and be_false back in RSpec 4?

[pirj]

be_nil would work even without BeNil, as a regular predicate matcher for a nil? predicate defined on Kernel. be_nil is a convenience matcher that provides more readable failure messages, e.g. expected: nil vs expected :not_nil.nil? to be true.

true?/false? are not even defined on TrueClass/FalseClass. Would using special be_true/be_false matchers improve their failure messages compared to be(true)/be(false)?

[marcandre]

Can probably be closed now that #1239 has been merged.

FWIW, I also find be nil to be preferable to be_nil