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

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 committed Sep 16, 2016
1 parent caaf0e2 commit de8b611
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 de8b611

Please sign in to comment.