From 44959aaf860042b58f38e56d4c1b1f35470f6cb9 Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Thu, 5 Sep 2024 13:54:31 -0400 Subject: [PATCH 1/4] Add cop UpdateIndexArgument --- config/default.yml | 4 ++ .../cop/chewy/update_index_argument.rb | 57 +++++++++++++++++++ .../cop/chewy/update_index_argument_spec.rb | 57 +++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 lib/rubocop/cop/chewy/update_index_argument.rb create mode 100644 spec/rubocop/cop/chewy/update_index_argument_spec.rb diff --git a/config/default.yml b/config/default.yml index e0f0f02..1b04bf9 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4,6 +4,10 @@ Chewy/FieldType: Include: - app/chewy/**/* +Chewy/UpdateIndexArgument: + Description: 'Prevent single expression in update_index block.' + Enabled: true + Grape/PreferNamespace: Description: 'Prevent usage of namespace aliases.' Enabled: true diff --git a/lib/rubocop/cop/chewy/update_index_argument.rb b/lib/rubocop/cop/chewy/update_index_argument.rb new file mode 100644 index 0000000..1efdde9 --- /dev/null +++ b/lib/rubocop/cop/chewy/update_index_argument.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Chewy + # This cop checks for the use of block with single expression in `update_index` method + # and suggests to use method symbol as second argument instead of block. + # + # # bad + # update_index('index_name') { self } + # + # # good + # update_index('index_name', :self) + # + class UpdateIndexArgument < Base + extend AutoCorrector + + MSG = 'Use method symbol as second argument instead of block.' + + RESTRICT_ON_SEND = %i[update_index].freeze + + def_node_matcher :block_with_single_expression?, <<~PATTERN + (block + (send nil? :update_index (str _) $...) + (args) + $...) + PATTERN + + def on_block(node) + block_with_single_expression?(node) do |existing_args, method_name| + return if method_name.last.children.compact.size > 1 + + method_name = method_name.first.source + add_offense(node.loc.expression) do |corrector| + corrector.replace(node.loc.expression, corrected_code(node, existing_args, method_name)) + end + end + end + + private + + def corrected_code(node, existing_args, method_name) + method_call = node.children[0] # The update_index method call + index_name = method_call.children[2].source + + if existing_args.any? + # If there are additional arguments (like if: :update?), insert method_name before them + "update_index(#{index_name}, :#{method_name}, #{existing_args.map(&:source).join(', ')})" + else + # If no additional arguments, simply add method_name as the second argument + "update_index(#{index_name}, :#{method_name})" + end + end + end + end + end +end diff --git a/spec/rubocop/cop/chewy/update_index_argument_spec.rb b/spec/rubocop/cop/chewy/update_index_argument_spec.rb new file mode 100644 index 0000000..97ea0dc --- /dev/null +++ b/spec/rubocop/cop/chewy/update_index_argument_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Chewy::UpdateIndexArgument, :config do + it 'registers an offense when using block with single expression' do + expect_offense(<<~RUBY) + update_index('index_name') { self } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + RUBY + + expect_correction(<<~RUBY) + update_index('index_name', :self) + RUBY + + expect_offense(<<~RUBY) + update_index('index_name', if: :update?) { self } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + RUBY + + expect_correction(<<~RUBY) + update_index('index_name', :self, if: :update?) + RUBY + + expect_offense(<<~RUBY) + update_index('index_name') { some_ids } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + RUBY + + expect_correction(<<~RUBY) + update_index('index_name', :some_ids) + RUBY + + expect_offense(<<~RUBY) + update_index('index_name', if: :update?) { some_ids } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + RUBY + + expect_correction(<<~RUBY) + update_index('index_name', :some_ids, if: :update?) + RUBY + end + + it 'does not register an offense when no block' do + expect_no_offenses(<<~RUBY) + update_index('index_name', :self) + RUBY + end + + it 'does not register an offense when the bock containe multiple expression' do + expect_no_offenses(<<~RUBY) + update_index('index_name') { something.call } + RUBY + + expect_no_offenses(<<~RUBY) + update_index('index_name') { something(id) } + RUBY + end +end From cd705c025555f7df637f7d2fea4cb7305f120418 Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Thu, 5 Sep 2024 13:55:12 -0400 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28b4b0c..aadc006 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ # main * Add Add Sidekiq/PerformInline ([#82](https://github.com/petalmd/rubocop-petal/pull/82)) +* Add Add Chewy/UpdateIndexArgument ([#83](https://github.com/petalmd/rubocop-petal/pull/83)) # v1.4.0 (2024-06-14) From 8596ccb56e8a5559e3beea17684e1e9451f42f9a Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Thu, 5 Sep 2024 13:55:33 -0400 Subject: [PATCH 3/4] rubocop --- .../cop/chewy/update_index_argument_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/rubocop/cop/chewy/update_index_argument_spec.rb b/spec/rubocop/cop/chewy/update_index_argument_spec.rb index 97ea0dc..8a4acb5 100644 --- a/spec/rubocop/cop/chewy/update_index_argument_spec.rb +++ b/spec/rubocop/cop/chewy/update_index_argument_spec.rb @@ -3,8 +3,8 @@ RSpec.describe RuboCop::Cop::Chewy::UpdateIndexArgument, :config do it 'registers an offense when using block with single expression' do expect_offense(<<~RUBY) - update_index('index_name') { self } - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + update_index('index_name') { self } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. RUBY expect_correction(<<~RUBY) @@ -12,8 +12,8 @@ RUBY expect_offense(<<~RUBY) - update_index('index_name', if: :update?) { self } - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + update_index('index_name', if: :update?) { self } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. RUBY expect_correction(<<~RUBY) @@ -21,8 +21,8 @@ RUBY expect_offense(<<~RUBY) - update_index('index_name') { some_ids } - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + update_index('index_name') { some_ids } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. RUBY expect_correction(<<~RUBY) @@ -30,8 +30,8 @@ RUBY expect_offense(<<~RUBY) - update_index('index_name', if: :update?) { some_ids } - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. + update_index('index_name', if: :update?) { some_ids } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use method symbol as second argument instead of block. RUBY expect_correction(<<~RUBY) From 2853138a6f78799511803fd6d77dc51d3f98caaa Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Thu, 5 Sep 2024 13:56:45 -0400 Subject: [PATCH 4/4] comment --- lib/rubocop/cop/chewy/update_index_argument.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/rubocop/cop/chewy/update_index_argument.rb b/lib/rubocop/cop/chewy/update_index_argument.rb index 1efdde9..8a151b3 100644 --- a/lib/rubocop/cop/chewy/update_index_argument.rb +++ b/lib/rubocop/cop/chewy/update_index_argument.rb @@ -19,6 +19,7 @@ class UpdateIndexArgument < Base RESTRICT_ON_SEND = %i[update_index].freeze + # @!method block_with_single_expression?(node) def_node_matcher :block_with_single_expression?, <<~PATTERN (block (send nil? :update_index (str _) $...) @@ -40,14 +41,12 @@ def on_block(node) private def corrected_code(node, existing_args, method_name) - method_call = node.children[0] # The update_index method call + method_call = node.children[0] index_name = method_call.children[2].source if existing_args.any? - # If there are additional arguments (like if: :update?), insert method_name before them "update_index(#{index_name}, :#{method_name}, #{existing_args.map(&:source).join(', ')})" else - # If no additional arguments, simply add method_name as the second argument "update_index(#{index_name}, :#{method_name})" end end