Add cop for checking assingments to ignored_columns

Setting `ignored_columns` may overwrite previous assignments, and that
is problematic because it can un-ignore a previously set column list.
Since it is not a problem to have a duplicate column in the list,
simply recommend always appending.

Author: fsateler

changelog/new_set_ignored_columns.md

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

config/default.yml

@@ -764,6 +764,12 @@ Rails/ScopeArgs:
   Include:
     - app/models/**/*.rb
 
+Rails/SetIgnoredColumns:
+  Description: 'Looks for assignments of `ignored_columns` that override previous assignments.'
+  Enabled: pending
+  SafeAutoCorrect: false
+  VersionAdded: '<<next>>'
+
 Rails/ShortI18n:
   Description: 'Use the short form of the I18n methods: `t` instead of `translate` and `l` instead of `localize`.'
   StyleGuide: 'https://rails.rubystyle.guide/#short-i18n'

lib/rubocop/cop/rails/set_ignored_columns.rb

@@ -0,0 +1,54 @@
+# 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 SetIgnoredColumns < Base
+        extend AutoCorrector
+        MSG = 'This assignment to `ignored_columns` may overwrite previous ones.'
+
+        def_node_matcher :ignored_columns_assign, <<-PATTERN
+          (send self :ignored_columns= ...)
+        PATTERN
+
+        def on_send(node)
+          ignored_columns_assign(node) do
+            add_offense(node) do |corrector|
+              corrector.replace(node.loc.operator, '+=')
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -92,6 +92,7 @@
 require_relative 'rails/save_bang'
 require_relative 'rails/schema_comment'
 require_relative 'rails/scope_args'
+require_relative 'rails/set_ignored_columns'
 require_relative 'rails/short_i18n'
 require_relative 'rails/skips_model_validations'
 require_relative 'rails/squished_sql_heredocs'

spec/rubocop/cop/rails/set_ignored_columns_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::SetIgnoredColumns, :config do
+  it 'registers an offense when using `ignored_columns` once' do
+    expect_offense(<<~RUBY)
+      class User < ActiveRecord::Base
+        self.ignored_columns = [:one]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This assignment to `ignored_columns` may overwrite previous ones.
+      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]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This assignment to `ignored_columns` may overwrite previous ones.
+        self.ignored_columns = [:two]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This assignment to `ignored_columns` may overwrite previous ones.
+      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]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This assignment to `ignored_columns` may overwrite previous ones.
+      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