Skip to content

Commit

Permalink
Add #to_d support to BigDecimalWithNumericArgument
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
leoarnold committed Nov 16, 2021
1 parent 51409ce commit 5406db2
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -312,3 +312,4 @@
[@ghiculescu]: https://github.com/ghiculescu
[@mfbmina]: https://github.com/mfbmina
[@mvz]: https://github.com/mvz
[@leoarnold]: https://github.com/leoarnold
Empty file added changelog/.gitkeep
Empty file.
1 change: 1 addition & 0 deletions 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][])
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -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'

Expand Down
4 changes: 4 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Expand Up @@ -99,10 +99,14 @@ than from Numeric for BigDecimal.
----
# bad
BigDecimal(1, 2)
1.to_d(2)
BigDecimal(1.2, 3, exception: true)
1.2.to_d(3, exception: true)
# good
BigDecimal('1', 2)
BigDecimal('1', 2)
BigDecimal('1.2', 3, exception: true)
BigDecimal('1.2', 3, exception: true)
----

Expand Down
35 changes: 23 additions & 12 deletions lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb
Expand Up @@ -10,36 +10,47 @@ module Performance
# @example
# # bad
# BigDecimal(1, 2)
# 1.to_d(2)
# BigDecimal(1.2, 3, exception: true)
# 1.2.to_d(3, exception: true)
#
# # good
# BigDecimal('1', 2)
# BigDecimal('1', 2)
# BigDecimal('1.2', 3, exception: true)
# BigDecimal('1.2', 3, 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
Expand Down
Expand Up @@ -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)
Expand All @@ -15,30 +26,92 @@
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)
BigDecimal('1.5', exception: true)
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

Expand All @@ -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

0 comments on commit 5406db2

Please sign in to comment.