From 6bb23b47e59cfcc26be17c98561f07390a852fb2 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 6 Jun 2024 19:14:00 +0300 Subject: [PATCH] [Fix #1281] Fix `WhereRange` autocorrect for complex expressions --- .../fix_where_range_complex_expressions.md | 1 + lib/rubocop/cop/rails/where_range.rb | 105 ++++++++++++------ spec/rubocop/cop/rails/where_range_spec.rb | 11 ++ 3 files changed, 81 insertions(+), 36 deletions(-) create mode 100644 changelog/fix_where_range_complex_expressions.md diff --git a/changelog/fix_where_range_complex_expressions.md b/changelog/fix_where_range_complex_expressions.md new file mode 100644 index 0000000000..215be32c91 --- /dev/null +++ b/changelog/fix_where_range_complex_expressions.md @@ -0,0 +1 @@ +* [#1281](https://github.com/rubocop/rubocop-rails/issues/1281): Fix `WhereRange` autocorrect for complex expressions. ([@fatkodima][]) diff --git a/lib/rubocop/cop/rails/where_range.rb b/lib/rubocop/cop/rails/where_range.rb index 18c73cfe3b..4926e5b6a6 100644 --- a/lib/rubocop/cop/rails/where_range.rb +++ b/lib/rubocop/cop/rails/where_range.rb @@ -86,47 +86,63 @@ def where_not?(node) # rubocop:disable Metrics def extract_column_and_value(template_node, values_node) - value = - case template_node.value - when GTEQ_ANONYMOUS_RE - "#{values_node[0].source}.." - when LTEQ_ANONYMOUS_RE - range_operator = range_operator(Regexp.last_match(2)) - "#{range_operator}#{values_node[0].source}" if target_ruby_version >= 2.7 - when RANGE_ANONYMOUS_RE - if values_node.size >= 2 - range_operator = range_operator(Regexp.last_match(2)) - "#{values_node[0].source}#{range_operator}#{values_node[1].source}" - end - when GTEQ_NAMED_RE - value_node = values_node[0] + case template_node.value + when GTEQ_ANONYMOUS_RE + lhs = values_node[0] + operator = '..' + when LTEQ_ANONYMOUS_RE + if target_ruby_version >= 2.7 + operator = range_operator(Regexp.last_match(2)) + rhs = values_node[0] + end + when RANGE_ANONYMOUS_RE + if values_node.size >= 2 + lhs = values_node[0] + operator = range_operator(Regexp.last_match(2)) + rhs = values_node[1] + end + when GTEQ_NAMED_RE + value_node = values_node[0] - if value_node.hash_type? - pair = find_pair(value_node, Regexp.last_match(2)) - "#{pair.value.source}.." if pair - end - when LTEQ_NAMED_RE - value_node = values_node[0] - - if value_node.hash_type? - pair = find_pair(value_node, Regexp.last_match(2)) - if pair && target_ruby_version >= 2.7 - range_operator = range_operator(Regexp.last_match(2)) - "#{range_operator}#{pair.value.source}" - end + if value_node.hash_type? + pair = find_pair(value_node, Regexp.last_match(2)) + lhs = pair.value + operator = '..' + end + when LTEQ_NAMED_RE + value_node = values_node[0] + + if value_node.hash_type? + pair = find_pair(value_node, Regexp.last_match(2)) + if pair && target_ruby_version >= 2.7 + operator = range_operator(Regexp.last_match(2)) + rhs = pair.value end - when RANGE_NAMED_RE - value_node = values_node[0] - - if value_node.hash_type? - range_operator = range_operator(Regexp.last_match(3)) - pair1 = find_pair(value_node, Regexp.last_match(2)) - pair2 = find_pair(value_node, Regexp.last_match(4)) - "#{pair1.value.source}#{range_operator}#{pair2.value.source}" if pair1 && pair2 + end + when RANGE_NAMED_RE + value_node = values_node[0] + + if value_node.hash_type? + pair1 = find_pair(value_node, Regexp.last_match(2)) + pair2 = find_pair(value_node, Regexp.last_match(4)) + + if pair1 && pair2 + lhs = pair1.value + operator = range_operator(Regexp.last_match(3)) + rhs = pair2.value end end + end - [Regexp.last_match(1), value] if value + if lhs + lhs_source = parentheses_needed?(lhs) ? "(#{lhs.source})" : lhs.source + end + + if rhs + rhs_source = parentheses_needed?(rhs) ? "(#{rhs.source})" : rhs.source + end + + [Regexp.last_match(1), "#{lhs_source}#{operator}#{rhs_source}"] if operator end # rubocop:enable Metrics @@ -151,6 +167,23 @@ def build_good_method(method_name, column, value) "#{method_name}(#{column}: #{value})" end end + + def parentheses_needed?(node) + !parentheses_not_needed?(node) + end + + def parentheses_not_needed?(node) + node.variable? || + node.literal? || + node.reference? || + node.const_type? || + node.begin_type? || + parenthesized_call_node?(node) + end + + def parenthesized_call_node?(node) + node.call_type? && (node.arguments.empty? || node.parenthesized_call?) + end end end end diff --git a/spec/rubocop/cop/rails/where_range_spec.rb b/spec/rubocop/cop/rails/where_range_spec.rb index d118b76f83..2f9869a5dc 100644 --- a/spec/rubocop/cop/rails/where_range_spec.rb +++ b/spec/rubocop/cop/rails/where_range_spec.rb @@ -159,6 +159,17 @@ foo.not('column >= ?', value) RUBY end + + it 'wraps complex expressions by parentheses' do + expect_offense(<<~RUBY) + Model.where('column >= ?', true ? 1 : 2) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: (true ? 1 : 2)..)` instead of manually constructing SQL. + RUBY + + expect_correction(<<~RUBY) + Model.where(column: (true ? 1 : 2)..) + RUBY + end end context 'Ruby >= 2.6', :ruby26, unsupported_on: :prism do