From d3e4268136201f60deba051939b5b92e6f2c3168 Mon Sep 17 00:00:00 2001 From: Dave Corson-Knowles Date: Sun, 13 Oct 2024 05:22:40 -0700 Subject: [PATCH 1/3] Complete branch coverage for VoidExpect and ContextWording --- spec/rubocop/cop/rspec/context_wording_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/rubocop/cop/rspec/context_wording_spec.rb b/spec/rubocop/cop/rspec/context_wording_spec.rb index 5896110af..340d10181 100644 --- a/spec/rubocop/cop/rspec/context_wording_spec.rb +++ b/spec/rubocop/cop/rspec/context_wording_spec.rb @@ -214,4 +214,17 @@ end end end + + context 'when `AllowedPatterns:` and `Prefixes:` are both empty' do + let(:cop_config) do + { 'Prefixes' => [], 'AllowedPatterns' => [] } + end + + it 'skips any description' do + expect_no_offenses(<<~RUBY) + context 'arbitrary text' do + end + RUBY + end + end end From 5a75070cf80910788fb6523094cfa007cb27f600 Mon Sep 17 00:00:00 2001 From: ydah Date: Tue, 22 Oct 2024 13:56:33 +0900 Subject: [PATCH 2/3] Change `RSpec/ContextWording` cop to always report an offense when both `Prefixes` and `AllowedPatterns` are empty see: https://github.com/rubocop/rubocop-rspec/pull/1972#discussion_r1808522184 --- CHANGELOG.md | 2 ++ docs/modules/ROOT/pages/cops_rspec.adoc | 3 +++ lib/rubocop/cop/rspec/context_wording.rb | 24 ++++++++++++------- .../rubocop/cop/rspec/context_wording_spec.rb | 7 +++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93499ceaf..cd89a961c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Change `RSpec/ContextWording` cop to always report an offense when both `Prefixes` and `AllowedPatterns` are empty. ([@ydah]) + ## 3.1.0 (2024-10-01) - Add `RSpec/StringAsInstanceDoubleConstant` to check for and correct strings used as instance_doubles. ([@corsonknowles]) 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/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/context_wording_spec.rb b/spec/rubocop/cop/rspec/context_wording_spec.rb index 340d10181..de3394b53 100644 --- a/spec/rubocop/cop/rspec/context_wording_spec.rb +++ b/spec/rubocop/cop/rspec/context_wording_spec.rb @@ -220,9 +220,10 @@ { 'Prefixes' => [], 'AllowedPatterns' => [] } end - it 'skips any description' do - expect_no_offenses(<<~RUBY) - context 'arbitrary text' do + 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 From 130c5641f1a1afff88513d0a8278656d157ab88f Mon Sep 17 00:00:00 2001 From: ydah Date: Wed, 23 Oct 2024 21:28:04 +0900 Subject: [PATCH 3/3] Fix an error for `RSpec/ChangeByZero` when `change (...) .by (0)` and `change (...)`, concatenated with `and` and `or` fix: https://github.com/rubocop/rubocop-rspec/issues/1983 --- CHANGELOG.md | 1 + lib/rubocop/cop/rspec/change_by_zero.rb | 6 +++- spec/rubocop/cop/rspec/change_by_zero_spec.rb | 30 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd89a961c..e7dd725c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Master (Unreleased) - 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/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/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