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

Problems with Style/TernaryParentheses autocorrection #3450

Closed
benhutton opened this issue Aug 29, 2016 · 1 comment
Closed

Problems with Style/TernaryParentheses autocorrection #3450

benhutton opened this issue Aug 29, 2016 · 1 comment
Labels

Comments

@benhutton
Copy link

benhutton commented Aug 29, 2016

When updating to the latest rubocop version, we ran rubocop -a and ended up with two bad lines:

-        (defined? thing) ? thing : 'Message'
+        defined? thing ? thing : 'Message'
-        state: (states.include? status) ? status : 'pending',
+        state: states.include? status ? status : 'pending',

And perhaps there are other, better, ways to write these lines.

Obviously, a "real" problem is that the parens are in the wrong place. It should be defined?(thing) instead of (defined? thing). It should be states.include?(status) instead of (states.include? status).

But it would be preferable to not attempt a bad autocorrect and just flag it as an error and let me sort it out myself.


Expected behavior

Should not attempt to autocorrect

Actual behavior

Attempts to autocorrect to invalid code

Steps to reproduce the problem

Have code which matches the above "-" lines, and then run rubocop -a

RuboCop version

Include the output of rubocop -V:

$ rubocop -V
0.42.0 (using Parser 2.3.1.2, running on ruby 2.3.1 x86_64-linux)
@Drenmi
Copy link
Collaborator

Drenmi commented Aug 30, 2016

@benhutton: You are correct. We should never auto-correct in a way that breaks code, or changes its meaning.

I implemented this cop originally. I will see if I can fix this when I have time (which may or may not be anytime soon.) If someone else feels like giving it a go, please feel free to do so. 😀

@bbatsov bbatsov added the bug label Sep 7, 2016
Drenmi added a commit to Drenmi/rubocop that referenced this issue Sep 16, 2016
…afe corrections

This cop, when configured to omit parentheses, would remove them even when the
condition contained an unparenthesized method call, e.g.:

```
foo = (bar? baz) ? 'Yes' : 'No'
```

which would change the semantics of the expression. This change fixes that.
Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Oct 15, 2016
…afe corrections (rubocop#3505)

This cop, when configured to omit parentheses, would remove them even when the
condition contained an unparenthesized method call, e.g.:

```
foo = (bar? baz) ? 'Yes' : 'No'
```

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

No branches or pull requests

3 participants