From eed7439be0c90c7d79e264240f91ad0a924d5a80 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Fri, 16 Dec 2022 01:15:55 +0300 Subject: [PATCH 1/5] Extract Capybara cops https://github.com/rubocop/rubocop-rspec/discussions/1440 Extracted to https://github.com/pirj/rubocop-capybara --- CHANGELOG.md | 1 + config/obsoletion.yml | 9 + lib/rubocop-rspec.rb | 3 +- .../capybara/current_path_expectation.rb | 92 +----- lib/rubocop/cop/rspec/capybara/match_style.rb | 26 +- .../cop/rspec/capybara/negation_matcher.rb | 77 +---- .../cop/rspec/capybara/specific_actions.rb | 60 +--- .../cop/rspec/capybara/specific_finders.rb | 73 +---- .../cop/rspec/capybara/specific_matcher.rb | 48 +-- .../cop/rspec/capybara/visibility_matcher.rb | 42 +-- lib/rubocop/cop/rspec/mixin/capybara_help.rb | 80 ----- lib/rubocop/cop/rspec/mixin/css_selector.rb | 146 --------- rubocop-rspec.gemspec | 1 + .../capybara/current_path_expectation_spec.rb | 130 -------- .../cop/rspec/capybara/match_style_spec.rb | 56 ---- .../rspec/capybara/negation_matcher_spec.rb | 97 ------ .../rspec/capybara/specific_actions_spec.rb | 234 -------------- .../rspec/capybara/specific_finders_spec.rb | 125 -------- .../rspec/capybara/specific_matcher_spec.rb | 287 ------------------ .../rspec/capybara/visibility_matcher_spec.rb | 113 ------- 20 files changed, 27 insertions(+), 1673 deletions(-) delete mode 100644 lib/rubocop/cop/rspec/mixin/capybara_help.rb delete mode 100644 lib/rubocop/cop/rspec/mixin/css_selector.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/current_path_expectation_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/match_style_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb delete mode 100644 spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f9bb7b55..be7388822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/config/obsoletion.yml b/config/obsoletion.yml index 38f27529a..c2f02f8f7 100644 --- a/config/obsoletion.yml +++ b/config/obsoletion.yml @@ -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 diff --git a/lib/rubocop-rspec.rb b/lib/rubocop-rspec.rb index 0f7e5346d..055c8da42 100644 --- a/lib/rubocop-rspec.rb +++ b/lib/rubocop-rspec.rb @@ -4,6 +4,7 @@ require 'yaml' require 'rubocop' +require 'rubocop-capybara' require_relative 'rubocop/rspec' require_relative 'rubocop/rspec/inject' @@ -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' diff --git a/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb b/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb index 6b9b023ba..d1e420647 100644 --- a/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb +++ b/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb @@ -29,95 +29,9 @@ module Capybara # # 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 + # @!parse class CurrentPathExpectation < ::RuboCop::Cop::Base; end + class CurrentPathExpectation < + ::RuboCop::Cop::Capybara::CurrentPathExpectation end end end diff --git a/lib/rubocop/cop/rspec/capybara/match_style.rb b/lib/rubocop/cop/rspec/capybara/match_style.rb index 13b289154..4a0062e66 100644 --- a/lib/rubocop/cop/rspec/capybara/match_style.rb +++ b/lib/rubocop/cop/rspec/capybara/match_style.rb @@ -29,30 +29,8 @@ module Capybara # # good # expect(page).to match_style(display: 'block') # - class MatchStyle < Base - extend AutoCorrector - - MSG = 'Use `%s` instead of `%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 + # @!parse class MatchStyle < ::RuboCop::Cop::Base; end + class MatchStyle < ::RuboCop::Cop::Capybara::MatchStyle end end end diff --git a/lib/rubocop/cop/rspec/capybara/negation_matcher.rb b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb index 57e3c7596..0c73807ef 100644 --- a/lib/rubocop/cop/rspec/capybara/negation_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb @@ -24,81 +24,8 @@ module Capybara # expect(page).to have_no_selector # expect(page).to have_no_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 - ].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 - (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.parent) - - 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) - (style == :have_no && not_to?(node)) || - (style == :not_to && have_no?(node)) - 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 + # @!parse class NegationMatcher < ::RuboCop::Cop::Base; end + class NegationMatcher < ::RuboCop::Cop::Capybara::NegationMatcher end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_actions.rb b/lib/rubocop/cop/rspec/capybara/specific_actions.rb index a25a4eb9d..e245c2bdc 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_actions.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_actions.rb @@ -20,64 +20,8 @@ module Capybara # click_link(exact_text: 'foo') # find('div').click_button # - class SpecificActions < ::RuboCop::Cop::Base - MSG = "Prefer `%s` over `find('%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 - - def on_send(node) - click_on_selector(node.receiver) do |arg| - next unless supported_selector?(arg) - # Always check the last selector in the case of multiple selectors - # separated by whitespace. - # because the `.click` is executed on the element to - # which the last selector points. - next unless (selector = last_selector(arg)) - next unless (action = specific_action(selector)) - next unless CapybaraHelp.specific_option?(node.receiver, arg, - action) - next unless CapybaraHelp.specific_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 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 + # @!parse class SpecificActions < ::RuboCop::Cop::Base; end + class SpecificActions < ::RuboCop::Cop::Capybara::SpecificActions end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_finders.rb b/lib/rubocop/cop/rspec/capybara/specific_finders.rb index 805e827e5..bac35d3a0 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_finders.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_finders.rb @@ -15,77 +15,8 @@ module Capybara # find_by_id('some-id') # find_by_id('some-id', visible: true) # - class SpecificFinders < ::RuboCop::Cop::Base - extend AutoCorrector - - include RangeHelp - - MSG = 'Prefer `find_by` over `find`.' - RESTRICT_ON_SEND = %i[find].freeze - - # @!method find_argument(node) - def_node_matcher :find_argument, <<~PATTERN - (send _ :find (str $_) ...) - PATTERN - - def on_send(node) - find_argument(node) do |arg| - next if CssSelector.multiple_selectors?(arg) - - on_attr(node, arg) if attribute?(arg) - on_id(node, arg) if CssSelector.id?(arg) - end - end - - private - - def on_attr(node, arg) - return unless (id = CssSelector.attributes(arg)['id']) - - register_offense(node, replaced_arguments(arg, id)) - end - - def on_id(node, arg) - register_offense(node, "'#{arg.to_s.delete('#')}'") - end - - def attribute?(arg) - CssSelector.attribute?(arg) && - CssSelector.common_attributes?(arg) - end - - def register_offense(node, arg_replacement) - add_offense(offense_range(node)) do |corrector| - corrector.replace(node.loc.selector, 'find_by_id') - corrector.replace(node.first_argument.loc.expression, - arg_replacement) - end - end - - def replaced_arguments(arg, id) - options = to_options(CssSelector.attributes(arg)) - options.empty? ? id : "#{id}, #{options}" - end - - def to_options(attrs) - attrs.each.map do |key, value| - next if key == 'id' - - "#{key}: #{value}" - end.compact.join(', ') - end - - def offense_range(node) - range_between(node.loc.selector.begin_pos, end_pos(node)) - end - - def end_pos(node) - if node.loc.end - node.loc.end.end_pos - else - node.loc.expression.end_pos - end - end + # @!parse class SpecificFinders < ::RuboCop::Cop::Base; end + class SpecificFinders < ::RuboCop::Cop::Capybara::SpecificFinders end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb index e9ef2c01e..f8ecd6075 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb @@ -26,52 +26,8 @@ module Capybara # 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 - - def on_send(node) - first_argument(node) do |arg| - next unless (matcher = specific_matcher(arg)) - next if CssSelector.multiple_selectors?(arg) - next unless CapybaraHelp.specific_option?(node, arg, matcher) - next unless CapybaraHelp.specific_pseudo_classes?(arg) - - 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 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 + # @!parse class SpecificMatcher < ::RuboCop::Cop::Base; end + class SpecificMatcher < ::RuboCop::Cop::Capybara::SpecificMatcher end end end diff --git a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb index 99f3a669d..e9bd5379d 100644 --- a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb @@ -26,46 +26,8 @@ module Capybara # 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 + # @!parse class VisibilityMatcher < ::RuboCop::Cop::Base; end + class VisibilityMatcher < ::RuboCop::Cop::Capybara::VisibilityMatcher end end end diff --git a/lib/rubocop/cop/rspec/mixin/capybara_help.rb b/lib/rubocop/cop/rspec/mixin/capybara_help.rb deleted file mode 100644 index 95a0acbf2..000000000 --- a/lib/rubocop/cop/rspec/mixin/capybara_help.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - module RSpec - # Help methods for capybara. - module CapybaraHelp - module_function - - # @param node [RuboCop::AST::SendNode] - # @param locator [String] - # @param element [String] - # @return [Boolean] - def specific_option?(node, locator, element) - attrs = CssSelector.attributes(locator).keys - return false unless replaceable_element?(node, element, attrs) - - attrs.all? do |attr| - CssSelector.specific_options?(element, attr) - end - end - - # @param locator [String] - # @return [Boolean] - def specific_pseudo_classes?(locator) - CssSelector.pseudo_classes(locator).all? do |pseudo_class| - replaceable_pseudo_class?(pseudo_class, locator) - end - end - - # @param pseudo_class [String] - # @param locator [String] - # @return [Boolean] - def replaceable_pseudo_class?(pseudo_class, locator) - return false unless CssSelector.specific_pesudo_classes?(pseudo_class) - - case pseudo_class - when 'not()' then replaceable_pseudo_class_not?(locator) - else true - end - end - - # @param locator [String] - # @return [Boolean] - def replaceable_pseudo_class_not?(locator) - locator.scan(/not\(.*?\)/).all? do |negation| - CssSelector.attributes(negation).values.all? do |v| - v.is_a?(TrueClass) || v.is_a?(FalseClass) - end - end - end - - # @param node [RuboCop::AST::SendNode] - # @param element [String] - # @param attrs [Array] - # @return [Boolean] - def replaceable_element?(node, element, attrs) - case element - when 'link' then replaceable_to_link?(node, attrs) - else true - end - end - - # @param node [RuboCop::AST::SendNode] - # @param attrs [Array] - # @return [Boolean] - def replaceable_to_link?(node, attrs) - include_option?(node, :href) || attrs.include?('href') - end - - # @param node [RuboCop::AST::SendNode] - # @param option [Symbol] - # @return [Boolean] - def include_option?(node, option) - node.each_descendant(:sym).find { |opt| opt.value == option } - end - end - end - end -end diff --git a/lib/rubocop/cop/rspec/mixin/css_selector.rb b/lib/rubocop/cop/rspec/mixin/css_selector.rb deleted file mode 100644 index 3fc2603fe..000000000 --- a/lib/rubocop/cop/rspec/mixin/css_selector.rb +++ /dev/null @@ -1,146 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - module RSpec - # Helps parsing css selector. - module CssSelector - COMMON_OPTIONS = %w[ - above below left_of right_of near count minimum maximum between text - 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 - # id?('#some-id') # => true - # id?('.some-class') # => false - def id?(selector) - selector.start_with?('#') - end - - # @param selector [String] - # @return [Boolean] - # @example - # attribute?('[attribute]') # => true - # attribute?('attribute') # => false - def attribute?(selector) - selector.start_with?('[') - end - - # @param selector [String] - # @return [Array] - # @example - # attributes('a[foo-bar_baz]') # => {"foo-bar_baz=>true} - # attributes('button[foo][bar]') # => {"foo"=>true, "bar"=>true} - # attributes('table[foo=bar]') # => {"foo"=>"'bar'"} - def attributes(selector) - selector.scan(/\[(.*?)\]/).flatten.to_h do |attr| - key, value = attr.split('=') - [key, normalize_value(value)] - end - end - - # @param selector [String] - # @return [Boolean] - # @example - # common_attributes?('a[focused]') # => true - # common_attributes?('button[focused][visible]') # => true - # common_attributes?('table[id=some-id]') # => true - # common_attributes?('h1[invalid]') # => false - def common_attributes?(selector) - attributes(selector).keys.difference(COMMON_OPTIONS).none? - end - - # @param selector [String] - # @return [Array] - # @example - # pseudo_classes('button:not([disabled])') # => ['not()'] - # pseudo_classes('a:enabled:not([valid])') # => ['enabled', 'not()'] - def pseudo_classes(selector) - # Attributes must be excluded or else the colon in the `href`s URL - # will also be picked up as pseudo classes. - # "a:not([href='http://example.com']):enabled" => "a:not():enabled" - ignored_attribute = selector.gsub(/\[.*?\]/, '') - # "a:not():enabled" => ["not()", "enabled"] - ignored_attribute.scan(/:([^:]*)/).flatten - end - - # @param selector [String] - # @return [Boolean] - # @example - # multiple_selectors?('a.cls b#id') # => true - # multiple_selectors?('a.cls') # => false - def multiple_selectors?(selector) - selector.match?(/[ >,+~]/) - end - - # @param value [String] - # @return [Boolean, String] - # @example - # normalize_value('true') # => true - # normalize_value('false') # => false - # normalize_value(nil) # => false - # normalize_value("foo") # => "'foo'" - def normalize_value(value) - case value - when 'true' then true - when 'false' then false - when nil then true - else "'#{value}'" - end - end - end - end - end -end diff --git a/rubocop-rspec.gemspec b/rubocop-rspec.gemspec index b96c36678..c72e2ce0a 100644 --- a/rubocop-rspec.gemspec +++ b/rubocop-rspec.gemspec @@ -38,4 +38,5 @@ Gem::Specification.new do |spec| } spec.add_runtime_dependency 'rubocop', '~> 1.33' + spec.add_runtime_dependency 'rubocop-capybara' end diff --git a/spec/rubocop/cop/rspec/capybara/current_path_expectation_spec.rb b/spec/rubocop/cop/rspec/capybara/current_path_expectation_spec.rb deleted file mode 100644 index 5fbed5008..000000000 --- a/spec/rubocop/cop/rspec/capybara/current_path_expectation_spec.rb +++ /dev/null @@ -1,130 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::CurrentPathExpectation do - it 'flags violations for `expect(current_path)`' do - expect_offense(<<~RUBY) - expect(current_path).to eq "/callback" - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path "/callback" - RUBY - end - - it 'flags violations for `expect(page.current_path)`' do - expect_offense(<<-RUBY) - expect(page.current_path).to eq("/callback") - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - end - - it 'registers an offense when a variable is used' do - expect_offense(<<~RUBY) - expect(current_path).to eq expected_path - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path expected_path, ignore_query: true - RUBY - end - - it 'preserves parentheses' do - expect_offense(<<~RUBY) - expect(current_path).to eq(expected_path) - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path(expected_path, ignore_query: true) - RUBY - end - - it 'registers an offense with arguments' do - expect_offense(<<~RUBY) - expect(current_path).to eq(expected_path(f: :b)) - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path(expected_path(f: :b), ignore_query: true) - RUBY - end - - it 'registers an offense with method calls' do - expect_offense(<<~RUBY) - expect(page.current_path).to eq(foo(bar).path) - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path(foo(bar).path, ignore_query: true) - RUBY - end - - it 'registers an offense with negation' do - expect_offense(<<~RUBY) - expect(current_path).not_to eq expected_path - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_no_current_path expected_path, ignore_query: true - RUBY - end - - it 'registers an offense with to_not negation alias' do - expect_offense(<<~RUBY) - expect(current_path).to_not eq expected_path - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_no_current_path expected_path, ignore_query: true - RUBY - end - - it 'registers an offense with `match`' do - expect_offense(<<~RUBY) - expect(page.current_path).to match(/regexp/i) - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_current_path(/regexp/i) - RUBY - end - - it 'registers an offense with `match` with a string argument' do - expect_offense(<<~RUBY) - expect(page.current_path).to match("string/") - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_correction(<<~'RUBY') - expect(page).to have_current_path(/string\//) - RUBY - end - - it "doesn't flag a violation for other expectations" do - expect_no_offenses(<<-RUBY) - expect(current_user).to eq(user) - RUBY - end - - it "doesn't flag a violation for other references to `current_path`" do - expect_no_offenses(<<-RUBY) - current_path = WalkingRoute.last.path - RUBY - end - - it 'ignores `match` with a variable, but does not autocorrect' do - expect_offense(<<-RUBY) - expect(page.current_path).to match(variable) - ^^^^^^ Do not set an RSpec expectation on `current_path` in Capybara feature specs - instead, use the `have_current_path` matcher on `page` - RUBY - - expect_no_corrections - end -end diff --git a/spec/rubocop/cop/rspec/capybara/match_style_spec.rb b/spec/rubocop/cop/rspec/capybara/match_style_spec.rb deleted file mode 100644 index 12600a334..000000000 --- a/spec/rubocop/cop/rspec/capybara/match_style_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::MatchStyle, :config do - it 'registers an offense when using `assert_style`' do - expect_offense(<<~RUBY) - page.find(:css, '#first').assert_style(display: 'block') - ^^^^^^^^^^^^ Use `assert_matches_style` instead of `assert_style`. - RUBY - - expect_correction(<<~RUBY) - page.find(:css, '#first').assert_matches_style(display: 'block') - RUBY - end - - it 'registers an offense when using `has_style?`' do - expect_offense(<<~RUBY) - expect(page.find(:css, 'first') - .has_style?(display: 'block')).to be true - ^^^^^^^^^^ Use `matches_style?` instead of `has_style?`. - RUBY - - expect_correction(<<~RUBY) - expect(page.find(:css, 'first') - .matches_style?(display: 'block')).to be true - RUBY - end - - it 'registers an offense when using `have_style`' do - expect_offense(<<~RUBY) - expect(page).to have_style(display: 'block') - ^^^^^^^^^^ Use `match_style` instead of `have_style`. - RUBY - - expect_correction(<<~RUBY) - expect(page).to match_style(display: 'block') - RUBY - end - - it 'does not register an offense when using `assert_matches_style`' do - expect_no_offenses(<<~RUBY) - page.find(:css, '#first').assert_matches_style(display: 'block') - RUBY - end - - it 'does not register an offense when using `matches_style?`' do - expect_no_offenses(<<~RUBY) - expect(page.find(:css, 'first').matches_style?(display: 'block')).to be true - RUBY - end - - it 'does not register an offense when using `match_style`' do - expect_no_offenses(<<~RUBY) - expect(page).to match_style(display: 'block') - RUBY - end -end diff --git a/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb deleted file mode 100644 index f84702c1d..000000000 --- a/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb +++ /dev/null @@ -1,97 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::NegationMatcher, :config do - let(:cop_config) { { 'EnforcedStyle' => enforced_style } } - - context 'with EnforcedStyle `have_no`' do - let(:enforced_style) { 'have_no' } - - %i[selector css xpath text title current_path link button - field checked_field unchecked_field select table - sibling ancestor].each do |matcher| - it 'registers an offense when using ' \ - '`expect(...).not_to have_#{matcher}`' do - expect_offense(<<~RUBY, matcher: matcher) - expect(page).not_to have_#{matcher} - ^^^^^^^^^^^^^{matcher} Use `expect(...).to have_no_#{matcher}`. - expect(page).not_to have_#{matcher}('a') - ^^^^^^^^^^^^^{matcher} Use `expect(...).to have_no_#{matcher}`. - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_no_#{matcher} - expect(page).to have_no_#{matcher}('a') - RUBY - end - - it 'does not register an offense when using ' \ - '`expect(...).to have_no_#{matcher}`' do - expect_no_offenses(<<~RUBY) - expect(page).to have_no_#{matcher} - RUBY - end - end - - it 'registers an offense when using ' \ - '`expect(...).not_to have_text` with heredoc' do - expect_offense(<<~RUBY) - expect(page).not_to have_text(exact_text: <<~TEXT) - ^^^^^^^^^^^^^^^^ Use `expect(...).to have_no_text`. - foo - TEXT - RUBY - - expect_correction(<<~RUBY) - expect(page).to have_no_text(exact_text: <<~TEXT) - foo - TEXT - RUBY - end - end - - context 'with EnforcedStyle `not_to`' do - let(:enforced_style) { 'not_to' } - - %i[selector css xpath text title current_path link button - field checked_field unchecked_field select table - sibling ancestor].each do |matcher| - it 'registers an offense when using ' \ - '`expect(...).to have_no_#{matcher}`' do - expect_offense(<<~RUBY, matcher: matcher) - expect(page).to have_no_#{matcher} - ^^^^^^^^^^^^{matcher} Use `expect(...).not_to have_#{matcher}`. - expect(page).to have_no_#{matcher}('a') - ^^^^^^^^^^^^{matcher} Use `expect(...).not_to have_#{matcher}`. - RUBY - - expect_correction(<<~RUBY) - expect(page).not_to have_#{matcher} - expect(page).not_to have_#{matcher}('a') - RUBY - end - - it 'does not register an offense when using ' \ - '`expect(...).not_to have_#{matcher}`' do - expect_no_offenses(<<~RUBY) - expect(page).not_to have_#{matcher} - RUBY - end - - it 'registers an offense when using ' \ - '`expect(...).to have_no_text` with heredoc' do - expect_offense(<<~RUBY) - expect(page).to have_no_text(exact_text: <<~TEXT) - ^^^^^^^^^^^^^^^ Use `expect(...).not_to have_text`. - foo - TEXT - RUBY - - expect_correction(<<~RUBY) - expect(page).not_to have_text(exact_text: <<~TEXT) - foo - TEXT - RUBY - end - end - end -end diff --git a/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb deleted file mode 100644 index 84ee67d26..000000000 --- a/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb +++ /dev/null @@ -1,234 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::SpecificActions, :config do - it 'does not register an offense for find and click action when ' \ - 'first argument is link' do - expect_no_offenses(<<~RUBY) - find('a').click - RUBY - end - - it 'registers an offense when using find and click action when ' \ - 'first argument is link with href' do - expect_offense(<<~RUBY) - find('a', href: 'http://example.com').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[href]").click - ^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[href='http://example.com']").click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - RUBY - end - - it 'registers an offense when using find and click action when ' \ - 'first argument is button' do - expect_offense(<<~RUBY) - find('button').click - ^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using find and click action when ' \ - 'first argument is button with class' do - expect_offense(<<~RUBY) - find('button.cls').click - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using find and click action when ' \ - 'consecutive chain methods' do - expect_offense(<<~RUBY) - find("a").find('button').click - ^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using find and click action with ' \ - 'other argument' do - expect_offense(<<~RUBY) - find('button', exact_text: 'foo').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using find and click aciton when ' \ - 'first argument is multiple selector ` `' do - expect_offense(<<~RUBY) - find('div button').click - ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is multiple selector `,`' do - expect_no_offenses(<<-RUBY) - find('button,a').click - find('a, button').click - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is multiple selector `>`' do - expect_no_offenses(<<-RUBY) - find('button>a').click - find('a > button').click - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is multiple selector `+`' do - expect_no_offenses(<<-RUBY) - find('button+a').click - find('a + button').click - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is multiple selector `~`' do - expect_no_offenses(<<-RUBY) - find('button~a').click - find('a ~ button').click - RUBY - end - - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name value - title type].each do |attr| - it 'registers an offense for abstract action when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `click_button`' do - expect_offense(<<-RUBY, attr: attr) - find("button[#{attr}=foo]").click - ^^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find("button[#{attr}]").click - ^^^^^^^^^^^^^^{attr}^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - end - - %i[above below left_of right_of near count minimum maximum between text id - class style visible obscured exact exact_text normalize_ws match wait - filter_set focused alt title download].each do |attr| - it 'does not register an offense for abstract action when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `click_link` without `href`' do - expect_no_offenses(<<-RUBY, attr: attr) - find("a[#{attr}=foo]").click - find("a[#{attr}]").click - RUBY - end - - it 'registers an offense for abstract action when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `click_link` with attribute `href`' do - expect_offense(<<-RUBY, attr: attr) - find("a[#{attr}=foo][href]").click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[#{attr}][href='http://example.com']").click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - RUBY - end - - it 'registers an offense for abstract action when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `click_link` with option `href`' do - expect_offense(<<-RUBY, attr: attr) - find("a[#{attr}=foo]", href: 'http://example.com').click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[#{attr}]", text: 'foo', href: 'http://example.com').click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - RUBY - end - end - - it 'registers an offense when using abstract action with ' \ - 'first argument is element with multiple replaceable attributes' do - expect_offense(<<-RUBY) - find('button[disabled][name="foo"]', exact_text: 'foo').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using abstract action with state' do - expect_offense(<<-RUBY) - find('button[disabled]', exact_text: 'foo').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using abstract action with ' \ - 'first argument is element with replaceable pseudo-classes' do - expect_offense(<<-RUBY) - find('button:not([disabled])', exact_text: 'bar').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find('button:not([disabled][visible])').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'registers an offense when using abstract action with ' \ - 'first argument is element with multiple replaceable pseudo-classes' do - expect_offense(<<-RUBY) - find('button:not([disabled]):enabled').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find('button:not([disabled=false]):disabled').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find('button:not([disabled]):not([disabled])').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - RUBY - end - - it 'does not register an offense when using abstract action with ' \ - 'first argument is element with replaceable pseudo-classes' \ - 'and not boolean attributes' do - expect_no_offenses(<<-RUBY) - find('button:not([name="foo"][disabled])').click - RUBY - end - - it 'does not register an offense when using abstract action with ' \ - 'first argument is element with multiple nonreplaceable pseudo-classes' do - expect_no_offenses(<<-RUBY) - find('button:first-of-type:not([disabled])').click - RUBY - end - - it 'does not register an offense for abstract action when ' \ - 'first argument is element with nonreplaceable attributes' do - expect_no_offenses(<<-RUBY) - find('button[data-disabled]').click - find('button[foo=bar]').click - find('button[foo-bar=baz]', exact_text: 'foo').click - RUBY - end - - it 'does not register an offense for abstract action when ' \ - 'first argument is element with multiple nonreplaceable attributes' do - expect_no_offenses(<<-RUBY) - find('button[disabled][foo]').click - find('button[foo][disabled]').click - find('button[foo][disabled][bar]').click - find('button[disabled][foo=bar]').click - find('button[disabled=foo][bar]', exact_text: 'foo').click - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is not a replaceable element' do - expect_no_offenses(<<-RUBY) - find('article').click - find('body').click - RUBY - end - - it 'does not register an offense for find and click aciton when ' \ - 'first argument is not an element' do - expect_no_offenses(<<-RUBY) - find('.a').click - find('#button').click - find('[a]').click - RUBY - end -end diff --git a/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb deleted file mode 100644 index fc2bda62a..000000000 --- a/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb +++ /dev/null @@ -1,125 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::SpecificFinders, :config do - it 'registers an offense when using `find`' do - expect_offense(<<~RUBY) - find('#some-id') - ^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id('some-id') - RUBY - end - - it 'registers an offense when using `find` with no parentheses' do - expect_offense(<<~RUBY) - find "#some-id" - ^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id 'some-id' - RUBY - end - - it 'registers an offense when using `find` and other args' do - expect_offense(<<~RUBY) - find('#some-id', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id('some-id', exact_text: 'foo') - RUBY - end - - it 'registers an offense when using `find` and other args ' \ - 'with no parentheses' do - expect_offense(<<~RUBY) - find '#some-id', exact_text: 'foo' - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id 'some-id', exact_text: 'foo' - RUBY - end - - it 'registers an offense when using `find` with method chain' do - expect_offense(<<~RUBY) - find('#some-id').find('#other-id').find('#another-id') - ^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - ^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - ^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id('some-id').find_by_id('other-id').find_by_id('another-id') - RUBY - end - - it 'registers an offense when using `find ' \ - 'with argument is attribute specified id' do - expect_offense(<<~RUBY) - find('[id=some-id]') - ^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - find('[visible][id=some-id]') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - find('[id=some-id][class=some-cls][focused]') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - RUBY - - expect_correction(<<~RUBY) - find_by_id('some-id') - find_by_id('some-id', visible: true) - find_by_id('some-id', class: 'some-cls', focused: true) - RUBY - end - - it 'does not register an offense when using `find ' \ - 'with argument is attribute not specified id' do - expect_no_offenses(<<~RUBY) - find('[visible]') - find('[class=some-cls][visible]') - RUBY - end - - it 'does not register an offense when using `find ' \ - 'with argument is element with id' do - expect_no_offenses(<<~RUBY) - find('h1#some-id') - RUBY - end - - it 'does not register an offense when using `find ' \ - 'with argument is element with attribute specified id' do - expect_no_offenses(<<~RUBY) - find('h1[id=some-id]') - RUBY - end - - it 'does not register an offense when using `find` ' \ - 'with argument is not id' do - expect_no_offenses(<<~RUBY) - find('a.some-id') - find('.some-id') - RUBY - end - - it 'does not register an offense when using `find_by_id`' do - expect_no_offenses(<<~RUBY) - find_by_id('some-id') - RUBY - end - - it 'does not register an offense when using `find` ' \ - 'with argument is id with multiple matcher' do - expect_no_offenses(<<~RUBY) - find('#some-id body') - find('#some-id>h1') - find('#some-id,h2') - find('#some-id+option') - RUBY - end -end diff --git a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb deleted file mode 100644 index 0a605cdd4..000000000 --- a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb +++ /dev/null @@ -1,287 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::SpecificMatcher do - it 'does not register an offense for abstract matcher when ' \ - 'first argument is not a replaceable element' do - expect_no_offenses(<<-RUBY) - expect(page).to have_selector('article') - expect(page).to have_no_selector('body') - expect(page).to have_css('tbody') - RUBY - end - - it 'does not register an offense for abstract matcher when ' \ - 'first argument is not an element' do - expect_no_offenses(<<-RUBY) - expect(page).to have_no_css('.a') - expect(page).to have_selector('#button') - expect(page).to have_no_selector('[table]') - RUBY - end - - it 'registers an offense when using `have_selector`' do - expect_offense(<<-RUBY) - expect(page).to have_selector('button') - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_selector`. - expect(page).to have_selector('table') - ^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_table` over `have_selector`. - expect(page).to have_selector('select') - ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_select` over `have_selector`. - expect(page).to have_selector('input') - ^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_selector`. - RUBY - end - - it 'registers an offense when using `have_no_selector`' do - expect_offense(<<-RUBY) - expect(page).to have_no_selector('button') - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_no_button` over `have_no_selector`. - RUBY - end - - it 'registers an offense when using `have_css`' do - expect_offense(<<-RUBY) - expect(page).to have_css('button') - ^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using `have_no_css`' do - expect_offense(<<-RUBY) - expect(page).to have_no_css('button') - ^^^^^^^^^^^^^^^^^^^^^ Prefer `have_no_button` over `have_no_css`. - RUBY - end - - it 'registers an offense when using abstract matcher and other args' do - expect_offense(<<-RUBY) - expect(page).to have_css('button', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using abstract matcher with class selector' do - expect_offense(<<-RUBY) - expect(page).to have_css('button.cls') - ^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button.cls', text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using abstract matcher with id selector' do - expect_offense(<<-RUBY) - expect(page).to have_css('button#id') - ^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button#id', text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name value - title type].each do |attr| - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_button`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("button[#{attr}=foo]") - ^^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css("button[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_button` over `have_css`. - RUBY - end - end - - %i[above below left_of right_of near count minimum maximum between text id - class style visible obscured exact exact_text normalize_ws match wait - filter_set focused alt title download].each do |attr| - it 'does not register an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_link` without `href`' do - expect_no_offenses(<<-RUBY, attr: attr) - expect(page).to have_css("a") - expect(page).to have_css("a[#{attr}=foo]") - expect(page).to have_css("a[#{attr}]") - RUBY - end - - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_link` with attribute `href`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("a[#{attr}=foo][href]") - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - expect(page).to have_css("a[#{attr}][href='http://example.com']") - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - RUBY - end - - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_link` with option `href`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("a[#{attr}=foo]", href: 'http://example.com') - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - expect(page).to have_css("a[#{attr}]", text: 'foo', href: 'http://example.com') - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - RUBY - end - end - - it 'registers an offense for abstract matcher when ' \ - 'first argument is element with replaceable attributes href ' \ - 'for `have_link`' do - expect_offense(<<-RUBY) - expect(page).to have_css("a[href]") - ^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - expect(page).to have_css("a[href='http://example.com']") - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - RUBY - end - - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused caption with_cols cols with_rows - rows].each do |attr| - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_table`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("table[#{attr}=foo]") - ^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_table` over `have_css`. - expect(page).to have_css("table[#{attr}]") - ^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_table` over `have_css`. - RUBY - end - end - - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name placeholder options - enabled_options disabled_options selected with_selected - multiple with_options].each do |attr| - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_select`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("select[#{attr}=foo]") - ^^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_select` over `have_css`. - expect(page).to have_css("select[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_select` over `have_css`. - RUBY - end - end - - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set checked unchecked disabled valid name - placeholder validation_message ].each do |attr| - it 'registers an offense for abstract matcher when ' \ - "first argument is element with replaceable attributes #{attr} " \ - 'for `have_field`' do - expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("input[#{attr}=foo]") - ^^^^^^^^^^^^^^^^^^{attr}^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css("input[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^ Prefer `have_field` over `have_css`. - RUBY - end - end - - it 'registers an offense when using abstract matcher with ' \ - 'first argument is element with multiple replaceable attributes' do - expect_offense(<<-RUBY) - expect(page).to have_css('button[disabled][name="foo"]', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using abstract matcher with state' do - expect_offense(<<-RUBY) - expect(page).to have_css('button[disabled]', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using abstract matcher with ' \ - 'first argument is element with replaceable pseudo-classes' do - expect_offense(<<-RUBY) - expect(page).to have_css('button:not([disabled])', exact_text: 'bar') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button:not([disabled][visible])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - RUBY - end - - it 'registers an offense when using abstract matcher with ' \ - 'first argument is element with multiple replaceable pseudo-classes' do - expect_offense(<<-RUBY) - expect(page).to have_css('button:not([disabled]):enabled') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button:not([disabled=false]):disabled') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button:not([disabled]):not([disabled])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('input:not([unchecked]):checked') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css('input:not([unchecked=false]):unchecked') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css('input:not([unchecked]):not([unchecked])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. - RUBY - end - - it 'does not register an offense when using abstract matcher with ' \ - 'first argument is element with replaceable pseudo-classes' \ - 'and not boolean attributes' do - expect_no_offenses(<<-RUBY) - expect(page).to have_css('button:not([name="foo"][disabled])') - RUBY - end - - it 'does not register an offense when using abstract matcher with ' \ - 'first argument is element with multiple nonreplaceable pseudo-classes' do - expect_no_offenses(<<-RUBY) - expect(page).to have_css('button:first-of-type:not([disabled])') - RUBY - end - - it 'does not register an offense for abstract matcher when ' \ - 'first argument is element with nonreplaceable attributes' do - expect_no_offenses(<<-RUBY) - expect(page).to have_css('button[data-disabled]') - expect(page).to have_css('button[foo=bar]') - expect(page).to have_css('button[foo-bar=baz]', exact_text: 'foo') - RUBY - end - - it 'does not register an offense for abstract matcher when ' \ - 'first argument is element with multiple nonreplaceable attributes' do - expect_no_offenses(<<-RUBY) - expect(page).to have_css('button[disabled][foo]') - expect(page).to have_css('button[foo][disabled]') - expect(page).to have_css('button[foo][disabled][bar]') - expect(page).to have_css('button[disabled][foo=bar]') - expect(page).to have_css('button[disabled=foo][bar]', exact_text: 'foo') - RUBY - end - - it 'does not register an offense for abstract matcher when ' \ - 'first argument is element with sub matcher' do - expect_no_offenses(<<-RUBY) - expect(page).to have_css('button body') - expect(page).to have_css('a,h1') - expect(page).to have_css('table>tr') - expect(page).to have_css('select+option') - RUBY - end - - it 'does not register an offense for abstract matcher when ' \ - 'first argument is dstr' do - expect_no_offenses(<<-'RUBY') - expect(page).to have_css(%{a[href="#{foo}"]}, text: "bar") - RUBY - end -end diff --git a/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb deleted file mode 100644 index c6c493679..000000000 --- a/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::RSpec::Capybara::VisibilityMatcher do - it 'registers an offense when using `visible: true`' do - expect_offense(<<-RUBY) - expect(page).to have_selector('.my_element', visible: true) - ^^^^^^^^^^^^^ Use `:visible` instead of `true`. - RUBY - end - - it 'registers an offense when using `visible: false`' do - expect_offense(<<-RUBY) - expect(page).to have_selector('.my_element', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - RUBY - end - - it 'recognizes multiple matchers' do - expect_offense(<<-RUBY) - expect(page).to have_css('.profile', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_xpath('.//profile', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_link('news', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_button('login', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_field('name', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_select('sauce', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_table('arrivals', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_checked_field('cat', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_unchecked_field('cat', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - RUBY - end - - it 'recognizes multiple negative matchers' do - expect_offense(<<-RUBY) - expect(page).to have_no_css('.profile', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_xpath('.//profile', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_link('news', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_button('login', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_field('name', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_select('sauce', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_table('arrivals', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_checked_field('cat', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - expect(page).to have_no_unchecked_field('cat', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - RUBY - end - - it 'registers an offense when using a selector`' do - expect_offense(<<-RUBY) - expect(page).to have_selector(:css, '.my_element', visible: false) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - RUBY - end - - it 'registers an offense when using a using multiple options`' do - expect_offense(<<-RUBY) - expect(page).to have_selector('.my_element', count: 1, visible: false, normalize_ws: true) - ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. - RUBY - end - - it 'does not register an offense when no options are given`' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element') - RUBY - end - - it 'does not register an offense when using `visible: :all`' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element', visible: :all) - RUBY - end - - it 'does not register an offense when using `visible: :visible`' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element', visible: :visible) - RUBY - end - - it 'does not register an offense when using `visible: :hidden`' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element', visible: :hidden) - RUBY - end - - it 'does not register an offense when using other options' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element', normalize_ws: true) - RUBY - end - - it 'does not register an offense when using multiple options' do - expect_no_offenses(<<~RUBY) - expect(page).to have_selector('.my_element', count: 1, normalize_ws: true) - RUBY - end -end From 15954bdc05a7cc83ec83a38eb5eb9dd102b64854 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 18 Dec 2022 14:40:26 +0300 Subject: [PATCH 2/5] Fix default conf spec `load_file` throws obsoletion errors --- spec/project/default_config_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/project/default_config_spec.rb b/spec/project/default_config_spec.rb index 1adbbdb68..08f45b60f 100644 --- a/spec/project/default_config_spec.rb +++ b/spec/project/default_config_spec.rb @@ -2,7 +2,7 @@ RSpec.describe 'config/default.yml' do subject(:default_config) do - RuboCop::ConfigLoader.load_file('config/default.yml') + RuboCop::ConfigLoader.load_yaml_configuration('config/default.yml') end let(:namespaces) do From cc98af08285305b17f5fe876de64570d4124c1fc Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 8 Jan 2023 14:27:07 +0300 Subject: [PATCH 3/5] Fix obsoletion errors by only enlisting the new cop ...and soft-aliasing the old one with a constant pointing to a class --- .../ROOT/pages/cops_rspec_capybara.adoc | 13 +++-- .../capybara/current_path_expectation.rb | 56 +++++++++---------- lib/rubocop/cop/rspec/capybara/match_style.rb | 56 +++++++++---------- .../cop/rspec/capybara/negation_matcher.rb | 46 +++++++-------- .../cop/rspec/capybara/specific_actions.rb | 38 ++++++------- .../cop/rspec/capybara/specific_finders.rb | 28 +++++----- .../cop/rspec/capybara/specific_matcher.rb | 50 ++++++++--------- .../cop/rspec/capybara/visibility_matcher.rb | 51 ++++++++--------- lib/rubocop/rspec/config_formatter.rb | 10 ++++ tasks/cops_documentation.rake | 39 +++++++++++-- 10 files changed, 215 insertions(+), 172 deletions(-) diff --git a/docs/modules/ROOT/pages/cops_rspec_capybara.adoc b/docs/modules/ROOT/pages/cops_rspec_capybara.adoc index 25f7f9950..bb87b6b6e 100644 --- a/docs/modules/ROOT/pages/cops_rspec_capybara.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_capybara.adoc @@ -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]. diff --git a/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb b/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb index d1e420647..41fcd2170 100644 --- a/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb +++ b/lib/rubocop/cop/rspec/capybara/current_path_expectation.rb @@ -4,35 +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') - # - # @!parse class CurrentPathExpectation < ::RuboCop::Cop::Base; end - class CurrentPathExpectation < + # @!parse + # # 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 end diff --git a/lib/rubocop/cop/rspec/capybara/match_style.rb b/lib/rubocop/cop/rspec/capybara/match_style.rb index 4a0062e66..b18102d45 100644 --- a/lib/rubocop/cop/rspec/capybara/match_style.rb +++ b/lib/rubocop/cop/rspec/capybara/match_style.rb @@ -4,34 +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') - # - # @!parse class MatchStyle < ::RuboCop::Cop::Base; end - class MatchStyle < ::RuboCop::Cop::Capybara::MatchStyle - 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 diff --git a/lib/rubocop/cop/rspec/capybara/negation_matcher.rb b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb index 0c73807ef..7c9c51702 100644 --- a/lib/rubocop/cop/rspec/capybara/negation_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb @@ -4,29 +4,29 @@ module RuboCop module Cop module RSpec module Capybara - # Enforces use of `have_no_*` or `not_to` for negated expectations. - # - # @example EnforcedStyle: not_to (default) - # # bad - # expect(page).to have_no_selector - # expect(page).to have_no_css('a') - # - # # good - # expect(page).not_to have_selector - # expect(page).not_to have_css('a') - # - # @example EnforcedStyle: have_no - # # bad - # expect(page).not_to have_selector - # expect(page).not_to have_css('a') - # - # # good - # expect(page).to have_no_selector - # expect(page).to have_no_css('a') - # - # @!parse class NegationMatcher < ::RuboCop::Cop::Base; end - class NegationMatcher < ::RuboCop::Cop::Capybara::NegationMatcher - end + # @!parse + # # Enforces use of `have_no_*` or `not_to` for negated expectations. + # # + # # @example EnforcedStyle: not_to (default) + # # # bad + # # expect(page).to have_no_selector + # # expect(page).to have_no_css('a') + # # + # # # good + # # expect(page).not_to have_selector + # # expect(page).not_to have_css('a') + # # + # # @example EnforcedStyle: have_no + # # # bad + # # expect(page).not_to have_selector + # # expect(page).not_to have_css('a') + # # + # # # good + # # expect(page).to have_no_selector + # # expect(page).to have_no_css('a') + # # + # class NegationMatcher < ::RuboCop::Cop::Base; end + NegationMatcher = ::RuboCop::Cop::Capybara::NegationMatcher end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_actions.rb b/lib/rubocop/cop/rspec/capybara/specific_actions.rb index e245c2bdc..93cea5a7f 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_actions.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_actions.rb @@ -4,25 +4,25 @@ 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 - # - # @!parse class SpecificActions < ::RuboCop::Cop::Base; end - class SpecificActions < ::RuboCop::Cop::Capybara::SpecificActions - end + # @!parse + # # 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 < ::RuboCop::Cop::Base; end + SpecificActions = ::RuboCop::Cop::Capybara::SpecificActions end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_finders.rb b/lib/rubocop/cop/rspec/capybara/specific_finders.rb index bac35d3a0..41c346e5a 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_finders.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_finders.rb @@ -4,20 +4,20 @@ module RuboCop module Cop module RSpec module Capybara - # Checks if there is a more specific finder offered by Capybara. - # - # @example - # # bad - # find('#some-id') - # find('[visible][id=some-id]') - # - # # good - # find_by_id('some-id') - # find_by_id('some-id', visible: true) - # - # @!parse class SpecificFinders < ::RuboCop::Cop::Base; end - class SpecificFinders < ::RuboCop::Cop::Capybara::SpecificFinders - end + # @!parse + # # Checks if there is a more specific finder offered by Capybara. + # # + # # @example + # # # bad + # # find('#some-id') + # # find('[visible][id=some-id]') + # # + # # # good + # # find_by_id('some-id') + # # find_by_id('some-id', visible: true) + # # + # class SpecificFinders < ::RuboCop::Cop::Base; end + SpecificFinders = ::RuboCop::Cop::Capybara::SpecificFinders end end end diff --git a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb index f8ecd6075..e6f5fa4d6 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb @@ -4,31 +4,31 @@ module RuboCop module Cop module RSpec 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') - # - # # 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') - # - # @!parse class SpecificMatcher < ::RuboCop::Cop::Base; end - class SpecificMatcher < ::RuboCop::Cop::Capybara::SpecificMatcher - end + # @!parse + # # 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; end + SpecificMatcher = ::RuboCop::Cop::Capybara::SpecificMatcher end end end diff --git a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb index e9bd5379d..bb63f8b73 100644 --- a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb @@ -4,31 +4,32 @@ module RuboCop module Cop module RSpec 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) - # - # @!parse class VisibilityMatcher < ::RuboCop::Cop::Base; end - class VisibilityMatcher < ::RuboCop::Cop::Capybara::VisibilityMatcher - end + # @!parse + # # 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; end + VisibilityMatcher = ::RuboCop::Cop::Capybara::VisibilityMatcher end end end diff --git a/lib/rubocop/rspec/config_formatter.rb b/lib/rubocop/rspec/config_formatter.rb index 443daca56..2d7553c86 100644 --- a/lib/rubocop/rspec/config_formatter.rb +++ b/lib/rubocop/rspec/config_formatter.rb @@ -8,6 +8,15 @@ module RSpec class ConfigFormatter EXTENSION_ROOT_DEPARTMENT = %r{^(RSpec/)}.freeze SUBDEPARTMENTS = %(RSpec/Capybara RSpec/FactoryBot RSpec/Rails) + EXTRACTED_COPS = %( + RSpec/Capybara/CurrentPathExpectation + RSpec/Capybara/MatchStyle + RSpec/Capybara/NegationMatcher + RSpec/Capybara/SpecificActions + RSpec/Capybara/SpecificFinders + RSpec/Capybara/SpecificMatcher + RSpec/Capybara/VisibilityMatcher + ) AMENDMENTS = %(Metrics/BlockLength) COP_DOC_BASE_URL = 'https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/' @@ -29,6 +38,7 @@ def dump def unified_config cops.each_with_object(config.dup) do |cop, unified| next if SUBDEPARTMENTS.include?(cop) || AMENDMENTS.include?(cop) + next if EXTRACTED_COPS.include?(cop) replace_nil(unified[cop]) unified[cop].merge!(descriptions.fetch(cop)) diff --git a/tasks/cops_documentation.rake b/tasks/cops_documentation.rake index 897ad7ad7..3cf06e777 100644 --- a/tasks/cops_documentation.rake +++ b/tasks/cops_documentation.rake @@ -12,10 +12,41 @@ end desc 'Generate docs of all cops departments' task generate_cops_documentation: :yard_for_generate_documentation do - generator = CopsDocumentationGenerator.new( - departments: %w[RSpec/Capybara RSpec/FactoryBot RSpec/Rails RSpec] - ) - generator.call + RuboCop::Cop::Registry.with_temporary_global do + global = RuboCop::Cop::Registry.global + # We set a constant for the old cop class, and this skips the + # `Base.inherited` hook that enlists those old cops. + %w[ + RuboCop::Cop::RSpec::Capybara::CurrentPathExpectation + RuboCop::Cop::RSpec::Capybara::MatchStyle + RuboCop::Cop::RSpec::Capybara::NegationMatcher + RuboCop::Cop::RSpec::Capybara::SpecificActions + RuboCop::Cop::RSpec::Capybara::SpecificFinders + RuboCop::Cop::RSpec::Capybara::SpecificMatcher + RuboCop::Cop::RSpec::Capybara::VisibilityMatcher + ].each do |extracted_cop| + cop = Class.const_get(extracted_cop) + class << cop + def badge + RuboCop::Cop::Badge.for(name) + end + + def name + super.sub('::Capybara::', '::RSpec::Capybara::') + end + + def department + :'RSpec/Capybara' + end + end + global.enlist(cop) + end + + generator = CopsDocumentationGenerator.new( + departments: %w[RSpec/Capybara RSpec/FactoryBot RSpec/Rails RSpec] + ) + generator.call + end end desc 'Syntax check for the documentation comments' From 3799e63e1fb5941e4f6a72f0f6d03be4aa5d2c37 Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Sat, 14 Jan 2023 22:08:23 +0900 Subject: [PATCH 4/5] Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc This PR is fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc. The following code: ```ruby expect(foo) .to be_something(<<~TEXT) bar TEXT ``` This would be corrected as follows: ```ruby expect(foo.something?(<<~TEXT)) .to be(true) bar TEXT ``` --- CHANGELOG.md | 1 + docs/modules/ROOT/pages/cops_rspec.adoc | 11 +++ lib/rubocop/cop/rspec/predicate_matcher.rb | 25 +++++- .../cop/rspec/predicate_matcher_spec.rb | 87 +++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be7388822..fd78cb5db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - 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]) +- Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc. ([@ydah]) ## 2.17.0 (2023-01-13) diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 71d5c3c9d..ccda069ce 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -3968,6 +3968,17 @@ expect(foo).to be_something # good - the above code is rewritten to it by this cop expect(foo.something?).to be(true) + +# bad - no autocorrect +expect(foo) + .to be_something(<<~TEXT) + bar + TEXT + +# good +expect(foo.something?(<<~TEXT)).to be(true) + bar +TEXT ---- ==== Strict: false, EnforcedStyle: explicit diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index 62fa497b3..872e823cc 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -118,7 +118,7 @@ def true?(to_symbol, matcher) end # A helper for `explicit` style - module ExplicitHelper + module ExplicitHelper # rubocop:disable Metrics/ModuleLength include RuboCop::RSpec::Language extend NodePattern::Macros @@ -150,11 +150,23 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength predicate_matcher?(node) do |actual, matcher| add_offense(node, message: message_explicit(matcher)) do |corrector| + next if uncorrectable_matcher?(node, matcher) + corrector_explicit(corrector, node, actual, matcher, matcher) end end end + def uncorrectable_matcher?(node, matcher) + heredoc_argument?(matcher) && !same_line?(node, matcher) + end + + def heredoc_argument?(matcher) + matcher.arguments.select do |arg| + %i[str dstr xstr].include?(arg.type) + end.any?(&:heredoc?) + end + # @!method predicate_matcher?(node) def_node_matcher :predicate_matcher?, <<-PATTERN (send @@ -271,6 +283,17 @@ def replacement_matcher(node) # # good - the above code is rewritten to it by this cop # expect(foo.something?).to be(true) # + # # bad - no autocorrect + # expect(foo) + # .to be_something(<<~TEXT) + # bar + # TEXT + # + # # good + # expect(foo.something?(<<~TEXT)).to be(true) + # bar + # TEXT + # # @example Strict: false, EnforcedStyle: explicit # # bad # expect(foo).to be_something diff --git a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb index ac80e32fc..149a45c6e 100644 --- a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb @@ -93,6 +93,21 @@ RUBY end + it 'registers an offense for a predicate method with heredoc' do + expect_offense(<<~RUBY) + expect(foo.include?(<<~TEXT)).to be_truthy + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `include` matcher over `include?`. + bar + TEXT + RUBY + + expect_correction(<<~RUBY) + expect(foo).to include(<<~TEXT) + bar + TEXT + RUBY + end + it 'registers an offense for a predicate method with a block' do expect_offense(<<~RUBY) expect(foo.all?(&:present?)).to be_truthy @@ -312,6 +327,78 @@ RUBY end + it 'registers an offense for a predicate method with heredoc' do + expect_offense(<<~RUBY) + expect(foo).to include(<<~TEXT) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + bar + TEXT + RUBY + + expect_correction(<<~RUBY) + expect(foo.include?(<<~TEXT)).to #{matcher_true} + bar + TEXT + RUBY + end + + it 'registers an offense for a predicate method with ' \ + 'heredoc and multiline expect' do + expect_offense(<<~RUBY) + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(<<~TEXT) + bar + TEXT + + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(bar, <<~TEXT, 'baz') + bar + TEXT + RUBY + + expect_no_corrections + end + + it 'registers an offense for a predicate method with ' \ + 'heredoc include #{} and multiline expect' do + expect_offense(<<~'RUBY') + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(<<~TEXT) + #{bar} + TEXT + + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(bar, <<~TEXT, 'baz') + #{bar} + TEXT + RUBY + + expect_no_corrections + end + + it 'registers an offense for a predicate method with ' \ + 'heredoc surrounded by back ticks and multiline expect' do + expect_offense(<<~'RUBY') + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(<<~`COMMAND`) + pwd + COMMAND + + expect(foo) + ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. + .to include(bar, <<~COMMAND, 'baz') + pwd + COMMAND + RUBY + + expect_no_corrections + end + it 'registers an offense for a predicate method with a block' do expect_offense(<<~RUBY) expect(foo).to be_all { |x| x.present? } From 5113d6f96ad536979710b361e3d6c101daa84da6 Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Sun, 15 Jan 2023 07:16:03 +0900 Subject: [PATCH 5/5] Fix a false positive for `RSpec/PredicateMatcher` when `include` with multiple arguments This PR is fix a false positive for `RSpec/PredicateMatcher` when `include` with multiple argument following code: ```ruby expect(foo).to include(foo, bar) ``` `RSpec/PredicateMatcher` autocorrects it as follows: ```ruby expect(foo.include?(foo, bar)).to be(true) ``` However, this is a SyntaxError. ``` ArgumentError: wrong number of arguments (given 2, expected 1) ``` --- CHANGELOG.md | 1 + lib/rubocop/cop/rspec/predicate_matcher.rb | 11 +++++ .../cop/rspec/predicate_matcher_spec.rb | 44 ++++++++++++------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd78cb5db..b7d0d72da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Fix a false positive for `RSpec/ContextMethod` when multi-line context with `#` at the beginning. ([@ydah]) - Extract Capybara cops to a separate repository. ([@pirj]) - Fix an incorrect autocorrect for `RSpec/PredicateMatcher` when multiline expect and predicate method with heredoc. ([@ydah]) +- Fix a false positive for `RSpec/PredicateMatcher` when `include` with multiple argument. ([@ydah]) ## 2.17.0 (2023-01-13) diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index 872e823cc..e49461b9e 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -149,6 +149,8 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength return if part_of_ignored_node?(node) predicate_matcher?(node) do |actual, matcher| + next unless replaceable_matcher?(matcher) + add_offense(node, message: message_explicit(matcher)) do |corrector| next if uncorrectable_matcher?(node, matcher) @@ -157,6 +159,15 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength end end + def replaceable_matcher?(matcher) + case matcher.method_name.to_s + when 'include' + matcher.arguments.one? + else + true + end + end + def uncorrectable_matcher?(node, matcher) heredoc_argument?(matcher) && !same_line?(node, matcher) end diff --git a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb index 149a45c6e..24b4351a4 100644 --- a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb @@ -95,14 +95,14 @@ it 'registers an offense for a predicate method with heredoc' do expect_offense(<<~RUBY) - expect(foo.include?(<<~TEXT)).to be_truthy - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `include` matcher over `include?`. + expect(foo.something?(<<~TEXT)).to be_truthy + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`. bar TEXT RUBY expect_correction(<<~RUBY) - expect(foo).to include(<<~TEXT) + expect(foo).to be_something(<<~TEXT) bar TEXT RUBY @@ -346,14 +346,14 @@ 'heredoc and multiline expect' do expect_offense(<<~RUBY) expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(<<~TEXT) + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(<<~TEXT) bar TEXT expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(bar, <<~TEXT, 'baz') + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(bar, <<~TEXT, 'baz') bar TEXT RUBY @@ -365,14 +365,14 @@ 'heredoc include #{} and multiline expect' do expect_offense(<<~'RUBY') expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(<<~TEXT) + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(<<~TEXT) #{bar} TEXT expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(bar, <<~TEXT, 'baz') + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(bar, <<~TEXT, 'baz') #{bar} TEXT RUBY @@ -384,14 +384,14 @@ 'heredoc surrounded by back ticks and multiline expect' do expect_offense(<<~'RUBY') expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(<<~`COMMAND`) + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(<<~`COMMAND`) pwd COMMAND expect(foo) - ^^^^^^^^^^^ Prefer using `include?` over `include` matcher. - .to include(bar, <<~COMMAND, 'baz') + ^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. + .to be_something(bar, <<~COMMAND, 'baz') pwd COMMAND RUBY @@ -399,6 +399,20 @@ expect_no_corrections end + it 'does not register an offense for a `include` ' \ + 'with no argument' do + expect_no_offenses(<<~RUBY) + expect(foo).to include + RUBY + end + + it 'does not register an offense for a `include` ' \ + 'with multiple arguments' do + expect_no_offenses(<<~RUBY) + expect(foo).to include(foo, bar) + RUBY + end + it 'registers an offense for a predicate method with a block' do expect_offense(<<~RUBY) expect(foo).to be_all { |x| x.present? }