From cea510a68b8c7534dbdb303dbbfc0e1c5ecaad39 Mon Sep 17 00:00:00 2001 From: ydah Date: Sat, 23 Mar 2024 03:19:56 +0900 Subject: [PATCH] Move the department associated with `Capybara::RSpecMatchers` to `Capybara/RSpec/*` --- CHANGELOG.md | 1 + config/default.yml | 35 +++++ lib/rubocop-capybara.rb | 1 + .../cop/capybara/current_path_expectation.rb | 136 +--------------- lib/rubocop/cop/capybara/negation_matcher.rb | 99 +----------- .../rspec/current_path_expectation.rb | 146 ++++++++++++++++++ .../cop/capybara/rspec/negation_matcher.rb | 107 +++++++++++++ .../cop/capybara/rspec/specific_matcher.rb | 96 ++++++++++++ .../cop/capybara/rspec/visibility_matcher.rb | 73 +++++++++ lib/rubocop/cop/capybara/specific_matcher.rb | 88 +---------- .../cop/capybara/visibility_matcher.rb | 65 +------- lib/rubocop/cop/capybara_cops.rb | 4 + .../current_path_expectation_spec.rb | 2 +- .../{ => rspec}/negation_matcher_spec.rb | 2 +- .../{ => rspec}/specific_matcher_spec.rb | 2 +- .../{ => rspec}/visibility_matcher_spec.rb | 2 +- 16 files changed, 492 insertions(+), 367 deletions(-) create mode 100644 lib/rubocop/cop/capybara/rspec/current_path_expectation.rb create mode 100644 lib/rubocop/cop/capybara/rspec/negation_matcher.rb create mode 100644 lib/rubocop/cop/capybara/rspec/specific_matcher.rb create mode 100644 lib/rubocop/cop/capybara/rspec/visibility_matcher.rb rename spec/rubocop/cop/capybara/{ => rspec}/current_path_expectation_spec.rb (99%) rename spec/rubocop/cop/capybara/{ => rspec}/negation_matcher_spec.rb (98%) rename spec/rubocop/cop/capybara/{ => rspec}/specific_matcher_spec.rb (99%) rename spec/rubocop/cop/capybara/{ => rspec}/visibility_matcher_spec.rb (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index a926b460..5b82029a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Edge (Unreleased) +- **breaking** Move the department associated with `Capybara::RSpecMatchers` to `Capybara/RSpec/*`. ([@ydah]) - Fix a false negative for `Capybara/NegationMatcher` when using `to_not`. ([@ydah]) ## 2.20.0 (2024-01-03) diff --git a/config/default.yml b/config/default.yml index 97f888a5..51c3c086 100644 --- a/config/default.yml +++ b/config/default.yml @@ -26,6 +26,7 @@ Capybara/CurrentPathExpectation: VersionAdded: '1.18' VersionChanged: '2.0' Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/CurrentPathExpectation + VersionRemoved: '3.0' Capybara/MatchStyle: Description: Checks for usage of deprecated style methods. @@ -43,6 +44,7 @@ Capybara/NegationMatcher: - have_no - not_to Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/NegationMatcher + VersionRemoved: '3.0' Capybara/RedundantWithinFind: Description: Checks for redundant `within find(...)` calls. @@ -67,6 +69,7 @@ Capybara/SpecificMatcher: Enabled: pending VersionAdded: '2.12' Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/SpecificMatcher + VersionRemoved: '3.0' Capybara/VisibilityMatcher: Description: Checks for boolean visibility in Capybara finders. @@ -74,11 +77,19 @@ Capybara/VisibilityMatcher: VersionAdded: '1.39' VersionChanged: '2.0' Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/VisibilityMatcher + VersionRemoved: '3.0' Capybara/RSpec: Enabled: true Include: *1 +Capybara/RSpec/CurrentPathExpectation: + Description: Checks that no expectations are set on Capybara's `current_path`. + Enabled: true + VersionAdded: '1.18' + VersionChanged: '2.0' + Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/CurrentPathExpectation + Capybara/RSpec/HaveSelector: Description: Use `have_css` or `have_xpath` instead of `have_selector`. Enabled: pending @@ -86,6 +97,17 @@ Capybara/RSpec/HaveSelector: VersionAdded: '2.19' Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/RSpec/HaveSelector +Capybara/RSpec/NegationMatcher: + Description: Enforces use of `have_no_*` or `not_to` for negated expectations. + Enabled: pending + VersionAdded: '2.14' + VersionChanged: '2.20' + EnforcedStyle: have_no + SupportedStyles: + - have_no + - not_to + Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/NegationMatcher + Capybara/RSpec/PredicateMatcher: Description: Prefer using predicate matcher over using predicate method directly. Enabled: pending @@ -97,3 +119,16 @@ Capybara/RSpec/PredicateMatcher: - explicit VersionAdded: '2.19' Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/RSpec/PredicateMatcher + +Capybara/RSpec/SpecificMatcher: + Description: Checks for there is a more specific matcher offered by Capybara. + Enabled: pending + VersionAdded: '2.12' + Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/SpecificMatcher + +Capybara/RSpec/VisibilityMatcher: + Description: Checks for boolean visibility in Capybara finders. + Enabled: true + VersionAdded: '1.39' + VersionChanged: '2.0' + Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/VisibilityMatcher diff --git a/lib/rubocop-capybara.rb b/lib/rubocop-capybara.rb index 57f417b8..4e4fc3d3 100644 --- a/lib/rubocop-capybara.rb +++ b/lib/rubocop-capybara.rb @@ -5,6 +5,7 @@ require 'rubocop' +require_relative 'rubocop/capybara/' require_relative 'rubocop/cop/capybara/mixin/capybara_help' require_relative 'rubocop/cop/capybara/mixin/css_attributes_parser' require_relative 'rubocop/cop/capybara/mixin/css_selector' diff --git a/lib/rubocop/cop/capybara/current_path_expectation.rb b/lib/rubocop/cop/capybara/current_path_expectation.rb index 6f23ac7a..42223a6b 100644 --- a/lib/rubocop/cop/capybara/current_path_expectation.rb +++ b/lib/rubocop/cop/capybara/current_path_expectation.rb @@ -1,140 +1,18 @@ # frozen_string_literal: true +require_relative './rspec/current_path_expectation' + module RuboCop module Cop 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') - # expect(page.current_path).to eq('/callback') + # IMPORTANT: This cop is deprecated and will be removed in + # RuboCop Capybara 3.0. + # Please use `Capybara/RSpec/CurrentPathExpectation` instead. # - # # good - # expect(page).to have_current_path('/callback', ignore_query: true) - # - # # bad (does not support autocorrection when `match` with a variable) - # expect(page).to match(variable) - # - class CurrentPathExpectation < ::RuboCop::Cop::Base - extend AutoCorrector - include RangeHelp - - 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_node_matcher(node) - def_node_matcher :regexp_node_matcher, <<~PATTERN - (send - #expectation_set_on_current_path ${:to :to_not :not_to} - $(send nil? :match ${str dstr xstr})) - 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_node_matcher(node.parent) do |to_sym, matcher_node, regexp| - rewrite_expectation(corrector, node, to_sym, matcher_node) - convert_regexp_node_to_literal(corrector, matcher_node, regexp) - end - end - - def rewrite_expectation(corrector, node, to_symbol, matcher_node) - corrector.replace(node.first_argument, '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_argument_parentheses(corrector, matcher_node.first_argument) - add_ignore_query_options(corrector, node, matcher_node) - end - - def convert_regexp_node_to_literal(corrector, matcher_node, regexp_node) - str_node = matcher_node.first_argument - regexp_expr = regexp_node_to_regexp_expr(regexp_node) - corrector.replace(str_node, regexp_expr) - end - - def regexp_node_to_regexp_expr(regexp_node) - if regexp_node.xstr_type? - "/\#{`#{regexp_node.value.value}`}/" - else - Regexp.new(regexp_node.value).inspect - end - end - - def add_argument_parentheses(corrector, arg_node) - return unless method_call_with_no_parentheses?(arg_node) - - first_argument_range = range_with_surrounding_space( - arg_node.first_argument.source_range, side: :left - ) - corrector.insert_before(first_argument_range, '(') - corrector.insert_after(arg_node.last_argument, ')') - end - - def method_call_with_no_parentheses?(arg_node) - arg_node.send_type? && arg_node.arguments? && !arg_node.parenthesized? - 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 `match` matcher. - def add_ignore_query_options(corrector, node, matcher_node) - return if matcher_node.method?(:match) - - expectation_node = node.parent.last_argument - expectation_last_child = expectation_node.children.last - corrector.insert_after(expectation_last_child, ', ignore_query: true') - end + class CurrentPathExpectation < + RuboCop::Cop::Capybara::RSpec::CurrentPathExpectation end end end diff --git a/lib/rubocop/cop/capybara/negation_matcher.rb b/lib/rubocop/cop/capybara/negation_matcher.rb index a2834498..4a561650 100644 --- a/lib/rubocop/cop/capybara/negation_matcher.rb +++ b/lib/rubocop/cop/capybara/negation_matcher.rb @@ -1,104 +1,17 @@ # frozen_string_literal: true +require_relative './rspec/negation_matcher' + module RuboCop module Cop module Capybara # Enforces use of `have_no_*` or `not_to` for negated expectations. # - # @example EnforcedStyle: have_no (default) - # # bad - # expect(page).not_to have_selector 'a' - # expect(page).not_to have_css('a') - # - # # good - # expect(page).to have_no_selector 'a' - # expect(page).to have_no_css('a') - # - # @example EnforcedStyle: not_to - # # bad - # expect(page).to have_no_selector 'a' - # expect(page).to have_no_css('a') - # - # # good - # expect(page).not_to have_selector 'a' - # expect(page).not_to have_css('a') + # IMPORTANT: This cop is deprecated and will be removed in + # RuboCop Capybara 3.0. + # Please use `Capybara/RSpec/NegationMatcher` instead. # - class NegationMatcher < ::RuboCop::Cop::Base - extend AutoCorrector - include ConfigurableEnforcedStyle - - MSG = 'Use `expect(...).%s %s`.' - CAPYBARA_MATCHERS = %w[ - selector css xpath text title current_path link button - field checked_field unchecked_field select table - sibling ancestor content - ].freeze - POSITIVE_MATCHERS = - Set.new(CAPYBARA_MATCHERS) { |element| :"have_#{element}" }.freeze - NEGATIVE_MATCHERS = - Set.new(CAPYBARA_MATCHERS) { |element| :"have_no_#{element}" } - .freeze - RESTRICT_ON_SEND = (POSITIVE_MATCHERS + NEGATIVE_MATCHERS).freeze - - # @!method not_to?(node) - def_node_matcher :not_to?, <<~PATTERN - (send ... {:not_to :to_not} - (send nil? %POSITIVE_MATCHERS ...)) - PATTERN - - # @!method have_no?(node) - def_node_matcher :have_no?, <<~PATTERN - (send ... :to - (send nil? %NEGATIVE_MATCHERS ...)) - PATTERN - - def on_send(node) - return unless offense?(node) - - matcher = node.method_name.to_s - add_offense(offense_range(node), - message: message(matcher)) do |corrector| - corrector.replace(node.parent.loc.selector, replaced_runner) - corrector.replace(node.loc.selector, - replaced_matcher(matcher)) - end - end - - private - - def offense?(node) - node.arguments? && - ((style == :have_no && not_to?(node.parent)) || - (style == :not_to && have_no?(node.parent))) - end - - def offense_range(node) - node.parent.loc.selector.with(end_pos: node.loc.selector.end_pos) - end - - def message(matcher) - format(MSG, - runner: replaced_runner, - matcher: replaced_matcher(matcher)) - end - - def replaced_runner - case style - when :have_no - 'to' - when :not_to - 'not_to' - end - end - - def replaced_matcher(matcher) - case style - when :have_no - matcher.sub('have_', 'have_no_') - when :not_to - matcher.sub('have_no_', 'have_') - end - end + class NegationMatcher < RuboCop::Cop::Capybara::RSpec::NegationMatcher end end end diff --git a/lib/rubocop/cop/capybara/rspec/current_path_expectation.rb b/lib/rubocop/cop/capybara/rspec/current_path_expectation.rb new file mode 100644 index 00000000..1a643bd8 --- /dev/null +++ b/lib/rubocop/cop/capybara/rspec/current_path_expectation.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Capybara + module RSpec + # 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') + # expect(page.current_path).to eq('/callback') + # + # # good + # expect(page).to have_current_path('/callback', ignore_query: true) + # + # # bad (does not support autocorrection when `match` with a variable) + # expect(page).to match(variable) + # + class CurrentPathExpectation < ::RuboCop::Cop::Base + extend AutoCorrector + include RangeHelp + + 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_node_matcher(node) + def_node_matcher :regexp_node_matcher, <<~PATTERN + (send + #expectation_set_on_current_path ${:to :to_not :not_to} + $(send nil? :match ${str dstr xstr})) + 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_node_matcher(node.parent) do |to_sym, matcher_node, regexp| + rewrite_expectation(corrector, node, to_sym, matcher_node) + convert_regexp_node_to_literal(corrector, matcher_node, regexp) + end + end + + def rewrite_expectation(corrector, node, to_symbol, matcher_node) + corrector.replace(node.first_argument, '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_argument_parentheses(corrector, matcher_node.first_argument) + add_ignore_query_options(corrector, node, matcher_node) + end + + def convert_regexp_node_to_literal(corrector, matcher_node, + regexp_node) + str_node = matcher_node.first_argument + regexp_expr = regexp_node_to_regexp_expr(regexp_node) + corrector.replace(str_node, regexp_expr) + end + + def regexp_node_to_regexp_expr(regexp_node) + if regexp_node.xstr_type? + "/\#{`#{regexp_node.value.value}`}/" + else + Regexp.new(regexp_node.value).inspect + end + end + + def add_argument_parentheses(corrector, arg_node) + return unless method_call_with_no_parentheses?(arg_node) + + first_argument_range = range_with_surrounding_space( + arg_node.first_argument.source_range, side: :left + ) + corrector.insert_before(first_argument_range, '(') + corrector.insert_after(arg_node.last_argument, ')') + end + + def method_call_with_no_parentheses?(arg_node) + arg_node.send_type? && arg_node.arguments? && + !arg_node.parenthesized? + 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 `match` matcher. + def add_ignore_query_options(corrector, node, matcher_node) + return if matcher_node.method?(:match) + + expectation_node = node.parent.last_argument + expectation_last_child = expectation_node.children.last + corrector.insert_after(expectation_last_child, + ', ignore_query: true') + end + end + end + end + end +end diff --git a/lib/rubocop/cop/capybara/rspec/negation_matcher.rb b/lib/rubocop/cop/capybara/rspec/negation_matcher.rb new file mode 100644 index 00000000..f4ebd8a7 --- /dev/null +++ b/lib/rubocop/cop/capybara/rspec/negation_matcher.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Capybara + module RSpec + # Enforces use of `have_no_*` or `not_to` for negated expectations. + # + # @example EnforcedStyle: have_no (default) + # # bad + # expect(page).not_to have_selector 'a' + # expect(page).not_to have_css('a') + # + # # good + # expect(page).to have_no_selector 'a' + # expect(page).to have_no_css('a') + # + # @example EnforcedStyle: not_to + # # bad + # expect(page).to have_no_selector 'a' + # expect(page).to have_no_css('a') + # + # # good + # expect(page).not_to have_selector 'a' + # expect(page).not_to have_css('a') + # + class NegationMatcher < ::RuboCop::Cop::Base + extend AutoCorrector + include ConfigurableEnforcedStyle + + MSG = 'Use `expect(...).%s %s`.' + CAPYBARA_MATCHERS = %w[ + selector css xpath text title current_path link button + field checked_field unchecked_field select table + sibling ancestor content + ].freeze + POSITIVE_MATCHERS = + Set.new(CAPYBARA_MATCHERS) { |element| :"have_#{element}" }.freeze + NEGATIVE_MATCHERS = + Set.new(CAPYBARA_MATCHERS) { |element| :"have_no_#{element}" } + .freeze + RESTRICT_ON_SEND = (POSITIVE_MATCHERS + NEGATIVE_MATCHERS).freeze + + # @!method not_to?(node) + def_node_matcher :not_to?, <<~PATTERN + (send ... {:not_to :to_not} + (send nil? %POSITIVE_MATCHERS ...)) + PATTERN + + # @!method have_no?(node) + def_node_matcher :have_no?, <<~PATTERN + (send ... :to + (send nil? %NEGATIVE_MATCHERS ...)) + PATTERN + + def on_send(node) + return unless offense?(node) + + matcher = node.method_name.to_s + add_offense(offense_range(node), + message: message(matcher)) do |corrector| + corrector.replace(node.parent.loc.selector, replaced_runner) + corrector.replace(node.loc.selector, + replaced_matcher(matcher)) + end + end + + private + + def offense?(node) + node.arguments? && + ((style == :have_no && not_to?(node.parent)) || + (style == :not_to && have_no?(node.parent))) + end + + def offense_range(node) + node.parent.loc.selector.with(end_pos: node.loc.selector.end_pos) + end + + def message(matcher) + format(MSG, + runner: replaced_runner, + matcher: replaced_matcher(matcher)) + end + + def replaced_runner + case style + when :have_no + 'to' + when :not_to + 'not_to' + end + end + + def replaced_matcher(matcher) + case style + when :have_no + matcher.sub('have_', 'have_no_') + when :not_to + matcher.sub('have_no_', 'have_') + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/capybara/rspec/specific_matcher.rb b/lib/rubocop/cop/capybara/rspec/specific_matcher.rb new file mode 100644 index 00000000..cefe9f95 --- /dev/null +++ b/lib/rubocop/cop/capybara/rspec/specific_matcher.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Capybara + module RSpec + # Checks for there is a more specific matcher offered by Capybara. + # + # @example + # + # # bad + # expect(page).to have_selector('button') + # expect(page).to have_no_selector('button.cls') + # expect(page).to have_css('button') + # expect(page).to have_no_css('a.cls', href: 'http://example.com') + # expect(page).to have_css('table.cls') + # expect(page).to have_css('select') + # expect(page).to have_css('input', exact_text: 'foo') + # + # # good + # expect(page).to have_button + # expect(page).to have_no_button(class: 'cls') + # expect(page).to have_button + # expect(page).to have_no_link('foo', class: 'cls', href: 'http://example.com') + # expect(page).to have_table(class: 'cls') + # expect(page).to have_select + # expect(page).to have_field('foo') + # + class SpecificMatcher < ::RuboCop::Cop::Base + MSG = 'Prefer `%s` over `%s`.' + RESTRICT_ON_SEND = %i[have_selector have_no_selector have_css + have_no_css].freeze + SPECIFIC_MATCHER = { + 'button' => 'button', + 'a' => 'link', + 'table' => 'table', + 'select' => 'select', + 'input' => 'field' + }.freeze + + # @!method first_argument(node) + def_node_matcher :first_argument, <<~PATTERN + (send nil? _ (str $_) ... ) + PATTERN + + # @!method text_with_regexp?(node) + def_node_search :text_with_regexp?, <<~PATTERN + (pair (sym {:text :exact_text}) (regexp ...)) + PATTERN + + def on_send(node) + first_argument(node) do |arg| + next unless (matcher = specific_matcher(arg)) + next if CssSelector.multiple_selectors?(arg) + next unless replaceable?(node, arg, matcher) + + add_offense(node, message: message(node, matcher)) + end + end + + private + + def specific_matcher(arg) + splitted_arg = arg[/^\w+/, 0] + SPECIFIC_MATCHER[splitted_arg] + end + + def replaceable?(node, arg, matcher) + replaceable_attributes?(arg) && + !text_with_regexp?(node) && + CapybaraHelp.replaceable_option?(node, arg, matcher) && + CapybaraHelp.replaceable_pseudo_classes?(arg) + end + + def replaceable_attributes?(selector) + CapybaraHelp.replaceable_attributes?( + CssSelector.attributes(selector) + ) + end + + def message(node, matcher) + format(MSG, + good_matcher: good_matcher(node, matcher), + bad_matcher: node.method_name) + end + + def good_matcher(node, matcher) + node.method_name + .to_s + .gsub(/selector|css/, matcher.to_s) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/capybara/rspec/visibility_matcher.rb b/lib/rubocop/cop/capybara/rspec/visibility_matcher.rb new file mode 100644 index 00000000..badc7f34 --- /dev/null +++ b/lib/rubocop/cop/capybara/rspec/visibility_matcher.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Capybara + module RSpec + # 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`. + # Read more in + # https://www.rubydoc.info/gems/capybara/Capybara%2FNode%2FFinders:all[the documentation]. + # + # @example + # # bad + # expect(page).to have_selector('.foo', visible: false) + # expect(page).to have_css('.foo', visible: true) + # expect(page).to have_link('my link', visible: false) + # + # # good + # expect(page).to have_selector('.foo', visible: :visible) + # expect(page).to have_css('.foo', visible: :all) + # expect(page).to have_link('my link', visible: :hidden) + # + class VisibilityMatcher < ::RuboCop::Cop::Base + MSG_FALSE = 'Use `:all` or `:hidden` instead of `false`.' + MSG_TRUE = 'Use `:visible` instead of `true`.' + CAPYBARA_MATCHER_METHODS = %w[ + button + checked_field + css + field + link + select + selector + table + unchecked_field + xpath + ].flat_map do |element| + ["have_#{element}".to_sym, "have_no_#{element}".to_sym] + end + + RESTRICT_ON_SEND = CAPYBARA_MATCHER_METHODS + + # @!method visible_true?(node) + def_node_matcher :visible_true?, <<~PATTERN + (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) true) ...>)) + PATTERN + + # @!method visible_false?(node) + def_node_matcher :visible_false?, <<~PATTERN + (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) false) ...>)) + PATTERN + + def on_send(node) + visible_false?(node) { |arg| add_offense(arg, message: MSG_FALSE) } + visible_true?(node) { |arg| add_offense(arg, message: MSG_TRUE) } + end + + private + + def capybara_matcher?(method_name) + CAPYBARA_MATCHER_METHODS.include? method_name + end + end + end + end + end +end diff --git a/lib/rubocop/cop/capybara/specific_matcher.rb b/lib/rubocop/cop/capybara/specific_matcher.rb index d8e9fc51..cadaff6e 100644 --- a/lib/rubocop/cop/capybara/specific_matcher.rb +++ b/lib/rubocop/cop/capybara/specific_matcher.rb @@ -1,93 +1,17 @@ # frozen_string_literal: true +require_relative './rspec/specific_matcher' + module RuboCop module Cop module Capybara # Checks for there is a more specific matcher offered by Capybara. # - # @example - # - # # bad - # expect(page).to have_selector('button') - # expect(page).to have_no_selector('button.cls') - # expect(page).to have_css('button') - # expect(page).to have_no_css('a.cls', href: 'http://example.com') - # expect(page).to have_css('table.cls') - # expect(page).to have_css('select') - # expect(page).to have_css('input', exact_text: 'foo') + # IMPORTANT: This cop is deprecated and will be removed in + # RuboCop Capybara 3.0. + # Please use `Capybara/RSpec/SpecificMatcher` instead. # - # # good - # expect(page).to have_button - # expect(page).to have_no_button(class: 'cls') - # expect(page).to have_button - # expect(page).to have_no_link('foo', class: 'cls', href: 'http://example.com') - # expect(page).to have_table(class: 'cls') - # expect(page).to have_select - # expect(page).to have_field('foo') - # - class SpecificMatcher < ::RuboCop::Cop::Base - MSG = 'Prefer `%s` over `%s`.' - RESTRICT_ON_SEND = %i[have_selector have_no_selector have_css - have_no_css].freeze - SPECIFIC_MATCHER = { - 'button' => 'button', - 'a' => 'link', - 'table' => 'table', - 'select' => 'select', - 'input' => 'field' - }.freeze - - # @!method first_argument(node) - def_node_matcher :first_argument, <<~PATTERN - (send nil? _ (str $_) ... ) - PATTERN - - # @!method text_with_regexp?(node) - def_node_search :text_with_regexp?, <<~PATTERN - (pair (sym {:text :exact_text}) (regexp ...)) - PATTERN - - def on_send(node) - first_argument(node) do |arg| - next unless (matcher = specific_matcher(arg)) - next if CssSelector.multiple_selectors?(arg) - next unless replaceable?(node, arg, matcher) - - add_offense(node, message: message(node, matcher)) - end - end - - private - - def specific_matcher(arg) - splitted_arg = arg[/^\w+/, 0] - SPECIFIC_MATCHER[splitted_arg] - end - - def replaceable?(node, arg, matcher) - replaceable_attributes?(arg) && - !text_with_regexp?(node) && - CapybaraHelp.replaceable_option?(node, arg, matcher) && - CapybaraHelp.replaceable_pseudo_classes?(arg) - end - - def replaceable_attributes?(selector) - CapybaraHelp.replaceable_attributes?( - CssSelector.attributes(selector) - ) - end - - def message(node, matcher) - format(MSG, - good_matcher: good_matcher(node, matcher), - bad_matcher: node.method_name) - end - - def good_matcher(node, matcher) - node.method_name - .to_s - .gsub(/selector|css/, matcher.to_s) - end + class SpecificMatcher < RuboCop::Cop::Capybara::RSpec::SpecificMatcher end end end diff --git a/lib/rubocop/cop/capybara/visibility_matcher.rb b/lib/rubocop/cop/capybara/visibility_matcher.rb index c06e38a8..bc5d0ccf 100644 --- a/lib/rubocop/cop/capybara/visibility_matcher.rb +++ b/lib/rubocop/cop/capybara/visibility_matcher.rb @@ -1,70 +1,17 @@ # frozen_string_literal: true +require_relative './rspec/current_path_expectation' + module RuboCop module Cop module Capybara # 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`. - # Read more in - # https://www.rubydoc.info/gems/capybara/Capybara%2FNode%2FFinders:all[the documentation]. - # - # @example - # # bad - # expect(page).to have_selector('.foo', visible: false) - # expect(page).to have_css('.foo', visible: true) - # expect(page).to have_link('my link', visible: false) - # - # # good - # expect(page).to have_selector('.foo', visible: :visible) - # expect(page).to have_css('.foo', visible: :all) - # expect(page).to have_link('my link', visible: :hidden) + # IMPORTANT: This cop is deprecated and will be removed in + # RuboCop Capybara 3.0. + # Please use `Capybara/RSpec/VisibilityMatcher` instead. # - class VisibilityMatcher < ::RuboCop::Cop::Base - MSG_FALSE = 'Use `:all` or `:hidden` instead of `false`.' - MSG_TRUE = 'Use `:visible` instead of `true`.' - CAPYBARA_MATCHER_METHODS = %w[ - button - checked_field - css - field - link - select - selector - table - unchecked_field - xpath - ].flat_map do |element| - ["have_#{element}".to_sym, "have_no_#{element}".to_sym] - end - - RESTRICT_ON_SEND = CAPYBARA_MATCHER_METHODS - - # @!method visible_true?(node) - def_node_matcher :visible_true?, <<~PATTERN - (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) true) ...>)) - PATTERN - - # @!method visible_false?(node) - def_node_matcher :visible_false?, <<~PATTERN - (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) false) ...>)) - PATTERN - - def on_send(node) - visible_false?(node) { |arg| add_offense(arg, message: MSG_FALSE) } - visible_true?(node) { |arg| add_offense(arg, message: MSG_TRUE) } - end - - private - - def capybara_matcher?(method_name) - CAPYBARA_MATCHER_METHODS.include? method_name - end + class VisibilityMatcher < RuboCop::Cop::Capybara::RSpec::VisibilityMatcher end end end diff --git a/lib/rubocop/cop/capybara_cops.rb b/lib/rubocop/cop/capybara_cops.rb index 1c386549..bca71e6e 100644 --- a/lib/rubocop/cop/capybara_cops.rb +++ b/lib/rubocop/cop/capybara_cops.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true +require_relative 'capybara/rspec/current_path_expectation' require_relative 'capybara/rspec/have_selector' +require_relative 'capybara/rspec/negation_matcher' require_relative 'capybara/rspec/predicate_matcher' +require_relative 'capybara/rspec/specific_matcher' +require_relative 'capybara/rspec/visibility_matcher' require_relative 'capybara/click_link_or_button_style' require_relative 'capybara/current_path_expectation' diff --git a/spec/rubocop/cop/capybara/current_path_expectation_spec.rb b/spec/rubocop/cop/capybara/rspec/current_path_expectation_spec.rb similarity index 99% rename from spec/rubocop/cop/capybara/current_path_expectation_spec.rb rename to spec/rubocop/cop/capybara/rspec/current_path_expectation_spec.rb index 8d61ea7f..f6f2b40b 100644 --- a/spec/rubocop/cop/capybara/current_path_expectation_spec.rb +++ b/spec/rubocop/cop/capybara/rspec/current_path_expectation_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Capybara::CurrentPathExpectation do +RSpec.describe RuboCop::Cop::Capybara::RSpec::CurrentPathExpectation do it 'flags offenses for `expect(current_path)`' do expect_offense(<<~RUBY) expect(current_path).to eq "/callback" diff --git a/spec/rubocop/cop/capybara/negation_matcher_spec.rb b/spec/rubocop/cop/capybara/rspec/negation_matcher_spec.rb similarity index 98% rename from spec/rubocop/cop/capybara/negation_matcher_spec.rb rename to spec/rubocop/cop/capybara/rspec/negation_matcher_spec.rb index c7999de3..55efb2c8 100644 --- a/spec/rubocop/cop/capybara/negation_matcher_spec.rb +++ b/spec/rubocop/cop/capybara/rspec/negation_matcher_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Capybara::NegationMatcher do +RSpec.describe RuboCop::Cop::Capybara::RSpec::NegationMatcher do let(:cop_config) { { 'EnforcedStyle' => enforced_style } } context 'with EnforcedStyle `have_no`' do diff --git a/spec/rubocop/cop/capybara/specific_matcher_spec.rb b/spec/rubocop/cop/capybara/rspec/specific_matcher_spec.rb similarity index 99% rename from spec/rubocop/cop/capybara/specific_matcher_spec.rb rename to spec/rubocop/cop/capybara/rspec/specific_matcher_spec.rb index 08fd1a55..d5439972 100644 --- a/spec/rubocop/cop/capybara/specific_matcher_spec.rb +++ b/spec/rubocop/cop/capybara/rspec/specific_matcher_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Capybara::SpecificMatcher do +RSpec.describe RuboCop::Cop::Capybara::RSpec::SpecificMatcher do it 'does not register an offense for abstract matcher when ' \ 'first argument is not a replaceable element' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/capybara/visibility_matcher_spec.rb b/spec/rubocop/cop/capybara/rspec/visibility_matcher_spec.rb similarity index 98% rename from spec/rubocop/cop/capybara/visibility_matcher_spec.rb rename to spec/rubocop/cop/capybara/rspec/visibility_matcher_spec.rb index e232fca9..f18d80a2 100644 --- a/spec/rubocop/cop/capybara/visibility_matcher_spec.rb +++ b/spec/rubocop/cop/capybara/rspec/visibility_matcher_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Capybara::VisibilityMatcher do +RSpec.describe RuboCop::Cop::Capybara::RSpec::VisibilityMatcher do it 'registers an offense when using `visible: true`' do expect_offense(<<~RUBY) expect(page).to have_selector('.my_element', visible: true)