From 80799c2c9a32109e18e9fffc5ae3ff024704a64f Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Sat, 15 Oct 2022 11:48:43 +0900 Subject: [PATCH] Add new `RSpec/Capybara/NegationMatcher` cop Resolve: https://github.com/rubocop/rubocop-rspec/issues/378 --- .rubocop.yml | 2 + CHANGELOG.md | 1 + config/default.yml | 10 ++ docs/modules/ROOT/pages/cops.adoc | 1 + .../ROOT/pages/cops_rspec_capybara.adoc | 56 ++++++++++ .../cop/rspec/capybara/negation_matcher.rb | 105 ++++++++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + .../rspec/capybara/negation_matcher_spec.rb | 97 ++++++++++++++++ 8 files changed, 273 insertions(+) create mode 100644 lib/rubocop/cop/rspec/capybara/negation_matcher.rb create mode 100644 spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 8a0d8885e..ba382ebf0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -124,6 +124,8 @@ RSpec/SubjectDeclaration: Enabled: true RSpec/VerifiedDoubleReference: Enabled: true +RSpec/Capybara/NegationMatcher: + Enabled: true RSpec/Capybara/SpecificFinders: Enabled: true RSpec/Capybara/SpecificMatcher: diff --git a/CHANGELOG.md b/CHANGELOG.md index 57318ba7e..6645ca1dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Add `require_implicit` style to `RSpec/ImplicitSubject`. ([@r7kamura][]) * Fix a false positive for `RSpec/Capybara/SpecificMatcher` when `have_css("a")` without attribute. ([@ydah][]) * Update `RSpec/ExampleWording` cop to raise error for insufficient descriptions. ([@akrox58][]) +* Add new `RSpec/Capybara/NegationMatcher` cop. ([@ydah][]) ## 2.13.2 (2022-09-23) diff --git a/config/default.yml b/config/default.yml index 86d99a964..f0e0c8d04 100644 --- a/config/default.yml +++ b/config/default.yml @@ -869,6 +869,16 @@ RSpec/Capybara/FeatureMethods: VersionChanged: '2.0' Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/FeatureMethods +RSpec/Capybara/NegationMatcher: + Description: Enforces use of `have_no_*` or `not_to` for negated expectations. + Enabled: pending + VersionAdded: '2.14' + EnforcedStyle: not_to + SupportedStyles: + - have_no + - not_to + Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher + RSpec/Capybara/SpecificFinders: Description: Checks if there is a more specific finder offered by Capybara. Enabled: pending diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index c5df14b87..15d8c56ee 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -95,6 +95,7 @@ * xref:cops_rspec_capybara.adoc#rspeccapybara/currentpathexpectation[RSpec/Capybara/CurrentPathExpectation] * xref:cops_rspec_capybara.adoc#rspeccapybara/featuremethods[RSpec/Capybara/FeatureMethods] +* xref:cops_rspec_capybara.adoc#rspeccapybara/negationmatcher[RSpec/Capybara/NegationMatcher] * xref:cops_rspec_capybara.adoc#rspeccapybara/specificfinders[RSpec/Capybara/SpecificFinders] * xref:cops_rspec_capybara.adoc#rspeccapybara/specificmatcher[RSpec/Capybara/SpecificMatcher] * xref:cops_rspec_capybara.adoc#rspeccapybara/visibilitymatcher[RSpec/Capybara/VisibilityMatcher] diff --git a/docs/modules/ROOT/pages/cops_rspec_capybara.adoc b/docs/modules/ROOT/pages/cops_rspec_capybara.adoc index 47b9c9d1f..48dd3fc2e 100644 --- a/docs/modules/ROOT/pages/cops_rspec_capybara.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_capybara.adoc @@ -112,6 +112,62 @@ end * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/FeatureMethods +== RSpec/Capybara/NegationMatcher + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Yes +| 2.14 +| - +|=== + +Enforces use of `have_no_*` or `not_to` for negated expectations. + +=== Examples + +==== EnforcedStyle: not_to (default) + +[source,ruby] +---- +# 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') +---- + +==== EnforcedStyle: have_no + +[source,ruby] +---- +# 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') +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| EnforcedStyle +| `not_to` +| `have_no`, `not_to` +|=== + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher + == RSpec/Capybara/SpecificFinders |=== diff --git a/lib/rubocop/cop/rspec/capybara/negation_matcher.rb b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb new file mode 100644 index 000000000..2d6b9acb2 --- /dev/null +++ b/lib/rubocop/cop/rspec/capybara/negation_matcher.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +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') + # + class NegationMatcher < 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 + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index a4d0021a4..0b64456f2 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -2,6 +2,7 @@ require_relative 'rspec/capybara/current_path_expectation' require_relative 'rspec/capybara/feature_methods' +require_relative 'rspec/capybara/negation_matcher' require_relative 'rspec/capybara/specific_finders' require_relative 'rspec/capybara/specific_matcher' require_relative 'rspec/capybara/visibility_matcher' diff --git a/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb new file mode 100644 index 000000000..f84702c1d --- /dev/null +++ b/spec/rubocop/cop/rspec/capybara/negation_matcher_spec.rb @@ -0,0 +1,97 @@ +# 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