Merge pull request #1323 from Earlopain/where-equal-not

[Fix #1199] Make `Rails/WhereEquals` aware of `where.not(...)`

Author: koic

changelog/fix_make_rails_where_equals_aware_of_not.md

@@ -0,0 +1 @@
+* [#1199](https://github.com/rubocop/rubocop-rails/issues/1199): Make `Rails/WhereEquals` aware of `where.not(...)`. ([@earlopain][])

config/default.yml

@@ -1185,12 +1185,12 @@ Rails/Validation:
     - app/models/**/*.rb
 
 Rails/WhereEquals:
-  Description: 'Pass conditions to `where` as a hash instead of manually constructing SQL.'
+  Description: 'Pass conditions to `where` and `where.not` as a hash instead of manually constructing SQL.'
   StyleGuide: 'https://rails.rubystyle.guide/#hash-conditions'
   Enabled: 'pending'
   SafeAutoCorrect: false
   VersionAdded: '2.9'
-  VersionChanged: '2.10'
+  VersionChanged: <<next>>
 
 Rails/WhereExists:
   Description: 'Prefer `exists?(...)` over `where(...).exists?`.'

lib/rubocop/cop/rails/where_equals.rb

@@ -4,7 +4,8 @@ module RuboCop
   module Cop
     module Rails
       # Identifies places where manually constructed SQL
-      # in `where` can be replaced with `where(attribute: value)`.
+      # in `where` and `where.not` can be replaced with
+      # `where(attribute: value)` and `where.not(attribute: value)`.
       #
       # @safety
       #   This cop's autocorrection is unsafe because is may change SQL.
@@ -13,6 +14,7 @@ module Rails
       # @example
       #   # bad
       #   User.where('name = ?', 'Gabe')
+      #   User.where.not('name = ?', 'Gabe')
       #   User.where('name = :name', name: 'Gabe')
       #   User.where('name IS NULL')
       #   User.where('name IN (?)', ['john', 'jane'])
@@ -21,6 +23,7 @@ module Rails
       #
       #   # good
       #   User.where(name: 'Gabe')
+      #   User.where.not(name: 'Gabe')
       #   User.where(name: nil)
       #   User.where(name: ['john', 'jane'])
       #   User.where(users: { name: 'Gabe' })
@@ -29,16 +32,18 @@ class WhereEquals < Base
         extend AutoCorrector
 
         MSG = 'Use `%<good_method>s` instead of manually constructing SQL.'
-        RESTRICT_ON_SEND = %i[where].freeze
+        RESTRICT_ON_SEND = %i[where not].freeze
 
         def_node_matcher :where_method_call?, <<~PATTERN
           {
-            (call _ :where (array $str_type? $_ ?))
-            (call _ :where $str_type? $_ ?)
+            (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_method_call?(node) do |template_node, value_node|
             value_node = value_node.first
 
@@ -47,7 +52,7 @@ def on_send(node)
             column, value = extract_column_and_value(template_node, value_node)
             return unless value
 
-            good_method = build_good_method(column, value)
+            good_method = build_good_method(node.method_name, column, value)
             message = format(MSG, good_method: good_method)
 
             add_offense(range, message: message) do |corrector|
@@ -88,15 +93,21 @@ def extract_column_and_value(template_node, value_node)
           [Regexp.last_match(1), value]
         end
 
-        def build_good_method(column, value)
+        def build_good_method(method_name, column, value)
           if column.include?('.')
             table, column = column.split('.')
 
-            "where(#{table}: { #{column}: #{value} })"
+            "#{method_name}(#{table}: { #{column}: #{value} })"
           else
-            "where(#{column}: #{value})"
+            "#{method_name}(#{column}: #{value})"
           end
         end
+
+        def where_not?(node)
+          return false unless (receiver = node.receiver)
+
+          receiver.send_type? && receiver.method?(:where)
+        end
       end
     end
   end

spec/rubocop/cop/rails/where_equals_spec.rb

@@ -12,6 +12,17 @@
     RUBY
   end
 
+  it 'registers an offense and corrects when using `=` and anonymous placeholder with not' do
+    expect_offense(<<~RUBY)
+      User.where.not('name = ?', 'Gabe')
+                 ^^^^^^^^^^^^^^^^^^^^^^^ Use `not(name: 'Gabe')` instead of manually constructing SQL.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      User.where.not(name: 'Gabe')
+    RUBY
+  end
+
   it 'registers an offense and corrects when using `=` and anonymous placeholder with safe navigation' do
     expect_offense(<<~RUBY)
       User&.where('name = ?', 'Gabe')
@@ -200,4 +211,10 @@
       User.where("name IN (:names)", )
     RUBY
   end
+
+  it 'does not register an offense when using `not` not preceded by `where`' do
+    expect_no_offenses(<<~RUBY)
+      users.not('name = ?', 'Gabe')
+    RUBY
+  end
 end