Skip to content

Commit

Permalink
[Fix #7902] Change Link/NumberConversion to detect symbol form of c…
Browse files Browse the repository at this point in the history
…onversion methods

Closes #7902
  • Loading branch information
tejasbubane authored and bbatsov committed Jan 26, 2021
1 parent 68a75b3 commit 7be96b3
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog/change_number_conversion_to_handle_symbols.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#7902](https://github.com/rubocop-hq/rubocop/issues/7902): Change `Lint/NumberConversion` to detect symbol form of conversion methods. ([@tejasbubane][])
47 changes: 41 additions & 6 deletions lib/rubocop/cop/lint/number_conversion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ module Lint
# '10'.to_i
# '10.2'.to_f
# '10'.to_c
# ['1', '2', '3'].map(&:to_i)
# foo.try(:to_f)
# bar.send(:to_c)
#
# # good
#
# Integer('10', 10)
# Float('10.2')
# Complex('10')
# ['1', '2', '3'].map { |i| Integer(i, 10) }
# foo.try { |i| Float(i) }
# bar.send { |i| Complex(i) }
#
# @example IgnoredMethods: [minutes]
#
Expand All @@ -52,22 +58,33 @@ class NumberConversion < Base
}.freeze
MSG = 'Replace unsafe number conversion with number '\
'class parsing, instead of using '\
'%<number_object>s.%<to_method>s, use stricter '\
'%<current>s, use stricter '\
'%<corrected_method>s.'
RESTRICT_ON_SEND = CONVERSION_METHOD_CLASS_MAPPING.keys.freeze
METHODS = CONVERSION_METHOD_CLASS_MAPPING.keys.map(&:inspect).join(' ')

def_node_matcher :to_method, <<~PATTERN
(send $_ ${:to_i :to_f :to_c})
(send $_ ${#{METHODS}})
PATTERN

def_node_matcher :to_method_symbol, <<~PATTERN
{(send _ $_ ${(sym ${#{METHODS}})} ...)
(send _ $_ ${(block_pass (sym ${#{METHODS}}))} ...)}
PATTERN

def on_send(node)
handle_conversion_method(node)
handle_as_symbol(node)
end

private

def handle_conversion_method(node)
to_method(node) do |receiver, to_method|
next if receiver.nil? || ignore_receiver?(receiver)

message = format(
MSG,
number_object: receiver.source,
to_method: to_method,
current: "#{receiver.source}.#{to_method}",
corrected_method: correct_method(node, receiver)
)
add_offense(node, message: message) do |corrector|
Expand All @@ -76,13 +93,31 @@ def on_send(node)
end
end

private
def handle_as_symbol(node)
to_method_symbol(node) do |receiver, sym_node, to_method|
next if receiver.nil?

message = format(
MSG,
current: sym_node.source,
corrected_method: correct_sym_method(to_method)
)
add_offense(node, message: message) do |corrector|
corrector.replace(sym_node, correct_sym_method(to_method))
end
end
end

def correct_method(node, receiver)
format(CONVERSION_METHOD_CLASS_MAPPING[node.method_name],
number_object: receiver.source)
end

def correct_sym_method(to_method)
body = format(CONVERSION_METHOD_CLASS_MAPPING[to_method], number_object: 'i')
"{ |i| #{body} }"
end

def ignore_receiver?(receiver)
if receiver.send_type? && ignored_method?(receiver.method_name)
true
Expand Down
35 changes: 35 additions & 0 deletions spec/rubocop/cop/lint/number_conversion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,41 @@
end
end

context 'to_method in symbol form' do
it 'registers offense and autocorrects' do
expect_offense(<<~RUBY)
"1,2,3,foo,5,6,7,8".split(',').map(&:to_i)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Replace unsafe number conversion with number class parsing, instead of using &:to_i, use stricter { |i| Integer(i, 10) }.
RUBY

expect_correction(<<~RUBY)
"1,2,3,foo,5,6,7,8".split(',').map({ |i| Integer(i, 10) })
RUBY
end

it 'registers offense with try' do
expect_offense(<<~RUBY)
"foo".try(:to_f)
^^^^^^^^^^^^^^^^ Replace unsafe number conversion with number class parsing, instead of using :to_f, use stricter { |i| Float(i) }.
RUBY

expect_correction(<<~RUBY)
"foo".try({ |i| Float(i) })
RUBY
end

it 'registers offense with send' do
expect_offense(<<~RUBY)
"foo".send(:to_c)
^^^^^^^^^^^^^^^^^ Replace unsafe number conversion with number class parsing, instead of using :to_c, use stricter { |i| Complex(i) }.
RUBY

expect_correction(<<~RUBY)
"foo".send({ |i| Complex(i) })
RUBY
end
end

context 'IgnoredClasses' do
let(:cop_config) { { 'IgnoredClasses' => %w[Time DateTime] } }

Expand Down

0 comments on commit 7be96b3

Please sign in to comment.