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

Extract Capybara cops #1519

Merged
merged 3 commits into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Fix a false negative for `RSpec/Pending` when using skipped in metadata is multiline string. ([@ydah])
- Fix a false positive for `RSpec/NoExpectationExample` when using skipped in metadata is multiline string. ([@ydah])
- Fix a false positive for `RSpec/ContextMethod` when multi-line context with `#` at the beginning. ([@ydah])
- Extract Capybara cops to a separate repository. ([@pirj])

## 2.17.0 (2023-01-13)

Expand Down
9 changes: 9 additions & 0 deletions config/obsoletion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,12 @@ changed_parameters:
parameters: IgnoredPatterns
alternative: AllowedPatterns
severity: warning

renamed:
RSpec/Capybara/CurrentPathExpectation: Capybara/CurrentPathExpectation
RSpec/Capybara/MatchStyle: Capybara/MatchStyle
RSpec/Capybara/NegationMatcher: Capybara/NegationMatcher
RSpec/Capybara/SpecificActions: Capybara/SpecificActions
RSpec/Capybara/SpecificFinders: Capybara/SpecificFinders
RSpec/Capybara/SpecificMatcher: Capybara/SpecificMatcher
RSpec/Capybara/VisibilityMatcher: Capybara/VisibilityMatcher
13 changes: 7 additions & 6 deletions docs/modules/ROOT/pages/cops_rspec_capybara.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,13 @@ expect(page).to have_field('foo')

Checks for boolean visibility in Capybara finders.

Capybara lets you find elements that match a certain visibility using
the `:visible` option. `:visible` accepts both boolean and symbols as
values, however using booleans can have unwanted effects. `visible:
false` does not find just invisible elements, but both visible and
invisible elements. For expressiveness and clarity, use one of the
symbol values, `:all`, `:hidden` or `:visible`.
Capybara lets you find elements that match a certain visibility
using the `:visible` option. `:visible` accepts both boolean and
symbols as values, however using booleans can have unwanted
effects. `visible: false` does not find just invisible elements,
but both visible and invisible elements. For expressiveness and
clarity, use one of the # symbol values, `:all`, `:hidden` or
`:visible`.
Read more in
https://www.rubydoc.info/gems/capybara/Capybara%2FNode%2FFinders:all[the documentation].

Expand Down
3 changes: 1 addition & 2 deletions lib/rubocop-rspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'yaml'

require 'rubocop'
require 'rubocop-capybara'

require_relative 'rubocop/rspec'
require_relative 'rubocop/rspec/inject'
Expand All @@ -17,8 +18,6 @@

require_relative 'rubocop/rspec/factory_bot/language'

require_relative 'rubocop/cop/rspec/mixin/capybara_help'
require_relative 'rubocop/cop/rspec/mixin/css_selector'
require_relative 'rubocop/cop/rspec/mixin/final_end_location'
require_relative 'rubocop/cop/rspec/mixin/inside_example_group'
require_relative 'rubocop/cop/rspec/mixin/metadata'
Expand Down
144 changes: 29 additions & 115 deletions lib/rubocop/cop/rspec/capybara/current_path_expectation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,121 +4,35 @@ module RuboCop
module Cop
module RSpec
module Capybara
# Checks that no expectations are set on Capybara's `current_path`.
#
# The
# https://www.rubydoc.info/github/teamcapybara/capybara/master/Capybara/RSpecMatchers#have_current_path-instance_method[`have_current_path` matcher]
# should be used on `page` to set expectations on Capybara's
# current path, since it uses
# https://github.com/teamcapybara/capybara/blob/master/README.md#asynchronous-javascript-ajax-and-friends[Capybara's waiting functionality]
# which ensures that preceding actions (like `click_link`) have
# completed.
#
# This cop does not support autocorrection in some cases.
#
# @example
# # bad
# expect(current_path).to eq('/callback')
#
# # good
# expect(page).to have_current_path('/callback')
#
# # bad (does not support autocorrection)
# expect(page.current_path).to match(variable)
#
# # good
# expect(page).to have_current_path('/callback')
#
class CurrentPathExpectation < ::RuboCop::Cop::Base
extend AutoCorrector

MSG = 'Do not set an RSpec expectation on `current_path` in ' \
'Capybara feature specs - instead, use the ' \
'`have_current_path` matcher on `page`'

RESTRICT_ON_SEND = %i[expect].freeze

# @!method expectation_set_on_current_path(node)
def_node_matcher :expectation_set_on_current_path, <<-PATTERN
(send nil? :expect (send {(send nil? :page) nil?} :current_path))
PATTERN

# Supported matchers: eq(...) / match(/regexp/) / match('regexp')
# @!method as_is_matcher(node)
def_node_matcher :as_is_matcher, <<-PATTERN
(send
#expectation_set_on_current_path ${:to :to_not :not_to}
${(send nil? :eq ...) (send nil? :match (regexp ...))})
PATTERN

# @!method regexp_str_matcher(node)
def_node_matcher :regexp_str_matcher, <<-PATTERN
(send
#expectation_set_on_current_path ${:to :to_not :not_to}
$(send nil? :match (str $_)))
PATTERN

def self.autocorrect_incompatible_with
[Style::TrailingCommaInArguments]
end

def on_send(node)
expectation_set_on_current_path(node) do
add_offense(node.loc.selector) do |corrector|
next unless node.chained?

autocorrect(corrector, node)
end
end
end

private

def autocorrect(corrector, node)
as_is_matcher(node.parent) do |to_sym, matcher_node|
rewrite_expectation(corrector, node, to_sym, matcher_node)
end

regexp_str_matcher(node.parent) do |to_sym, matcher_node, regexp|
rewrite_expectation(corrector, node, to_sym, matcher_node)
convert_regexp_str_to_literal(corrector, matcher_node, regexp)
end
end

def rewrite_expectation(corrector, node, to_symbol, matcher_node)
current_path_node = node.first_argument
corrector.replace(current_path_node, 'page')
corrector.replace(node.parent.loc.selector, 'to')
matcher_method = if to_symbol == :to
'have_current_path'
else
'have_no_current_path'
end
corrector.replace(matcher_node.loc.selector, matcher_method)
add_ignore_query_options(corrector, node)
end

def convert_regexp_str_to_literal(corrector, matcher_node, regexp_str)
str_node = matcher_node.first_argument
regexp_expr = Regexp.new(regexp_str).inspect
corrector.replace(str_node, regexp_expr)
end

# `have_current_path` with no options will include the querystring
# while `page.current_path` does not.
# This ensures the option `ignore_query: true` is added
# except when the expectation is a regexp or string
def add_ignore_query_options(corrector, node)
expectation_node = node.parent.last_argument
expectation_last_child = expectation_node.children.last
return if %i[regexp str].include?(expectation_last_child.type)

corrector.insert_after(
expectation_last_child,
', ignore_query: true'
)
end
end
# @!parse
Copy link
Member Author

Choose a reason for hiding this comment

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

This hack is needed to:

  1. Only enlist the cop once to avoid obsoletion errors (obsoletion warning will be shown)
  2. Parse YARD cop docs for the old cop name for cop docs generation

# # Checks that no expectations are set on Capybara's `current_path`.
# #
# # The
# # https://www.rubydoc.info/github/teamcapybara/capybara/master/Capybara/RSpecMatchers#have_current_path-instance_method[`have_current_path` matcher]
# # should be used on `page` to set expectations on Capybara's
# # current path, since it uses
# # https://github.com/teamcapybara/capybara/blob/master/README.md#asynchronous-javascript-ajax-and-friends[Capybara's waiting functionality]
# # which ensures that preceding actions (like `click_link`) have
# # completed.
# #
# # This cop does not support autocorrection in some cases.
# #
# # @example
# # # bad
# # expect(current_path).to eq('/callback')
# #
# # # good
# # expect(page).to have_current_path('/callback')
# #
# # # bad (does not support autocorrection)
# # expect(page.current_path).to match(variable)
# #
# # # good
# # expect(page).to have_current_path('/callback')
# #
# class CurrentPathExpectation < ::RuboCop::Cop::Base; end
CurrentPathExpectation =
::RuboCop::Cop::Capybara::CurrentPathExpectation
end
end
end
Expand Down
78 changes: 28 additions & 50 deletions lib/rubocop/cop/rspec/capybara/match_style.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,34 @@ module RuboCop
module Cop
module RSpec
module Capybara
# Checks for usage of deprecated style methods.
#
# @example when using `assert_style`
# # bad
# page.find(:css, '#first').assert_style(display: 'block')
#
# # good
# page.find(:css, '#first').assert_matches_style(display: 'block')
#
# @example when using `has_style?`
# # bad
# expect(page.find(:css, 'first')
# .has_style?(display: 'block')).to be true
#
# # good
# expect(page.find(:css, 'first')
# .matches_style?(display: 'block')).to be true
#
# @example when using `have_style`
# # bad
# expect(page).to have_style(display: 'block')
#
# # good
# expect(page).to match_style(display: 'block')
#
class MatchStyle < Base
extend AutoCorrector

MSG = 'Use `%<good>s` instead of `%<bad>s`.'
RESTRICT_ON_SEND = %i[assert_style has_style? have_style].freeze
PREFERRED_METHOD = {
'assert_style' => 'assert_matches_style',
'has_style?' => 'matches_style?',
'have_style' => 'match_style'
}.freeze

def on_send(node)
method_node = node.loc.selector
add_offense(method_node) do |corrector|
corrector.replace(method_node,
PREFERRED_METHOD[method_node.source])
end
end

private

def message(node)
format(MSG, good: PREFERRED_METHOD[node.source], bad: node.source)
end
end
# @!parse
# # Checks for usage of deprecated style methods.
# #
# # @example when using `assert_style`
# # # bad
# # page.find(:css, '#first').assert_style(display: 'block')
# #
# # # good
# # page.find(:css, '#first').assert_matches_style(display: 'block')
# #
# # @example when using `has_style?`
# # # bad
# # expect(page.find(:css, 'first')
# # .has_style?(display: 'block')).to be true
# #
# # # good
# # expect(page.find(:css, 'first')
# # .matches_style?(display: 'block')).to be true
# #
# # @example when using `have_style`
# # # bad
# # expect(page).to have_style(display: 'block')
# #
# # # good
# # expect(page).to match_style(display: 'block')
# #
# class MatchStyle < ::RuboCop::Cop::Base; end
MatchStyle = ::RuboCop::Cop::Capybara::MatchStyle
end
end
end
Expand Down