Add new Rails/WhereNot cop

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### New features
 
 * [#52](https://github.com/rubocop-hq/rubocop-rails/issues/52): Add new `Rails/AfterCommitOverride` cop. ([@fatkodima][])
+* [#274](https://github.com/rubocop-hq/rubocop-rails/pull/274): Add new `Rails/WhereNot` cop. ([@fatkodima][])
 
 ## 2.7.1 (2020-07-26)
 

config/default.yml

@@ -687,6 +687,12 @@ Rails/WhereExists:
   Enabled: 'pending'
   VersionAdded: '2.7'
 
+Rails/WhereNot:
+  Description: 'Use `where.not(...)` instead of manually constructing negated SQL in `where`.'
+  StyleGuide: 'https://rails.rubystyle.guide/#where-not'
+  Enabled: 'pending'
+  VersionAdded: '2.8'
+
 # Accept `redirect_to(...) and return` and similar cases.
 Style/AndOr:
   EnforcedStyle: conditionals

docs/modules/ROOT/pages/cops.adoc

@@ -80,5 +80,6 @@
 * xref:cops_rails.adoc#railsunknownenv[Rails/UnknownEnv]
 * xref:cops_rails.adoc#railsvalidation[Rails/Validation]
 * xref:cops_rails.adoc#railswhereexists[Rails/WhereExists]
+* xref:cops_rails.adoc#railswherenot[Rails/WhereNot]
 
 // END_COP_LIST

docs/modules/ROOT/pages/cops_rails.adoc

@@ -4134,3 +4134,39 @@ User.exists?(name: 'john')
 User.where('length(name) > 10').exists?
 user.posts.exists?(published: true)
 ----
+
+== Rails/WhereNot
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| Yes
+| Yes
+| 2.8
+| -
+|===
+
+This cop identifies places where manually constructed SQL
+in `where` can be replaced with `where.not(...)`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+User.where('name != ?', 'Gabe')
+User.where('name != :name', name: 'Gabe')
+User.where('name IS NOT NULL')
+User.where('name NOT IN (?)', ['john', 'jane'])
+User.where('name NOT IN (:names)', names: ['john', 'jane'])
+
+# good
+User.where.not(name: 'Gabe')
+User.where.not(name: nil)
+User.where.not(name: ['john', 'jane'])
+----
+
+=== References
+
+* https://rails.rubystyle.guide/#where-not

lib/rubocop/cop/rails/where_not.rb

@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cop identifies places where manually constructed SQL
+      # in `where` can be replaced with `where.not(...)`.
+      #
+      # @example
+      #   # bad
+      #   User.where('name != ?', 'Gabe')
+      #   User.where('name != :name', name: 'Gabe')
+      #   User.where('name IS NOT NULL')
+      #   User.where('name NOT IN (?)', ['john', 'jane'])
+      #   User.where('name NOT IN (:names)', names: ['john', 'jane'])
+      #
+      #   # good
+      #   User.where.not(name: 'Gabe')
+      #   User.where.not(name: nil)
+      #   User.where.not(name: ['john', 'jane'])
+      #
+      class WhereNot < Cop
+        include RangeHelp
+
+        MSG = 'Use `%<good_method>s` instead of manually constructing negated SQL in `where`.'
+
+        def_node_matcher :where_method_call?, <<~PATTERN
+          {
+            (send _ :where (array $str_type? $_ ?))
+            (send _ :where $str_type? $_ ?)
+          }
+        PATTERN
+
+        def on_send(node)
+          where_method_call?(node) do |template_node, value_node|
+            value_node = value_node.first
+
+            range = offense_range(node)
+
+            column_and_value = extract_column_and_value(template_node, value_node)
+            return unless column_and_value
+
+            good_method = build_good_method(*column_and_value)
+            message = format(MSG, good_method: good_method)
+
+            add_offense(node, location: range, message: message)
+          end
+        end
+
+        def autocorrect(node)
+          where_method_call?(node) do |template_node, value_node|
+            value_node = value_node.first
+
+            lambda do |corrector|
+              range = offense_range(node)
+
+              column, value = *extract_column_and_value(template_node, value_node)
+              replacement = build_good_method(column, value)
+
+              corrector.replace(range, replacement)
+            end
+          end
+        end
+
+        NOT_EQ_ANONYMOUS_RE = /\A([\w.]+)\s+!=\s+\?\z/.freeze                  # column != ?
+        NOT_IN_ANONYMOUS_RE = /\A([\w.]+)\s+NOT\s+IN\s+\(\?\)\z/i.freeze       # column NOT IN (?)
+        NOT_EQ_NAMED_RE     = /\A([\w.]+)\s+!=\s+:(\w+)\z/.freeze              # column != :column
+        NOT_IN_NAMED_RE     = /\A([\w.]+)\s+NOT\s+IN\s+\(:(\w+)\)\z/i.freeze   # column NOT IN (:column)
+        IS_NOT_NULL_RE      = /\A([\w.]+)\s+IS\s+NOT\s+NULL\z/i.freeze         # column IS NOT NULL
+
+        private
+
+        def offense_range(node)
+          range_between(node.loc.selector.begin_pos, node.loc.expression.end_pos)
+        end
+
+        def extract_column_and_value(template_node, value_node)
+          value =
+            case template_node.value
+            when NOT_EQ_ANONYMOUS_RE, NOT_IN_ANONYMOUS_RE
+              value_node.source
+            when NOT_EQ_NAMED_RE, NOT_IN_NAMED_RE
+              return unless value_node.hash_type?
+
+              pair = value_node.pairs.find { |p| p.key.value.to_sym == Regexp.last_match(2).to_sym }
+              pair.value.source
+            when IS_NOT_NULL_RE
+              'nil'
+            else
+              return
+            end
+
+          [Regexp.last_match(1), value]
+        end
+
+        def build_good_method(column, value)
+          if column.include?('.')
+            "where.not('#{column}' => #{value})"
+          else
+            "where.not(#{column}: #{value})"
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -82,3 +82,4 @@
 require_relative 'rails/unknown_env'
 require_relative 'rails/validation'
 require_relative 'rails/where_exists'
+require_relative 'rails/where_not'

spec/rubocop/cop/rails/where_not_spec.rb

@@ -0,0 +1,163 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::WhereNot do
+  subject(:cop) { described_class.new }
+
+  it 'registers an offense and corrects when using `!=` and anonymous placeholder' do
+    expect_offense(<<~RUBY)
+      User.where('name != ?', 'Gabe')
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: 'Gabe')
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `!=` and named placeholder' do
+    expect_offense(<<~RUBY)
+      User.where('name != :name', name: 'Gabe')
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: 'Gabe')
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `IS NOT NULL`' do
+    expect_offense(<<~RUBY)
+      User.where('name IS NOT NULL')
+           ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: nil)` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: nil)
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `NOT IN` and anonymous placeholder' do
+    expect_offense(<<~RUBY)
+      User.where("name NOT IN (?)", ['john', 'jane'])
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: ['john', 'jane'])` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: ['john', 'jane'])
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using `NOT IN` and named placeholder' do
+    expect_offense(<<~RUBY)
+      User.where("name NOT IN (:names)", names: ['john', 'jane'])
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: ['john', 'jane'])` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: ['john', 'jane'])
+    RUBY
+  end
+
+  it 'registers an offense and corrects when using namespaced columns' do
+    expect_offense(<<~RUBY)
+      Course.where('enrollments.student_id != ?', student.id)
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      Course.where.not('enrollments.student_id' => student.id)
+    RUBY
+  end
+
+  context 'with array arguments' do
+    it 'registers an offense and corrects when using `!=` and anonymous placeholder' do
+      expect_offense(<<~RUBY)
+        User.where(['name != ?', 'Gabe'])
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.where.not(name: 'Gabe')
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `!=` and named placeholder' do
+      expect_offense(<<~RUBY)
+        User.where(['name != :name', { name: 'Gabe' }])
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.where.not(name: 'Gabe')
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `IS NOT NULL`' do
+      expect_offense(<<~RUBY)
+        User.where(['name IS NOT NULL'])
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: nil)` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.where.not(name: nil)
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `NOT IN` and anonymous placeholder' do
+      expect_offense(<<~RUBY)
+        User.where(["name NOT IN (?)", ['john', 'jane']])
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: ['john', 'jane'])` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.where.not(name: ['john', 'jane'])
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using `NOT IN` and named placeholder' do
+      expect_offense(<<~RUBY)
+        User.where(["name NOT IN (:names)", names: ['john', 'jane']])
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not(name: ['john', 'jane'])` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        User.where.not(name: ['john', 'jane'])
+      RUBY
+    end
+
+    it 'registers an offense and corrects when using namespaced columns' do
+      expect_offense(<<~RUBY)
+        Course.where(['enrollments.student_id != ?', student.id])
+               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where.not('enrollments.student_id' => student.id)` instead of manually constructing negated SQL in `where`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        Course.where.not('enrollments.student_id' => student.id)
+      RUBY
+    end
+  end
+
+  it 'does not register an offense when using `where.not`' do
+    expect_no_offenses(<<~RUBY)
+      User.where.not(name: nil)
+    RUBY
+  end
+
+  it 'does not register an offense when not using template string' do
+    expect_no_offenses(<<~RUBY)
+      User.where(name: 'john')
+    RUBY
+  end
+
+  it 'does not register an offense when template string does not contain negation' do
+    expect_no_offenses(<<~RUBY)
+      User.where('name = ?', 'john')
+    RUBY
+  end
+
+  it 'does not register an offense when template string contains negation and additional boolean logic' do
+    expect_no_offenses(<<~RUBY)
+      User.where('name != ? AND age != ?', 'john', 19)
+    RUBY
+  end
+end