From edca3cfb2e7e415f56c62b9f4bb9f7bac9e674e8 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Sat, 17 Sep 2016 16:53:13 +0800 Subject: [PATCH] [Fix #3450] Prevent `Style/TernaryParentheses` from making unsafe corrections (#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. --- CHANGELOG.md | 1 + lib/rubocop/cop/style/ternary_parentheses.rb | 21 ++++++++++++++++-- .../cop/style/ternary_parentheses_spec.rb | 22 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d04dccb0c3e..5261bef11e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/cop/style/ternary_parentheses.rb b/lib/rubocop/cop/style/ternary_parentheses.rb index 5a2b4303f3f3..c7c3593373ff 100644 --- a/lib/rubocop/cop/style/ternary_parentheses.rb +++ b/lib/rubocop/cop/style/ternary_parentheses.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/style/ternary_parentheses_spec.rb b/spec/rubocop/cop/style/ternary_parentheses_spec.rb index ffb7106a7195..25935432e575 100644 --- a/spec/rubocop/cop/style/ternary_parentheses_spec.rb +++ b/spec/rubocop/cop/style/ternary_parentheses_spec.rb @@ -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