Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a false positive for RSpec/ExpectActual with rspec-rails #1764

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

naveg
Copy link
Contributor

@naveg naveg commented Jan 4, 2024

rspec-rails comes with routing matchers that use literals in the expect().

Fixes #759


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Updated documentation.
  • Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

If you have created a new cop:

  • Added the new cop to config/default.yml.
  • The cop is configured as Enabled: pending in config/default.yml.
  • The cop is configured as Enabled: true in .rubocop.yml.
  • The cop documents examples of good and bad code.
  • The tests assert both that bad code is reported and that good code is not reported.
  • Set VersionAdded: "<<next>>" in default/config.yml.

If you have modified an existing cop's configuration options:

  • Set VersionChanged: "<<next>>" in config/default.yml.

@naveg naveg requested a review from a team as a code owner January 4, 2024 23:49
@naveg naveg force-pushed the naveg/expect-actual-rails-routing branch 2 times, most recently from 01ac15d to a2e2460 Compare January 4, 2024 23:57
Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thank you!

lib/rubocop/cop/rspec/expect_actual.rb Outdated Show resolved Hide resolved
@@ -50,6 +50,7 @@ class ExpectActual < Base
regexp
].freeze

SKIPPED_MATCHERS = %i[route_to be_routable].freeze
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are planning to extract the rspec-rails specific parts from rubocop-rspec, I don’t think we should hardcode these matchers names. Can we either have (the non-existing) rubocop-rspec-rails inject them, or maybe just add a SkippedMatchers to the configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we wait to see how the rails extraction takes shape before doing that? It's true that this is rails-specific, but it's also highly unlikely to matter, since we'd only miss adding offenses for non-rails matchers that happened to be named "route_to" or "be_routable".

@naveg naveg force-pushed the naveg/expect-actual-rails-routing branch 2 times, most recently from c53c48e to eaf04e6 Compare January 5, 2024 20:56
@naveg naveg requested a review from bquorning January 6, 2024 18:11
CHANGELOG.md Outdated Show resolved Hide resolved
@naveg naveg force-pushed the naveg/expect-actual-rails-routing branch from eaf04e6 to d81260f Compare January 9, 2024 18:29
rspec-rails comes with routing matchers that use literals in the
`expect()`

Fixes rubocop#759
@naveg naveg force-pushed the naveg/expect-actual-rails-routing branch from d81260f to 30eef5c Compare January 9, 2024 18:30
@pirj pirj merged commit 5a23de1 into rubocop:master Jan 9, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RSpec/ExpectActual false positive against routing specs (when using a negated matcher)
3 participants