Add cop for checking assingments to ignored_columns

changelog/new_ignored_columns_assignment.md

@@ -0,0 +1 @@
+* [#771](https://github.com/rubocop/rubocop-rails/pull/771): Add new `Rails/IgnoredColumnsAssignment` cop. ([@kkitadate][])

config/default.yml

@@ -537,6 +537,13 @@ Rails/I18nLocaleTexts:
   Enabled: pending
   VersionAdded: '2.14'
 
+Rails/IgnoredColumnsAssignment:
+  Description: 'Looks for assignments of `ignored_columns` that override previous assignments.'
+  StyleGuide: 'https://rails.rubystyle.guide/#append-ignored-columns'
+  Enabled: pending
+  SafeAutoCorrect: false
+  VersionAdded: '<<next>>'
+
 Rails/IgnoredSkipActionFilterOption:
   Description: 'Checks that `if` and `only` (or `except`) are not used together as options of `skip_*` action filter.'
   Reference: 'https://api.rubyonrails.org/classes/AbstractController/Callbacks/ClassMethods.html#method-i-_normalize_callback_options'

lib/rubocop/cop/rails/ignored_columns_assignment.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cops looks for assignments of `ignored_columns` that may override previous
+      # assignments.
+      #
+      # Overwriting previous assignments is usually a mistake, since it will
+      # un-ignore the first set of columns. Since duplicate column names is not
+      # a problem, it is better to simply append to the list.
+      #
+      # @example
+      #
+      #   # bad
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns = [:one]
+      #   end
+      #
+      #   # bad
+      #   class User < ActiveRecord::Base
+      #     self.ignored_columns = [:one, :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 IgnoredColumnsAssignment < Base
+        extend AutoCorrector
+
+        MSG = 'Use `+=` instead of `=`.'
+        RESTRICT_ON_SEND = %i[ignored_columns=].freeze
+
+        def on_send(node)
+          add_offense(node.loc.operator) do |corrector|
+            corrector.replace(node.loc.operator, '+=')
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -61,6 +61,7 @@
 require_relative 'rails/i18n_lazy_lookup'
 require_relative 'rails/i18n_locale_assignment'
 require_relative 'rails/i18n_locale_texts'
+require_relative 'rails/ignored_columns_assignment'
 require_relative 'rails/ignored_skip_action_filter_option'
 require_relative 'rails/index_by'
 require_relative 'rails/index_with'

spec/rubocop/cop/rails/ignored_columns_assignment_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::IgnoredColumnsAssignment, :config do
+  it 'registers an offense when using `ignored_columns` once' do
+    expect_offense(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+                             ^ Use `+=` instead of `=`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns += [:one]
+      end
+    RUBY
+  end
+
+  it 'registers an offense when setting `ignored_columns` twice' do
+    expect_offense(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+                             ^ Use `+=` instead of `=`.
+        self.ignored_columns = [:two]
+                             ^ Use `+=` instead of `=`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns += [:one]
+        self.ignored_columns += [:two]
+      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]
+                             ^ Use `+=` instead of `=`.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns += [:one]
+        self.ignored_columns += [:two]
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when appending to `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