Skip to content

Commit

Permalink
Add new Rails/WhereRange cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Apr 30, 2024
1 parent 28b274b commit 5a2e7df
Show file tree
Hide file tree
Showing 5 changed files with 367 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_where_range_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1272](https://github.com/rubocop/rubocop-rails/pull/1272): Add new `Rails/WhereRange` cop. ([@fatkodima][])
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,12 @@ Rails/WhereNotWithMultipleConditions:
VersionAdded: '2.17'
VersionChanged: '2.18'

Rails/WhereRange:
Description: 'Use ranges in `where` instead of manually constructing SQL.'
StyleGuide: 'https://rails.rubystyle.guide/#where-ranges'
Enabled: pending
VersionAdded: '<<next>>'

# Accept `redirect_to(...) and return` and similar cases.
Style/AndOr:
EnforcedStyle: conditionals
Expand Down
157 changes: 157 additions & 0 deletions lib/rubocop/cop/rails/where_range.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Identifies places where manually constructed SQL
# in `where` can be replaced with ranges.
#
# @example
# # bad
# User.where('age >= ?', 18)
# User.where.not('age >= ?', 18)
# User.where('age < ?', 18)
# User.where('age >= ? AND age < ?', 18, 21)
# User.where('age >= :start', start: 18)
# User.where('users.age >= ?', 18)
#
# # good
# User.where(age: 18..)
# User.where.not(age: 18..)
# User.where(age: ...18)
# User.where(age: 18...21)
# User.where(users: { age: 18.. })
#
# # good
# # There are no beginless ranges in ruby.
# User.where('age > ?', 18)
#
class WhereRange < Base
include RangeHelp
extend AutoCorrector
extend TargetRubyVersion
extend TargetRailsVersion

MSG = 'Use `%<good_method>s` instead of manually constructing SQL.'

RESTRICT_ON_SEND = %i[where not].freeze

# column >= ?
GTEQ_ANONYMOUS_RE = /\A([\w.]+)\s+>=\s+\?\z/.freeze
# column <[=] ?
LTEQ_ANONYMOUS_RE = /\A([\w.]+)\s+(<=?)\s+\?\z/.freeze
# column >= ? AND column <[=] ?
RANGE_ANONYMOUS_RE = /\A([\w.]+)\s+>=\s+\?\s+AND\s+\1\s+(<=?)\s+\?\z/i.freeze
# column >= :value
GTEQ_NAMED_RE = /\A([\w.]+)\s+>=\s+:(\w+)\z/.freeze
# column <[=] :value
LTEQ_NAMED_RE = /\A([\w.]+)\s+(<=?)\s+:(\w+)\z/.freeze
# column >= :value1 AND column <[=] :value2
RANGE_NAMED_RE = /\A([\w.]+)\s+>=\s+:(\w+)\s+AND\s+\1\s+(<=?)\s+:(\w+)\z/i.freeze

minimum_target_ruby_version 2.6
minimum_target_rails_version 6.0

def_node_matcher :where_range_call?, <<~PATTERN
{
(call _ {:where :not} (array $str_type? $_ +))
(call _ {:where :not} $str_type? $_ +)
}
PATTERN

def on_send(node)
return if node.method?(:not) && !where_not?(node)

where_range_call?(node) do |template_node, values_node|
column, value = extract_column_and_value(template_node, values_node)

return unless column

range = offense_range(node)
good_method = build_good_method(node.method_name, column, value)
message = format(MSG, good_method: good_method)

add_offense(range, message: message) do |corrector|
corrector.replace(range, good_method)
end
end
end

private

def where_not?(node)
receiver = node.receiver
receiver&.send_type? && receiver&.method?(:where)
end

# 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]

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
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
end

[Regexp.last_match(1), value] if value
end
# rubocop:enable Metrics

def range_operator(comparison_operator)
comparison_operator == '<' ? '...' : '..'
end

def find_pair(hash_node, value)
hash_node.pairs.find { |pair| pair.key.value.to_sym == value.to_sym }
end

def offense_range(node)
range_between(node.loc.selector.begin_pos, node.source_range.end_pos)
end

def build_good_method(method_name, column, value)
if column.include?('.')
table, column = column.split('.')

"#{method_name}(#{table}: { #{column}: #{value} })"
else
"#{method_name}(#{column}: #{value})"
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,4 @@
require_relative 'rails/where_missing'
require_relative 'rails/where_not'
require_relative 'rails/where_not_with_multiple_conditions'
require_relative 'rails/where_range'
202 changes: 202 additions & 0 deletions spec/rubocop/cop/rails/where_range_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::WhereRange, :config do
context 'Ruby <= 2.5', :ruby25 do
it 'does not register an offense when using anonymous `>=`' do
expect_no_offenses(<<~RUBY)
Model.where('column >= ?', value)
RUBY
end

it 'does not register an offense when using anonymous `<=`' do
expect_no_offenses(<<~RUBY)
Model.where('column <= ?', value)
RUBY
end
end

context 'Rails 5.1', :rails51 do
it 'does not register an offense when using anonymous `>=`' do
expect_no_offenses(<<~RUBY)
Model.where('column >= ?', value)
RUBY
end
end

context 'Rails 6.0', :rails60 do
context 'Ruby >= 2.6', :ruby26 do
it 'registers an offense and corrects when using anonymous `>=`' do
expect_offense(<<~RUBY)
Model.where('column >= ?', value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value..)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value..)
RUBY
end

it 'registers an offense and corrects when using named `>=`' do
expect_offense(<<~RUBY)
Model.where('column >= :min', min: value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value..)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value..)
RUBY
end

it 'registers an offense and corrects when using anonymous `>=` with an explicit table' do
expect_offense(<<~RUBY)
Model.where('table.column >= ?', value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(table: { column: value.. })` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(table: { column: value.. })
RUBY
end

it 'does not register an offense when using anonymous `>`' do
expect_no_offenses(<<~RUBY)
Model.where('column > ?', value)
RUBY
end

it 'registers an offense and corrects when using anonymous `>= AND <`' do
expect_offense(<<~RUBY)
Model.where('column >= ? AND column < ?', value1, value2)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value1...value2)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value1...value2)
RUBY
end

it 'registers an offense and corrects when using anonymous `>= AND <=`' do
expect_offense(<<~RUBY)
Model.where('column >= ? AND column <= ?', value1, value2)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value1..value2)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value1..value2)
RUBY
end

it 'registers an offense and corrects when using named `>= AND <`' do
expect_offense(<<~RUBY)
Model.where('column >= :min AND column < :max', min: value1, max: value2)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value1...value2)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value1...value2)
RUBY
end

it 'does not register an offense when using different columns' do
expect_no_offenses(<<~RUBY)
Model.where('column1 >= ? AND column2 < ?', value1, value2)
RUBY
end

it 'does not register an offense when using named `>= AND <` and placeholders do not exist' do
expect_no_offenses(<<~RUBY)
Model.where('column >= :min AND column < :max', min: value)
RUBY
end

it 'registers an offense and corrects when using `>=` with an array' do
expect_offense(<<~RUBY)
Model.where(['column >= ?', value])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value..)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value..)
RUBY
end

it 'registers an offense and corrects when using `>= AND <=` with an array' do
expect_offense(<<~RUBY)
Model.where(['column >= ? AND column <= ?', value1, value2])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: value1..value2)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: value1..value2)
RUBY
end

it 'registers an offense and corrects when using `where.not`' do
expect_offense(<<~RUBY)
Model.where.not('column >= ?', value)
^^^^^^^^^^^^^^^^^^^^^^^^^ Use `not(column: value..)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where.not(column: value..)
RUBY
end

it 'does not register an offense when using ranges' do
expect_no_offenses(<<~RUBY)
Model.where(column: value..)
RUBY
end

it 'does not register an offense when using `not` with ranges' do
expect_no_offenses(<<~RUBY)
Model.where.not(column: value..)
RUBY
end

it 'does not register an offense when using `not` not preceding by `where`' do
expect_no_offenses(<<~RUBY)
foo.not('column >= ?', value)
RUBY
end
end

context 'Ruby >= 2.6', :ruby26, unsupported_on: :prism do
it 'does not register an offense when using anonymous `<=`' do
expect_no_offenses(<<~RUBY)
Model.where('column <= ?', value)
RUBY
end

it 'does not register an offense when using anonymous `<`' do
expect_no_offenses(<<~RUBY)
Model.where('column < ?', value)
RUBY
end
end

context 'Ruby >= 2.7', :ruby27 do
it 'registers an offense and corrects when using anonymous `<=`' do
expect_offense(<<~RUBY)
Model.where('column <= ?', value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: ..value)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: ..value)
RUBY
end

it 'registers an offense and corrects when using anonymous `<`' do
expect_offense(<<~RUBY)
Model.where('column < ?', value)
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(column: ...value)` instead of manually constructing SQL.
RUBY

expect_correction(<<~RUBY)
Model.where(column: ...value)
RUBY
end
end
end
end

0 comments on commit 5a2e7df

Please sign in to comment.