Added Cop to blacklist polymorphic associations

One should really use a separate table instead of using polymorphic
associations.

See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11168 for
more information.

Author: Yorick Peterse

app/models/award_emoji.rb

@@ -5,7 +5,7 @@ class AwardEmoji < ActiveRecord::Base
   include Participable
   include GhostUser
 
-  belongs_to :awardable, polymorphic: true
+  belongs_to :awardable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :user
 
   validates :awardable, :user, presence: true

app/models/deployment.rb

@@ -4,7 +4,7 @@ class Deployment < ActiveRecord::Base
   belongs_to :project, required: true, validate: true
   belongs_to :environment, required: true, validate: true
   belongs_to :user
-  belongs_to :deployable, polymorphic: true
+  belongs_to :deployable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :sha, presence: true
   validates :ref, presence: true

app/models/event.rb

@@ -47,7 +47,7 @@ class Event < ActiveRecord::Base
 
   belongs_to :author, class_name: "User"
   belongs_to :project
-  belongs_to :target, polymorphic: true
+  belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   # For Hash only
   serialize :data # rubocop:disable Cop/ActiverecordSerialize

app/models/label_link.rb

@@ -1,7 +1,7 @@
 class LabelLink < ActiveRecord::Base
   include Importable
 
-  belongs_to :target, polymorphic: true
+  belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :label
 
   validates :target, presence: true, unless: :importing?

app/models/member.rb

@@ -8,7 +8,7 @@ class Member < ActiveRecord::Base
 
   belongs_to :created_by, class_name: "User"
   belongs_to :user
-  belongs_to :source, polymorphic: true
+  belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   delegate :name, :username, :email, to: :user, prefix: true
 

app/models/note.rb

@@ -41,7 +41,7 @@ class Note < ActiveRecord::Base
   participant :author
 
   belongs_to :project
-  belongs_to :noteable, polymorphic: true, touch: true
+  belongs_to :noteable, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :author, class_name: "User"
   belongs_to :updated_by, class_name: "User"
   belongs_to :last_edited_by, class_name: 'User'

app/models/notification_setting.rb

@@ -4,7 +4,7 @@ class NotificationSetting < ActiveRecord::Base
   default_value_for :level, NotificationSetting.levels[:global]
 
   belongs_to :user
-  belongs_to :source, polymorphic: true
+  belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :project, foreign_key: 'source_id'
 
   validates :user, presence: true

app/models/redirect_route.rb

@@ -1,5 +1,5 @@
 class RedirectRoute < ActiveRecord::Base
-  belongs_to :source, polymorphic: true
+  belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :source, presence: true
 

app/models/route.rb

@@ -1,5 +1,5 @@
 class Route < ActiveRecord::Base
-  belongs_to :source, polymorphic: true
+  belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :source, presence: true
 

app/models/sent_notification.rb

@@ -2,7 +2,7 @@ class SentNotification < ActiveRecord::Base
   serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize
 
   belongs_to :project
-  belongs_to :noteable, polymorphic: true
+  belongs_to :noteable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :recipient, class_name: "User"
 
   validates :project, :recipient, presence: true

app/models/subscription.rb

@@ -1,7 +1,7 @@
 class Subscription < ActiveRecord::Base
   belongs_to :user
   belongs_to :project
-  belongs_to :subscribable, polymorphic: true
+  belongs_to :subscribable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :user, :subscribable, presence: true
 

app/models/todo.rb

@@ -22,7 +22,7 @@ class Todo < ActiveRecord::Base
   belongs_to :author, class_name: "User"
   belongs_to :note
   belongs_to :project
-  belongs_to :target, polymorphic: true, touch: true
+  belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :user
 
   delegate :name, :email, to: :author, prefix: true, allow_nil: true

app/models/upload.rb

@@ -2,7 +2,7 @@ class Upload < ActiveRecord::Base
   # Upper limit for foreground checksum processing
   CHECKSUM_THRESHOLD = 100.megabytes
 
-  belongs_to :model, polymorphic: true
+  belongs_to :model, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :size, presence: true
   validates :path, presence: true

app/models/user_agent_detail.rb

@@ -1,5 +1,5 @@
 class UserAgentDetail < ActiveRecord::Base
-  belongs_to :subject, polymorphic: true
+  belongs_to :subject, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :user_agent, :ip_address, :subject_id, :subject_type, presence: true
 

rubocop/cop/activerecord_serialize.rb

@@ -1,24 +1,18 @@
+require_relative '../model_helpers'
+
 module RuboCop
   module Cop
     # Cop that prevents the use of `serialize` in ActiveRecord models.
     class ActiverecordSerialize < RuboCop::Cop::Cop
+      include ModelHelpers
+
       MSG = 'Do not store serialized data in the database, use separate columns and/or tables instead'.freeze
 
       def on_send(node)
-        return unless in_models?(node)
+        return unless in_model?(node)
 
         add_offense(node, :selector) if node.children[1] == :serialize
       end
-
-      def models_path
-        File.join(Dir.pwd, 'app', 'models')
-      end
-
-      def in_models?(node)
-        path = node.location.expression.source_buffer.name
-
-        path.start_with?(models_path)
-      end
     end
   end
 end

rubocop/cop/polymorphic_associations.rb

@@ -0,0 +1,23 @@
+require_relative '../model_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that prevents the use of polymorphic associations
+    class PolymorphicAssociations < RuboCop::Cop::Cop
+      include ModelHelpers
+
+      MSG = 'Do not use polymorphic associations, use separate tables instead'.freeze
+
+      def on_send(node)
+        return unless in_model?(node)
+        return unless node.children[1] == :belongs_to
+
+        node.children.last.each_node(:pair) do |pair|
+          key_name = pair.children[0].children[0]
+
+          add_offense(pair, :expression) if key_name == :polymorphic
+        end
+      end
+    end
+  end
+end

rubocop/model_helpers.rb

@@ -0,0 +1,11 @@
+module RuboCop
+  module ModelHelpers
+    # Returns true if the given node originated from the models directory.
+    def in_model?(node)
+      path = node.location.expression.source_buffer.name
+      models_path = File.join(Dir.pwd, 'app', 'models')
+
+      path.start_with?(models_path)
+    end
+  end
+end

rubocop/rubocop.rb

@@ -2,6 +2,7 @@
 require_relative 'cop/gem_fetcher'
 require_relative 'cop/activerecord_serialize'
 require_relative 'cop/redirect_with_status'
+require_relative 'cop/polymorphic_associations'
 require_relative 'cop/migration/add_column'
 require_relative 'cop/migration/add_column_with_default_to_large_table'
 require_relative 'cop/migration/add_concurrent_foreign_key'

spec/rubocop/cop/activerecord_serialize_spec.rb

@@ -10,7 +10,7 @@
 
   context 'inside the app/models directory' do
     it 'registers an offense when serialize is used' do
-      allow(cop).to receive(:in_models?).and_return(true)
+      allow(cop).to receive(:in_model?).and_return(true)
 
       inspect_source(cop, 'serialize :foo')
 
@@ -23,7 +23,7 @@
 
   context 'outside the app/models directory' do
     it 'does nothing' do
-      allow(cop).to receive(:in_models?).and_return(false)
+      allow(cop).to receive(:in_model?).and_return(false)
 
       inspect_source(cop, 'serialize :foo')
 

spec/rubocop/cop/polymorphic_associations_spec.rb

@@ -0,0 +1,33 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/polymorphic_associations'
+
+describe RuboCop::Cop::PolymorphicAssociations do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'inside the app/models directory' do
+    it 'registers an offense when polymorphic: true is used' do
+      allow(cop).to receive(:in_model?).and_return(true)
+
+      inspect_source(cop, 'belongs_to :foo, polymorphic: true')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+      end
+    end
+  end
+
+  context 'outside the app/models directory' do
+    it 'does nothing' do
+      allow(cop).to receive(:in_model?).and_return(false)
+
+      inspect_source(cop, 'belongs_to :foo, polymorphic: true')
+
+      expect(cop.offenses).to be_empty
+    end
+  end
+end