Add RuboCop::Cop::Betterment::NoStrictLoading cops

* Add ByDefaultForModels to ensure models do not set
  self.strict_loading_by_default.
* Add ForAssociations to ensure model associations do not set
  :strict_loading option.

The purpose of this cop is to make sure to highlight any places where
we explicitly set strict loading, as opposed to relyong on global
settings. The idea will be to capture all the offenses in a
.rubocop_todo.yml so we can burn down and fix all the cases over time.

lib/rubocop/cop/betterment.rb

@@ -11,6 +11,7 @@
 require 'rubocop/cop/betterment/implicit_redirect_type'
 require 'rubocop/cop/betterment/internals_protection'
 require 'rubocop/cop/betterment/memoization_with_arguments'
+require 'rubocop/cop/betterment/no_strict_loading'
 require 'rubocop/cop/betterment/non_standard_actions'
 require 'rubocop/cop/betterment/non_standard_controller'
 require 'rubocop/cop/betterment/redirect_status'

lib/rubocop/cop/betterment/no_strict_loading.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Betterment
+      module NoStrictLoading
+        # This cop ensures that `self.strict_loading_by_default = <any value>` is not set in ActiveRecord models.
+        class ByDefaultForModels < Base
+          extend AutoCorrector
+
+          MSG = 'Do not set `self.strict_loading_by_default` in ActiveRecord models.'
+
+          # @!method strict_loading_by_default_set?(node)
+          def_node_matcher :strict_loading_by_default_set?, <<~PATTERN
+            (send self :strict_loading_by_default= _)
+          PATTERN
+
+          def on_send(node)
+            return unless strict_loading_by_default_set?(node)
+
+            add_offense(node.source_range) do |corrector|
+              buffer    = node.source_range.source_buffer
+              start_pos = node.source_range.begin_pos - node.source_range.column
+
+              # Find end of line including any extra blank line
+              end_pos = start_pos
+              end_pos += 1 while end_pos < buffer.source.length && (buffer.source[end_pos] != "\n" || buffer.source[end_pos + 1] == "\n")
+
+              corrector.remove(Parser::Source::Range.new(buffer, start_pos, end_pos + 1))
+            end
+          end
+        end
+
+        # This cop ensures that `strict_loading: <any value>` is not set in ActiveRecord associations.
+        class ForAssociations < Base
+          extend AutoCorrector
+
+          MSG = 'Do not set `:strict_loading` in ActiveRecord associations.'
+
+          ASSOCIATION_METHODS = %i(belongs_to has_and_belongs_to_many has_many has_one).freeze
+
+          # @!method association_with_strict_loading?(node)
+          def_node_matcher :association_with_strict_loading?, <<~PATTERN
+            (send nil? {#{ASSOCIATION_METHODS.map(&:inspect).join(' ')}} _ (hash <(pair (sym :strict_loading) _) ...>))
+          PATTERN
+
+          def on_send(node) # rubocop:disable Metrics/PerceivedComplexity
+            return unless association_with_strict_loading?(node)
+
+            hash_node = node.last_argument
+            return unless hash_node.hash_type?
+
+            hash_node.pairs.each do |pair|
+              next unless pair.key.sym_type? && pair.key.value.equal?(:strict_loading)
+
+              add_offense(pair.source_range) do |corrector|
+                if hash_node.pairs.one?
+                  # Remove from comma after symbol to end of hash
+                  range = hash_node.source_range.adjust(begin_pos: -2)
+                  corrector.remove(range)
+                else
+                  # Create new hash with remaining pairs
+                  new_source = (hash_node.pairs - [pair]).map(&:source).join(', ')
+                  corrector.replace(hash_node, new_source)
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/betterment/no_strict_loading/by_default_for_models_spec.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Betterment::NoStrictLoading::ByDefaultForModels, :config do
+  context 'when `self.strict_loading_by_default` is set' do
+    context 'when the class has no code before or after self.strict_loading_by_default' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class MyModel < ApplicationRecord
+            self.strict_loading_by_default = true
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not set `self.strict_loading_by_default` in ActiveRecord models.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class MyModel < ApplicationRecord
+          end
+        RUBY
+      end
+    end
+
+    context 'when the class has a comment after self.strict_loading_by_default' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class MyModel < ApplicationRecord
+            self.strict_loading_by_default = true # Set to true to enable strict_loading_by_default
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not set `self.strict_loading_by_default` in ActiveRecord models.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class MyModel < ApplicationRecord
+          end
+        RUBY
+      end
+    end
+
+    context 'when the class has code before self.strict_loading_by_default' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class MyModel < ApplicationRecord
+            self.table_name = :my_models
+            self.strict_loading_by_default = true
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not set `self.strict_loading_by_default` in ActiveRecord models.
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class MyModel < ApplicationRecord
+            self.table_name = :my_models
+          end
+        RUBY
+      end
+    end
+
+    context 'when the class has code after self.strict_loading_by_default' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class MyModel < ApplicationRecord
+            self.strict_loading_by_default = true
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not set `self.strict_loading_by_default` in ActiveRecord models.
+
+            belongs_to :user
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class MyModel < ApplicationRecord
+            belongs_to :user
+          end
+        RUBY
+      end
+    end
+  end
+
+  it 'does not register an offense when `self.strict_loading_by_default` is not set' do
+    expect_no_offenses(<<~RUBY)
+      class MyModel < ApplicationRecord
+      end
+    RUBY
+  end
+end

spec/rubocop/cop/betterment/no_strict_loading/for_associations_spec.rb

@@ -0,0 +1,113 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Betterment::NoStrictLoading::ForAssociations, :config do
+  context 'when `strict_loading` is set in an association' do
+    it 'registers an offense for belongs_to' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          belongs_to :user, strict_loading: false # preserve this comment
+                            ^^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          belongs_to :user # preserve this comment
+        end
+      RUBY
+    end
+
+    it 'registers an offense for has_and_belongs_to_many' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_and_belongs_to_many :tags, strict_loading: true # preserve this comment
+                                         ^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_and_belongs_to_many :tags # preserve this comment
+        end
+      RUBY
+    end
+
+    it 'registers an offense for has_many' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts, strict_loading: true # preserve this comment
+                           ^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts # preserve this comment
+        end
+      RUBY
+    end
+
+    it 'registers an offense for has_one' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_one :account, strict_loading: true # preserve this comment
+                            ^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_one :account # preserve this comment
+        end
+      RUBY
+    end
+
+    it 'preserves other options' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts, strict_loading: true, dependent: :destroy # preserve this comment
+                           ^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts, dependent: :destroy # preserve this comment
+        end
+      RUBY
+    end
+
+    it 'preserves multiline options' do
+      expect_offense(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts,        # preserve this comment
+            strict_loading: true, # remove this comment
+            ^^^^^^^^^^^^^^^^^^^^ Do not set `:strict_loading` in ActiveRecord associations.
+            dependent: :destroy   # preserve this comment
+        end
+      RUBY
+
+      expect_correction(<<~RUBY)
+        class MyModel < ApplicationRecord
+          has_many :posts,        # preserve this comment
+            dependent: :destroy   # preserve this comment
+        end
+      RUBY
+    end
+  end
+
+  context 'when `strict_loading` is not set in an association' do
+    it 'does not register an offense' do
+      expect_no_offenses(<<~RUBY)
+        class MyModel < ApplicationRecord
+          belongs_to :user              # comment
+          has_many :posts               # comment
+          has_and_belongs_to_many :tags # comment
+          has_one :account              # comment
+        end
+      RUBY
+    end
+  end
+end