Skip to content

Commit

Permalink
Support AllCops:ActiveSupportExtensionsEnabled for Style/SymbolProc
Browse files Browse the repository at this point in the history
Follow up d39f073.

This PR supports `AllCops:ActiveSupportExtensionsEnabled` instead of a TODO comment in `Style/SymbolProc` cop.
  • Loading branch information
koic authored and bbatsov committed May 16, 2024
1 parent 47ee67c commit 10cf5f0
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12890](https://github.com/rubocop/rubocop/issues/12890): Support `ActiveSupportExtensionsEnabled` for `Style/SymbolProc`. ([@koic][])
2 changes: 1 addition & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5449,7 +5449,7 @@ Style/SymbolProc:
Enabled: true
Safe: false
VersionAdded: '0.26'
VersionChanged: '1.40'
VersionChanged: '<<next>>'
AllowMethodsWithArguments: false
# A list of method names to be always allowed by the check.
# The names should be fairly unique, otherwise you'll end up ignoring lots of code.
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/metrics/utils/code_length_calculator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ def build_foldable_checks(types) # rubocop:disable Metrics/MethodLength
types.map do |type|
case type
when :array
->(node) { node.array_type? }
lambda(&:array_type?)
when :hash
->(node) { node.hash_type? }
lambda(&:hash_type?)
when :heredoc
->(node) { heredoc_node?(node) }
when :method_call
->(node) { node.call_type? }
lambda(&:call_type?)
else
raise Warning, "Unknown foldable type: #{type.inspect}. " \
"Valid foldable types are: #{FOLDABLE_TYPES.join(', ')}."
Expand Down
37 changes: 32 additions & 5 deletions lib/rubocop/cop/style/symbol_proc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,23 @@ module Style
# # good
# something.map { |s| s.upcase }
#
# @example AllCops:ActiveSupportExtensionsEnabled: false (default)
# # bad
# ->(x) { x.foo }
# proc { |x| x.foo }
# Proc.new { |x| x.foo }
#
# # good
# lambda(&:foo)
# proc(&:foo)
# Proc.new(&:foo)
#
# @example AllCops:ActiveSupportExtensionsEnabled: true
# # good
# ->(x) { x.foo }
# proc { |x| x.foo }
# Proc.new { |x| x.foo }
#
class SymbolProc < Base
include CommentsHelp
include RangeHelp
Expand All @@ -129,6 +146,7 @@ class SymbolProc < Base

MSG = 'Pass `&:%<method>s` as an argument to `%<block_method>s` instead of a block.'
SUPER_TYPES = %i[super zsuper].freeze
LAMBDA_OR_PROC = %i[lambda proc].freeze

# @!method proc_node?(node)
def_node_matcher :proc_node?, '(send (const {nil? cbase} :Proc) :new)'
Expand All @@ -151,13 +169,12 @@ def self.autocorrect_incompatible_with
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def on_block(node)
symbol_proc?(node) do |dispatch_node, arguments_node, method_name|
# TODO: Rails-specific handling that we should probably make
# configurable - https://github.com/rubocop/rubocop/issues/1485
# we should allow lambdas & procs
return if proc_node?(dispatch_node)
if active_support_extensions_enabled?
return if proc_node?(dispatch_node)
return if LAMBDA_OR_PROC.include?(dispatch_node.method_name)
end
return if unsafe_hash_usage?(dispatch_node)
return if unsafe_array_usage?(dispatch_node)
return if %i[lambda proc].include?(dispatch_node.method_name)
return if allowed_method_name?(dispatch_node.method_name)
return if allow_if_method_has_argument?(node.send_node)
return if node.block_type? && destructuring_block_argument?(arguments_node)
Expand Down Expand Up @@ -206,6 +223,8 @@ def autocorrect(corrector, node)
end

def autocorrect_without_args(corrector, node)
autocorrect_lambda_block(corrector, node) if node.send_node.lambda_literal?

corrector.replace(block_range_with_space(node), "(&:#{node.body.method_name})")
end

Expand All @@ -218,6 +237,14 @@ def autocorrect_with_args(corrector, node, args, method_name)
corrector.remove(block_range_with_space(node))
end

def autocorrect_lambda_block(corrector, node)
send_node_loc = node.send_node.loc
corrector.replace(send_node_loc.selector, 'lambda')

range = range_between(send_node_loc.selector.end_pos, node.loc.begin.end_pos - 2)
corrector.remove(range)
end

def block_range_with_space(node)
block_range = range_between(begin_pos_for_replacement(node), node.loc.end.end_pos)
range_with_surrounding_space(block_range, side: :left)
Expand Down
175 changes: 152 additions & 23 deletions spec/rubocop/cop/style/symbol_proc_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@
RUBY
end

it 'registers an offense for a block with parameterless method call on param and no space between method name and opening brace' do
expect_offense(<<~RUBY)
foo.map{ |a| a.nil? }
^^^^^^^^^^^^^^ Pass `&:nil?` as an argument to `map` instead of a block.
RUBY

expect_correction(<<~RUBY)
foo.map(&:nil?)
RUBY
end

it 'registers an offense for safe navigation operator' do
expect_offense(<<~RUBY)
coll&.map { |e| e.upcase }
Expand All @@ -38,20 +49,76 @@
expect_no_offenses('something { |x, y| x.method }')
end

it 'accepts lambda with 1 argument' do
expect_no_offenses('->(x) { x.method }')
end
context 'when `AllCops/ActiveSupportExtensionsEnabled: true`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => true })
end

it 'accepts proc with 1 argument' do
expect_no_offenses('proc { |x| x.method }')
end
it 'accepts lambda with 1 argument' do
expect_no_offenses('->(x) { x.method }')
end

it 'accepts Proc.new with 1 argument' do
expect_no_offenses('Proc.new { |x| x.method }')
it 'accepts proc with 1 argument' do
expect_no_offenses('proc { |x| x.method }')
end

it 'accepts Proc.new with 1 argument' do
expect_no_offenses('Proc.new { |x| x.method }')
end

it 'accepts ::Proc.new with 1 argument' do
expect_no_offenses('::Proc.new { |x| x.method }')
end
end

it 'accepts ::Proc.new with 1 argument' do
expect_no_offenses('::Proc.new { |x| x.method }')
context 'when `AllCops/ActiveSupportExtensionsEnabled: false`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => false })
end

it 'registers lambda `->` with 1 argument' do
expect_offense(<<~RUBY)
->(x) { x.method }
^^^^^^^^^^^^ Pass `&:method` as an argument to `lambda` instead of a block.
RUBY

expect_correction(<<~RUBY)
lambda(&:method)
RUBY
end

it 'registers proc with 1 argument' do
expect_offense(<<~RUBY)
proc { |x| x.method }
^^^^^^^^^^^^^^^^ Pass `&:method` as an argument to `proc` instead of a block.
RUBY

expect_correction(<<~RUBY)
proc(&:method)
RUBY
end

it 'registers Proc.new with 1 argument' do
expect_offense(<<~RUBY)
Proc.new { |x| x.method }
^^^^^^^^^^^^^^^^ Pass `&:method` as an argument to `new` instead of a block.
RUBY

expect_correction(<<~RUBY)
Proc.new(&:method)
RUBY
end

it 'registers ::Proc.new with 1 argument' do
expect_offense(<<~RUBY)
::Proc.new { |x| x.method }
^^^^^^^^^^^^^^^^ Pass `&:method` as an argument to `new` instead of a block.
RUBY

expect_correction(<<~RUBY)
::Proc.new(&:method)
RUBY
end
end

context 'when AllowedMethods is enabled' do
Expand Down Expand Up @@ -412,24 +479,86 @@
expect_no_offenses('something { _1 + _2 }')
end

it 'accepts lambda with 1 numbered parameter' do
expect_no_offenses('-> { _1.method }')
end
context 'when `AllCops/ActiveSupportExtensionsEnabled: true`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => true })
end

it 'accepts proc with 1 numbered parameter' do
expect_no_offenses('proc { _1.method }')
end
it 'accepts lambda with 1 numbered parameter' do
expect_no_offenses('-> { _1.method }')
end

it 'accepts block with only second numbered parameter' do
expect_no_offenses('something { _2.first }')
end
it 'accepts proc with 1 numbered parameter' do
expect_no_offenses('proc { _1.method }')
end

it 'accepts Proc.new with 1 numbered parameter' do
expect_no_offenses('Proc.new { _1.method }')
it 'accepts block with only second numbered parameter' do
expect_no_offenses('something { _2.first }')
end

it 'accepts Proc.new with 1 numbered parameter' do
expect_no_offenses('Proc.new { _1.method }')
end

it 'accepts ::Proc.new with 1 numbered parameter' do
expect_no_offenses('::Proc.new { _1.method }')
end
end

it 'accepts ::Proc.new with 1 numbered parameter' do
expect_no_offenses('::Proc.new { _1.method }')
context 'when `AllCops/ActiveSupportExtensionsEnabled: false`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => false })
end

it 'registers lambda with 1 numbered parameter' do
expect_offense(<<~RUBY)
-> { _1.method }
^^^^^^^^^^^^^ Pass `&:method` as an argument to `lambda` instead of a block.
RUBY

expect_correction(<<~RUBY)
lambda(&:method)
RUBY
end

it 'registers proc with 1 numbered parameter' do
expect_offense(<<~RUBY)
proc { _1.method }
^^^^^^^^^^^^^ Pass `&:method` as an argument to `proc` instead of a block.
RUBY

expect_correction(<<~RUBY)
proc(&:method)
RUBY
end

it 'does not register block with only second numbered parameter' do
expect_no_offenses(<<~RUBY)
something { _2.first }
RUBY
end

it 'registers Proc.new with 1 numbered parameter' do
expect_offense(<<~RUBY)
Proc.new { _1.method }
^^^^^^^^^^^^^ Pass `&:method` as an argument to `new` instead of a block.
RUBY

expect_correction(<<~RUBY)
Proc.new(&:method)
RUBY
end

it 'registers ::Proc.new with 1 numbered parameter' do
expect_offense(<<~RUBY)
::Proc.new { _1.method }
^^^^^^^^^^^^^ Pass `&:method` as an argument to `new` instead of a block.
RUBY

expect_correction(<<~RUBY)
::Proc.new(&:method)
RUBY
end
end

context 'AllowComments: true' do
Expand Down

0 comments on commit 10cf5f0

Please sign in to comment.