From 1ad1f1296db95ba1e72a03916be2471616dfa733 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 7 Sep 2021 01:27:02 +0900 Subject: [PATCH] Fix an error for `Lint/NumberConversion` This PR fixes the following error for `Lint/NumberConversion` when using nested number conversion methods. ## Before ```console % cat example.rb var.to_i.to_f % bundle exec rubocop --only Lint/NumberConversion -A (snip) Inspecting 1 file An error occurred while Lint/NumberConversion cop was inspecting /Users/koic/src/github.com/koic/rubocop-issues/number/example.rb:1:0. To see the complete backtrace run rubocop -d. W Offenses: example.rb:1:1: W: [Corrected] Lint/NumberConversion: Replace unsafe number conversion with number class parsing, instead of using var.to_i.to_f, use stricter Float(var.to_i).var.to_i.to_f ^^^^^^^^^^^^^ example.rb:1:7: W: [Corrected] Lint/NumberConversion: Replace unsafe number conversion with number class parsing, instead of using var.to_i, use stricter Integer(var, 10).Float(var.to_i) ^^^^^^^^ 1 file inspected, 2 offenses detected, 2 offenses corrected 1 error occurred: An error occurred while Lint/NumberConversion cop was inspecting /Users/koic/src/github.com/koic/rubocop-issues/number/example.rb:1:0. Errors are usually caused by RuboCop bugs. Please, report your problems to RuboCop's issue tracker. % cat example.rb Float(Integer(var, 10)) ``` ## After ```console % cat example.rb var.to_i.to_f % bundle exec rubocop --only Lint/NumberConversion -A (snip) Offenses: example.rb:1:1: W: [Corrected] Lint/NumberConversion: Replace unsafe number conversion with number class parsing, instead of using var.to_i, use stricter Integer(var, 10).var.to_i.to_f ^^^^^^^^ 1 file inspected, 1 offense detected, 1 offense corrected % cat example.rb Integer(var, 10).to_f ``` The result of `Integer()` is an `Integer` object, so `to_f` does not need to be replaced with `Float()`. Accordingly, this PR will be changed to be accepted if the receiver is a numeric literal that is an obvious numeric (e.g. `42.to_f`). --- .../fix_error_for_lint_number_conversion.md | 1 + lib/rubocop/cop/lint/number_conversion.rb | 8 ++- .../cop/lint/number_conversion_spec.rb | 61 +++++++++++++++---- 3 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 changelog/fix_error_for_lint_number_conversion.md diff --git a/changelog/fix_error_for_lint_number_conversion.md b/changelog/fix_error_for_lint_number_conversion.md new file mode 100644 index 00000000000..c6a59a19740 --- /dev/null +++ b/changelog/fix_error_for_lint_number_conversion.md @@ -0,0 +1 @@ +* [#10067](https://github.com/rubocop/rubocop/pull/10067): Fix an error for `Lint/NumberConversion` when using nested number conversion methods. ([@koic][]) diff --git a/lib/rubocop/cop/lint/number_conversion.rb b/lib/rubocop/cop/lint/number_conversion.rb index 9a092316864..a61bc973d3b 100644 --- a/lib/rubocop/cop/lint/number_conversion.rb +++ b/lib/rubocop/cop/lint/number_conversion.rb @@ -60,6 +60,7 @@ class NumberConversion < Base 'class parsing, instead of using '\ '`%s`, use stricter '\ '`%s`.' + CONVERSION_METHODS = %i[Integer Float Complex to_i to_f to_c].freeze METHODS = CONVERSION_METHOD_CLASS_MAPPING.keys.map(&:inspect).join(' ') # @!method to_method(node) @@ -127,7 +128,8 @@ def remove_parentheses(corrector, node) end def ignore_receiver?(receiver) - if receiver.send_type? && ignored_method?(receiver.method_name) + if receiver.numeric_type? || (receiver.send_type? && + (conversion_method?(receiver.method_name) || ignored_method?(receiver.method_name))) true elsif (receiver = top_receiver(receiver)) receiver.const_type? && ignored_class?(receiver.const_name) @@ -142,6 +144,10 @@ def top_receiver(node) receiver end + def conversion_method?(method_name) + CONVERSION_METHODS.include?(method_name) + end + def ignored_classes cop_config.fetch('IgnoredClasses', []) end diff --git a/spec/rubocop/cop/lint/number_conversion_spec.rb b/spec/rubocop/cop/lint/number_conversion_spec.rb index 1c6a7dae320..0a4e6e36235 100644 --- a/spec/rubocop/cop/lint/number_conversion_spec.rb +++ b/spec/rubocop/cop/lint/number_conversion_spec.rb @@ -13,17 +13,6 @@ RUBY end - it 'when using `#to_i` for integer' do - expect_offense(<<~RUBY) - 10.to_i - ^^^^^^^ Replace unsafe number conversion with number class parsing, instead of using `10.to_i`, use stricter `Integer(10, 10)`. - RUBY - - expect_correction(<<~RUBY) - Integer(10, 10) - RUBY - end - it 'when using `#to_f`' do expect_offense(<<~RUBY) "10.2".to_f @@ -46,6 +35,27 @@ RUBY end + it 'when using `#to_i` for number literals' do + expect_no_offenses(<<~RUBY) + 42.to_i + 42.0.to_i + RUBY + end + + it 'when using `#to_f` for number literals' do + expect_no_offenses(<<~RUBY) + 42.to_f + 42.0.to_f + RUBY + end + + it 'when using `#to_c` for number literals' do + expect_no_offenses(<<~RUBY) + 42.to_c + 42.0.to_c + RUBY + end + it 'when `#to_i` called on a variable' do expect_offense(<<~RUBY) string_value = '10' @@ -163,6 +173,35 @@ RUBY end + it 'registers an offense when using nested number conversion methods' do + expect_offense(<<~RUBY) + var.to_i.to_f + ^^^^^^^^ Replace unsafe number conversion with number class parsing, instead of using `var.to_i`, use stricter `Integer(var, 10)`. + RUBY + + expect_correction(<<~RUBY) + Integer(var, 10).to_f + RUBY + end + + it 'does not register an offense when using `Integer` constructor' do + expect_no_offenses(<<~RUBY) + Integer(var, 10).to_f + RUBY + end + + it 'does not register an offense when using `Float` constructor' do + expect_no_offenses(<<~RUBY) + Float(var).to_i + RUBY + end + + it 'does not register an offense when using `Complex` constructor' do + expect_no_offenses(<<~RUBY) + Complex(var).to_f + RUBY + end + it 'registers offense with send' do expect_offense(<<~RUBY) "foo".send(:to_c)