From 6f20945899d905344dba76c3cd6a5db614a5d256 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 6 Sep 2024 14:28:24 +0900 Subject: [PATCH] [Fix #454] Fix false positives for `Performance/BigDecimalWithNumericArgument` Fixes #454. This PR fixes false positives for `Performance/BigDecimalWithNumericArgument` when using BigDecimal 3.1+. The bad and good examples for this cop are reversed between BigDecimal 3.0 and 3.1. Since BigDecimal 3.1 is the default gem in Ruby 3.1, this PR reverses the bad and good examples when targeting Ruby 3.1+. So, the goal of this PR is to address many cases where the meaning was entirely reversed. To avoid introducing excessive complexity related to version dependencies, detection is disabled for Ruby 3.0 and below. Ideally, a solution that prioritizes the BigDecimal version would be best in the future, but this PR does not take that into account. --- .../big_decimal_with_numeric_argument.rb | 39 +-- .../big_decimal_with_numeric_argument_spec.rb | 254 +++++++++--------- 2 files changed, 153 insertions(+), 140 deletions(-) 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 1909b6568e..2041f32c79 100644 --- a/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb +++ b/lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb @@ -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 - 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 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 2fc9ba4fdd..e15247db6b 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 @@ -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