Skip to content

Commit

Permalink
[Fix #12311] Fix false negatives for Style/RedundantParentheses
Browse files Browse the repository at this point in the history
Fixes #12311.

This PR fixes false negatives for `Style/RedundantParentheses`
when parentheses around logical operator keywords in method definition.
  • Loading branch information
koic authored and bbatsov committed Nov 9, 2023
1 parent e312ea9 commit 80fce12
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12311](https://github.com/rubocop/rubocop/issues/12311): Fix false negatives for `Style/RedundantParentheses` when parentheses around logical operator keywords in method definition. ([@koic][])
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/preceding_following_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def aligned_token?(range, line)
end

def aligned_operator?(range, line)
(aligned_identical?(range, line) || aligned_assignment?(range, line))
aligned_identical?(range, line) || aligned_assignment?(range, line)
end

def aligned_words?(range, line)
Expand Down
9 changes: 8 additions & 1 deletion lib/rubocop/cop/style/redundant_parentheses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class RedundantParentheses < Base
include Parentheses
extend AutoCorrector

ALLOWED_NODE_TYPES = %i[and or send splat kwsplat].freeze

# @!method square_brackets?(node)
def_node_matcher :square_brackets?, '(send {(send _recv _msg) str array hash} :[] ...)'

Expand Down Expand Up @@ -144,11 +146,16 @@ def find_offense_message(begin_node, node)
return 'a constant' if node.const_type?
return 'an interpolated expression' if interpolation?(begin_node)

return if begin_node.chained? || !begin_node.parent.nil?
return if begin_node.chained?

if node.and_type? || node.or_type?
return if ALLOWED_NODE_TYPES.include?(begin_node.parent&.type)
return if begin_node.parent&.if_type? && begin_node.parent&.ternary?

'a logical expression'
elsif node.respond_to?(:comparison_method?) && node.comparison_method?
return unless begin_node.parent.nil?

'a comparison expression'
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/redundant_string_escape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def percent_w_upper_literal?(node)
end

def percent_array_literal?(node)
(percent_w_literal?(node) || percent_w_upper_literal?(node))
percent_w_literal?(node) || percent_w_upper_literal?(node)
end

def heredoc_with_disabled_interpolation?(node)
Expand Down
74 changes: 73 additions & 1 deletion spec/rubocop/cop/style/redundant_parentheses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -493,15 +493,87 @@ def x
expect_no_offenses('(a...b)')
end

it 'accepts parentheses around logical operator keywords' do
it 'registers parentheses around `||` logical operator keywords in method definition' do
expect_offense(<<~RUBY)
def foo
(x || y)
^^^^^^^^ Don't use parentheses around a logical expression.
end
RUBY

expect_correction(<<~RUBY)
def foo
x || y
end
RUBY
end

it 'registers parentheses around `&&` logical operator keywords in method definition' do
expect_offense(<<~RUBY)
def foo
(x && y)
^^^^^^^^ Don't use parentheses around a logical expression.
end
RUBY

expect_correction(<<~RUBY)
def foo
x && y
end
RUBY
end

it 'accepts parentheses around arithmetic operator' do
expect_no_offenses('x - (y || z)')
end

it 'accepts parentheses around logical operator keywords (`and`, `and`, `or`)' do
expect_no_offenses('(1 and 2) and (3 or 4)')
end

it 'accepts parentheses around logical operator keywords (`or`, `or`, `and`)' do
expect_no_offenses('(1 or 2) or (3 and 4)')
end

it 'accepts parentheses around comparison operator keywords' do
# Parentheses are redundant, but respect user's intentions for readability.
expect_no_offenses('x && (y == z)')
end

it 'accepts parentheses around logical operator in splat' do
# Parentheses are redundant, but respect user's intentions for readability.
expect_no_offenses('x = *(y || z)')
end

it 'accepts parentheses around logical operator in double splat' do
# Parentheses are redundant, but respect user's intentions for readability.
expect_no_offenses('x(**(y || z))')
end

it 'accepts parentheses around logical operator in ternary operator' do
# Parentheses are redundant, but respect user's intentions for readability.
expect_no_offenses('cond ? x : (y || z)')
end

it 'registers parentheses around logical operator in `if`...`else`' do
expect_offense(<<~RUBY)
if cond
x
else
(y || z)
^^^^^^^^ Don't use parentheses around a logical expression.
end
RUBY

expect_correction(<<~RUBY)
if cond
x
else
y || z
end
RUBY
end

it 'accepts parentheses around a method call with parenthesized logical expression receiver' do
expect_no_offenses('(x || y).z')
end
Expand Down

0 comments on commit 80fce12

Please sign in to comment.