Merge pull request #469 from koic/fix_false_positives_for_performance…

…_big_decimal_with_numeric_argument_cop

[Fix #468] Fix false positives for `Performance/BigDecimalWithNumericArgument`

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][])

lib/rubocop/cop/performance/big_decimal_with_numeric_argument.rb

@@ -3,53 +3,74 @@
 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
         extend TargetRubyVersion
 
         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

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