Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fix #454] Fix false positives for Performance/BigDecimalWithNumericArgument #463

Merged
merged 1 commit into from
Sep 7, 2024
Merged

[Fix #454] Fix false positives for Performance/BigDecimalWithNumericArgument #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,48 @@
module RuboCop
module Cop
module Performance
# Identifies places where numeric argument to BigDecimal should be
# converted to string. Initializing from String is faster
# than from Numeric for BigDecimal.
# Identifies places where string argument to `BigDecimal` should be
# converted to numeric. Initializing from Integer is faster
# than from String for BigDecimal.
#
# @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)
#
# # 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
extend TargetRubyVersion

minimum_target_ruby_version 3.1
Copy link
Contributor

@Earlopain Earlopain Sep 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use requires_gem API here? bigdecimal is becoming a bundled gem so many already have it in their lockfile and the ruby version is barely relevant anymore. This would stop this cop from doing something if the user doesn't use bundler, or for those who somehow don't have it in their lockfile yet but I think this is the way to go

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, a solution that prioritizes the BigDecimal version would be best in the future, but this PR does not take that into account.

As the testing methodology for that approach doesn't seem to be established, it is not included in this PR. The focus is currently on resolving the more common cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. You already meant requires_gem wit with that


MSG = 'Convert numeric literal to string and pass it to `BigDecimal`.'
MSG = 'Convert string literal to numeric 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? ...)
(send nil? :BigDecimal $str_type? ...)
PATTERN

def_node_matcher :to_d?, <<~PATTERN
(send [!nil? $numeric_type?] :to_d ...)
(send [!nil? $str_type?] :to_d ...)
PATTERN

def on_send(node)
if (numeric = big_decimal_with_numeric_argument?(node))
add_offense(numeric.source_range) do |corrector|
corrector.wrap(numeric, "'", "'")
if (string = big_decimal_with_numeric_argument?(node))
add_offense(string.source_range) do |corrector|
corrector.replace(string, string.value)
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(', ')
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(', ')

corrector.replace(node, "BigDecimal(#{big_decimal_args})")
end
Expand Down
254 changes: 132 additions & 122 deletions spec/rubocop/cop/performance/big_decimal_with_numeric_argument_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,129 +1,139 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::BigDecimalWithNumericArgument, :config do
it 'registers an offense and corrects when using `BigDecimal` with integer' do
expect_offense(<<~RUBY)
BigDecimal(1)
^ Convert numeric literal to string and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal('1')
RUBY
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`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(1)
RUBY
end

it 'registers an offense and corrects when using `String#to_d`' do
expect_offense(<<~RUBY)
'1'.to_d
^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(1)
RUBY
end

it 'registers an offense and corrects when using `BigDecimal` with float string' do
expect_offense(<<~RUBY)
BigDecimal('1.5', exception: true)
^^^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(1.5, exception: true)
RUBY
end

it 'registers an offense and corrects when using float `String#to_d`' do
expect_offense(<<~RUBY)
'1.5'.to_d(exception: true)
^^^^^ Convert string literal to numeric 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 string and precision' do
expect_offense(<<~RUBY)
BigDecimal('3.14', 1)
^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(3.14, 1)
RUBY
end

it 'registers an offense when using float `String#to_d` with precision' do
expect_offense(<<~RUBY)
'3.14'.to_d(1)
^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(3.14, 1)
RUBY
end

it 'registers an offense when using `BigDecimal` with float string and non-literal precision' do
expect_offense(<<~RUBY)
precision = 1
BigDecimal('3.14', precision)
^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
precision = 1
BigDecimal(3.14, precision)
RUBY
end

it 'registers an offense when using float `String#to_d` with non-literal precision' do
expect_offense(<<~RUBY)
precision = 1
'3.14'.to_d(precision)
^^^^^^ Convert string literal to numeric 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 string, 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`.
RUBY

expect_correction(<<~RUBY)
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
expect_offense(<<~RUBY)
'3.14'.to_d(1, exception: true)
^^^^^^ Convert string literal to numeric and pass it to `BigDecimal`.
RUBY

expect_correction(<<~RUBY)
BigDecimal(3.14, 1, exception: true)
RUBY
end

it 'does not register an offense when using `BigDecimal` with integer' do
expect_no_offenses(<<~RUBY)
BigDecimal(1)
RUBY
end

it 'does not register an offense when using `Integer#to_d`' do
expect_no_offenses(<<~RUBY)
1.to_d
RUBY
end
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)
BigDecimal('1')
RUBY
end

it 'registers an offense and corrects when using `BigDecimal` with float' do
expect_offense(<<~RUBY)
BigDecimal(1.5, 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 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 '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 '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

it 'does not register an offense when using `BigDecimal` with string' do
expect_no_offenses(<<~RUBY)
BigDecimal('1')
RUBY
end

it 'does not register an offense when using `String#to_d`' do
expect_no_offenses(<<~RUBY)
'1'.to_d
RUBY
context 'when Ruby <= 3.0', :ruby30, unsupported_on: :prism do
it 'does not register an offense and corrects when using `BigDecimal` with string' do
expect_no_offenses(<<~RUBY)
BigDecimal('1')
RUBY
end
end
end