Rubocop-rspec: 'RSpec/PredicateMatcher' explicit style conflicts with Rails 'have_http_status' matcher

[yorkxin]

Background

Our engineering team prefer expect(object.good?).to be(true) over expect(object).to be_good so that it's easier to search for "good?" method name across the whole code base.

Therefore, we are using the following settings for RSpec/PredicateMatcher:

RSpec/PredicateMatcher:
  Strict: true
  EnforcedStyle: explicit

However, this cop also finds the rspec-rails have_http_status matcher should be changed:

Instead of:

expect(response).to have_http_status(:success)

The cop would suggest:

expect(response.has_http_status?(:success)).to be(true)

Which results in method not found of Response#has_http_status?.

Questions

Is this the expected behavior? If so, what settings should I use to allow have_http_status?

Current Workaround

Our current workaround is to use a dirty hack to add have_http_status to built-in matchers list?:

# .rubocoprc.rb
RuboCop::Cop::RSpec::ExplicitHelper::BUILT_IN_MATCHERS =
  RuboCop::Cop::RSpec::ExplicitHelper::BUILT_IN_MATCHERS.dup.push('have_http_status').freeze

# .rubocop.yml 
require: 
- ./rubocoprc.rb

[pirj]

@chitsaou I believe have_http_status clashes with predicate matchers, and technically auto-correction is correct.

I don't see a reason not to include have_http_status to BUILT_IN_MATCHERS by default.
However, it's a special case, different from the other built-in matchers. Ignoring this offence might not be the best choice, as it's still possible to correct the example:

- expect(response).to have_http_status(:success)
+ expect(response.status).to be(:success)

Which approach would you prefer?

[dgollahon]

We should double check for more common have_ matchers in rspec-rails or other common matching tools and make sure we haven't overlooked any others.

It might possibly make sense to add a user-customizable ignore list as well for custom matchers.

Also, I added a note to #721 that we should mark this cop as unsafe.

[pirj]

shoulda-matchers:

Also those:

have_secure_password corresponds to has_secure_password
have_and_belong_to_many tests your has_and_belongs_to_many associations.
have_db_column tests that the table that backs your model has a specific column.
have_db_index tests that the table that backs your model has an index on a specific column.
have_many tests your has_many associations.
have_one tests your has_one associations.
have_readonly_attribute tests usage of the attr_readonly macro.

rspec-rails:

• have_http_status
• have_rendered
• have_enqueued_mail - block matcher

I don't see any common pattern here, probably they should all be excluded.

[yorkxin]

Thanks for all of your replies. It's true that there might be other have_* matchers from libraries, such as these from webmock:

have_been_requested
have_not_been_made
have_requested(method, uri)
have_not_requested(method, uri)

Wouldn't it be great if we can configure ignore list for this cop?

[pirj]

@chitsaou #838 has just been released in 1.37.0 allowing you to configure RSpec/PredicateMatcher to ignore specific matchers:

RSpec/PredicateMatcher:
  AllowedExplicitMatchers:
    - have_http_status
    - have_been_made

Do you think this is sufficient?

On a side note, it would be incredible to add such configuration to corresponding projects (e.g. webmock and shoulda-matchers), however, I'm afraid it will override each other.
@Darhazer do you think it's a reasonable approach to avoid pulling the list of such third-party matchers into default rubocop-rspec configuration? Is it possible with the current RuboCop's configuration system?

[Darhazer]

I don't think it's currently possible.
It would be nice to provide such an API for 3rd party extensions to adjust rubocop config, although it could become rather complicated.

For the time being, we will need to pull the list of matchers from popular gems.

[yorkxin]

@pirj This new option of RSpec/PredicateMatcher cop allows us to adjust the whitelist. Really appreciate your help and the works from @mkrawc. You're awesome 🎉

I agree that managing a list might be hard, but at least we have a start :)

Thank you all again!

[pirj]

reasonable approach to avoid pulling the list of such third-party matchers into default rubocop-rspec configuration

Stumbled across inherit_mode:

However, advanced users can still merge arrays using the inherit_mode setting.

If you guys are interested, we may add something like:

inherit_mode:
  merge:
    - RSpec/PredicateMatcher/AllowedExplicitMatchers

add your settings to a third-party gem and see if it's being pulled.

Would you like to hack on it @chitsaou ?