Skip to content

Commit

Permalink
[Fix #11908] Support AllowedReceivers for Style/CollectionMethods
Browse files Browse the repository at this point in the history
Fixes #11908.

This PR supports `AllowedReceivers` for `Style/CollectionMethods`.
For example, by setting `AllowedReceivers: ['params']` in RuboCop Rails or user configuration,
the following false positive for `params` can be prevented:

```ruby
def selection_params(current, last_category = nil, params = {})
  params.merge(
    category_id: current.is_a?(Category) ? current.to_param : last_category.to_param,
    site_id: current.is_a?(Site) ? current.to_param : nil,
  ).delete_if { |_k, v| v.nil? }
end
```

The configuration to `params` is not set in the RuboCop core due to Rails specific issues.
So if this approach is accepted, I will add it to RuboCop Rails's default configuration.
  • Loading branch information
koic authored and bbatsov committed Jun 2, 2023
1 parent 713e7da commit 356fc46
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11908](/~https://github.com/rubocop/rubocop/issues/11908): Support `AllowedReceivers` for `Style/CollectionMethods`. ([@koic][])
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3362,6 +3362,7 @@ Style/CollectionCompact:
Safe: false
VersionAdded: '1.2'
VersionChanged: '1.3'
AllowedReceivers: []

# Align with the style guide.
Style/CollectionMethods:
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
require_relative 'rubocop/cop/mixin/allowed_identifiers'
require_relative 'rubocop/cop/mixin/allowed_methods'
require_relative 'rubocop/cop/mixin/allowed_pattern'
require_relative 'rubocop/cop/mixin/allowed_receivers'
require_relative 'rubocop/cop/mixin/auto_corrector' # rubocop:todo Naming/InclusiveLanguage
require_relative 'rubocop/cop/mixin/check_assignment'
require_relative 'rubocop/cop/mixin/check_line_breakable'
Expand Down
34 changes: 34 additions & 0 deletions lib/rubocop/cop/mixin/allowed_receivers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

module RuboCop
module Cop
# This module encapsulates the ability to allow certain receivers in a cop.
module AllowedReceivers
def allowed_receiver?(receiver)
receiver_name = receiver_name(receiver)

allowed_receivers.include?(receiver_name)
end

def receiver_name(receiver)
if receiver.receiver && !receiver.receiver.const_type?
return receiver_name(receiver.receiver)
end

if receiver.send_type?
if receiver.receiver
"#{receiver_name(receiver.receiver)}.#{receiver.method_name}"
else
receiver.method_name.to_s
end
else
receiver.source
end
end

def allowed_receivers
cop_config.fetch('AllowedReceivers', [])
end
end
end
end
6 changes: 6 additions & 0 deletions lib/rubocop/cop/style/collection_compact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ module Style
# # good
# hash.compact!
#
# @example AllowedReceivers: ['params']
# # good
# params.reject(&:nil?)
#
class CollectionCompact < Base
include AllowedReceivers
include RangeHelp
extend AutoCorrector
extend TargetRubyVersion
Expand Down Expand Up @@ -76,6 +81,7 @@ class CollectionCompact < Base

def on_send(node)
return unless (range = offense_range(node))
return if allowed_receiver?(node.receiver)
if (target_ruby_version <= 3.0 || node.method?(:delete_if)) && to_enum_method?(node)
return
end
Expand Down
23 changes: 1 addition & 22 deletions lib/rubocop/cop/style/hash_each_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module Style
# execute(sql).keys.each { |v| p v }
# execute(sql).values.each { |v| p v }
class HashEachMethods < Base
include AllowedReceivers
include Lint::UnusedArgument
extend AutoCorrector

Expand Down Expand Up @@ -116,28 +117,6 @@ def correct_args(node, corrector)
def kv_range(outer_node)
outer_node.receiver.loc.selector.join(outer_node.loc.selector)
end

def allowed_receiver?(receiver)
receiver_name = receiver_name(receiver)

allowed_receivers.include?(receiver_name)
end

def receiver_name(receiver)
if receiver.send_type?
if receiver.receiver
"#{receiver_name(receiver.receiver)}.#{receiver.method_name}"
else
receiver.method_name.to_s
end
else
receiver.source
end
end

def allowed_receivers
cop_config.fetch('AllowedReceivers', [])
end
end
end
end
Expand Down
23 changes: 23 additions & 0 deletions spec/rubocop/cop/style/collection_compact_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,27 @@ def foo(params)
RUBY
end
end

context "when `AllowedReceivers: ['params']`" do
let(:cop_config) { { 'AllowedReceivers' => ['params'] } }

it 'does not register an offense when receiver is `params` method' do
expect_no_offenses(<<~RUBY)
params.reject { |param| param.nil? }
RUBY
end

it 'does not register an offense when method chained receiver is `params` method' do
expect_no_offenses(<<~RUBY)
params.merge(key: value).delete_if { |_k, v| v.nil? }
RUBY
end

it 'registers an offense when receiver is not allowed name' do
expect_offense(<<~RUBY)
foo.reject { |param| param.nil? }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact` instead of `reject { |param| param.nil? }`.
RUBY
end
end
end

0 comments on commit 356fc46

Please sign in to comment.