-
-
Notifications
You must be signed in to change notification settings - Fork 277
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: Fix false negative for RSpec/Dialect
when specified Capybara-specific methods
#1952
base: master
Are you sure you want to change the base?
Fix: Fix false negative for RSpec/Dialect
when specified Capybara-specific methods
#1952
Conversation
…pecific methods fixed rubocop#1951 `RuboCop::Cop::RSpec::Dialect#rspec_method?` detects if node's second child is an RSpec block using `RuboCop::RSpec::Language::ALL.all`. However, `RuboCop::Cop::RSpec::Dialect#rspec_method?` returned nil because the following Capybara-specific methods were not set in config/default.yml. - given - given! - background I remove the line that checks if it’s a `rspec_method?` and add a `inside_example_group?` check. cf. rubocop#1951 (comment) Due to the above, it wouldn't filter out are calls with an explicit receiver where the receiver is not RSpec, eg `foo.describe`. so the following examples were removed. - allows calling methods named xxx in examples
Should I also change the following document as it no longer checks for rubocop-rspec/lib/rubocop/cop/rspec/dialect.rb Lines 8 to 23 in fa5da43
|
Maybe. I find it hard to understand what the “based” means in “dialect can be based on the following RSpec methods”, though. Can you think of a simpler description? |
RSpec.context 'context' do | ||
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `describe` over `context`. | ||
it 'tests common context invocations' do | ||
expect(request.context).to be_empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those ones were here to ensure that context
with an explicit receiver is not detected. Can you keep it in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply 🙏
Those ones were here to ensure that
context
with an explicit receiver is not detected. Can you keep it in some way?
In fa5da43 the check using rspec_method?
has been removed, so explicit receivers are also detected. To avoid detection, I can't think of any ideas other than to do the following.
def on_send(node)
return unless inside_example_group?(node)
+ return unless rspec?(node.children.first)
return unless preferred_methods[node.method_name]
Do you have any good ideas?
@sanfrecce-osaka ping ;) |
How about the following? - # Enforces custom RSpec dialects.
+ # Enforces custom RSpec dialects based on your config.
#
- # A dialect can be based on the following RSpec methods:
+ # `Dialects` are the following examples:
#
# - describe, context, feature, example_group
# - xdescribe, xcontext, xfeature
# - fdescribe, fcontext, ffeature
# - shared_examples, shared_examples_for, shared_context
# - it, specify, example, scenario, its
# - fit, fspecify, fexample, fscenario, focus
# - xit, xspecify, xexample, xscenario, skip
# - pending
# - prepend_before, before, append_before,
# - around
# - prepend_after, after, append_after
# - let, let!
# - subject, subject!
# - expect, is_expected, expect_any_instance_of
#
# By default all of the RSpec methods and aliases are allowed. By setting
# a config like:
#
# RSpec/Dialect:
# PreferredMethods:
# context: describe |
fixed #1951
RuboCop::Cop::RSpec::Dialect#rspec_method?
detects if node's second child is an RSpec block usingRuboCop::RSpec::Language::ALL.all
. However,RuboCop::Cop::RSpec::Dialect#rspec_method?
returned nil because the following Capybara-specific methods were not set in config/default.yml.I remove the line that checks if it’s a
rspec_method?
and add ainside_example_group?
check.cf. #1951 (comment)
Due to the above, it wouldn't filter out are calls with an explicit receiver where the receiver is not RSpec, eg
foo.describe
. so the following examples were removed.Before submitting the PR make sure the following are checked:
master
(if not - rebase it).CHANGELOG.md
if the new code introduces user-observable changes.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:
config/default.yml
.Enabled: pending
inconfig/default.yml
.Enabled: true
in.rubocop.yml
.VersionAdded: "<<next>>"
indefault/config.yml
.If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"
inconfig/default.yml
.