Add cop for checking duplicate assingments to ignored_columns

Overwriting previous assignments is usually a mistake, since it will
un-ignore the first set of columns

Author: fsateler

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* [#514](https://github.com/rubocop/rubocop-rails/pull/514): Add new `Rails/DuplicateIgnoredColumns` cop. ([@fsateler][])
+
 ## 2.11.1 (2021-06-25)
 
 ### Bug fixes
@@ -417,3 +421,4 @@
 [@nvasilevski]: https://github.com/nvasilevski
 [@skryukov]: https://github.com/skryukov
 [@johnsyweb]: https://github.com/johnsyweb
+[@fsateler]: https://github.com/fsateler

config/default.yml

@@ -236,6 +236,11 @@ Rails/DelegateAllowBlank:
   Enabled: true
   VersionAdded: '0.44'
 
+Rails/DuplicateIgnoredColumns:
+  Description: 'Looks for assignments of `ignored_columns` that override previous assignments.'
+  Enabled: pending
+  VersionAdded: '2.12'
+
 Rails/DynamicFindBy:
   Description: 'Use `find_by` instead of dynamic `find_by_*`.'
   StyleGuide: 'https://rails.rubystyle.guide#find_by'

docs/modules/ROOT/pages/cops.adoc

@@ -39,6 +39,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railsdefaultscope[Rails/DefaultScope]
 * xref:cops_rails.adoc#railsdelegate[Rails/Delegate]
 * xref:cops_rails.adoc#railsdelegateallowblank[Rails/DelegateAllowBlank]
+* xref:cops_rails.adoc#railsduplicateignoredcolumns[Rails/DuplicateIgnoredColumns]
 * xref:cops_rails.adoc#railsdynamicfindby[Rails/DynamicFindBy]
 * xref:cops_rails.adoc#railseagerevaluationlogmessage[Rails/EagerEvaluationLogMessage]
 * xref:cops_rails.adoc#railsenumhash[Rails/EnumHash]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1156,6 +1156,52 @@ delegate :foo, to: :bar, allow_blank: true
 delegate :foo, to: :bar, allow_nil: true
 ----
 
+== Rails/DuplicateIgnoredColumns
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| Yes
+| No
+| 2.12
+| -
+|===
+
+This cops looks for assignments of `ignored_columns` that override previous
+assignments.
+
+Overwriting previous assignments is usually a mistake, since it will
+un-ignore the first set of columns
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+class User < ActiveRecord::Base
+  self.ignored_columns = [:one]
+  self.ignored_columns = [:two]
+end
+
+# bad
+class User < ActiveRecord::Base
+  self.ignored_columns += [:one]
+  self.ignored_columns = [:two]
+end
+
+# good
+class User < ActiveRecord::Base
+  self.ignored_columns = [:one, :two]
+end
+
+# good
+class User < ActiveRecord::Base
+  self.ignored_columns = [:one]
+  self.ignored_columns += [:two]
+end
+----
+
 == Rails/DynamicFindBy
 
 |===

lib/rubocop/cop/rails/duplicate_ignored_columns.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cops looks for assignments of `ignored_columns` that override previous
+      # assignments.
+      #
+      # Overwriting previous assignments is usually a mistake, since it will
+      # un-ignore the first set of columns
+      #
+      # @example
+      #
+      #   # bad
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns = [:one]
+      #     self.ignored_columns = [:two]
+      #   end
+      #
+      #   # bad
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns += [:one]
+      #     self.ignored_columns = [:two]
+      #   end
+      #
+      #   # good
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns = [:one, :two]
+      #   end
+      #
+      #   # good
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns = [:one]
+      #     self.ignored_columns += [:two]
+      #   end
+      #
+      class DuplicateIgnoredColumns < Base
+        MSG = 'This assignment to `ignored_columns` overwrites previous ones.'
+
+        def_node_matcher :ignored_columns_assign, <<-PATTERN
+          (send self :ignored_columns= ...)
+        PATTERN
+        def_node_matcher :ignored_columns_append, <<-PATTERN
+          (op-asgn
+            (send self :ignored_columns)
+            :+ ...
+          )
+        PATTERN
+
+        def on_op_asgn(node)
+          ignored_columns_append(node) do
+            @seen_before = true
+          end
+        end
+
+        def on_send(node)
+          ignored_columns_assign(node) do
+            add_offense(node.loc.selector) if @seen_before
+            @seen_before = true
+          end
+        end
+
+        def on_new_investigation
+          @seen_before = false
+          super
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -28,6 +28,7 @@
 require_relative 'rails/default_scope'
 require_relative 'rails/delegate'
 require_relative 'rails/delegate_allow_blank'
+require_relative 'rails/duplicate_ignored_columns'
 require_relative 'rails/dynamic_find_by'
 require_relative 'rails/eager_evaluation_log_message'
 require_relative 'rails/enum_hash'

spec/rubocop/cop/rails/duplicate_ignored_columns_spec.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::DuplicateIgnoredColumns, :config do
+  it 'registers an offense when setting `ignored_columns` twice' do
+    expect_offense(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+        self.ignored_columns = [:two]
+             ^^^^^^^^^^^^^^^ This assignment to `ignored_columns` overwrites previous ones.
+      end
+    RUBY
+  end
+
+  it 'registers an offense when setting `ignored_columns` after appending' do
+    expect_offense(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns += [:one]
+        self.ignored_columns = [:two]
+             ^^^^^^^^^^^^^^^ This assignment to `ignored_columns` overwrites previous ones.
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when using `ignored_columns` once' do
+    expect_no_offenses(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+      end
+    RUBY
+  end
+  it 'does not register an offense when setting `ignored_columns` and then appending' do
+    expect_no_offenses(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+        self.ignored_columns += [:two]
+      end
+    RUBY
+  end
+end