From 8651f14250d0ea93132e1feabc1ccb5864ca00e9 Mon Sep 17 00:00:00 2001 From: Leo Arnold Date: Sat, 13 Nov 2021 20:35:59 +0100 Subject: [PATCH] Add #to_d support to BigDecimalWithNumericArgument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The [`bigdecimal/util`](https://github.com/ruby/bigdecimal/blob/v3.0.2/lib/bigdecimal/util.rb) library adds `#to_d` to several numeric types, where ```ruby numeric.to_d(*args) ``` is equivalent to ```ruby BigDecimal(numeric, *args) ``` and the performance improvement of `BigDecimalWithNumericArgument` therefore equally applies here. Also, the exception of `Float` with specified precision does not seem to make sense and is therefore dropped: ``` Warming up -------------------------------------- BigDecimal("1.2") 266.340k i/100ms BigDecimal("1.2", 9) 261.542k i/100ms BigDecimal(1.2, 9) 44.366k i/100ms 1.2.to_d 42.809k i/100ms 1.2.to_d(9) 43.915k i/100ms Calculating ------------------------------------- BigDecimal("1.2") 2.634M (± 1.8%) i/s - 13.317M in 5.056712s BigDecimal("1.2", 9) 2.510M (± 5.4%) i/s - 12.554M in 5.016970s BigDecimal(1.2, 9) 330.664k (±32.5%) i/s - 1.464M in 5.055963s 1.2.to_d 183.891k (± 7.3%) i/s - 941.798k in 5.148850s 1.2.to_d(9) 189.455k (± 9.4%) i/s - 966.130k in 5.149545s ``` Generated with: ```ruby require 'bigdecimal' require 'bigdecimal/util' require 'benchmark/ips' numeric = 1.2 string = numeric.to_s prec = rand(7..11) Benchmark.ips do |ips| ips.report("BigDecimal(\"#{string}\")") { BigDecimal(string) } ips.report("BigDecimal(\"#{string}\", #{prec})") { BigDecimal(string, prec) } ips.report("BigDecimal(#{numeric}, #{prec})") { BigDecimal(numeric, prec) } ips.report("#{numeric}.to_d") { numeric.to_d } ips.report("#{numeric}.to_d(#{prec})") { numeric.to_d(prec) } end ``` --- ...d_to_d_to_BigDecimalWithNumericArgument.md | 1 + config/default.yml | 2 +- docs/modules/ROOT/pages/cops_performance.adoc | 4 + .../big_decimal_with_numeric_argument.rb | 35 ++++--- .../big_decimal_with_numeric_argument_spec.rb | 95 +++++++++++++++++-- 5 files changed, 116 insertions(+), 21 deletions(-) create mode 100644 changelog/new_add_to_d_to_BigDecimalWithNumericArgument.md diff --git a/changelog/new_add_to_d_to_BigDecimalWithNumericArgument.md b/changelog/new_add_to_d_to_BigDecimalWithNumericArgument.md new file mode 100644 index 0000000000..e4055bae19 --- /dev/null +++ b/changelog/new_add_to_d_to_BigDecimalWithNumericArgument.md @@ -0,0 +1 @@ +* [#269](https://github.com/rubocop/rubocop-performance/pull/269): Add `#to_d` support to `BigDecimalWithNumericArgument`. ([@leoarnold][]) diff --git a/config/default.yml b/config/default.yml index af19a0cf67..56ea1ab56b 100644 --- a/config/default.yml +++ b/config/default.yml @@ -17,7 +17,7 @@ Performance/ArraySemiInfiniteRangeSlice: VersionAdded: '1.9' Performance/BigDecimalWithNumericArgument: - Description: 'Convert numeric argument to string before passing to BigDecimal.' + Description: 'Convert numeric literal to string and pass it to `BigDecimal`.' Enabled: 'pending' VersionAdded: '1.7' diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 83f3b1e54a..8bfbc4b175 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -99,11 +99,15 @@ than from Numeric for BigDecimal. ---- # bad BigDecimal(1, 2) +4.to_d(6) BigDecimal(1.2, 3, exception: true) +4.5.to_d(6, exception: true) # good BigDecimal('1', 2) +BigDecimal('4', 6) BigDecimal('1.2', 3, exception: true) +BigDecimal('4.5', 6, exception: true) ---- == Performance/BindCall 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 1248544cf2..dfddc2e59f 100644 --- a/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb +++ b/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb @@ -10,36 +10,47 @@ module Performance # @example # # bad # BigDecimal(1, 2) + # 4.to_d(6) # BigDecimal(1.2, 3, exception: true) + # 4.5.to_d(6, exception: true) # # # good # BigDecimal('1', 2) + # BigDecimal('4', 6) # BigDecimal('1.2', 3, exception: true) + # BigDecimal('4.5', 6, exception: true) # class BigDecimalWithNumericArgument < Base extend AutoCorrector - MSG = 'Convert numeric argument to string before passing to `BigDecimal`.' - RESTRICT_ON_SEND = %i[BigDecimal].freeze + MSG = 'Convert numeric literal to string 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 $numeric_type? ...) PATTERN + def_node_matcher :to_d?, <<~PATTERN + (send [!nil? $numeric_type?] :to_d ...) + PATTERN + def on_send(node) - return unless (numeric = big_decimal_with_numeric_argument?(node)) - return if numeric.float_type? && specifies_precision?(node) + if (numeric = big_decimal_with_numeric_argument?(node)) + add_offense(numeric.source_range) do |corrector| + corrector.wrap(numeric, "'", "'") + end + elsif (numeric_to_d = to_d?(node)) + add_offense(numeric_to_d.source_range) do |corrector| + big_decimal_args = node + .arguments + .map(&:source) + .unshift("'#{numeric_to_d.source}'") + .join(', ') - add_offense(numeric.source_range) do |corrector| - corrector.wrap(numeric, "'", "'") + corrector.replace(node, "BigDecimal(#{big_decimal_args})") + end end end - - private - - def specifies_precision?(node) - node.arguments.size > 1 && !node.arguments[1].hash_type? - end 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 95255823cc..2fc9ba4fdd 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 @@ -4,7 +4,18 @@ it 'registers an offense and corrects when using `BigDecimal` with integer' do expect_offense(<<~RUBY) BigDecimal(1) - ^ Convert numeric argument to string before passing to `BigDecimal`. + ^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('1') + RUBY + end + + it 'registers an offense and corrects when using `Integer#to_d`' do + expect_offense(<<~RUBY) + 1.to_d + ^ Convert numeric literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) @@ -15,7 +26,7 @@ it 'registers an offense and corrects when using `BigDecimal` with float' do expect_offense(<<~RUBY) BigDecimal(1.5, exception: true) - ^^^ Convert numeric argument to string before passing to `BigDecimal`. + ^^^ Convert numeric literal to string and pass it to `BigDecimal`. RUBY expect_correction(<<~RUBY) @@ -23,22 +34,84 @@ RUBY end - it 'does not register an offense when using `BigDecimal` with float and precision' do - expect_no_offenses(<<~RUBY) + it 'registers an offense and corrects when using `Float#to_d`' do + expect_offense(<<~RUBY) + 1.5.to_d(exception: true) + ^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('1.5', exception: true) + RUBY + end + + it 'registers an offense when using `BigDecimal` with float and precision' do + expect_offense(<<~RUBY) BigDecimal(3.14, 1) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('3.14', 1) RUBY end - it 'does not register an offense when using `BigDecimal` with float and non-literal precision' do - expect_no_offenses(<<~RUBY) + it 'registers an offense when using `Float#to_d` with precision' do + expect_offense(<<~RUBY) + 3.14.to_d(1) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('3.14', 1) + RUBY + end + + it 'registers an offense when using `BigDecimal` with float and non-literal precision' do + expect_offense(<<~RUBY) precision = 1 BigDecimal(3.14, precision) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + precision = 1 + BigDecimal('3.14', precision) RUBY end - it 'does not register an offense when using `BigDecimal` with float, precision, and a keyword argument' do - expect_no_offenses(<<~RUBY) + it 'registers an offense when using `Float#to_d` with non-literal precision' do + expect_offense(<<~RUBY) + precision = 1 + 3.14.to_d(precision) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + precision = 1 + BigDecimal('3.14', precision) + RUBY + end + + it 'registers an offense when using `BigDecimal` with float, precision, and a keyword argument' do + expect_offense(<<~RUBY) BigDecimal(3.14, 1, exception: true) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('3.14', 1, exception: true) + RUBY + end + + it 'registers an offense when using `Float#to_d` with precision and a keyword argument' do + expect_offense(<<~RUBY) + 3.14.to_d(1, exception: true) + ^^^^ Convert numeric literal to string and pass it to `BigDecimal`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('3.14', 1, exception: true) RUBY end @@ -47,4 +120,10 @@ BigDecimal('1') RUBY end + + it 'does not register an offense when using `String#to_d`' do + expect_no_offenses(<<~RUBY) + '1'.to_d + RUBY + end end