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

Allow Style/TernaryParentheses to override Style/RedundantParentheses when parentheses are required #10648

Closed
jmuhlfel opened this issue May 18, 2022 · 2 comments · Fixed by #10874

Comments

@jmuhlfel
Copy link

Is your feature request related to a problem? Please describe.

Style/TernaryParentheses has infinite loop protection to prevent it from endlessly undoing corrections for Style/RedundantParentheses when both cops are enabled. This makes sense, but it has an unfortunate side effect: if Style/TernaryParentheses is set to require parentheses (via EnforcedStyle: require_parentheses or require_parentheses_when_complex), that preference is overridden and no offenses are flagged on ternary expressions that lack parentheses. To illustrate:

# rubocop.yml
Style/RedundantParentheses:
  Enabled: true

Style/TernaryParentheses:
  Enabled: true
  EnforcedStyle: require_parentheses_when_complex

# foo.rb
foo = bar.empty? || baz.empty? ? "a" : "b" # no Style/TernaryParentheses offense is registered

I figured this out by reading the source for both cops - I don't think it's documented anywhere.

Describe the solution you'd like

I propose moving the infinite loop protection to Style/RedundantParentheses: have it skip flagging offenses on ternary expressions when Style/TernaryParentheses is set to require parentheses. That way, rubocop can properly enforce the configurations for both cops.

Describe alternatives you've considered

The only alternative I'm aware of is to disable Style/RedundantParentheses, but it's a good cop to have and I'd prefer to have it enabled.

Additional context

I've been a rubocop user for years and very much appreciate the work the maintainers do. I'd be glad to put a PR together if you folks are onboard with this idea but don't have the bandwidth.

dvandersluis added a commit to dvandersluis/rubocop that referenced this issue Aug 5, 2022
… 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.
@dvandersluis
Copy link
Member

Hi @jmuhlfel thanks for the report, I've opened #10874.

@jmuhlfel
Copy link
Author

jmuhlfel commented Aug 5, 2022

Thanks very much @dvandersluis!

bbatsov pushed a commit that referenced this issue Aug 6, 2022
…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.
WJWH pushed a commit to WJWH/rubocop that referenced this issue Aug 8, 2022
… 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants