diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f95e435c..4c296f91e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Change `RSpec/ScatteredSetup` to allow `around` hooks to be scattered. ([@ydah]) - Fix an error `RSpec/ChangeByZero` cop when without expect block. ([@lee266]) - Fix a false positive for `RSpec/DescribedClass` when `SkipBlocks` is true and numblocks are used. ([@earlopain]) +- Add support for `itblock` from Ruby 3.4. ([@earlopain]) ## 3.5.0 (2025-02-16) diff --git a/lib/rubocop/cop/rspec/around_block.rb b/lib/rubocop/cop/rspec/around_block.rb index c10d8f183..68d43aabb 100644 --- a/lib/rubocop/cop/rspec/around_block.rb +++ b/lib/rubocop/cop/rspec/around_block.rb @@ -41,6 +41,11 @@ class AroundBlock < Base (numblock (send nil? :around sym ?) ...) PATTERN + # @!method hook_itblock(node) + def_node_matcher :hook_itblock, <<~PATTERN + (itblock (send nil? :around sym ?) ...) + PATTERN + # @!method find_arg_usage(node) def_node_search :find_arg_usage, <<~PATTERN {(send $... {:call :run}) (send _ _ $...) (yield $...) (block-pass $...)} @@ -51,14 +56,20 @@ def on_block(node) if example_proxy.nil? add_no_arg_offense(node) else - check_for_unused_proxy(node, example_proxy) + check_for_unused_proxy(node, example_proxy, example_proxy.name) end end end def on_numblock(node) hook_numblock(node) do - check_for_numblock(node) + check_for_unused_proxy(node, node.children.last, :_1) + end + end + + def on_itblock(node) + hook_itblock(node) do + check_for_unused_proxy(node, node.children.last, :it) end end @@ -68,25 +79,14 @@ def add_no_arg_offense(node) add_offense(node, message: MSG_NO_ARG) end - def check_for_unused_proxy(block, proxy) - find_arg_usage(block) do |usage| - return if usage.include?(s(:lvar, proxy.name)) - end - - add_offense( - proxy, - message: format(MSG_UNUSED_ARG, arg: proxy.name) - ) - end - - def check_for_numblock(block) + def check_for_unused_proxy(block, range, name) find_arg_usage(block) do |usage| - return if usage.include?(s(:lvar, :_1)) + return if usage.include?(s(:lvar, name)) end add_offense( - block.children.last, - message: format(MSG_UNUSED_ARG, arg: :_1) + range, + message: format(MSG_UNUSED_ARG, arg: name) ) end end diff --git a/lib/rubocop/cop/rspec/empty_line_after_hook.rb b/lib/rubocop/cop/rspec/empty_line_after_hook.rb index cfcd8d30a..a2383e17c 100644 --- a/lib/rubocop/cop/rspec/empty_line_after_hook.rb +++ b/lib/rubocop/cop/rspec/empty_line_after_hook.rb @@ -68,6 +68,7 @@ def on_block(node) end alias on_numblock on_block + alias on_itblock on_block private diff --git a/lib/rubocop/cop/rspec/expect_in_hook.rb b/lib/rubocop/cop/rspec/expect_in_hook.rb index 4474c707b..6cdddd15e 100644 --- a/lib/rubocop/cop/rspec/expect_in_hook.rb +++ b/lib/rubocop/cop/rspec/expect_in_hook.rb @@ -38,6 +38,7 @@ def on_block(node) end alias on_numblock on_block + alias on_itblock on_block private diff --git a/lib/rubocop/cop/rspec/hook_argument.rb b/lib/rubocop/cop/rspec/hook_argument.rb index 654dd036f..67ca7bc1d 100644 --- a/lib/rubocop/cop/rspec/hook_argument.rb +++ b/lib/rubocop/cop/rspec/hook_argument.rb @@ -89,6 +89,7 @@ def on_block(node) end alias on_numblock on_block + alias on_itblock on_block private diff --git a/lib/rubocop/cop/rspec/hooks_before_examples.rb b/lib/rubocop/cop/rspec/hooks_before_examples.rb index e8aeeebb5..1cb016492 100644 --- a/lib/rubocop/cop/rspec/hooks_before_examples.rb +++ b/lib/rubocop/cop/rspec/hooks_before_examples.rb @@ -38,14 +38,12 @@ class HooksBeforeExamples < Base } PATTERN - def on_block(node) + def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler return unless example_group_with_body?(node) check_hooks(node.body) if multiline_block?(node.body) end - alias on_numblock on_block - private def multiline_block?(block) diff --git a/lib/rubocop/cop/rspec/iterated_expectation.rb b/lib/rubocop/cop/rspec/iterated_expectation.rb index 2eb7ee6aa..37c84f79b 100644 --- a/lib/rubocop/cop/rspec/iterated_expectation.rb +++ b/lib/rubocop/cop/rspec/iterated_expectation.rb @@ -36,6 +36,13 @@ class IteratedExpectation < Base ) PATTERN + # @!method each_itblock?(node) + def_node_matcher :each_itblock?, <<~PATTERN + (itblock + (send ... :each) _ $(...) + ) + PATTERN + # @!method expectation?(node) def_node_matcher :expectation?, <<~PATTERN (send (send nil? :expect (lvar %)) :to ...) @@ -43,22 +50,30 @@ class IteratedExpectation < Base def on_block(node) each?(node) do |arg, body| - if single_expectation?(body, arg) || only_expectations?(body, arg) - add_offense(node.send_node) - end + check_expectation(node, body, arg) end end def on_numblock(node) each_numblock?(node) do |body| - if single_expectation?(body, :_1) || only_expectations?(body, :_1) - add_offense(node.send_node) - end + check_expectation(node, body, :_1) + end + end + + def on_itblock(node) + each_itblock?(node) do |body| + check_expectation(node, body, :it) end end private + def check_expectation(node, body, arg) + if single_expectation?(body, arg) || only_expectations?(body, arg) + add_offense(node.send_node) + end + end + def single_expectation?(body, arg) expectation?(body, arg) end diff --git a/lib/rubocop/cop/rspec/mixin/metadata.rb b/lib/rubocop/cop/rspec/mixin/metadata.rb index 325747433..e7f68df76 100644 --- a/lib/rubocop/cop/rspec/mixin/metadata.rb +++ b/lib/rubocop/cop/rspec/mixin/metadata.rb @@ -27,7 +27,7 @@ module Metadata (send (lvar %) #Hooks.all _ $...) PATTERN - def on_block(node) + def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler rspec_configure(node) do |block_var| metadata_in_block(node, block_var) do |metadata_arguments| on_metadata_arguments(metadata_arguments) @@ -38,7 +38,6 @@ def on_block(node) on_metadata_arguments(metadata_arguments) end end - alias on_numblock on_block def on_metadata(_symbols, _hash) raise ::NotImplementedError diff --git a/lib/rubocop/cop/rspec/no_expectation_example.rb b/lib/rubocop/cop/rspec/no_expectation_example.rb index a800636df..b9a44b13e 100644 --- a/lib/rubocop/cop/rspec/no_expectation_example.rb +++ b/lib/rubocop/cop/rspec/no_expectation_example.rb @@ -96,6 +96,7 @@ def on_block(node) end alias on_numblock on_block + alias on_itblock on_block end end end diff --git a/lib/rubocop/cop/rspec/redundant_around.rb b/lib/rubocop/cop/rspec/redundant_around.rb index 7522c9ce0..3e20482f1 100644 --- a/lib/rubocop/cop/rspec/redundant_around.rb +++ b/lib/rubocop/cop/rspec/redundant_around.rb @@ -28,6 +28,7 @@ def on_block(node) end end alias on_numblock on_block + alias on_itblock on_block def on_send(node) return unless match_redundant_around_hook_send?(node) diff --git a/lib/rubocop/cop/rspec/skip_block_inside_example.rb b/lib/rubocop/cop/rspec/skip_block_inside_example.rb index 7137fd10a..95e0afa1f 100644 --- a/lib/rubocop/cop/rspec/skip_block_inside_example.rb +++ b/lib/rubocop/cop/rspec/skip_block_inside_example.rb @@ -34,6 +34,7 @@ def on_block(node) end alias on_numblock on_block + alias on_itblock on_block private diff --git a/rubocop-rspec.gemspec b/rubocop-rspec.gemspec index 5296dd3e2..64413118e 100644 --- a/rubocop-rspec.gemspec +++ b/rubocop-rspec.gemspec @@ -39,5 +39,5 @@ Gem::Specification.new do |spec| } spec.add_dependency 'lint_roller', '~> 1.1' - spec.add_dependency 'rubocop', '~> 1.72', '>= 1.72.1' + spec.add_dependency 'rubocop', '~> 1.75' end diff --git a/spec/rubocop/cop/rspec/around_block_spec.rb b/spec/rubocop/cop/rspec/around_block_spec.rb index 301f452b4..d95b474bd 100644 --- a/spec/rubocop/cop/rspec/around_block_spec.rb +++ b/spec/rubocop/cop/rspec/around_block_spec.rb @@ -178,4 +178,66 @@ end end end + + context 'when Ruby 3.4', :ruby34, unsupported_on: :parser do + context 'when the yielded value is unused' do + it 'registers an offense' do + expect_offense(<<~RUBY) + around { it } + ^^ You should call `it.call` or `it.run`. + RUBY + end + end + + context 'when a method other than #run or #call is called' do + it 'registers an offense' do + expect_offense(<<~RUBY) + around do + it.inspect + ^^^^^^^^^^ You should call `it.call` or `it.run`. + end + RUBY + end + end + + context 'when #run is called' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + around do + it.run + end + RUBY + end + end + + context 'when #call is called' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + around do + it.call + end + RUBY + end + end + + context 'when used as a block arg' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + around do + 1.times(&it) + end + RUBY + end + end + + context 'when passed to another method' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + around do + something_that_might_run_test(it, another_arg) + end + RUBY + end + end + end end diff --git a/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb index 49d8bd327..847d64675 100644 --- a/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb +++ b/spec/rubocop/cop/rspec/empty_line_after_hook_spec.rb @@ -483,5 +483,37 @@ RUBY end end + + context 'when Ruby 3.4', :ruby34 do + it 'registers an offense for empty line after `around` hook' do + expect_offense(<<~RUBY) + RSpec.describe User do + around { it.run } + ^^^^^^^^^^^^^^^^^ Add an empty line after `around`. + it { does_something } + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe User do + around { it.run } + + it { does_something } + end + RUBY + end + + it 'does not register an offense for multiline `around` block' do + expect_no_offenses(<<~RUBY) + RSpec.describe User do + around do + it.run + end + + it { does_something } + end + RUBY + end + end end end diff --git a/spec/rubocop/cop/rspec/expect_in_hook_spec.rb b/spec/rubocop/cop/rspec/expect_in_hook_spec.rb index 49f56435a..e3722944e 100644 --- a/spec/rubocop/cop/rspec/expect_in_hook_spec.rb +++ b/spec/rubocop/cop/rspec/expect_in_hook_spec.rb @@ -90,4 +90,20 @@ RUBY end end + + context 'when Ruby 3.4', :ruby34 do + it 'adds an offense for `expect` in `around` hook' do + expect_offense(<<~RUBY) + around do + expect(something).to eq('foo') + ^^^^^^ Do not use `expect` in `around` hook + is_expected(something).to eq('foo') + ^^^^^^^^^^^ Do not use `is_expected` in `around` hook + expect_any_instance_of(Something).to receive(:foo) + ^^^^^^^^^^^^^^^^^^^^^^ Do not use `expect_any_instance_of` in `around` hook + it.run + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rspec/hook_argument_spec.rb b/spec/rubocop/cop/rspec/hook_argument_spec.rb index 6a7fbd6e8..137f1a965 100644 --- a/spec/rubocop/cop/rspec/hook_argument_spec.rb +++ b/spec/rubocop/cop/rspec/hook_argument_spec.rb @@ -262,29 +262,59 @@ context 'when Ruby 2.7', :ruby27 do it 'does not flag :example for hooks' do expect_no_offenses(<<~RUBY) - around(:example) { _1 } + around(:example) { _1.run } RUBY end it 'detects :each for hooks' do expect_offense(<<~RUBY) - around(:each) { _1 } + around(:each) { _1.run } ^^^^^^^^^^^^^ Use `:example` for RSpec hooks. RUBY expect_correction(<<~RUBY) - around(:example) { _1 } + around(:example) { _1.run } RUBY end it 'detects hooks without default scopes' do expect_offense(<<~RUBY) - around { _1 } + around { _1.run } ^^^^^^ Use `:example` for RSpec hooks. RUBY expect_correction(<<~RUBY) - around(:example) { _1 } + around(:example) { _1.run } + RUBY + end + end + + context 'when Ruby 3.4', :ruby34 do + it 'does not flag :example for hooks' do + expect_no_offenses(<<~RUBY) + around(:example) { it.run } + RUBY + end + + it 'detects :each for hooks' do + expect_offense(<<~RUBY) + around(:each) { it.run } + ^^^^^^^^^^^^^ Use `:example` for RSpec hooks. + RUBY + + expect_correction(<<~RUBY) + around(:example) { it.run } + RUBY + end + + it 'detects hooks without default scopes' do + expect_offense(<<~RUBY) + around { it.run } + ^^^^^^ Use `:example` for RSpec hooks. + RUBY + + expect_correction(<<~RUBY) + around(:example) { it.run } RUBY end end diff --git a/spec/rubocop/cop/rspec/hooks_before_examples_spec.rb b/spec/rubocop/cop/rspec/hooks_before_examples_spec.rb index 180041b03..a194e6a89 100644 --- a/spec/rubocop/cop/rspec/hooks_before_examples_spec.rb +++ b/spec/rubocop/cop/rspec/hooks_before_examples_spec.rb @@ -227,4 +227,65 @@ RUBY end end + + context 'when Ruby 3.4', :ruby34 do + it 'flags `around` after `it`' do + expect_offense(<<~RUBY) + RSpec.describe User do + it { is_expected.to be_after_around_hook } + around { it } + ^^^^^^^^^^^^^ Move `around` above the examples in the group. + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe User do + around { it } + it { is_expected.to be_after_around_hook } + end + RUBY + end + + it 'flags `around` after `context`' do + expect_offense(<<~RUBY) + RSpec.describe User do + context 'a context' do + it { is_expected.to be_after_around_hook } + end + + around { it } + ^^^^^^^^^^^^^ Move `around` above the examples in the group. + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe User do + around { it } + context 'a context' do + it { is_expected.to be_after_around_hook } + end + + end + RUBY + end + + it 'flags `around` after `include_examples`' do + expect_offense(<<~RUBY) + RSpec.describe User do + include_examples('should be after around-hook') + + around { it } + ^^^^^^^^^^^^^ Move `around` above the examples in the group. + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe User do + around { it } + include_examples('should be after around-hook') + + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rspec/iterated_expectation_spec.rb b/spec/rubocop/cop/rspec/iterated_expectation_spec.rb index 81b379071..97a7d7eb7 100644 --- a/spec/rubocop/cop/rspec/iterated_expectation_spec.rb +++ b/spec/rubocop/cop/rspec/iterated_expectation_spec.rb @@ -164,4 +164,83 @@ RUBY end end + + context 'when Ruby 3.4', :ruby34, unsupported_on: :parser do + it 'flags `each` with an expectation' do + expect_offense(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { expect(it).to be_valid } + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. + end + RUBY + end + + it 'flags `each` when expectation calls method with arguments' do + expect_offense(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { expect(it).to be_a(User) } + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. + end + RUBY + end + + it 'ignores `each` without expectation' do + expect_no_offenses(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { allow(it).to receive(:method) } + end + RUBY + end + + it 'flags `each` with multiple expectations' do + expect_offense(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each do + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using the `all` matcher instead of iterating over an array. + expect(it).to receive(:method) + expect(it).to receive(:other_method) + end + end + RUBY + end + + it 'ignore `each` when the body does not contain only expectations' do + expect_no_offenses(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each do + allow(Something).to receive(:method).and_return(it) + expect(it).to receive(:method) + expect(it).to receive(:other_method) + end + end + RUBY + end + + it 'ignores `each` with expectation on property' do + expect_no_offenses(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { expect(it.name).to be } + end + RUBY + end + + it 'ignores assignments in the iteration' do + expect_no_offenses(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each { array = array.concat(it) } + end + RUBY + end + + it 'ignores `each` when there is a negative expectation' do + expect_no_offenses(<<~RUBY) + it 'validates users' do + [user1, user2, user3].each do + expect(it).not_to receive(:method) + expect(it).to receive(:other_method) + end + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rspec/no_expectation_example_spec.rb b/spec/rubocop/cop/rspec/no_expectation_example_spec.rb index 06c7e5221..d5f81aa68 100644 --- a/spec/rubocop/cop/rspec/no_expectation_example_spec.rb +++ b/spec/rubocop/cop/rspec/no_expectation_example_spec.rb @@ -238,4 +238,19 @@ end end end + + context 'when Ruby 3.4', :ruby34 do + context 'with no expectation example with it' do + it 'registers an offense' do + expect_offense(<<~RUBY) + RSpec.describe Foo do + it { it } + ^^^^^^^^^ No expectation found in this example. + + it { expect(baz).to be_truthy } + end + RUBY + end + end + end end diff --git a/spec/rubocop/cop/rspec/redundant_around_spec.rb b/spec/rubocop/cop/rspec/redundant_around_spec.rb index d85dfa734..f5fba259c 100644 --- a/spec/rubocop/cop/rspec/redundant_around_spec.rb +++ b/spec/rubocop/cop/rspec/redundant_around_spec.rb @@ -107,4 +107,33 @@ RUBY end end + + context 'when Ruby 3.4', :ruby34 do + context 'with another node in itblock `around`' do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + around do + it.run + + foo + end + RUBY + end + end + + context 'with redundant itblock `around`' do + it 'registers offense' do + expect_offense(<<~RUBY) + around do + ^^^^^^^^^ Remove redundant `around` hook. + it.run + end + RUBY + + expect_correction(<<~RUBY) + + RUBY + end + end + end end diff --git a/spec/rubocop/cop/rspec/skip_block_inside_example_spec.rb b/spec/rubocop/cop/rspec/skip_block_inside_example_spec.rb index 1126eb84e..01362d86f 100644 --- a/spec/rubocop/cop/rspec/skip_block_inside_example_spec.rb +++ b/spec/rubocop/cop/rspec/skip_block_inside_example_spec.rb @@ -22,6 +22,17 @@ RUBY end + it 'registers an offense when using `skip` with an itblock', :ruby34 do + expect_offense(<<~RUBY) + it 'does something' do + skip 'not yet implemented' do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't pass a block to `skip` inside examples. + it + end + end + RUBY + end + it 'does not register an offense when using `skip` without a block' do expect_no_offenses(<<~RUBY) it 'does something' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b39d1e6fe..c021bfb48 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -44,6 +44,11 @@ module SpecHelper config.include_context 'with default RSpec/Language config', :config config.include_context 'smoke test', type: :cop_spec + + # whitequark/parser is syntax compatible with 2.0 to 3.3. + if ENV['PARSER_ENGINE'] != 'parser_prism' + config.filter_run_excluding unsupported_on: :parser + end end $LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))