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 #10648] Allow Style/TernaryParentheses to take priority over Style/RedundantParentheses for requiring parentheses #10874

Merged
merged 1 commit into from Aug 6, 2022
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
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