diff options
-rw-r--r-- | app/models/award_emoji.rb | 2 | ||||
-rw-r--r-- | app/models/deployment.rb | 2 | ||||
-rw-r--r-- | app/models/event.rb | 2 | ||||
-rw-r--r-- | app/models/label_link.rb | 2 | ||||
-rw-r--r-- | app/models/member.rb | 2 | ||||
-rw-r--r-- | app/models/note.rb | 2 | ||||
-rw-r--r-- | app/models/notification_setting.rb | 2 | ||||
-rw-r--r-- | app/models/redirect_route.rb | 2 | ||||
-rw-r--r-- | app/models/route.rb | 2 | ||||
-rw-r--r-- | app/models/sent_notification.rb | 2 | ||||
-rw-r--r-- | app/models/subscription.rb | 2 | ||||
-rw-r--r-- | app/models/todo.rb | 2 | ||||
-rw-r--r-- | app/models/upload.rb | 2 | ||||
-rw-r--r-- | app/models/user_agent_detail.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/activerecord_serialize.rb | 16 | ||||
-rw-r--r-- | rubocop/cop/polymorphic_associations.rb | 23 | ||||
-rw-r--r-- | rubocop/model_helpers.rb | 11 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 1 | ||||
-rw-r--r-- | spec/rubocop/cop/activerecord_serialize_spec.rb | 4 | ||||
-rw-r--r-- | spec/rubocop/cop/polymorphic_associations_spec.rb | 33 |
20 files changed, 89 insertions, 27 deletions
diff --git a/app/models/award_emoji.rb b/app/models/award_emoji.rb index 6ada6fae4eb..ebe60441603 100644 --- a/app/models/award_emoji.rb +++ b/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 diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 304179c0a97..85e7901dfee 100644 --- a/app/models/deployment.rb +++ b/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 diff --git a/app/models/event.rb b/app/models/event.rb index d6d39473774..fad6ff03927 100644 --- a/app/models/event.rb +++ b/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 diff --git a/app/models/label_link.rb b/app/models/label_link.rb index 51b5c2b1f4c..d68e1f54317 100644 --- a/app/models/label_link.rb +++ b/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? diff --git a/app/models/member.rb b/app/models/member.rb index 29f9d61e870..788a32dd8e3 100644 --- a/app/models/member.rb +++ b/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 diff --git a/app/models/note.rb b/app/models/note.rb index 563af47f314..244bf169c29 100644 --- a/app/models/note.rb +++ b/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' diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index e4726e62e93..42412a9a44b 100644 --- a/app/models/notification_setting.rb +++ b/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 diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb index 99812bcde53..964175ddab8 100644 --- a/app/models/redirect_route.rb +++ b/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 diff --git a/app/models/route.rb b/app/models/route.rb index be77b8b51a5..97e8a6ad9e9 100644 --- a/app/models/route.rb +++ b/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 diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index eed3ca7e179..edde7bedbab 100644 --- a/app/models/sent_notification.rb +++ b/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 diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 17869c8bac2..2f0c9640744 100644 --- a/app/models/subscription.rb +++ b/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 diff --git a/app/models/todo.rb b/app/models/todo.rb index b011001b235..696d139af74 100644 --- a/app/models/todo.rb +++ b/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 diff --git a/app/models/upload.rb b/app/models/upload.rb index 13987931b05..f194d7bdb80 100644 --- a/app/models/upload.rb +++ b/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 diff --git a/app/models/user_agent_detail.rb b/app/models/user_agent_detail.rb index 0949c6ef083..2d05fdd3e54 100644 --- a/app/models/user_agent_detail.rb +++ b/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 diff --git a/rubocop/cop/activerecord_serialize.rb b/rubocop/cop/activerecord_serialize.rb index bfa0cff9a67..9bdcc3b4c34 100644 --- a/rubocop/cop/activerecord_serialize.rb +++ b/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 diff --git a/rubocop/cop/polymorphic_associations.rb b/rubocop/cop/polymorphic_associations.rb new file mode 100644 index 00000000000..7d554704550 --- /dev/null +++ b/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 diff --git a/rubocop/model_helpers.rb b/rubocop/model_helpers.rb new file mode 100644 index 00000000000..309723dc34c --- /dev/null +++ b/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 diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 22815090508..dae30969abf 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -2,6 +2,7 @@ require_relative 'cop/custom_error_class' 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' diff --git a/spec/rubocop/cop/activerecord_serialize_spec.rb b/spec/rubocop/cop/activerecord_serialize_spec.rb index a303b16d264..5bd7e5fa926 100644 --- a/spec/rubocop/cop/activerecord_serialize_spec.rb +++ b/spec/rubocop/cop/activerecord_serialize_spec.rb @@ -10,7 +10,7 @@ describe RuboCop::Cop::ActiverecordSerialize do 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 @@ describe RuboCop::Cop::ActiverecordSerialize do 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') diff --git a/spec/rubocop/cop/polymorphic_associations_spec.rb b/spec/rubocop/cop/polymorphic_associations_spec.rb new file mode 100644 index 00000000000..49959aa6419 --- /dev/null +++ b/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 |