Skip to content

Commit

Permalink
[Fix rubocop#10648] Allow Style/TernaryParentheses to take priority…
Browse files Browse the repository at this point in the history
… over `Style/RedundantParentheses`.

`Style/TernaryParentheses` allows RuboCop to require ternary conditions are wrapped in parentheses, but this behaviour doesn't take effect when `Style/RedundantParentheses` is also enabled, in order to prevent an infinite loop.

However, it makes more sense for `TernaryParentheses` to take priority here and have `RedudantParentheses` allow the parentheses instead of creating an infinite autocorrection loop.

This change moves the infinite loop protection into `RedundantParentheses` instead.
  • Loading branch information
dvandersluis committed Aug 5, 2022
1 parent 4f9cc54 commit d267934
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 50 deletions.
1 change: 1 addition & 0 deletions changelog/change_allow_styleternaryparentheses_to_take.md
@@ -0,0 +1 @@
* [#10648](https://github.com/rubocop/rubocop/issues/10648): Allow `Style/TernaryParentheses` to take priority over `Style/RedundantParentheses` when parentheses are enforced. ([@dvandersluis][])
16 changes: 15 additions & 1 deletion lib/rubocop/cop/style/redundant_parentheses.rb
Expand Up @@ -58,7 +58,8 @@ def allowed_expression?(node)
allowed_ancestor?(node) ||
allowed_method_call?(node) ||
allowed_array_or_hash_element?(node) ||
allowed_multiple_expression?(node)
allowed_multiple_expression?(node) ||
allowed_ternary?(node)
end

def allowed_ancestor?(node)
Expand All @@ -80,6 +81,19 @@ def allowed_multiple_expression?(node)
!ancestor.begin_type? && !ancestor.def_type? && !ancestor.block_type?
end

def allowed_ternary?(node)
return unless node&.parent&.if_type?

node.parent.ternary? && ternary_parentheses_required?
end

def ternary_parentheses_required?
config = @config.for_cop('Style/TernaryParentheses')
allowed_styles = %w[require_parentheses require_parentheses_when_complex]

config.fetch('Enabled') && allowed_styles.include?(config['EnforcedStyle'])
end

def like_method_argument_parentheses?(node)
node.send_type? && node.arguments.one? &&
!node.arithmetic_operation? && node.first_argument.begin_type?
Expand Down
14 changes: 1 addition & 13 deletions lib/rubocop/cop/style/ternary_parentheses.rb
Expand Up @@ -71,7 +71,7 @@ def on_if(node)

return if only_closing_parenthesis_is_last_line?(condition)
return if condition_as_parenthesized_one_line_pattern_matching?(condition)
return unless node.ternary? && !infinite_loop? && offense?(node)
return unless node.ternary? && offense?(node)

message = message(node)

Expand Down Expand Up @@ -166,22 +166,10 @@ def require_parentheses_when_complex?
style == :require_parentheses_when_complex
end

def redundant_parentheses_enabled?
@config.for_cop('Style/RedundantParentheses').fetch('Enabled')
end

def parenthesized?(node)
node.begin_type?
end

# When this cop is configured to enforce parentheses and the
# `RedundantParentheses` cop is enabled, it will cause an infinite loop
# as they compete to add and remove the parentheses respectively.
def infinite_loop?
(require_parentheses? || require_parentheses_when_complex?) &&
redundant_parentheses_enabled?
end

def unsafe_autocorrect?(condition)
condition.children.any? do |child|
unparenthesized_method_call?(child) || below_ternary_precedence?(child)
Expand Down
26 changes: 26 additions & 0 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -2640,4 +2640,30 @@ module Foo#{trailing_whitespace}
]
RUBY
end

it 'properly autocorrects when `Style/TernaryParentheses` requires parentheses ' \
'that `Style/RedundantParentheses` would otherwise remove' do
source_file = Pathname('example.rb')
create_file(source_file, <<~RUBY)
foo ? bar : baz
RUBY

create_file('.rubocop.yml', <<~YAML)
Style/TernaryParentheses:
EnforcedStyle: require_parentheses
Style/RedundantParentheses:
Enabled: true
YAML

status = cli.run(
%w[--autocorrect-all --only] << %w[
Style/TernaryParentheses
Style/RedundantParentheses
].join(',')
)
expect(status).to eq(0)
expect(source_file.read).to eq(<<~RUBY)
(foo) ? bar : baz
RUBY
end
end
69 changes: 58 additions & 11 deletions spec/rubocop/cop/style/redundant_parentheses_spec.rb
Expand Up @@ -57,18 +57,65 @@
it_behaves_like 'redundant', '(retry)', 'retry', 'a keyword'
it_behaves_like 'redundant', '(self)', 'self', 'a keyword'

it 'registers an offense for parens around constant ternary condition' do
expect_offense(<<~RUBY)
(X) ? Y : N
^^^ Don't use parentheses around a constant.
(X)? Y : N
^^^ Don't use parentheses around a constant.
RUBY
context 'ternaries' do
let(:other_cops) do
{
'Style/TernaryParentheses' => {
'Enabled' => ternary_parentheses_enabled,
'EnforcedStyle' => ternary_parentheses_enforced_style
}
}
end
let(:ternary_parentheses_enabled) { true }
let(:ternary_parentheses_enforced_style) { nil }

context 'when Style/TernaryParentheses is not enabled' do
let(:ternary_parentheses_enabled) { false }

it 'registers an offense for parens around constant ternary condition' do
expect_offense(<<~RUBY)
(X) ? Y : N
^^^ Don't use parentheses around a constant.
(X)? Y : N
^^^ Don't use parentheses around a constant.
RUBY

expect_correction(<<~RUBY)
X ? Y : N
X ? Y : N
RUBY
end
end

expect_correction(<<~RUBY)
X ? Y : N
X ? Y : N
RUBY
context 'when Style/TernaryParentheses has EnforcedStyle: require_no_parentheses' do
let(:ternary_parentheses_enforced_style) { 'require_no_parentheses' }

it 'registers an offense for parens around ternary condition' do
expect_offense(<<~RUBY)
(X) ? Y : N
^^^ Don't use parentheses around a constant.
(X)? Y : N
^^^ Don't use parentheses around a constant.
RUBY

expect_correction(<<~RUBY)
X ? Y : N
X ? Y : N
RUBY
end
end

context 'when Style/TernaryParentheses has EnforcedStyle: require_parentheses' do
let(:ternary_parentheses_enforced_style) { 'require_parentheses' }

it_behaves_like 'plausible', '(X) ? Y : N'
end

context 'when Style/TernaryParentheses has EnforcedStyle: require_parentheses_when_complex' do
let(:ternary_parentheses_enforced_style) { 'require_parentheses_when_complex' }

it_behaves_like 'plausible', '(X) ? Y : N'
end
end

it_behaves_like 'keyword with return value', 'break'
Expand Down
25 changes: 0 additions & 25 deletions spec/rubocop/cop/style/ternary_parentheses_spec.rb
Expand Up @@ -2,11 +2,6 @@

RSpec.describe RuboCop::Cop::Style::TernaryParentheses, :config do
let(:redundant_parens_enabled) { false }
let(:other_cops) do
{
'Style/RedundantParentheses' => { 'Enabled' => redundant_parens_enabled }
}
end

shared_examples 'safe assignment disabled' do |style, message|
let(:cop_config) { { 'EnforcedStyle' => style, 'AllowSafeAssignment' => false } }
Expand Down Expand Up @@ -898,24 +893,4 @@
end
end
end

context 'when `RedundantParenthesis` would cause an infinite loop' do
let(:redundant_parens_enabled) { true }

context 'when `EnforcedStyle: require_parentheses`' do
let(:cop_config) { { 'EnforcedStyle' => 'require_parentheses' } }

it 'accepts' do
expect_no_offenses('foo = bar? ? a : b')
end
end

context 'when `EnforcedStyle: require_parentheses_when_complex`' do
let(:cop_config) { { 'EnforcedStyle' => 'require_parentheses_when_complex' } }

it 'accepts' do
expect_no_offenses('!condition.nil? ? foo : bar')
end
end
end
end

0 comments on commit d267934

Please sign in to comment.