diff --git a/CHANGELOG.md b/CHANGELOG.md index a93ca6537..b911f161d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## Master (Unreleased) - Fix `RSpec/VoidExpect` to only operate inside an example block. ([@corsonknowles]) +- Change `RSpec/ContextWording` cop to always report an offense when both `Prefixes` and `AllowedPatterns` are empty. ([@ydah]) +- Fix an error for `RSpec/ChangeByZero` when `change (...) .by (0)` and `change (...)`, concatenated with `and` and `or`. ([@ydah]) ## 3.1.0 (2024-10-01) diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 5556dd086..20015bc0a 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -720,6 +720,9 @@ the configuration to meet project needs. Other acceptable prefixes may include `if`, `unless`, `for`, `before`, `after`, or `during`. They may consist of multiple words if desired. +If both `Prefixes` and `AllowedPatterns` are empty, this cop will always +report an offense. So you need to set at least one of them. + This cop can be customized allowed context description pattern with `AllowedPatterns`. By default, there are no checking by pattern. diff --git a/lib/rubocop/cop/rspec/change_by_zero.rb b/lib/rubocop/cop/rspec/change_by_zero.rb index 1ee9a40b7..75d442c75 100644 --- a/lib/rubocop/cop/rspec/change_by_zero.rb +++ b/lib/rubocop/cop/rspec/change_by_zero.rb @@ -118,7 +118,11 @@ def register_offense(node, change_node) # rubocop:enable Metrics/MethodLength def compound_expectations?(node) - %i[and or & |].include?(node.parent.method_name) + if node.parent.send_type? + %i[and or & |].include?(node.parent.method_name) + else + node.parent.and_type? || node.parent.or_type? + end end def message(change_node) diff --git a/lib/rubocop/cop/rspec/context_wording.rb b/lib/rubocop/cop/rspec/context_wording.rb index 693c5c185..3fdba92f6 100644 --- a/lib/rubocop/cop/rspec/context_wording.rb +++ b/lib/rubocop/cop/rspec/context_wording.rb @@ -12,6 +12,9 @@ module RSpec # # @see http://www.betterspecs.org/#contexts # + # If both `Prefixes` and `AllowedPatterns` are empty, this cop will always + # report an offense. So you need to set at least one of them. + # # @example `Prefixes` configuration # # .rubocop.yml # # RSpec/ContextWording: @@ -58,7 +61,9 @@ module RSpec class ContextWording < Base include AllowedPattern - MSG = 'Context description should match %s.' + MSG_MATCH = 'Context description should match %s.' + MSG_ALWAYS = 'Current settings will always report an offense. Please ' \ + 'add allowed words to `Prefixes` or `AllowedPatterns`.' # @!method context_wording(node) def_node_matcher :context_wording, <<~PATTERN @@ -67,8 +72,7 @@ class ContextWording < Base def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler context_wording(node) do |context| - if bad_pattern?(context) - message = format(MSG, patterns: expect_patterns) + unless matches_allowed_pattern?(description(context)) add_offense(context, message: message) end end @@ -84,12 +88,6 @@ def prefix_regexes @prefix_regexes ||= prefixes.map { |pre| /^#{Regexp.escape(pre)}\b/ } end - def bad_pattern?(node) - return false if allowed_patterns.empty? - - !matches_allowed_pattern?(description(node)) - end - def description(context) if context.xstr_type? context.value.value @@ -98,6 +96,14 @@ def description(context) end end + def message + if allowed_patterns.empty? + MSG_ALWAYS + else + format(MSG_MATCH, patterns: expect_patterns) + end + end + def expect_patterns inspected = allowed_patterns.map do |pattern| pattern.inspect.gsub(/\A"|"\z/, '/') diff --git a/spec/rubocop/cop/rspec/change_by_zero_spec.rb b/spec/rubocop/cop/rspec/change_by_zero_spec.rb index 5f313895b..4e9604768 100644 --- a/spec/rubocop/cop/rspec/change_by_zero_spec.rb +++ b/spec/rubocop/cop/rspec/change_by_zero_spec.rb @@ -68,6 +68,10 @@ expect { foo }.to change { Foo.bar }.by(0).and change { Foo.baz }.by(0) ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change(Foo, :bar).by(0).and change(Foo, :baz) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. end RUBY @@ -84,6 +88,10 @@ expect { foo }.to change { Foo.bar }.by(0) & change { Foo.baz }.by(0) ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change(Foo, :bar).by(0) & change(Foo, :baz) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar } & change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. end RUBY @@ -100,6 +108,10 @@ expect { foo }.to change { Foo.bar }.by(0).or change { Foo.baz }.by(0) ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change(Foo, :bar).by(0).or change(Foo, :baz) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.or change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. end RUBY @@ -116,6 +128,10 @@ expect { foo }.to change { Foo.bar }.by(0) | change { Foo.baz }.by(0) ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change(Foo, :bar) | change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. + expect { foo }.to change { Foo.bar }.by(0) | change { Foo.baz } + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer negated matchers with compound expectations over `change.by(0)`. end RUBY @@ -244,6 +260,14 @@ ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. .and change { Foo.baz }.by(0) ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + expect { foo } + .to change(Foo, :bar) + .and change(Foo, :baz).by(0) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. + expect { foo } + .to change { Foo.bar } + .and change { Foo.baz }.by(0) + ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `not_change` with compound expectations over `change.by(0)`. end RUBY @@ -255,6 +279,12 @@ expect { foo } .to not_change { Foo.bar } .and not_change { Foo.baz } + expect { foo } + .to change(Foo, :bar) + .and not_change(Foo, :baz) + expect { foo } + .to change { Foo.bar } + .and not_change { Foo.baz } end RUBY end diff --git a/spec/rubocop/cop/rspec/context_wording_spec.rb b/spec/rubocop/cop/rspec/context_wording_spec.rb index 5896110af..de3394b53 100644 --- a/spec/rubocop/cop/rspec/context_wording_spec.rb +++ b/spec/rubocop/cop/rspec/context_wording_spec.rb @@ -214,4 +214,18 @@ end end end + + context 'when `AllowedPatterns:` and `Prefixes:` are both empty' do + let(:cop_config) do + { 'Prefixes' => [], 'AllowedPatterns' => [] } + end + + it 'always registers an offense' do + expect_offense(<<~RUBY) + context 'this is an incorrect context' do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Current settings will always report an offense. Please add allowed words to `Prefixes` or `AllowedPatterns`. + end + RUBY + end + end end