Style/TernaryParentheses autocorrect changes code intent

[petehamilton]

The Style/TernaryParentheses cop removes parentheses around ternary conditions when auto-correcting.

This week we had a case where applying this cop's autocorrect resulted in a change in code behaviour. I understand from other PRs/issues that autocorrects are intended to not change code's behaviour, only it's style.

Given code like (odd? x) ? "A" : "B", this cop transforms it into odd? x ? "A" : "B" which will return a different result.

---

Expected behavior

Given: (odd? x) ? "A" : "B"

I would expect the cop to do one of:

• Leave it as-is
• Register an offense
• Autocorrect to odd?(x) ? "A" : "B"

Actual behavior

The code is auto-corrected to odd? x ? "A" : "B" (interpreted as odd?(x ? "A" : "B"))

Steps to reproduce the problem

I've reproduced in a test which you can run: petehamilton@a9ec2ee

RuboCop version

$ rubocop -V
0.49.1 (using Parser 2.4.0.0, running on ruby 2.3.3 x86_64-darwin16)

Note: I originally thought this might be a similar issue to #4118 but on re-reading I think it's actually different so I've opened this issue to discuss

[rrosenblum]

Interesting, this does seem like a bug because the method call isn't using parenthesis. This appears to be a different issue than #4118.

The preferred syntax for this would be odd?(x) ? "A" : "B".

[Drenmi]

I agree with @rrosenblum.

I am inclined to say that this shouldn't be auto-corrected by this cop, because deciding whether #odd? should have parentheses has proven to be non-trivial, and is a concern of Style/MethodCallWithArgsParentheses, which would pick it up in the next pass (if enabled.)

So the fix would be to not auto-correct cases like these, IMHO.

[buehmann]

Hi, we ran into the same issue. When auto-correcting with the Style/TernaryParentheses cop, the behaviour of the code was changed. It seems like precedence of operators is not taken into account properly.

One class of cases was the one above (method call without parentheses), the other one was and or or in the condition:

(true or false) ? 'right' : 'wrong'  # => "right"

is rewritten into

true or false ? 'right' : 'wrong'    # => true

➜  rubocop (master) ✗ git describe
v0.52.1-153-g43df795dd
➜  rubocop (master) ✗ ruby --version
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin16]
➜  rubocop (master) ✗ cat original.rb
puts((a = (true or false) ? 'right' : 'wrong').inspect)
puts((a = (false and true) ? 'wrong' : 'right').inspect)
puts((a = ((0..1).include? 1) ? 'right' : 'wrong').inspect)
➜  rubocop (master) ✗ ruby original.rb
"right"
"right"
"right"
➜  rubocop (master) ✗ cp original.rb auto.rb
➜  rubocop (master) ✗ bundle exec bin/rubocop -a --only Style/TernaryParentheses auto.rb
Inspecting 1 file
C

Offenses:

auto.rb:1:11: C: [Corrected] Style/TernaryParentheses: Omit parentheses for ternary conditions.
puts((a = (true or false) ? 'right' : 'wrong').inspect)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
auto.rb:2:11: C: [Corrected] Style/TernaryParentheses: Omit parentheses for ternary conditions.
puts((a = (false and true) ? 'wrong' : 'right').inspect)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
auto.rb:3:11: C: [Corrected] Style/TernaryParentheses: Omit parentheses for ternary conditions.
puts((a = ((0..1).include? 1) ? 'right' : 'wrong').inspect)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 3 offenses detected, 3 offenses corrected
➜  rubocop (master) ✗ cat auto.rb
puts((a = true or false ? 'right' : 'wrong').inspect)
puts((a = false and true ? 'wrong' : 'right').inspect)
puts((a = (0..1).include? 1 ? 'right' : 'wrong').inspect)
➜  rubocop (master) ✗ ruby auto.rb
true
false
false

[jonas054]

This was fixed by #3505.

[kjeldahl]

I am still seeing this error in rubocop v0.62:

$ rubocop --only  Style/TernaryParentheses -a app/controllers/app_controller.rb 
Inspecting 1 file
C

Offenses:

app/controllers/app_controller.rb:63:5: C: [Corrected] Style/TernaryParentheses: Only use parentheses for ternary expressions with complex conditions.
    (%w(a b).include? params[:t]) ? "ab" : "c"
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

$ rubocop -v
0.62.0

It is corrected to this:

%w(a b).include? params[:t] ? "ab" : "c"

I have configured the Style/TernaryParentheses cop with EnforcedStyle: require_parentheses_when_complex

If I run @buehmann s example above with default configuration I get the same results as he does.
If the configuration is changed to require_parentheses_when_complex it fails in this way:

$ ruby auto.rb
"right"
"right"
false

$ cat auto.rb
puts((a = (true or false) ? 'right' : 'wrong').inspect)
puts((a = (false and true) ? 'wrong' : 'right').inspect)
puts((a = (0..1).include? (1) ? 'right' : 'wrong').inspect)

[jonas054]

Yes. There's still a bug there. 😞