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.

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

Autocorrect for Performance/BigDecimalWithNumericArgument cop converts an existing code to an invalid code #470

Closed
y-yagi opened this issue Sep 17, 2024 · 2 comments

Comments

@y-yagi
Copy link

y-yagi commented Sep 17, 2024

Expected behavior

Autocorrect for Performance/BigDecimalWithNumericArgument cop converts an existing code to a valid code

Actual behavior

Autocorrect for Performance/BigDecimalWithNumericArgument cop converts an existing code to an invalid code.

Steps to reproduce the problem

Sinc #463, autocorrect for Performance/BigDecimalWithNumericArgument cop converts string argument of BigDecimal to numeric. But, if a value is float, that generates an invalid code.

For example, if we have BigDecimal("0.8"), autocorrect converts it to BigDecimal(0.8), but this is an invalid code.

require 'bigdecimal'

BigDecimal(0.8)
(irb):3:in `BigDecimal': can't omit precision for a Float. (ArgumentError)

BigDecimal(0.8)
           ^^^
        from (irb):3:in `<main>'
        from <internal:kernel>:187:in `loop'
        from /home/yyagi/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/irb-1.14.0/exe/irb:9:in `<top (required)>'
        from /home/yyagi/.rbenv/versions/3.3.5/bin/irb:25:in `load'
        from /home/yyagi/.rbenv/versions/3.3.5/bin/irb:25:in `<main>' 

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:
You can see extension cop versions (e.g. rubocop-performance, rubocop-rails, and others) output by rubocop -V,
include them as well. Here's an example:

$ bundle exec rubocop -V
1.66.1 (using Parser 3.3.5.0, rubocop-ast 1.32.3, running on ruby 3.3.3) [x86_64-linux]
  - rubocop-factory_bot 2.26.1
  - rubocop-performance 1.22.0
  - rubocop-rails 2.26.1
  - rubocop-rspec 3.0.5
  - rubocop-rspec_rails 2.30.0

Additional Information

I understand that change is caused by #454. But, if a value is a float, I think BigDecimal("0.8") is still faster than BigDecimal(0.8, 0).

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem 'benchmark-ips'
  gem 'bigdecimal'
end

require 'benchmark/ips'
require 'bigdecimal'
require 'bigdecimal/util'

Benchmark.ips do |x|
  x.report('float string new')  { BigDecimal("0.8") }
  x.report('float new')         { BigDecimal(0.8, 0) }
  x.compare!
end
$ ruby bug_report_rubocop.rb  

Fetching gem metadata from https://rubygems.org/.
Resolving dependencies...
ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
Warming up --------------------------------------
    float string new   409.997k i/100ms
           float new   272.709k i/100ms
Calculating -------------------------------------
    float string new      4.128M (± 1.4%) i/s  (242.27 ns/i) -     20.910M in   5.066891s
           float new      2.716M (± 2.9%) i/s  (368.25 ns/i) -     13.635M in   5.025915s

Comparison:
    float string new:  4127550.6 i/s
           float new:  2715531.7 i/s - 1.52x  slower
@Earlopain
Copy link
Contributor

This should be fixed with #469, can you please update? It's already been released.

@y-yagi
Copy link
Author

y-yagi commented Sep 17, 2024

I missed it, thanks!

@y-yagi y-yagi closed this as completed Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants