Capybara/SpecificFinders conflicts with Rails/DynamicFindBy

[FunnyHector]

Capybara/SpecificFinders suggests the following change:

# bad
find('#some-id')
find('[visible][id=some-id]')

# good
find_by_id('some-id')
find_by_id('some-id', visible: true)

However, find_by_id is going to be flagged by Rails/DynamicFindBy.

These two cops can't be enabled together. If I had to choose from the two, Rails/DynamicFindBy would be my choice and Capybara/SpecificFinders would be disabled.

---

Expected behavior

Capybara/SpecificFinders could somehow respect Rails/DynamicFindBy.

Actual behavior

They have conflicting opinions on code like these.

RuboCop version

➜  rubocop -V
1.43.0 (using Parser 3.2.0.0, rubocop-ast 1.24.1, running on ruby 3.1.3) [arm64-darwin22]
  - rubocop-performance 1.15.2
  - rubocop-rails 2.14.2
  - rubocop-rspec 2.18.0

[ydah]

Hi, thanks for your feedback! I also recognize the issue.

FYI, rubocop-rails provides a measure to mitigate false positives for this capybara-derived method.

• [Fix #778] Fix a false positive for Rails/DynamicFindBy rubocop-rails#785
• https://docs.rubocop.org/rubocop-rails/cops_rails.html#configurable-attributes-15
• https://github.com/rubocop/rubocop-rails/blob/5425d8ac6ad714bbdc4968fe580704ba31a729f0/config/default.yml#L383-L388

How about doing the following to avoid this?

Rails/DynamicFindBy:
  AllowedMethods:
    - find_by_sql # default by rubocop-rails
    - find_by_token_for # default by rubocop-rails
    - find_by_id

[pirj]

I don't entirely understand, can you please provide an example of an offence raised by DynamicFinders?
It should ignore an implicit receiver and page, right? Also, when multiple arguments are passed.

[FunnyHector]

How about doing the following to avoid this?

@ydah Thanks for your suggestion. I've seen this somewhere, but I don't really want to whitelist those methods for Rails/DynamicFindBy. I think having find_by_id calls on ActiveRecord in production code is worse than not having a capybara derived method call in tests. Currently I've disabled Capybara/SpecificFinders for that reason.

[FunnyHector]

can you please provide an example of an offence raised by DynamicFinders?

@pirj I have this code:

expect(page.find("#id")).to be_checked

Capybara/SpecificFinders auto-corrects this to

expect(page.find_by_id('id')).to be_checked

which is flagged by Rails/DynamicFindBy:

Offenses:

spec/test.rb:9:14: C: [Correctable] Rails/DynamicFindBy: Use find_by instead of dynamic find_by_id.
      expect(page.find_by_id('id')).to be_checked
             ^^^^^^^^^^^^^^^^^^^^^

[ydah]

@FunnyHector Ah. I overlooked one thing, but if you keep rubocop-rails up-to-date, I don't think that problem will occur.
Some of the following changes to Rails/DynamicFindBy have already been incorporated into rubocop-rails, which should be at least 2.17.3 or higher.

• Ignore if number of arguments does not match on Rails/DynamicFindBy rubocop-rails#862
• False positive in Rails/DynamicFindBy rubocop-rails#778

[FunnyHector]

@ydah ah ye my rubocop-rails isn't very up to date. #862 should resolve this issue. Thanks to everyone for this! We can close this issue now I believe.

[johnpash]

Sorry for the hi-jack but I have a quick question.

Having just started using rubocop-capybara I was presented with this error. The exact error message is:
Prefer find_by over find.

But if the only option is to use find_by_id wouldn't it be better if the message read
Prefer find_by_id over find.

[ydah]

Thank you for your feedback! We will open the next Pull Request to improve the message.

• Fix an offense message for Capybara/SpecificFinders #32