-
-
Notifications
You must be signed in to change notification settings - Fork 263
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refs: https://rails.rubystyle.guide/#finding-missing-relationship-records This cop is check for the use `left_joins` and `where` method for finding missing relationship records. ```ruby # bad Foo.left_joins(:foo).where(foos: { id: nil }) # good Foo.where.missing(:foo) ```
- Loading branch information
Showing
5 changed files
with
274 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#744](https://github.com/rubocop/rubocop-rails/pull/744): Add new `Rails/WhereMissing` cop. ([@ydah][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
# Check for the use `left_joins` and `where` method for finding missing relationship records. | ||
# | ||
# This cop is enabled in Rails 6.1 or higher. | ||
# | ||
# @example | ||
# # bad | ||
# Foo.left_joins(:foo).where(foos: { id: nil }) | ||
# | ||
# # good | ||
# Foo.where.missing(:foo) | ||
# | ||
class WhereMissing < Base | ||
include RangeHelp | ||
extend AutoCorrector | ||
extend TargetRailsVersion | ||
|
||
MSG = 'Use `where.missing(:%<association>s)` instead of ' \ | ||
'`%<left_joins_method>s(:%<association>s).where(%<association>ss: { id: nil })`.' | ||
RESTRICT_ON_SEND = %i[left_joins left_outer_joins].freeze | ||
|
||
minimum_target_rails_version 6.1 | ||
|
||
# @!method where_node_and_argument(node) | ||
def_node_search :where_node_and_argument, <<~PATTERN | ||
$(send ... :where (hash <(pair $(sym _) (hash (pair (sym :id) (nil))))...> )) | ||
PATTERN | ||
|
||
# @!method missing_relationship?(node) | ||
def_node_search :missing_relationship?, <<~PATTERN | ||
(pair (sym _) (hash (pair (sym :id) (nil)))) | ||
PATTERN | ||
|
||
def on_send(node) | ||
return unless node.first_argument.sym_type? | ||
|
||
where_node_and_argument(node.ancestors.last) do |where_node, where_argument| | ||
next unless same_relationship?(where_argument, node.first_argument) | ||
|
||
range = range_between(node.loc.selector.begin_pos, node.loc.expression.end_pos) | ||
register_offense(node, where_node, range) | ||
break | ||
end | ||
end | ||
|
||
private | ||
|
||
def same_relationship?(where, left_joins) | ||
"#{left_joins.value}s" == where.value.to_s | ||
end | ||
|
||
def register_offense(node, where_node, range) | ||
add_offense(range, message: message(node)) do |corrector| | ||
corrector.replace(node.loc.selector, 'where.missing') | ||
if multi_condition?(where_node.first_argument) | ||
replace_where_method(corrector, where_node) | ||
else | ||
remove_where_method(corrector, node, where_node) | ||
end | ||
end | ||
end | ||
|
||
def replace_where_method(corrector, where_node) | ||
where_node.first_argument.children.each do |child| | ||
next unless missing_relationship?(child) | ||
|
||
corrector.remove(replace_range(child)) | ||
end | ||
end | ||
|
||
def replace_range(child) | ||
if (right_sibling = child.right_sibling) | ||
range_between(child.loc.expression.begin_pos, right_sibling.loc.expression.begin_pos) | ||
else | ||
range_between(child.left_sibling.loc.expression.end_pos, child.loc.expression.end_pos) | ||
end | ||
end | ||
|
||
def remove_where_method(corrector, node, where_node) | ||
range = range_between(where_node.loc.selector.begin_pos, where_node.loc.end.end_pos) | ||
if node.multiline? && !same_line?(node, where_node) | ||
range = range_by_whole_lines(range, include_final_newline: true) | ||
else | ||
corrector.remove(where_node.loc.dot) | ||
end | ||
|
||
corrector.remove(range) | ||
end | ||
|
||
def same_line?(left_joins_node, where_node) | ||
left_joins_node.loc.selector.line == where_node.loc.selector.line | ||
end | ||
|
||
def multi_condition?(where_arg) | ||
where_arg.children.count > 1 | ||
end | ||
|
||
def message(node) | ||
format(MSG, association: node.first_argument.value, left_joins_method: node.method_name) | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Rails::WhereMissing, :config do | ||
context 'Rails 6.1', :rails61 do | ||
it 'registers an offense when using `left_joins(foo).where(foos: {id: nil})`' do | ||
expect_offense(<<~RUBY) | ||
Foo.left_joins(:foo).where(foos: { id: nil }).where(bar: "bar") | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `left_outer_joins(foo).where(foos: {id: nil})`' do | ||
expect_offense(<<~RUBY) | ||
Foo.left_outer_joins(:foo).where(foos: { id: nil }).where(bar: "bar") | ||
^^^^^^^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_outer_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `where(foos: {id: nil}).left_joins(foo)`' do | ||
expect_offense(<<~RUBY) | ||
Foo.where(foos: { id: nil }).left_joins(:foo).where(bar: "bar") | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it "registers an offense when using `left_joins(foo).where(foos: {id: nil}, bar: 'bar')`" do | ||
expect_offense(<<~RUBY) | ||
Foo.left_joins(:foo).where(foos: { id: nil }, bar: "bar") | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it "registers an offense when using `left_joins(foo).where(bar: 'bar', foos: {id: nil})`" do | ||
expect_offense(<<~RUBY) | ||
Foo.left_joins(:foo).where(bar: "bar", foos: { id: nil }) | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `where(foos: {id: nil}).joins(:bar).left_joins(foo)`' do | ||
expect_offense(<<~RUBY) | ||
Foo.left_joins(:foo).joins(:bar).where(foos: { id: nil }) | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo).joins(:bar) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `left_joins(foo).where(foos: {id: nil})` with multi-line leading ' \ | ||
'dot method calls' do | ||
expect_offense(<<~RUBY) | ||
Foo | ||
.left_joins(:foo) | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
.where(foos: { id: nil }) | ||
.where(bar: "bar") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo | ||
.where.missing(:foo) | ||
.where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `left_joins(foo).where(foos: {id: nil})` with multi-line trailing ' \ | ||
'dot method calls' do | ||
expect_offense(<<~RUBY) | ||
Foo. | ||
left_joins(:foo). | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
where(foos: { id: nil }). | ||
where(bar: "bar") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo. | ||
where.missing(:foo). | ||
where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `left_joins(foo).where(foos: {id: nil})` and there is a line break after' \ | ||
'`left_joins.where`' do | ||
expect_offense(<<~RUBY) | ||
Foo.left_joins(:foo).where(foos: { id: nil }) | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
.where(bar: "bar") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo.where.missing(:foo) | ||
.where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `left_joins(foo).where(foos: {id: nil})` and there is a line break after' \ | ||
'`left_joins.where` and receiver' do | ||
expect_offense(<<~RUBY) | ||
Foo | ||
.left_joins(:foo).where(foos: { id: nil }) | ||
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`. | ||
.where(bar: "bar") | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
Foo | ||
.where.missing(:foo) | ||
.where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when not finding missing relationship records`' do | ||
expect_no_offenses(<<~RUBY) | ||
Foo.left_joins(:foo).where(bazs: { id: nil }) | ||
Foo.left_joins(:foo).where(foos: { name: nil }) | ||
Foo.left_joins(:foo).where(foos: { id: 1 }) | ||
Foo.left_joins(foo: :bar).where(bars: { id: nil }) | ||
RUBY | ||
end | ||
end | ||
|
||
context 'Rails 6.0', :rails60 do | ||
it 'does not register an offense when using `left_joins(foo).where(foos: {id: nil})`' do | ||
expect_no_offenses(<<~RUBY) | ||
Foo.left_joins(:foo).where(foos: { id: nil }).where(bar: "bar") | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `left_outer_joins(foo).where(foos: {id: nil})`' do | ||
expect_no_offenses(<<~RUBY) | ||
Foo.left_outer_joins(:foo).where(foos: { id: nil }).where(bar: "bar") | ||
RUBY | ||
end | ||
end | ||
end |