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

Third option for Style/TernaryParentheses #3451

Closed
benhutton opened this issue Aug 29, 2016 · 5 comments
Closed

Third option for Style/TernaryParentheses #3451

benhutton opened this issue Aug 29, 2016 · 5 comments
Labels
feature request good first issue Easy task, suitable for newcomers to the project hacktoberfest

Comments

@benhutton
Copy link

Currently (as of 0.42.0), rubocop gives 2 options for Style/TernaryParentheses

EnforcedStyle: require_no_parentheses (deafault)

@bad
foo = (bar?) ? a : b
foo = (bar.baz) ? a : b
foo = (bar && baz) ? a : b

@good
foo = bar? ? a : b
foo = bar.baz? ? a : b
foo = bar && baz ? a : b

and

EnforcedStyle: require_parentheses

@bad
foo = bar? ? a : b
foo = bar.baz? ? a : b
foo = bar && baz ? a : b

@good
foo = (bar?) ? a : b
foo = (bar.baz) ? a : b
foo = (bar && baz) ? a : b

Both of these are undesirable to our team...

Sometimes, parens are desired—the conditional is complex and the timing is not quite right to do a proper refactor. Parentheses greatly aid legibility and code comprehension and scannability. (Quick test: without looking, in Ruby's order of operations precedence, where does a ternary operator fall? How does it compare to &&? How about to =? What about to defined??)

Usually, parens are unnecessary. Which is why require_no_parentheses was chosen as the default, I'm sure.

So, what I'd like to see is a third option:

EnforcedStyle: require_parentheses_when_complex

@bad
foo = (bar?) ? a : b
foo = (bar.baz?) ? a : b
foo = bar && baz ? a : b

@good
foo = bar? ? a : b
foo = bar.baz ? a : b
foo = (bar && baz) ? a : b

Namely, I want to require parens when there is a non-unary operator as the first term in the ternary operator.

What do ya'll think?

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 7, 2016

Makes sense to me. I'd accept such a style if someone implements it.

@bbatsov bbatsov added feature request good first issue Easy task, suitable for newcomers to the project labels Sep 7, 2016
@Drenmi
Copy link
Collaborator

Drenmi commented Sep 8, 2016

This is a clear improvement from a similar suggestion made earlier, in that this one specifies, unambiguously, what "complex" means, but the suggested configuration name unfortunately doesn't give the user much indication of its specifics. 😀

@benhutton
Copy link
Author

@Drenmi by "complex" I was thinking the more basic definition of "compound":

image

Not necessarily "complicated", though that is part of the semantic range, too.

What about the word "compound"? So, require_parentheses_when_compound?

@Drenmi
Copy link
Collaborator

Drenmi commented Sep 9, 2016

I think whatever we come up with will be not specific enough, lest we make the name prohibitively long. 😅 I think let's just go with "complex", and include an explanation of the setting in default.yml. 😀

@swcraig
Copy link
Contributor

swcraig commented Nov 3, 2016

I have the cop working for this and will commit somewhat soon.

bbatsov added a commit that referenced this issue Nov 22, 2016
[Fix #3451] Add new `require_parentheses_when_complex` style to `Styl…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request good first issue Easy task, suitable for newcomers to the project hacktoberfest
Projects
None yet
Development

No branches or pull requests

4 participants