Skip to content

Commit 75ce9e0

Browse files
committed
[Fix #505] Add Cop for short-circuiting ands
1 parent 9661166 commit 75ce9e0

File tree

5 files changed

+118
-0
lines changed

5 files changed

+118
-0
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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][])

config/default.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,13 @@ Performance/SelectMap:
294294
Enabled: false
295295
VersionAdded: '1.11'
296296

297+
Performance/ShortCircuitAnd:
298+
Description: >-
299+
Use Ruby's short-circuiting behavior to evaluate cheaper
300+
expressions before expensive ones in `and` operations.
301+
Enabled: 'pending'
302+
VersionAdded: <<next>>
303+
297304
Performance/Size:
298305
Description: >-
299306
Use `size` instead of `count` for counting
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Performance
6+
# Identifies logical expressions where a cheaper operand
7+
# appears after a more expensive one (like a method call) in `and`/`&&` expressions.
8+
#
9+
# Reordering such expressions improves performance by leveraging Ruby's short-circuiting
10+
# behavior ensuring inexpensive checks are evaluated first.
11+
#
12+
# @example
13+
# # bad
14+
# costly_method? && local_var
15+
#
16+
# # good
17+
# local_var && costly_method?
18+
class ShortCircuitAnd < Base
19+
extend AutoCorrector
20+
21+
MSG = 'Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls.'
22+
23+
def on_and(and_node)
24+
if and_node.children.first.and_type? && and_node.children[1].variable?
25+
handle_child_and(and_node)
26+
elsif !and_node.children.first.variable? && and_node.children[1].variable?
27+
handle_leaf(and_node)
28+
end
29+
end
30+
31+
def handle_child_and(and_node)
32+
top_and_node = and_node
33+
var_node = and_node.children[1]
34+
loop do
35+
and_node = and_node.children.first
36+
unless and_node.children[1].variable?
37+
add_offense(top_and_node) do |corrector|
38+
corrector.swap(var_node, and_node.children[1])
39+
end
40+
end
41+
break unless and_node.children.first.and_type?
42+
end
43+
end
44+
45+
def handle_leaf(and_node)
46+
add_offense(and_node) do |corrector|
47+
corrector.swap(and_node.children.first, and_node.children[1])
48+
end
49+
end
50+
end
51+
end
52+
end
53+
end

lib/rubocop/cop/performance_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
require_relative 'performance/reverse_each'
4242
require_relative 'performance/reverse_first'
4343
require_relative 'performance/select_map'
44+
require_relative 'performance/short_circuit_and'
4445
require_relative 'performance/size'
4546
require_relative 'performance/sort_reverse'
4647
require_relative 'performance/squeeze'
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Performance::ShortCircuitAnd, :config do
4+
defs = <<~RUBY
5+
a = 42
6+
def foo?
7+
42
8+
end
9+
RUBY
10+
11+
shared_examples 'logical_and' do |logical_and|
12+
it "registers an offense for a single misordered use of `#{logical_and}`" do
13+
expect_offense(<<~RUBY, logical_and: logical_and)
14+
#{defs}
15+
foo? #{logical_and} a
16+
^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls.
17+
RUBY
18+
19+
expect_correction(<<~RUBY)
20+
#{defs}
21+
a #{logical_and} foo?
22+
RUBY
23+
end
24+
25+
it "doesn't register an offense for a single well ordered use of `#{logical_and}`" do
26+
expect_no_offenses(<<~RUBY)
27+
#{defs}
28+
a #{logical_and} foo?
29+
RUBY
30+
end
31+
32+
it "registers offenses for multiple misordered uses of `#{logical_and}`" do
33+
expect_offense(<<~RUBY, logical_and: logical_and)
34+
#{defs}
35+
foo? #{logical_and} a #{logical_and} foo? #{logical_and} a
36+
^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls.
37+
^^^^^^{logical_and}^^^^{logical_and}^^^^^^^{logical_and}^^ Use short-circuit logic with cheaper expressions first to avoid unnecessary method calls.
38+
RUBY
39+
40+
expect_correction(<<~RUBY)
41+
#{defs}
42+
a #{logical_and} a #{logical_and} foo? #{logical_and} foo?
43+
RUBY
44+
end
45+
46+
it "doesn't register offenses for multiple misordered uses of `#{logical_and}`" do
47+
expect_no_offenses(<<~RUBY)
48+
#{defs}
49+
a #{logical_and} a #{logical_and} foo? #{logical_and} foo?
50+
RUBY
51+
end
52+
end
53+
54+
include_examples 'logical_and', '&&'
55+
include_examples 'logical_and', 'and'
56+
end

0 commit comments

Comments
 (0)