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

inifinite loop in Ternary Parentheses #3347

Closed
guyisra opened this issue Jul 27, 2016 · 5 comments
Closed

inifinite loop in Ternary Parentheses #3347

guyisra opened this issue Jul 27, 2016 · 5 comments

Comments

@guyisra
Copy link

guyisra commented Jul 27, 2016

I've set

Style/TernaryParentheses:
  EnforcedStyle: require_parentheses

but then it get stuck in a loop due to some other rule:

72:9: C: [Corrected] Don't use parentheses around a method call.
        (first_name.present?) ? { 'firstName' => first_name } : {}
        ^^^^^^^^^^^^^^^^^^^^^
72:9: C: [Corrected] Use parentheses for ternary conditions.
        first_name.present? ? { 'firstName' => first_name } : {}

Infinite loop detected

Not sure what rule causes to remove the parentheses but I think that

  1. They should be mutually exclusive
  2. Ternary Parenthses should only be in an equality condition and/or when with assignment and not a method condition:
    res = var == "nope" ? true : false
    should be
    res = (var == "nope") ? true : false
@Drenmi
Copy link
Collaborator

Drenmi commented Jul 27, 2016

The problem with conditional styles is which one should take precedence.

I.e. you prefer to have parentheses on all ternaries except where the condition is a method call, but it is also likely that someone else might want to omit parentheses around all method calls, except when used as a condition in a ternary.

I'll sit down and think about the interactions of these two cops, and see if there are any configuration options that could make sense here.

@guyisra
Copy link
Author

guyisra commented Jul 27, 2016

one way is to tell the user of the conflict and to explictly remove one of the conflicting rules

@Drenmi
Copy link
Collaborator

Drenmi commented Jul 27, 2016

@jonas054, @bbatsov: What's the modus operandi for this kind of conflict?

@jonas054
Copy link
Collaborator

I think we have instances where a cop allows a certain construct if it sees (by checking another cop's configuration) that a manual or automatic correction to comply with the cop's own rule would be reversed by the other cop.

So in this case, Style/TernaryParentheses would check the configuration of Style/RedundantParentheses.

@Drenmi
Copy link
Collaborator

Drenmi commented Jul 31, 2016

@jonas054: Thanks! I guess ideally, down the line, cops shouldn't know about each other, but should be fine for now. 😀

Drenmi added a commit to Drenmi/rubocop that referenced this issue Jul 31, 2016
…` cop

When this cop was configured to enforce parentheses and the
`RedundantParentheses` cop was enabled, it would cause an infinite
loop as they compete to add and remove the parens respectively.

This change fixes that.
@bbatsov bbatsov closed this as completed in 37ac0a6 Aug 1, 2016
Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Oct 15, 2016
…` cop (rubocop#3354)

When this cop was configured to enforce parentheses and the
`RedundantParentheses` cop was enabled, it would cause an infinite
loop as they compete to add and remove the parens respectively.

This change fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants