Skip to content

Commit

Permalink
Add new RSpec/Capybara/SpecificActions
Browse files Browse the repository at this point in the history
Fix: #1326

We would like to eventually add autocorrect,
but as with `RSpec/Capybara/SpecificMatcher`,
the checks are complex and it would be better to introduce them
step by step.
  • Loading branch information
ydah committed Oct 20, 2022
1 parent 9806c0f commit ed59a61
Show file tree
Hide file tree
Showing 10 changed files with 460 additions and 31 deletions.
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ RSpec/VerifiedDoubleReference:
Enabled: true
RSpec/Capybara/NegationMatcher:
Enabled: true
RSpec/Capybara/SpecificActions:
Enabled: true
RSpec/Capybara/SpecificFinders:
Enabled: true
RSpec/Capybara/SpecificMatcher:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Improve `RSpec/NoExpectationExample` cop to ignore examples skipped or pending via metatada. ([@pirj][])
* Add `RSpec/FactoryBot/ConsistentParenthesesStyle` cop. ([@Liberatys][])
* Add `RSpec/Rails/InferredSpecType` cop. ([@r7kamura][])
* Add new `RSpec/Capybara/SpecificActions` cop. ([@ydah][])

## 2.13.2 (2022-09-23)

Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,12 @@ RSpec/Capybara/NegationMatcher:
- not_to
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher

RSpec/Capybara/SpecificActions:
Description: Checks for there is a more specific actions offered by Capybara.
Enabled: pending
VersionAdded: '2.14'
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/SpecificActions

RSpec/Capybara/SpecificFinders:
Description: Checks if there is a more specific finder offered by Capybara.
Enabled: pending
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
* xref:cops_rspec_capybara.adoc#rspeccapybara/currentpathexpectation[RSpec/Capybara/CurrentPathExpectation]
* xref:cops_rspec_capybara.adoc#rspeccapybara/featuremethods[RSpec/Capybara/FeatureMethods]
* xref:cops_rspec_capybara.adoc#rspeccapybara/negationmatcher[RSpec/Capybara/NegationMatcher]
* xref:cops_rspec_capybara.adoc#rspeccapybara/specificactions[RSpec/Capybara/SpecificActions]
* xref:cops_rspec_capybara.adoc#rspeccapybara/specificfinders[RSpec/Capybara/SpecificFinders]
* xref:cops_rspec_capybara.adoc#rspeccapybara/specificmatcher[RSpec/Capybara/SpecificMatcher]
* xref:cops_rspec_capybara.adoc#rspeccapybara/visibilitymatcher[RSpec/Capybara/VisibilityMatcher]
Expand Down
35 changes: 35 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec_capybara.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,41 @@ expect(page).to have_no_css('a')

* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher

== RSpec/Capybara/SpecificActions

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Pending
| Yes
| No
| 2.14
| -
|===

Checks for there is a more specific actions offered by Capybara.

=== Examples

[source,ruby]
----
# bad
find('a').click
find('button.cls').click
find('a', exact_text: 'foo').click
find('div button').click
# good
click_link
click_button(class: 'cls')
click_link(exact_text: 'foo')
find('div').click_button
----

=== References

* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/SpecificActions

== RSpec/Capybara/SpecificFinders

|===
Expand Down
130 changes: 130 additions & 0 deletions lib/rubocop/cop/rspec/capybara/specific_actions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
module Capybara
# Checks for there is a more specific actions offered by Capybara.
#
# @example
#
# # bad
# find('a').click
# find('button.cls').click
# find('a', exact_text: 'foo').click
# find('div button').click
#
# # good
# click_link
# click_button(class: 'cls')
# click_link(exact_text: 'foo')
# find('div').click_button
#
class SpecificActions < Base
MSG = "Prefer `%<good_action>s` over `find('%<selector>s').click`."
RESTRICT_ON_SEND = %i[click].freeze
SPECIFIC_ACTION = {
'button' => 'button',
'a' => 'link'
}.freeze

# @!method click_on_selector(node)
def_node_matcher :click_on_selector, <<-PATTERN
(send _ :find (str $_) ...)
PATTERN

# @!method option?(node)
def_node_search :option?, <<-PATTERN
(pair (sym %) _)
PATTERN

def on_send(node)
click_on_selector(node.receiver) do |arg|
next unless supported_selector?(arg)
next unless (selector = last_selector(arg))
next unless (action = specific_action(selector))
next unless specific_action_option?(node.receiver, arg, action)
next unless specific_action_pseudo_classes?(arg)

range = offense_range(node, node.receiver)
add_offense(range, message: message(action, selector))
end
end

private

def specific_action(selector)
SPECIFIC_ACTION[last_selector(selector)]
end

def specific_action_option?(node, arg, action)
attrs = CssSelector.attributes(arg).keys
return false unless replaceable_action?(node, action, attrs)

attrs.all? do |attr|
CssSelector.specific_options?(action, attr)
end
end

def specific_action_pseudo_classes?(arg)
CssSelector.pseudo_classes(arg).all? do |pseudo_class|
replaceable_pseudo_class?(pseudo_class, arg)
end
end

def replaceable_pseudo_class?(pseudo_class, arg)
unless CssSelector.specific_pesudo_classes?(pseudo_class)
return false
end

case pseudo_class
when 'not()' then replaceable_pseudo_class_not?(arg)
else true
end
end

def replaceable_pseudo_class_not?(arg)
arg.scan(/not\(.*?\)/).all? do |not_arg|
CssSelector.attributes(not_arg).values.all? do |v|
v.is_a?(TrueClass) || v.is_a?(FalseClass)
end
end
end

def replaceable_action?(node, action, attrs)
case action
when 'link' then replaceable_to_click_link?(node, attrs)
else true
end
end

def replaceable_to_click_link?(node, attrs)
option?(node, :href) || attrs.include?('href')
end

def supported_selector?(selector)
!selector.match?(/[>,+~]/)
end

def last_selector(arg)
arg.split.last[/^\w+/, 0]
end

def offense_range(node, receiver)
receiver.loc.selector.with(end_pos: node.loc.expression.end_pos)
end

def message(action, selector)
format(MSG,
good_action: good_action(action),
selector: selector)
end

def good_action(action)
"click_#{action}"
end
end
end
end
end
end
32 changes: 2 additions & 30 deletions lib/rubocop/cop/rspec/capybara/specific_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,6 @@ class SpecificMatcher < Base # rubocop:disable Metrics/ClassLength
'select' => 'select',
'input' => 'field'
}.freeze
SPECIFIC_MATCHER_OPTIONS = {
'button' => (
CssSelector::COMMON_OPTIONS + %w[disabled name value title type]
).freeze,
'link' => (
CssSelector::COMMON_OPTIONS + %w[href alt title download]
).freeze,
'table' => (
CssSelector::COMMON_OPTIONS + %w[
caption with_cols cols with_rows rows
]
).freeze,
'select' => (
CssSelector::COMMON_OPTIONS + %w[
disabled name placeholder options enabled_options
disabled_options selected with_selected multiple with_options
]
).freeze,
'field' => (
CssSelector::COMMON_OPTIONS + %w[
checked unchecked disabled valid name placeholder
validation_message readonly with type multiple
]
).freeze
}.freeze
SPECIFIC_MATCHER_PSEUDO_CLASSES = %w[
not() disabled enabled checked unchecked
].freeze

# @!method first_argument(node)
def_node_matcher :first_argument, <<-PATTERN
Expand Down Expand Up @@ -99,7 +71,7 @@ def specific_matcher_option?(node, arg, matcher)
return false unless replaceable_matcher?(node, matcher, attrs)

attrs.all? do |attr|
SPECIFIC_MATCHER_OPTIONS.fetch(matcher, []).include?(attr)
CssSelector.specific_options?(matcher, attr)
end
end

Expand All @@ -110,7 +82,7 @@ def specific_matcher_pseudo_classes?(arg)
end

def replaceable_pseudo_class?(pseudo_class, arg)
unless SPECIFIC_MATCHER_PSEUDO_CLASSES.include?(pseudo_class)
unless CssSelector.specific_pesudo_classes?(pseudo_class)
return false
end

Expand Down
49 changes: 48 additions & 1 deletion lib/rubocop/cop/rspec/mixin/css_selector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,56 @@ module CssSelector
id class style visible obscured exact exact_text normalize_ws match
wait filter_set focused
].freeze
SPECIFIC_OPTIONS = {
'button' => (
COMMON_OPTIONS + %w[disabled name value title type]
).freeze,
'link' => (
COMMON_OPTIONS + %w[href alt title download]
).freeze,
'table' => (
COMMON_OPTIONS + %w[
caption with_cols cols with_rows rows
]
).freeze,
'select' => (
COMMON_OPTIONS + %w[
disabled name placeholder options enabled_options
disabled_options selected with_selected multiple with_options
]
).freeze,
'field' => (
COMMON_OPTIONS + %w[
checked unchecked disabled valid name placeholder
validation_message readonly with type multiple
]
).freeze
}.freeze
SPECIFIC_PSEUDO_CLASSES = %w[
not() disabled enabled checked unchecked
].freeze

module_function

# @param element [String]
# @param attribute [String]
# @return [Boolean]
# @example
# specific_pesudo_classes?('button', 'name') # => true
# specific_pesudo_classes?('link', 'invalid') # => false
def specific_options?(element, attribute)
SPECIFIC_OPTIONS.fetch(element, []).include?(attribute)
end

# @param pseudo_class [String]
# @return [Boolean]
# @example
# specific_pesudo_classes?('disabled') # => true
# specific_pesudo_classes?('first-of-type') # => false
def specific_pesudo_classes?(pseudo_class)
SPECIFIC_PSEUDO_CLASSES.include?(pseudo_class)
end

# @param selector [String]
# @return [Boolean]
# @example
Expand Down Expand Up @@ -75,7 +122,7 @@ def pseudo_classes(selector)
# multiple_selectors?('a.cls b#id') # => true
# multiple_selectors?('a.cls') # => false
def multiple_selectors?(selector)
selector.match?(/[ >,+]/)
selector.match?(/[ >,+~]/)
end

# @param value [String]
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require_relative 'rspec/capybara/current_path_expectation'
require_relative 'rspec/capybara/feature_methods'
require_relative 'rspec/capybara/negation_matcher'
require_relative 'rspec/capybara/specific_actions'
require_relative 'rspec/capybara/specific_finders'
require_relative 'rspec/capybara/specific_matcher'
require_relative 'rspec/capybara/visibility_matcher'
Expand Down
Loading

0 comments on commit ed59a61

Please sign in to comment.