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

Add new Lint/FloatComparison cop #8432

Merged
merged 1 commit into from Aug 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
* [#8389](https://github.com/rubocop-hq/rubocop/pull/8389): Add new `Lint/DuplicateRescueException` cop. ([@fatkodima][])
* [#8430](https://github.com/rubocop-hq/rubocop/pull/8430): Add new `Lint/UnreachableLoop` cop. ([@fatkodima][])
* [#8412](https://github.com/rubocop-hq/rubocop/pull/8412): Add new `Style/OptionalBooleanParameter` cop. ([@fatkodima][])
* [#8432](https://github.com/rubocop-hq/rubocop/pull/8432): Add new `Lint/FloatComparison` cop. ([@fatkodima][])
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): Add new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8415](https://github.com/rubocop-hq/rubocop/pull/8415): Add new `Style/ExplicitBlockArgument` cop. ([@fatkodima][])
* [#8339](https://github.com/rubocop-hq/rubocop/pull/8339): Add `Config#for_badge` as an efficient way to get a cop's config merged with its department's. ([@marcandre][])
Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -1478,6 +1478,12 @@ Lint/FlipFlop:
Enabled: true
VersionAdded: '0.16'

Lint/FloatComparison:
Description: 'Checks for the presence of precise comparison of floating point numbers.'
StyleGuide: '#float-comparison'
Enabled: pending
VersionAdded: '0.89'

Lint/FloatOutOfRange:
Description: >-
Catches floating-point literals too large or small for Ruby to
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -206,6 +206,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintensurereturn[Lint/EnsureReturn]
* xref:cops_lint.adoc#linterbnewarguments[Lint/ErbNewArguments]
* xref:cops_lint.adoc#lintflipflop[Lint/FlipFlop]
* xref:cops_lint.adoc#lintfloatcomparison[Lint/FloatComparison]
* xref:cops_lint.adoc#lintfloatoutofrange[Lint/FloatOutOfRange]
* xref:cops_lint.adoc#lintformatparametermismatch[Lint/FormatParameterMismatch]
* xref:cops_lint.adoc#lintheredocmethodcallposition[Lint/HeredocMethodCallPosition]
Expand Down
45 changes: 45 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -1229,6 +1229,51 @@ end

* https://rubystyle.guide#no-flip-flops

== Lint/FloatComparison

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 0.89
| -
|===

This cop checks for the presence of precise comparison of floating point numbers.

Floating point values are inherently inaccurate, and comparing them for exact equality
is almost never the desired semantics. Comparison via the `==/!=` operators checks
floating-point value representation to be exactly the same, which is very unlikely
if you perform any arithmetic operations involving precision loss.

=== Examples

[source,ruby]
----
# bad
x == 0.1
x != 0.1

# good - using BigDecimal
x.to_d == 0.1.to_d

# good
(x - 0.1).abs < Float::EPSILON
fatkodima marked this conversation as resolved.
Show resolved Hide resolved

# good
tolerance = 0.0001
(x - 0.1).abs < tolerance

# Or some other epsilon based type of comparison:
# https://www.embeddeduse.com/2019/08/26/qt-compare-two-floats/
----

=== References

* https://rubystyle.guide#float-comparison

== Lint/FloatOutOfRange

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -264,6 +264,7 @@
require_relative 'rubocop/cop/lint/ensure_return'
require_relative 'rubocop/cop/lint/erb_new_arguments'
require_relative 'rubocop/cop/lint/flip_flop'
require_relative 'rubocop/cop/lint/float_comparison'
require_relative 'rubocop/cop/lint/float_out_of_range'
require_relative 'rubocop/cop/lint/format_parameter_mismatch'
require_relative 'rubocop/cop/lint/heredoc_method_call_position'
Expand Down
93 changes: 93 additions & 0 deletions lib/rubocop/cop/lint/float_comparison.rb
@@ -0,0 +1,93 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for the presence of precise comparison of floating point numbers.
#
# Floating point values are inherently inaccurate, and comparing them for exact equality
# is almost never the desired semantics. Comparison via the `==/!=` operators checks
# floating-point value representation to be exactly the same, which is very unlikely
# if you perform any arithmetic operations involving precision loss.
#
# @example
# # bad
# x == 0.1
# x != 0.1
#
# # good - using BigDecimal
# x.to_d == 0.1.to_d
#
# # good
# (x - 0.1).abs < Float::EPSILON
#
# # good
# tolerance = 0.0001
# (x - 0.1).abs < tolerance
#
# # Or some other epsilon based type of comparison:
# # https://www.embeddeduse.com/2019/08/26/qt-compare-two-floats/
#
class FloatComparison < Base
MSG = 'Avoid (in)equality comparisons of floats as they are unreliable.'

EQUALITY_METHODS = %i[== != eql? equal?].freeze
FLOAT_RETURNING_METHODS = %i[to_f Float fdiv].freeze
FLOAT_INSTANCE_METHODS = %i[@- abs magnitude modulo next_float prev_float quo].to_set.freeze

def on_send(node)
return unless EQUALITY_METHODS.include?(node.method_name)

lhs, _method, rhs = *node
add_offense(node) if float?(lhs) || float?(rhs)
end

private

def float?(node)
return false unless node

case node.type
when :float
true
when :send
check_send(node)
when :begin
float?(node.children.first)
else
false
end
end

# rubocop:disable Metrics/PerceivedComplexity
def check_send(node)
if node.arithmetic_operation?
lhs, _operation, rhs = *node
float?(lhs) || float?(rhs)
elsif FLOAT_RETURNING_METHODS.include?(node.method_name)
true
elsif node.receiver&.float_type?
if FLOAT_INSTANCE_METHODS.include?(node.method_name)
true
else
check_numeric_returning_method(node)
end
end
end
# rubocop:enable Metrics/PerceivedComplexity

def check_numeric_returning_method(node)
return false unless node.receiver

case node.method_name
when :angle, :arg, :phase
Float(node.receiver.source).negative?
when :ceil, :floor, :round, :truncate
precision = node.first_argument
precision&.int_type? && Integer(precision.source).positive?
end
end
end
end
end
end
66 changes: 66 additions & 0 deletions spec/rubocop/cop/lint/float_comparison_spec.rb
@@ -0,0 +1,66 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::FloatComparison do
subject(:cop) { described_class.new }

it 'registers an offense when comparing with float' do
offenses = inspect_source(<<~RUBY)
x == 0.1
0.1 == x
x != 0.1
0.1 != x
x.eql?(0.1)
0.1.eql?(x)
RUBY

expect(offenses.size).to eq(6)
end

it 'registers an offense when comparing with float returning method' do
offenses = inspect_source(<<~RUBY)
x == Float(1)
x == '0.1'.to_f
x == 1.fdiv(2)
RUBY

expect(offenses.size).to eq(3)
end

it 'registers an offense when comparing with arightmetic operator on floats' do
offenses = inspect_source(<<~RUBY)
x == 0.1 + y
x == y + Float('0.1')
x == y + z * (foo(arg) + '0.1'.to_f)
RUBY

expect(offenses.size).to eq(3)
end

it 'registers an offense when comparing with method on float receiver' do
expect_offense(<<~RUBY)
x == 0.1.abs
^^^^^^^^^^^^ Avoid (in)equality comparisons of floats as they are unreliable.
RUBY
end

it 'does not register an offense when comparing with float method '\
'that can return numeric and returns integer' do
expect_no_offenses(<<~RUBY)
x == 1.1.ceil
RUBY
end

it 'registers an offense when comparing with float method '\
'that can return numeric and returns float' do
expect_offense(<<~RUBY)
x == 1.1.ceil(1)
^^^^^^^^^^^^^^^^ Avoid (in)equality comparisons of floats as they are unreliable.
RUBY
end

it 'does not register an offense when comparing with float using epsilon' do
expect_no_offenses(<<~RUBY)
(x - 0.1) < epsilon
RUBY
end
end