Skip to content

Commit

Permalink
Fix a false positive for Rails/WhereMissing when left_joins(:foo)
Browse files Browse the repository at this point in the history
… and `where(foos: {id: nil})` separated by `or`, `and`

This PR is fix a false positive for `Rails/WhereMissing` when `left_joins(:foo)` and `where(foos: {id: nil})` separated by `or`, `and`.

It is necessary to consider the following cases, which are separated by `or` or `and`.

```ruby
Foo.left_joins(:foo).or(Foo.where(foos: {id: nil}))
Foo.where(foos: {id: nil}).and(Foo.left_joins(:foo))
```
  • Loading branch information
ydah committed Nov 21, 2022
1 parent 363ed1e commit 2783e35
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#871](https://github.com/rubocop/rubocop-rails/pull/871): Fix a false positive for `Rails/WhereMissing` when `left_joins(:foo)` and `where(foos: {id: nil})` separated by `or`, `and`. ([@ydah][])
14 changes: 12 additions & 2 deletions lib/rubocop/cop/rails/where_missing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class WhereMissing < Base
def on_send(node)
return unless node.first_argument.sym_type?

where_node_and_argument(root_receiver(node)) do |where_node, where_argument|
root_receiver = root_receiver(node)
where_node_and_argument(root_receiver) do |where_node, where_argument|
next unless root_receiver == root_receiver(where_node)
next unless same_relationship?(where_argument, node.first_argument)

range = range_between(node.loc.selector.begin_pos, node.loc.expression.end_pos)
Expand All @@ -50,7 +52,15 @@ def on_send(node)
private

def root_receiver(node)
node&.parent&.send_type? ? root_receiver(node.parent) : node
if node&.parent&.send_type?
if node.parent.method?(:or) || node.parent.method?(:and)
node
else
root_receiver(node.parent)
end
else
node
end
end

def same_relationship?(where, left_joins)
Expand Down
34 changes: 34 additions & 0 deletions spec/rubocop/cop/rails/where_missing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,40 @@ def test
RUBY
end

it 'does not register an offense when `left_joins(:foo)` and `where(foos: {id: nil})` separated by `or`' do
expect_no_offenses(<<~RUBY)
Foo.left_joins(:foo).or(Foo.where(foos: {id: nil}))
RUBY
end

it 'does not register an offense when `left_joins(:foo)` and `where(foos: {id: nil})` separated by `and`' do
expect_no_offenses(<<~RUBY)
Foo.where(foos: {id: nil}).and(Foo.left_joins(:foo))
RUBY
end

it 'registers an offense when using `left_joins(:foo).where(foo: {id: nil})` outside `or` conditional expression' do
expect_offense(<<~RUBY)
Foo.left_joins(:foo).where(foos: {id: nil}).or(Foo.where(bar: "bar").left_joins(:foo))
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`.
RUBY

expect_correction(<<~RUBY)
Foo.where.missing(:foo).or(Foo.where(bar: "bar").left_joins(:foo))
RUBY
end

it 'registers an offense when using `left_joins(:foo).where(foo: {id: nil})` within `and` conditional expression' do
expect_offense(<<~RUBY)
Foo.left_joins(:foo).where(bar: "bar").and(Foo.where(foos: {id: nil}).left_joins(:foo))
^^^^^^^^^^^^^^^^ Use `where.missing(:foo)` instead of `left_joins(:foo).where(foos: { id: nil })`.
RUBY

expect_correction(<<~RUBY)
Foo.left_joins(:foo).where(bar: "bar").and(Foo.where.missing(:foo))
RUBY
end

it "registers an offense when using `left_joins(:foo).where(foos: {id: nil}, bar: 'bar')` " \
"with other `left_joins(:foo).where(foos: {id: nil}, bar: 'bar')`" do
expect_offense(<<~RUBY)
Expand Down

0 comments on commit 2783e35

Please sign in to comment.