Merge pull request #12247 from koic/fix_false_negatives_for_style_red…

…undant_parentheses

Fix false negatives for `Style/RedundantParentheses`

changelog/fix_false_negatives_for_style_redundant_parentheses.md

@@ -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][])

lib/rubocop/cop/style/redundant_parentheses.rb

@@ -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]'
 

spec/rubocop/cop/style/redundant_parentheses_spec.rb

@@ -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'
@@ -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