diff --git a/changelog/new_add_new_rails_where_missing_cop.md b/changelog/new_add_new_rails_where_missing_cop.md new file mode 100644 index 0000000000..0f32e082c8 --- /dev/null +++ b/changelog/new_add_new_rails_where_missing_cop.md @@ -0,0 +1 @@ +* [#744](https://github.com/rubocop/rubocop-rails/pull/744): Add new `Rails/WhereMissing` cop. ([@ydah][]) diff --git a/config/default.yml b/config/default.yml index 27446c2fed..5939c7da70 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1003,6 +1003,12 @@ Rails/WhereExists: VersionAdded: '2.7' VersionChanged: '2.10' +Rails/WhereMissing: + Description: 'Check for the use `left_joins` and `where` method for finding missing relationship records.' + StyleGuide: 'https://rails.rubystyle.guide/#finding-missing-relationship-records' + Enabled: pending + VersionAdded: '<>' + Rails/WhereNot: Description: 'Use `where.not(...)` instead of manually constructing negated SQL in `where`.' StyleGuide: 'https://rails.rubystyle.guide/#hash-conditions' diff --git a/lib/rubocop/cop/rails/where_missing.rb b/lib/rubocop/cop/rails/where_missing.rb new file mode 100644 index 0000000000..150d46138e --- /dev/null +++ b/lib/rubocop/cop/rails/where_missing.rb @@ -0,0 +1,75 @@ +# 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` instead.' + RESTRICT_ON_SEND = %i[where].freeze + + minimum_target_rails_version 6.1 + + # @!method find_missing_relationship_record?(node) + def_node_search :find_missing_relationship_record?, <<~PATTERN + (send (send ... :left_joins (sym _)) :where (hash (pair (sym _) (hash (pair (sym :id) (nil)))))) + PATTERN + + # @!method left_joins_argument(node) + def_node_matcher :left_joins_argument, <<~PATTERN + (send (const nil? _) :left_joins (sym $_)) + PATTERN + + # @!method where_argument(node) + def_node_matcher :where_argument, <<~PATTERN + (send ... (hash (pair (sym $_) _))) + PATTERN + + def on_send(node) + return unless find_missing_relationship_record?(node) + + left_joins_node = node.receiver + return unless same_relationship?(node, left_joins_node) + + register_offense(node, left_joins_node) + end + + private + + def register_offense(node, left_joins_node) + range = range_between(left_joins_node.loc.selector.begin_pos, left_joins_node.parent.loc.end.end_pos) + add_offense(range) do |corrector| + autocorrect(node, left_joins_node, corrector) + end + end + + def autocorrect(node, left_joins_node, corrector) + corrector.replace(range_between(left_joins_node.loc.selector.begin_pos, node.loc.end.end_pos), + replaced_method(left_joins_node)) + end + + def same_relationship?(where_node, left_joins_node) + "#{left_joins_argument(left_joins_node)}s" == where_argument(where_node).to_s + end + + def replaced_method(node) + "where.missing(:#{left_joins_argument(node)})" + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index cf7c6c5769..e5266f51af 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -119,4 +119,5 @@ require_relative 'rails/validation' require_relative 'rails/where_equals' require_relative 'rails/where_exists' +require_relative 'rails/where_missing' require_relative 'rails/where_not' diff --git a/spec/rubocop/cop/rails/where_missing_spec.rb b/spec/rubocop/cop/rails/where_missing_spec.rb new file mode 100644 index 0000000000..f4450b7b24 --- /dev/null +++ b/spec/rubocop/cop/rails/where_missing_spec.rb @@ -0,0 +1,133 @@ +# 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` instead. + 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 leading ' \ + 'dot method calls' do + expect_offense(<<~RUBY) + Foo + .left_joins(:foo) + ^^^^^^^^^^^^^^^^ Use `where.missing` instead. + .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` instead. + 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` instead. + .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` instead. + .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 }) + 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_joins(foo).where(foos: {id: nil})` with multi-line leading ' \ + 'dot method calls' 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_joins(foo).where(foos: {id: nil})` with multi-line trailing ' \ + 'dot method calls' 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_joins(foo).where(foos: {id: nil})` and there is a line break ' \ + '`after left_joins.where`' 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_joins(foo).where(foos: {id: nil})` and there is a line break ' \ + '`after left_joins.where` and receiver' do + expect_no_offenses(<<~RUBY) + Foo + .left_joins(:foo).where(foos: { id: nil }) + .where(bar: "bar") + RUBY + end + end +end