Skip to content

Commit

Permalink
[Fix rubocop#3450] Prevent Style/TernaryParentheses from making uns…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Drenmi authored and Neodelf committed Oct 15, 2016
1 parent 4cb3016 commit edca3cf
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
* [#3423](https://github.com/bbatsov/rubocop/issues/3423): Checks if .rubocop is a file before parsing. ([@joejuzl][])
* [#3439](https://github.com/bbatsov/rubocop/issues/3439): Fix variable assignment check not working properly when a block is used in `Rails/SaveBang`. ([@QuinnHarris][])
* [#3401](https://github.com/bbatsov/rubocop/issues/3401): Read file contents in binary mode so `Style/EndOfLine` works on Windows. ([@jonas054][])
* [#3450](https://github.com/bbatsov/rubocop/issues/3450): Prevent `Style/TernaryParentheses` cop from making unsafe corrections. ([@drenmi][])

### Changes

Expand Down
21 changes: 19 additions & 2 deletions lib/rubocop/cop/style/ternary_parentheses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ def autocorrect(node)
corrector.insert_before(condition.source_range, '(')
corrector.insert_after(condition.source_range, ')')
else
# Don't correct if it's a safe assignment
unless safe_assignment?(condition)
unless safe_assignment?(condition) ||
unsafe_autocorrect?(condition)
corrector.remove(condition.loc.begin)
corrector.remove(condition.loc.end)
end
Expand Down Expand Up @@ -101,6 +101,23 @@ def infinite_loop?
require_parentheses? &&
redundant_parentheses_enabled?
end

def unsafe_autocorrect?(condition)
condition.children.any? do |child|
unparenthesized_method_call?(child)
end
end

def unparenthesized_method_call?(child)
argument = method_call_argument(child)

argument && argument !~ /^\(/
end

def_node_matcher :method_call_argument, <<-PATTERN
{(:defined? $...)
(send {(send ...) nil} _ $(send nil _)...)}
PATTERN
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cop/style/ternary_parentheses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,28 @@
'foo = (bar = baz = find_baz) ? a : b'
end
end

context 'with an unparenthesized method call condition' do
it_behaves_like 'code with offense',
'foo = (defined? bar) ? a : b',
'foo = (defined? bar) ? a : b'

it_behaves_like 'code with offense',
'foo = (baz? bar) ? a : b',
'foo = (baz? bar) ? a : b'

context 'when calling method on a receiver' do
it_behaves_like 'code with offense',
'foo = (baz.foo? bar) ? a : b',
'foo = (baz.foo? bar) ? a : b'
end

context 'when calling method with multiple arguments' do
it_behaves_like 'code with offense',
'foo = (baz.foo? bar, baz) ? a : b',
'foo = (baz.foo? bar, baz) ? a : b'
end
end
end

context 'when `RedundantParenthesis` would cause an infinite loop' do
Expand Down

0 comments on commit edca3cf

Please sign in to comment.