diff --git a/changelog/new_short_circuit_and_for_cost_effective_evaluation.md b/changelog/new_short_circuit_and_for_cost_effective_evaluation.md new file mode 100644 index 000000000..a37ff2e01 --- /dev/null +++ b/changelog/new_short_circuit_and_for_cost_effective_evaluation.md @@ -0,0 +1 @@ +* [#505](https://github.com/rubocop/rubocop-performance/issues/505): Add a new cop to enforce short-circuiting of logical `and`s by evaluating cheaper expressions first. ([@Am1ne77][]) diff --git a/config/default.yml b/config/default.yml index 42b2fc5b0..6205ffbdd 100644 --- a/config/default.yml +++ b/config/default.yml @@ -294,6 +294,13 @@ Performance/SelectMap: Enabled: false VersionAdded: '1.11' +Performance/ShortCircuitAnd: + Description: >- + Use Ruby's short-circuiting behavior to evaluate cheaper + expressions before expensive ones in `and` operations. + Enabled: 'pending' + VersionAdded: <> + Performance/Size: Description: >- Use `size` instead of `count` for counting diff --git a/lib/rubocop/cop/performance/short_circuit_and.rb b/lib/rubocop/cop/performance/short_circuit_and.rb new file mode 100644 index 000000000..2c75cadf4 --- /dev/null +++ b/lib/rubocop/cop/performance/short_circuit_and.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Identifies logical expressions where a cheaper operand + # appears after a more expensive one (like a method call) in `and`/`&&` expressions. + # + # Reordering such expressions improves performance by leveraging Ruby's short-circuiting + # behavior ensuring inexpensive checks are evaluated first. + # + # @example + # # bad + # costly_method? && local_var + # + # # good + # local_var && costly_method? + class ShortCircuitAnd < Base + extend AutoCorrector + + MSG = 'Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls.' + + def on_and(and_node) + if and_node.children.first.and_type? && and_node.children[1].variable? + handle_child_and(and_node) + elsif !and_node.children.first.variable? && and_node.children[1].variable? + handle_leaf(and_node) + end + end + + def handle_child_and(and_node) + top_and_node = and_node + var_node = and_node.children[1] + loop do + and_node = and_node.children.first + unless and_node.children[1].variable? + add_offense(top_and_node) do |corrector| + corrector.swap(var_node, and_node.children[1]) + end + end + break unless and_node.children.first.and_type? + end + end + + def handle_leaf(and_node) + add_offense(and_node) do |corrector| + corrector.swap(and_node.children.first, and_node.children[1]) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/performance/sum.rb b/lib/rubocop/cop/performance/sum.rb index f91a80039..2187199ba 100644 --- a/lib/rubocop/cop/performance/sum.rb +++ b/lib/rubocop/cop/performance/sum.rb @@ -154,7 +154,7 @@ def handle_sum_map_candidate(node) def empty_array_literal?(node) receiver = node.children.first - array_literal?(node) && receiver && receiver.children.empty? + receiver && array_literal?(node) && receiver.children.empty? end def array_literal?(node) diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 8a56be5db..2b4fe778c 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -41,6 +41,7 @@ require_relative 'performance/reverse_each' require_relative 'performance/reverse_first' require_relative 'performance/select_map' +require_relative 'performance/short_circuit_and' require_relative 'performance/size' require_relative 'performance/sort_reverse' require_relative 'performance/squeeze' diff --git a/spec/rubocop/cop/performance/short_circuit_and_spec.rb b/spec/rubocop/cop/performance/short_circuit_and_spec.rb new file mode 100644 index 000000000..a653557f3 --- /dev/null +++ b/spec/rubocop/cop/performance/short_circuit_and_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::ShortCircuitAnd, :config do + defs = <<~RUBY + a = 42 + def foo? + 42 + end + RUBY + + shared_examples 'logical_and' do |logical_and| + it "registers an offense for a single misordered use of `#{logical_and}`" do + expect_offense(<<~RUBY, logical_and: logical_and) + #{defs} + foo? #{logical_and} a + ^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls. + RUBY + + expect_correction(<<~RUBY) + #{defs} + a #{logical_and} foo? + RUBY + end + + it "doesn't register an offense for a single well ordered use of `#{logical_and}`" do + expect_no_offenses(<<~RUBY) + #{defs} + a #{logical_and} foo? + RUBY + end + + it "registers offenses for multiple misordered uses of `#{logical_and}`" do + expect_offense(<<~RUBY, logical_and: logical_and) + #{defs} + foo? #{logical_and} a #{logical_and} foo? #{logical_and} a + ^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls. + ^^^^^^{logical_and}^^^^{logical_and}^^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls. + RUBY + + expect_correction(<<~RUBY) + #{defs} + a #{logical_and} a #{logical_and} foo? #{logical_and} foo? + RUBY + end + + it "doesn't register offenses for multiple misordered uses of `#{logical_and}`" do + expect_no_offenses(<<~RUBY) + #{defs} + a #{logical_and} a #{logical_and} foo? #{logical_and} foo? + RUBY + end + end + + include_examples 'logical_and', '&&' + include_examples 'logical_and', 'and' +end