diff --git a/changelog/fix_false_positives_for_performance_big_decimal_with_numeric_argument.md b/changelog/fix_false_positives_for_performance_big_decimal_with_numeric_argument.md new file mode 100644 index 0000000000..6db6995a73 --- /dev/null +++ b/changelog/fix_false_positives_for_performance_big_decimal_with_numeric_argument.md @@ -0,0 +1 @@ +* [#468](https://github.com/rubocop/rubocop-performance/issues/468): Fix false positives for `Performance/BigDecimalWithNumericArgument` when using float argument for `BigDecimal`. ([@koic][]) diff --git a/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb b/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb index 2041f32c79..2aa52b6fc7 100644 --- a/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb +++ b/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb @@ -3,22 +3,28 @@ module RuboCop module Cop module Performance - # Identifies places where string argument to `BigDecimal` should be - # converted to numeric. Initializing from Integer is faster - # than from String for BigDecimal. + # Identifies places where a float argument to BigDecimal should be converted to a string. + # Initializing from String is faster than from Float for BigDecimal. + # + # Also identifies places where an integer string argument to BigDecimal should be converted to + # an integer. Initializing from Integer is faster than from String for BigDecimal. # # @example # # bad - # BigDecimal('1', 2) - # BigDecimal('4', 6) + # BigDecimal(1.2, 3, exception: true) + # 4.5.to_d(6, exception: true) + # + # # good # BigDecimal('1.2', 3, exception: true) # BigDecimal('4.5', 6, exception: true) # + # # bad + # BigDecimal('1', 2) + # BigDecimal('4', 6) + # # # good # BigDecimal(1, 2) # 4.to_d(6) - # BigDecimal(1.2, 3, exception: true) - # 4.5.to_d(6, exception: true) # class BigDecimalWithNumericArgument < Base extend AutoCorrector @@ -26,30 +32,45 @@ class BigDecimalWithNumericArgument < Base minimum_target_ruby_version 3.1 - MSG = 'Convert string literal to numeric and pass it to `BigDecimal`.' + MSG_FROM_FLOAT_TO_STRING = 'Convert float literal to string and pass it to `BigDecimal`.' + MSG_FROM_INTEGER_TO_STRING = 'Convert string literal to integer and pass it to `BigDecimal`.' RESTRICT_ON_SEND = %i[BigDecimal to_d].freeze - def_node_matcher :big_decimal_with_numeric_argument?, <<~PATTERN - (send nil? :BigDecimal $str_type? ...) + def_node_matcher :big_decimal_with_numeric_argument, <<~PATTERN + (send nil? :BigDecimal ${float_type? str_type?} ...) PATTERN - def_node_matcher :to_d?, <<~PATTERN - (send [!nil? $str_type?] :to_d ...) + def_node_matcher :to_d, <<~PATTERN + (send [!nil? ${float_type? str_type?}] :to_d ...) PATTERN + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength def on_send(node) - if (string = big_decimal_with_numeric_argument?(node)) - add_offense(string.source_range) do |corrector| - corrector.replace(string, string.value) + if (numeric = big_decimal_with_numeric_argument(node)) + if numeric.numeric_type? + add_offense(numeric, message: MSG_FROM_FLOAT_TO_STRING) do |corrector| + corrector.wrap(numeric, "'", "'") + end + elsif numeric.value.match?(/\A\d+\z/) + add_offense(numeric, message: MSG_FROM_INTEGER_TO_STRING) do |corrector| + corrector.replace(numeric, numeric.value) + end end - elsif (string_to_d = to_d?(node)) - add_offense(string_to_d.source_range) do |corrector| - big_decimal_args = node.arguments.map(&:source).unshift(string_to_d.value).join(', ') + elsif (numeric_to_d = to_d(node)) + if numeric_to_d.numeric_type? + add_offense(numeric_to_d, message: MSG_FROM_FLOAT_TO_STRING) do |corrector| + big_decimal_args = node.arguments.map(&:source).unshift("'#{numeric_to_d.source}'").join(', ') - corrector.replace(node, "BigDecimal(#{big_decimal_args})") + corrector.replace(node, "BigDecimal(#{big_decimal_args})") + end + elsif numeric_to_d.value.match?(/\A\d+\z/) + add_offense(numeric_to_d, message: MSG_FROM_INTEGER_TO_STRING) do |corrector| + corrector.replace(node, "#{numeric_to_d.value}.to_d") + end end end end + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength end end end diff --git a/spec/rubocop/cop/performance/big_decimal_with_numeric_argument_spec.rb b/spec/rubocop/cop/performance/big_decimal_with_numeric_argument_spec.rb index e15247db6b..fc9a691c9f 100644 --- a/spec/rubocop/cop/performance/big_decimal_with_numeric_argument_spec.rb +++ b/spec/rubocop/cop/performance/big_decimal_with_numeric_argument_spec.rb @@ -2,128 +2,128 @@ RSpec.describe RuboCop::Cop::Performance::BigDecimalWithNumericArgument, :config do context 'when Ruby >= 3.1', :ruby31 do - it 'registers an offense and corrects when using `BigDecimal` with string' do - expect_offense(<<~RUBY) - BigDecimal('1') - ^^^ Convert string literal to numeric and pass it to `BigDecimal`. + it 'does not register an offense and corrects when using `BigDecimal` with integer' do + expect_no_offenses(<<~RUBY) + BigDecimal(1) RUBY + end - expect_correction(<<~RUBY) - BigDecimal(1) + it 'does not register an offense and corrects when using `Integer#to_d` for integer' do + expect_no_offenses(<<~RUBY) + 1.to_d RUBY end - it 'registers an offense and corrects when using `String#to_d`' do + it 'registers an offense and corrects when using `BigDecimal` with float' do expect_offense(<<~RUBY) - '1'.to_d - ^^^ Convert string literal to numeric and pass it to `BigDecimal`. + BigDecimal(1.5, exception: true) + ^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(1) + BigDecimal('1.5', exception: true) RUBY end - it 'registers an offense and corrects when using `BigDecimal` with float string' do + it 'registers an offense and corrects when using `Float#to_d`' do expect_offense(<<~RUBY) - BigDecimal('1.5', exception: true) - ^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + 1.5.to_d(exception: true) + ^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(1.5, exception: true) + BigDecimal('1.5', exception: true) RUBY end - it 'registers an offense and corrects when using float `String#to_d`' do + it 'registers an offense when using `BigDecimal` with float and precision' do expect_offense(<<~RUBY) - '1.5'.to_d(exception: true) - ^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + BigDecimal(3.14, 1) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(1.5, exception: true) + BigDecimal('3.14', 1) RUBY end - it 'registers an offense when using `BigDecimal` with float string and precision' do + it 'registers an offense when using `Float#to_d` with precision' do expect_offense(<<~RUBY) - BigDecimal('3.14', 1) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + 3.14.to_d(1) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(3.14, 1) + BigDecimal('3.14', 1) RUBY end - it 'registers an offense when using float `String#to_d` with precision' do + it 'registers an offense when using `BigDecimal` with float and non-literal precision' do expect_offense(<<~RUBY) - '3.14'.to_d(1) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + precision = 1 + BigDecimal(3.14, precision) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(3.14, 1) + precision = 1 + BigDecimal('3.14', precision) RUBY end - it 'registers an offense when using `BigDecimal` with float string and non-literal precision' do + it 'registers an offense when using `Float#to_d` with non-literal precision' do expect_offense(<<~RUBY) precision = 1 - BigDecimal('3.14', precision) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + 3.14.to_d(precision) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) precision = 1 - BigDecimal(3.14, precision) + BigDecimal('3.14', precision) RUBY end - it 'registers an offense when using float `String#to_d` with non-literal precision' do + it 'registers an offense when using `BigDecimal` with float, precision, and a keyword argument' do expect_offense(<<~RUBY) - precision = 1 - '3.14'.to_d(precision) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + BigDecimal(3.14, 1, exception: true) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - precision = 1 - BigDecimal(3.14, precision) + BigDecimal('3.14', 1, exception: true) RUBY end - it 'registers an offense when using `BigDecimal` with float string, precision, and a keyword argument' do + it 'registers an offense when using `Float#to_d` with precision and a keyword argument' do expect_offense(<<~RUBY) - BigDecimal('3.14', 1, exception: true) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + 3.14.to_d(1, exception: true) + ^^^^ Convert float literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(3.14, 1, exception: true) + BigDecimal('3.14', 1, exception: true) RUBY end - it 'registers an offense when using float `String#to_d` with precision and a keyword argument' do + it 'registers an offense when using `BigDecimal` with integer string' do expect_offense(<<~RUBY) - '3.14'.to_d(1, exception: true) - ^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`. + BigDecimal('1') + ^^^ Convert string literal to integer and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) - BigDecimal(3.14, 1, exception: true) + BigDecimal(1) RUBY end - it 'does not register an offense when using `BigDecimal` with integer' do - expect_no_offenses(<<~RUBY) - BigDecimal(1) + it 'registers an offense when using `String#to_d` for integer string' do + expect_offense(<<~RUBY) + '1'.to_d + ^^^ Convert string literal to integer and pass it to `BigDecimal`. RUBY - end - it 'does not register an offense when using `Integer#to_d`' do - expect_no_offenses(<<~RUBY) + expect_correction(<<~RUBY) 1.to_d RUBY end