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/new_duplicate_ignored_columns.md

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

config/default.yml

@@ -251,6 +251,11 @@ Rails/DelegateAllowBlank:
   Enabled: true
   VersionAdded: '0.44'
 
+Rails/DuplicateIgnoredColumns:
+  Description: 'Looks for assignments of `ignored_columns` that override previous assignments.'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+
 Rails/DurationArithmetic:
   Description: 'Do not use duration as arithmetic operand with `Time.current`.'
   StyleGuide: 'https://rails.rubystyle.guide#duration-arithmetic'

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) 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

@@ -30,6 +30,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/duration_arithmetic'
 require_relative 'rails/dynamic_find_by'
 require_relative 'rails/eager_evaluation_log_message'

spec/rubocop/cop/rails/duplicate_ignored_columns_spec.rb

@@ -0,0 +1,40 @@
+# 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