diff --git a/config/default.yml b/config/default.yml index 606acb83d..492138f72 100644 --- a/config/default.yml +++ b/config/default.yml @@ -89,6 +89,13 @@ Performance/ConcurrentMonotonicTime: Enabled: pending VersionAdded: '1.12' +Performance/ConditionalDefinition: + Description: >- + Move conditional logic outside of the method body to + improve performance by avoiding repeated evaluation of conditions. + Enabled: pending + VersionAdded: '<>' + Performance/ConstantRegexp: Description: 'Finds regular expressions with dynamic components that are all constants.' Enabled: pending diff --git a/lib/rubocop/cop/performance/conditional_definition.rb b/lib/rubocop/cop/performance/conditional_definition.rb new file mode 100644 index 000000000..edc913223 --- /dev/null +++ b/lib/rubocop/cop/performance/conditional_definition.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Move conditional logic outside of the method body definition to + # improve performance by avoiding repeated evaluation of constant condition. + # + # @example + # # bad + # def answer + # if RUBY_VERSION < '4.0.' + # 42 + # else + # 239 + # end + # end + # + # # good + # if RUBY_VERSION < '4.0.' + # def answer + # 42 + # end + # else + # def answer + # 239 + # end + # end + class ConditionalDefinition < Base + MSG = 'Move conditional logic outside of the method body definition to ' \ + 'improve performance by avoiding repeated evaluation of constant condition.' + + CONSTANTS = Set[ + :RUBY_VERSION, + :RUBY_RELEASE_DATE, + :RUBY_PLATFORM, + :RUBY_PATCHLEVEL, + :RUBY_REVISION, + :RUBY_COPYRIGHT, + :RUBY_ENGINE, + :RUBY_ENGINE_VERSION, + :RUBY_DESCRIPTION + ].freeze + OPERATORS = Set[:<, :<=, :==, :>=, :>, :=~, :===].freeze + private_constant :CONSTANTS, :OPERATORS + + def on_def(node) + return unless dynamic_definition_in_body?(node) + + dynamic_definition_in_body?(node) do + add_offense(node) + end + end + + # @!method bad_method?(node) + def_node_matcher :dynamic_definition_in_body?, <<~PATTERN + (def _ (...) + (if + { + (send (const nil? CONSTANTS) OPERATORS _) + (send _ OPERATORS (const nil? CONSTANTS)) + } + _ _)) + PATTERN + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 8a56be5db..16a3a30b7 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -14,6 +14,7 @@ require_relative 'performance/collection_literal_in_loop' require_relative 'performance/compare_with_block' require_relative 'performance/concurrent_monotonic_time' +require_relative 'performance/conditional_definition' require_relative 'performance/constant_regexp' require_relative 'performance/count' require_relative 'performance/delete_prefix' diff --git a/spec/rubocop/cop/performance/conditional_definition_spec.rb b/spec/rubocop/cop/performance/conditional_definition_spec.rb new file mode 100644 index 000000000..c08202327 --- /dev/null +++ b/spec/rubocop/cop/performance/conditional_definition_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::ConditionalDefinition, :config do + %i[ + RUBY_VERSION + RUBY_PLATFORM + RUBY_PATCHLEVEL + RUBY_ENGINE + RUBY_DESCRIPTION + ].each do |constant| + %i[< <= === >= > =~ ===].each do |operator| + it "registers an offense when using `#{operator}` comparison with `#{constant}` as LHS" do + expect_offense(<<~RUBY) + def foo + ^^^^^^^ Move conditional logic outside of the method body definition to improve performance by avoiding repeated evaluation of constant condition. + if #{constant} #{operator} '3.0' + 1 + else + 2 + end + end + RUBY + end + + it "registers an offense when using `#{operator}` comparison with `#{constant}` as RHS" do + expect_offense(<<~RUBY) + def foo + ^^^^^^^ Move conditional logic outside of the method body definition to improve performance by avoiding repeated evaluation of constant condition. + if '3.0' #{operator} #{constant} + 1 + else + 2 + end + end + RUBY + end + end + end + + it 'does not register an offense when conditional logic is moved outside of method definition' do + expect_no_offenses(<<~RUBY) + if RUBY_VERSION < '4.0.' + def answer + 42 + end + else + def answer + 239 + end + end + RUBY + end +end