From 4d01b52228df7df564589908db53f43efab1f891 Mon Sep 17 00:00:00 2001 From: Parker Finch Date: Sun, 26 May 2019 14:07:42 -0400 Subject: [PATCH] [Fix #7063] Fix unsafe autocorrect for 'Style/TernaryParentheses' This takes the precedence of operators used into account when deciding to autocorrect. The issue is that if an operator of precedence below that of the ternary operator (e.g. "or") is used, then the existing autocorrect can change the semantics by removing parentheses. With this change, the autocorrect will no longer attempt to make this correction. --- CHANGELOG.md | 4 ++++ lib/rubocop/cop/style/ternary_parentheses.rb | 12 ++++++++++- .../cop/style/ternary_parentheses_spec.rb | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9044488c555..ec2f72f3c362 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### Bug fixes + +* [#7063](https://github.com/rubocop-hq/rubocop/issues/7063): Fix autocorrect in `Style/TernaryParentheses` cop. ([@parkerfinch][]) + ### Changes * [#5976](https://github.com/rubocop-hq/rubocop/issues/5976): Remove Rails cops. ([@koic][]) diff --git a/lib/rubocop/cop/style/ternary_parentheses.rb b/lib/rubocop/cop/style/ternary_parentheses.rb index e04b0561a282..6c93257b57b4 100644 --- a/lib/rubocop/cop/style/ternary_parentheses.rb +++ b/lib/rubocop/cop/style/ternary_parentheses.rb @@ -149,7 +149,8 @@ def infinite_loop? def unsafe_autocorrect?(condition) condition.children.any? do |child| - unparenthesized_method_call?(child) + unparenthesized_method_call?(child) || + below_ternary_precedence?(child) end end @@ -157,6 +158,15 @@ def unparenthesized_method_call?(child) method_name(child) =~ /^[a-z]/i && !child.parenthesized? end + def below_ternary_precedence?(child) + # Handle English "or", e.g. 'foo or bar ? a : b' + (child.or_type? && child.semantic_operator?) || + # Handle English "and", e.g. 'foo and bar ? a : b' + (child.and_type? && child.semantic_operator?) || + # Handle English "not", e.g. 'not foo ? a : b' + (child.send_type? && child.prefix_not?) + end + def_node_matcher :method_name, <<-PATTERN {($:defined? (send nil? _) ...) (send {_ nil?} $_ _ ...)} diff --git a/spec/rubocop/cop/style/ternary_parentheses_spec.rb b/spec/rubocop/cop/style/ternary_parentheses_spec.rb index 0d25371c322c..08cc8a7936ab 100644 --- a/spec/rubocop/cop/style/ternary_parentheses_spec.rb +++ b/spec/rubocop/cop/style/ternary_parentheses_spec.rb @@ -110,6 +110,14 @@ it_behaves_like 'code with offense', 'foo = bar && (baz || bar) ? a : b', 'foo = (bar && (baz || bar)) ? a : b' + + it_behaves_like 'code with offense', + 'foo = bar or baz ? a : b', + 'foo = bar or (baz) ? a : b' + + it_behaves_like 'code with offense', + 'not bar ? a : b', + 'not (bar) ? a : b' end context 'with an assignment condition' do @@ -168,6 +176,12 @@ it_behaves_like 'code without offense', 'foo = bar && (baz || bar) ? a : b' + + it_behaves_like 'code with offense', + 'foo = (foo or bar) ? a : b' + + it_behaves_like 'code with offense', + 'foo = (not bar) ? a : b' end context 'with an assignment condition' do @@ -248,6 +262,10 @@ it_behaves_like 'code with offense', 'foo = (bar[:baz]) ? a : b', 'foo = bar[:baz] ? a : b' + + it_behaves_like 'code with offense', + 'foo = bar or (baz) ? a : b', + 'foo = bar or baz ? a : b' end context 'with a complex condition' do @@ -255,6 +273,9 @@ 'foo = (bar.baz?) ? a : b', 'foo = bar.baz? ? a : b' + it_behaves_like 'code without offense', + 'foo = (baz or bar) ? a : b' + it_behaves_like 'code without offense', 'foo = (bar && (baz || bar)) ? a : b' end