Skip to content
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

Fix false negatives for Style/RedundantParentheses #12247

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12247](https://github.com/rubocop/rubocop/pull/12247): Fix false negatives for `Style/RedundantParentheses` when using logical or comparison expressions with redundant parentheses. ([@koic][])
26 changes: 21 additions & 5 deletions lib/rubocop/cop/style/redundant_parentheses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,32 @@ def method_chain_begins_with_hash_literal?(node)

def check(begin_node)
node = begin_node.children.first
return offense(begin_node, 'a keyword') if keyword_with_redundant_parentheses?(node)
return offense(begin_node, 'a literal') if disallowed_literal?(begin_node, node)
return offense(begin_node, 'a variable') if node.variable?
return offense(begin_node, 'a constant') if node.const_type?

return offense(begin_node, 'an interpolated expression') if interpolation?(begin_node)
if (message = find_offense_message(begin_node, node))
return offense(begin_node, message)
end

check_send(begin_node, node) if node.call_type?
end

# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def find_offense_message(begin_node, node)
return 'a keyword' if keyword_with_redundant_parentheses?(node)
return 'a literal' if disallowed_literal?(begin_node, node)
return 'a variable' if node.variable?
return 'a constant' if node.const_type?
return 'an interpolated expression' if interpolation?(begin_node)

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

if node.and_type? || node.or_type?
'a logical expression'
elsif node.respond_to?(:comparison_method?) && node.comparison_method?
'a comparison expression'
end
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

# @!method interpolation?(node)
def_node_matcher :interpolation?, '[^begin ^^dstr]'

Expand Down
32 changes: 31 additions & 1 deletion spec/rubocop/cop/style/redundant_parentheses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,19 @@
it_behaves_like 'redundant', '({0 => :a}[0])', '{0 => :a}[0]', 'a method call'
it_behaves_like 'redundant', '(x; y)', 'x; y', 'a method call'

it_behaves_like 'redundant', '(x && y)', 'x && y', 'a logical expression'
it_behaves_like 'redundant', '(x || y)', 'x || y', 'a logical expression'
it_behaves_like 'redundant', '(x and y)', 'x and y', 'a logical expression'
it_behaves_like 'redundant', '(x or y)', 'x or y', 'a logical expression'

it_behaves_like 'redundant', '(x == y)', 'x == y', 'a comparison expression'
it_behaves_like 'redundant', '(x === y)', 'x === y', 'a comparison expression'
it_behaves_like 'redundant', '(x != y)', 'x != y', 'a comparison expression'
it_behaves_like 'redundant', '(x > y)', 'x > y', 'a comparison expression'
it_behaves_like 'redundant', '(x >= y)', 'x >= y', 'a comparison expression'
it_behaves_like 'redundant', '(x < y)', 'x < y', 'a comparison expression'
it_behaves_like 'redundant', '(x <= y)', 'x <= y', 'a comparison expression'

it_behaves_like 'redundant', '(!x)', '!x', 'a unary operation'
it_behaves_like 'redundant', '(~x)', '~x', 'a unary operation'
it_behaves_like 'redundant', '(-x)', '-x', 'a unary operation'
Expand Down Expand Up @@ -480,10 +493,27 @@ def x
expect_no_offenses('(a...b)')
end

it 'accepts parentheses around operator keywords' do
it 'accepts parentheses around logical operator keywords' do
expect_no_offenses('(1 and 2) and (3 or 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 a method call with parenthesized logical expression receiver' do
expect_no_offenses('(x || y).z')
end

it 'accepts parentheses around a method call with parenthesized comparison expression receiver' do
expect_no_offenses('(x == y).zero?')
end

it 'accepts parentheses around single argument separated by semicolon' do
expect_no_offenses('x((prepare; perform))')
end

it 'registers an offense when there is space around the parentheses' do
expect_offense(<<~RUBY)
if x; y else (1) end
Expand Down