diff --git a/changelog/new_add_performancemumericpredicate_cop.md b/changelog/new_add_performancemumericpredicate_cop.md new file mode 100644 index 0000000000..a16c45f0e8 --- /dev/null +++ b/changelog/new_add_performancemumericpredicate_cop.md @@ -0,0 +1 @@ +* [#440](https://github.com/rubocop/rubocop-performance/pull/440): Add Performance::NumericPredicate cop. ([@miry][]) diff --git a/config/default.yml b/config/default.yml index a508a39827..17c4c5d8f7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -205,6 +205,12 @@ Performance/MethodObjectAsBlock: Enabled: pending VersionAdded: '1.9' +Performance/NumericPredicate: + Description: 'Use compare operator instead of `Numeric#positive?`, `Numeric#negative?`, or `Numeric#zero?`.' + Enabled: pending + Safe: false + VersionAdded: '<>' + Performance/OpenStruct: Description: 'Use `Struct` instead of `OpenStruct`.' Enabled: false diff --git a/lib/rubocop/cop/performance/numeric_predicate.rb b/lib/rubocop/cop/performance/numeric_predicate.rb new file mode 100644 index 0000000000..85dfc73d08 --- /dev/null +++ b/lib/rubocop/cop/performance/numeric_predicate.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Identifies places where numeric uses predicates `positive?`, and `negative?` should be + # converted to compare operator. + # + # @safety + # This cop is unsafe because it cannot be guaranteed that the receiver + # defines the predicates or can be compared to a number, which may lead + # to a false positive for non-standard classes. + # + # @example + # # bad + # 1.positive? + # 1.43.negative? + # -4.zero? + # + # # good + # 1 > 0 + # 1.43 < 0.0 + # -4 == 0 + # + class NumericPredicate < Base + extend AutoCorrector + + MSG = 'Use compare operator `%s` instead of `%s`.' + RESTRICT_ON_SEND = %i[positive? zero? negative?].freeze + REPLACEMENTS = { negative?: '<', positive?: '>', zero?: '==' }.freeze + + def_node_matcher :num_predicate?, <<~PATTERN + (send $numeric_type? ${:negative? :positive? :zero?}) + PATTERN + + def_node_matcher :instance_predicate?, <<~PATTERN + (send $!nil? ${:negative? :positive?}) + PATTERN + + def on_send(node) + return unless num_predicate?(node) || instance_predicate?(node) + return unless node.receiver + + good_method = build_good_method(node.receiver, node.method_name) + message = format(MSG, good: good_method, bad: node.source) + add_offense(node, message: message) do |corrector| + corrector.replace(node, good_method) + end + end + + private + + def build_good_method(receiver, method) + operation = REPLACEMENTS[method] + zero = receiver&.float_type? ? 0.0 : 0 + "#{receiver.source} #{operation} #{zero}" + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 2a18f26120..78cbc0a102 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -27,6 +27,7 @@ require_relative 'performance/map_compact' require_relative 'performance/map_method_chain' require_relative 'performance/method_object_as_block' +require_relative 'performance/numeric_predicate' require_relative 'performance/open_struct' require_relative 'performance/range_include' require_relative 'performance/io_readlines' diff --git a/spec/rubocop/cop/performance/numeric_predicate_spec.rb b/spec/rubocop/cop/performance/numeric_predicate_spec.rb new file mode 100644 index 0000000000..0495792be2 --- /dev/null +++ b/spec/rubocop/cop/performance/numeric_predicate_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::NumericPredicate, :config do + let(:message) { RuboCop::Cop::Performance::NumericPredicate::MSG } + + shared_examples 'common functionality' do |method, op| + it 'for integer' do + expect_offense(<<~RUBY, method: method) + 1.#{method} + ^^^{method} Use compare operator `1 #{op} 0` instead of `1.#{method}`. + RUBY + + expect_correction(<<~RUBY) + 1 #{op} 0 + RUBY + end + + it 'for float' do + expect_offense(<<~RUBY, method: method) + 1.2.#{method} + ^^^^^{method} Use compare operator `1.2 #{op} 0.0` instead of `1.2.#{method}`. + RUBY + + expect_correction(<<~RUBY) + 1.2 #{op} 0.0 + RUBY + end + + it 'ignore big decimal' do + next if method == 'zero?' + + expect_offense(<<~RUBY, method: method) + BigDecimal('1', 2).#{method} + ^^^^^^^^^^^^^^^^^^^^{method} Use compare operator `BigDecimal('1', 2) #{op} 0` instead of `BigDecimal('1', 2).#{method}`. + RUBY + + expect_correction(<<~RUBY) + BigDecimal('1', 2) #{op} 0 + RUBY + end + + it 'for variable' do + next if method == 'zero?' + + expect_offense(<<~RUBY, method: method) + foo = 1 + foo.#{method} + ^^^^^{method} Use compare operator `foo #{op} 0` instead of `foo.#{method}`. + RUBY + + expect_correction(<<~RUBY) + foo = 1 + foo #{op} 0 + RUBY + end + + it 'in condition statements' do + next if method == 'zero?' + + expect_offense(<<~RUBY, method: method) + foo = 1 + if foo.#{method} + ^^^^^{method} Use compare operator `foo #{op} 0` instead of `foo.#{method}`. + end + RUBY + + expect_correction(<<~RUBY) + foo = 1 + if foo #{op} 0 + end + RUBY + end + + it 'in map statement' do + next if method == 'zero?' + + expect_no_offenses(<<~RUBY, method: method) + foo = [1, 2, 3] + if foo.all?(&:#{method}) + end + RUBY + end + end + + it_behaves_like 'common functionality', 'positive?', '>' + it_behaves_like 'common functionality', 'negative?', '<' + it_behaves_like 'common functionality', 'zero?', '==' +end