Skip to content

Add new Performance/ConditionalDefinition cop #499

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<<next>>'

Performance/ConstantRegexp:
Description: 'Finds regular expressions with dynamic components that are all constants.'
Enabled: pending
Expand Down
68 changes: 68 additions & 0 deletions lib/rubocop/cop/performance/conditional_definition.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
53 changes: 53 additions & 0 deletions spec/rubocop/cop/performance/conditional_definition_spec.rb
Original file line number Diff line number Diff line change
@@ -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