Merge remote-tracking branch 'upstream/master' into 27762-add-default-artifacts-expiration

* upstream/master: (247 commits)
  Switched CONTRIBUTING.md style guide recommendation for method chaining
  Fix new offenses
  Stylistic tweaks
  Fix OAuth/SAML user blocking behavior
  Revert "Enable Style/DotPosition"
  Revert "Prefer leading style for Style/DotPosition"
  Revert "Enable Style/BarePercentLiterals"
  Manually correct autocorrect
  Move up delegate calls
  Exclude migrations from Style/MutableConstant
  ActiveSupport delegation is preferred over Forwardable
  Update haml_lint to work with newest rubocop
  Add explanations to cops
  Update rubocop and rubocop-rspec and regenerate .rubocop_todo.yml
  Update rubocop and rubocop-rspec and regenerate .rubocop_todo.yml
  Order cops alphabetically
  Don’t exclude some file in lib from rubocop
  Fix new offenses
  Enable Rails/Delegate
  Enable Style/WordArray
  ...

47 files changed, 181 insertions, 190 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 1349587fb0a..99759ec4cae 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -5,7 +5,7 @@ class ApplicationSetting < ActiveRecord::Base
   add_authentication_token_field :runners_registration_token
   add_authentication_token_field :health_check_access_token
 
-  CACHE_KEY = 'application_setting.last'
+  CACHE_KEY = 'application_setting.last'.freeze
   DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s*     # comma or semicolon, optionally surrounded by whitespace
                             |               # or
                             \s              # any whitespace character
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index bfba6425bfe..77aba91f65c 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -22,8 +22,10 @@ module Ci
     serialize :options
     serialize :yaml_variables, Gitlab::Serializer::Ci::Variables
 
+    delegate :name, to: :project, prefix: true
+
     validates :coverage, numericality: true, allow_blank: true
-    validates_presence_of :ref
+    validates :ref, presence: true
 
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
@@ -233,10 +235,6 @@ module Ci
       gl_project_id
     end
 
-    def project_name
-      project.name
-    end
-
     def repo_url
       auth = "gitlab-ci-token:#{ensure_token!}@"
       project.http_url_to_repo.sub(/^https?:\/\//) do |prefix|
@@ -257,7 +255,7 @@ module Ci
       return unless regex
 
       matches = text.scan(Regexp.new(regex)).last
-      matches = matches.last if matches.kind_of?(Array)
+      matches = matches.last if matches.is_a?(Array)
       coverage = matches.gsub(/\d+(\.\d+)?/).first
 
       if coverage.present?
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index dc4590a9923..80e11a5b58f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -14,9 +14,11 @@ module Ci
     has_many :builds, foreign_key: :commit_id
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
 
-    validates_presence_of :sha, unless: :importing?
-    validates_presence_of :ref, unless: :importing?
-    validates_presence_of :status, unless: :importing?
+    delegate :id, to: :project, prefix: true
+
+    validates :sha, presence: { unless: :importing? }
+    validates :ref, presence: { unless: :importing? }
+    validates :status, presence: { unless: :importing? }
     validate :valid_commit_sha, unless: :importing?
 
     after_create :keep_around_commits, unless: :importing?
@@ -93,8 +95,11 @@ module Ci
         .select("max(#{quoted_table_name}.id)")
         .group(:ref, :sha)
 
-      relation = ref ? where(ref: ref) : self
-      relation.where(id: max_id)
+      if ref
+        where(ref: ref, id: max_id.where(ref: ref))
+      else
+        where(id: max_id)
+      end
     end
 
     def self.latest_status(ref = nil)
@@ -150,10 +155,6 @@ module Ci
       builds.latest.with_artifacts_not_expired.includes(project: [:namespace])
     end
 
-    def project_id
-      project.id
-    end
-
     # For now the only user who participates is the user who triggered
     def participants(_current_user = nil)
       Array(user)
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 07a086b0aca..4863c34a6a6 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -4,8 +4,8 @@ module Ci
 
     RUNNER_QUEUE_EXPIRY_TIME = 60.minutes
     LAST_CONTACT_TIME = 1.hour.ago
-    AVAILABLE_SCOPES = %w[specific shared active paused online]
-    FORM_EDITABLE = %i[description tag_list active run_untagged locked]
+    AVAILABLE_SCOPES = %w[specific shared active paused online].freeze
+    FORM_EDITABLE = %i[description tag_list active run_untagged locked].freeze
 
     has_many :builds
     has_many :runner_projects, dependent: :destroy
diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb
index 1f9baeca5b1..234376a7e4c 100644
--- a/app/models/ci/runner_project.rb
+++ b/app/models/ci/runner_project.rb
@@ -5,6 +5,6 @@ module Ci
     belongs_to :runner
     belongs_to :project, foreign_key: :gl_project_id
 
-    validates_uniqueness_of :runner_id, scope: :gl_project_id
+    validates :runner_id, uniqueness: { scope: :gl_project_id }
   end
 end
diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb
index 62889fe80d8..39a1dd86241 100644
--- a/app/models/ci/trigger.rb
+++ b/app/models/ci/trigger.rb
@@ -7,8 +7,8 @@ module Ci
     belongs_to :project, foreign_key: :gl_project_id
     has_many :trigger_requests, dependent: :destroy
 
-    validates_presence_of :token
-    validates_uniqueness_of :token
+    validates :token, presence: true
+    validates :token, uniqueness: true
 
     before_validation :set_default_values
 
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 46f06733da1..0a18986ef26 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -22,12 +22,12 @@ class Commit
   DIFF_HARD_LIMIT_LINES = 50000
 
   # The SHA can be between 7 and 40 hex characters.
-  COMMIT_SHA_PATTERN = '\h{7,40}'
+  COMMIT_SHA_PATTERN = '\h{7,40}'.freeze
 
   class << self
     def decorate(commits, project)
       commits.map do |commit|
-        if commit.kind_of?(Commit)
+        if commit.is_a?(Commit)
           commit
         else
           self.new(commit, project)
@@ -105,7 +105,7 @@ class Commit
   end
 
   def diff_line_count
-    @diff_line_count ||= Commit::diff_line_count(raw_diffs)
+    @diff_line_count ||= Commit.diff_line_count(raw_diffs)
     @diff_line_count
   end
 
@@ -122,11 +122,12 @@ class Commit
   def full_title
     return @full_title if @full_title
 
-    if safe_message.blank?
-      @full_title = no_commit_message
-    else
-      @full_title = safe_message.split("\n", 2).first
-    end
+    @full_title =
+      if safe_message.blank?
+        no_commit_message
+      else
+        safe_message.split("\n", 2).first
+      end
   end
 
   # Returns the commits description
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 99a6326309d..fc750a3e5e9 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -10,10 +10,11 @@ class CommitStatus < ActiveRecord::Base
   belongs_to :user
 
   delegate :commit, to: :pipeline
+  delegate :sha, :short_sha, to: :pipeline
 
   validates :pipeline, presence: true, unless: :importing?
 
-  validates_presence_of :name
+  validates :name, presence: true
 
   alias_attribute :author, :user
 
@@ -102,8 +103,6 @@ class CommitStatus < ActiveRecord::Base
     end
   end
 
-  delegate :sha, :short_sha, to: :pipeline
-
   def before_sha
     pipeline.before_sha || Gitlab::Git::BLANK_SHA
   end
diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index a600f9c14c5..8ea95beed79 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -11,14 +11,15 @@ module CacheMarkdownField
   # Knows about the relationship between markdown and html field names, and
   # stores the rendering contexts for the latter
   class FieldData
-    extend Forwardable
-
     def initialize
       @data = {}
     end
 
-    def_delegators :@data, :[], :[]=
-    def_delegator :@data, :keys, :markdown_fields
+    delegate :[], :[]=, to: :@data
+
+    def markdown_fields
+      @data.keys
+    end
 
     def html_field(markdown_field)
       "#{markdown_field}_html"
@@ -45,7 +46,7 @@ module CacheMarkdownField
     Project
     Release
     Snippet
-  ]
+  ].freeze
 
   def self.caching_classes
     CACHING_CLASSES.map(&:constantize)
diff --git a/app/models/concerns/case_sensitivity.rb b/app/models/concerns/case_sensitivity.rb
index fe0cea8465f..034e9f40ff0 100644
--- a/app/models/concerns/case_sensitivity.rb
+++ b/app/models/concerns/case_sensitivity.rb
@@ -13,11 +13,12 @@ module CaseSensitivity
       params.each do |key, value|
         column = ActiveRecord::Base.connection.quote_table_name(key)
 
-        if cast_lower
-          condition = "LOWER(#{column}) = LOWER(:value)"
-        else
-          condition = "#{column} = :value"
-        end
+        condition =
+          if cast_lower
+            "LOWER(#{column}) = LOWER(:value)"
+          else
+            "#{column} = :value"
+          end
 
         criteria = criteria.where(condition, value: value)
       end
diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 431c0354969..aea359e70bb 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -1,12 +1,12 @@
 module HasStatus
   extend ActiveSupport::Concern
 
-  DEFAULT_STATUS = 'created'
-  AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped]
-  STARTED_STATUSES = %w[running success failed skipped]
-  ACTIVE_STATUSES = %w[pending running]
-  COMPLETED_STATUSES = %w[success failed canceled skipped]
-  ORDERED_STATUSES = %w[failed pending running canceled success skipped]
+  DEFAULT_STATUS = 'created'.freeze
+  AVAILABLE_STATUSES = %w[created pending running success failed canceled skipped].freeze
+  STARTED_STATUSES = %w[running success failed skipped].freeze
+  ACTIVE_STATUSES = %w[pending running].freeze
+  COMPLETED_STATUSES = %w[success failed canceled skipped].freeze
+  ORDERED_STATUSES = %w[failed pending running canceled success skipped].freeze
 
   class_methods do
     def status_sql
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 5f53c48fc88..37c727b5d9f 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -16,9 +16,9 @@ module Issuable
   include TimeTrackable
 
   # This object is used to gather issuable meta data for displaying
-  # upvotes, downvotes and notes count for issues and merge requests
+  # upvotes, downvotes, notes and closing merge requests count for issues and merge requests
   # lists avoiding n+1 queries and improving performance.
-  IssuableMeta = Struct.new(:upvotes, :downvotes, :notes_count)
+  IssuableMeta = Struct.new(:upvotes, :downvotes, :notes_count, :merge_requests_count)
 
   included do
     cache_markdown_field :title, pipeline: :single_line
@@ -46,6 +46,17 @@ module Issuable
 
     has_one :metrics
 
+    delegate :name,
+             :email,
+             to: :author,
+             prefix: true
+
+    delegate :name,
+             :email,
+             to: :assignee,
+             allow_nil: true,
+             prefix: true
+
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
 
@@ -68,21 +79,10 @@ module Issuable
 
     scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
     scope :join_project, -> { joins(:project) }
-    scope :inc_notes_with_associations, -> { includes(notes: [ :project, :author, :award_emoji ]) }
+    scope :inc_notes_with_associations, -> { includes(notes: [:project, :author, :award_emoji]) }
     scope :references_project, -> { references(:project) }
     scope :non_archived, -> { join_project.where(projects: { archived: false }) }
 
-    delegate :name,
-             :email,
-             to: :author,
-             prefix: true
-
-    delegate :name,
-             :email,
-             to: :assignee,
-             allow_nil: true,
-             prefix: true
-
     attr_mentionable :title, pipeline: :single_line
     attr_mentionable :description
 
@@ -182,7 +182,7 @@ module Issuable
     def grouping_columns(sort)
       grouping_columns = [arel_table[:id]]
 
-      if ["milestone_due_desc", "milestone_due_asc"].include?(sort)
+      if %w(milestone_due_desc milestone_due_asc).include?(sort)
         milestone_table = Milestone.arel_table
         grouping_columns << milestone_table[:id]
         grouping_columns << milestone_table[:due_date]
@@ -235,7 +235,7 @@ module Issuable
       # DEPRECATED
       repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
     }
-    hook_data.merge!(assignee: assignee.hook_attrs) if assignee
+    hook_data[:assignee] = assignee.hook_attrs if assignee
 
     hook_data
   end
diff --git a/app/models/concerns/reactive_service.rb b/app/models/concerns/reactive_service.rb
index e1f868a299b..713246039c1 100644
--- a/app/models/concerns/reactive_service.rb
+++ b/app/models/concerns/reactive_service.rb
@@ -5,6 +5,6 @@ module ReactiveService
     include ReactiveCaching
 
     # Default cache key: class name + project_id
-    self.reactive_cache_key = ->(service) { [ service.class.model_name.singular, service.project_id ] }
+    self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] }
   end
 end
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
index 7edb0acd56c..b9a2d812edd 100644
--- a/app/models/concerns/sortable.rb
+++ b/app/models/concerns/sortable.rb
@@ -46,11 +46,12 @@ module Sortable
         where("label_links.target_id = #{target_column}").
         reorder(nil)
 
-      if target_type_column
-        query = query.where("label_links.target_type = #{target_type_column}")
-      else
-        query = query.where(label_links: { target_type: target_type })
-      end
+      query =
+        if target_type_column
+          query.where("label_links.target_type = #{target_type_column}")
+        else
+          query.where(label_links: { target_type: target_type })
+        end
 
       query = query.where.not(title: excluded_labels) if excluded_labels.present?
 
diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb
index 79adc77c9e4..107e6764ba2 100644
--- a/app/models/concerns/spammable.rb
+++ b/app/models/concerns/spammable.rb
@@ -13,7 +13,7 @@ module Spammable
     attr_accessor :spam
     attr_accessor :spam_log
 
-    after_validation :check_for_spam, on: :create
+    after_validation :check_for_spam, on: [:create, :update]
 
     cattr_accessor :spammable_attrs, instance_accessor: false do
       []
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 559b3075905..895a91139c9 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -8,7 +8,7 @@ class DiffNote < Note
   validates :position, presence: true
   validates :diff_line, presence: true
   validates :line_code, presence: true, line_code: true
-  validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] }
+  validates :noteable_type, inclusion: { in: %w(Commit MergeRequest) }
   validates :resolved_by, presence: true, if: :resolved?
   validate :positions_complete
   validate :verify_supported
diff --git a/app/models/event.rb b/app/models/event.rb
index e5027df3f8a..4b8eac9accf 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -47,7 +47,7 @@ class Event < ActiveRecord::Base
     def contributions
       where("action = ? OR (target_type IN (?) AND action IN (?)) OR (target_type = ? AND action = ?)",
             Event::PUSHED,
-            ["MergeRequest", "Issue"], [Event::CREATED, Event::CLOSED, Event::MERGED],
+            %w(MergeRequest Issue), [Event::CREATED, Event::CLOSED, Event::MERGED],
             "Note", Event::COMMENTED)
     end
 
diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index 26712c19b5a..b973bbcd8da 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -43,7 +43,7 @@ class ExternalIssue
   end
 
   def reference_link_text(from_project = nil)
-    return "##{id}" if /^\d+$/.match(id)
+    return "##{id}" if id =~ /^\d+$/
 
     id
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d8826b65fcc..de90f19f854 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -15,8 +15,6 @@ class Issue < ActiveRecord::Base
   DueThisWeek   = DueDateStruct.new('Due This Week', 'week').freeze
   DueThisMonth  = DueDateStruct.new('Due This Month', 'month').freeze
 
-  ActsAsTaggableOn.strict_case_match = true
-
   belongs_to :project
   belongs_to :moved_to, class_name: 'Issue'
 
diff --git a/app/models/label.rb b/app/models/label.rb
index 5b6b9a7a736..f68a8c9cff2 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -11,7 +11,7 @@ class Label < ActiveRecord::Base
 
   cache_markdown_field :description, pipeline: :single_line
 
-  DEFAULT_COLOR = '#428BCA'
+  DEFAULT_COLOR = '#428BCA'.freeze
 
   default_value_for :color, DEFAULT_COLOR
 
diff --git a/app/models/member.rb b/app/models/member.rb
index d07f270b757..0545bd4eedf 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -10,6 +10,8 @@ class Member < ActiveRecord::Base
   belongs_to :user
   belongs_to :source, polymorphic: true
 
+  delegate :name, :username, :email, to: :user, prefix: true
+
   validates :user, presence: true, unless: :invite?
   validates :source, presence: true
   validates :user_id, uniqueness: { scope: [:source_type, :source_id],
@@ -73,8 +75,6 @@ class Member < ActiveRecord::Base
   after_destroy :post_destroy_hook, unless: :pending?
   after_commit :refresh_member_authorized_projects
 
-  delegate :name, :username, :email, to: :user, prefix: true
-
   default_value_for :notification_level, NotificationSetting.levels[:global]
 
   class << self
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 204f34f0269..446f9f8f8a7 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -1,11 +1,11 @@
 class GroupMember < Member
-  SOURCE_TYPE = 'Namespace'
+  SOURCE_TYPE = 'Namespace'.freeze
 
   belongs_to :group, foreign_key: 'source_id'
 
   # Make sure group member points only to group as it source
   default_value_for :source_type, SOURCE_TYPE
-  validates_format_of :source_type, with: /\ANamespace\z/
+  validates :source_type, format: { with: /\ANamespace\z/ }
   default_scope { where(source_type: SOURCE_TYPE) }
 
   def self.access_level_roles
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 008fff0857c..912820b51ac 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -1,5 +1,5 @@
 class ProjectMember < Member
-  SOURCE_TYPE = 'Project'
+  SOURCE_TYPE = 'Project'.freeze
 
   include Gitlab::ShellAdapter
 
@@ -7,7 +7,7 @@ class ProjectMember < Member
 
   # Make sure project member points only to project as it source
   default_value_for :source_type, SOURCE_TYPE
-  validates_format_of :source_type, with: /\AProject\z/
+  validates :source_type, format: { with: /\AProject\z/ }
   validates :access_level, inclusion: { in: Gitlab::Access.values }
   default_scope { where(source_type: SOURCE_TYPE) }
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 204d2b153ad..7eb875f1ef5 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -203,7 +203,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def diff_size
-    opts = diff_options || {}
+    # The `#diffs` method ends up at an instance of a class inheriting from
+    # `Gitlab::Diff::FileCollection::Base`, so use those options as defaults
+    # here too, to get the same diff size without performing highlighting.
+    #
+    opts = Gitlab::Diff::FileCollection::Base.default_options.merge(diff_options || {})
 
     raw_diffs(opts).size
   end
@@ -527,7 +531,7 @@ class MergeRequest < ActiveRecord::Base
     }
 
     if diff_head_commit
-      attrs.merge!(last_commit: diff_head_commit.hook_attrs)
+      attrs[:last_commit] = diff_head_commit.hook_attrs
     end
 
     attributes.merge!(attrs)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 70bad2a4396..baee00b8fcd 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -7,7 +7,7 @@ class MergeRequestDiff < ActiveRecord::Base
   COMMITS_SAFE_SIZE = 100
 
   # Valid types of serialized diffs allowed by Gitlab::Git::Diff
-  VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta]
+  VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta].freeze
 
   belongs_to :merge_request
 
diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb
index ab597c37947..daafb137be4 100644
--- a/app/models/merge_requests_closing_issues.rb
+++ b/app/models/merge_requests_closing_issues.rb
@@ -4,4 +4,12 @@ class MergeRequestsClosingIssues < ActiveRecord::Base
 
   validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true
   validates :issue_id, presence: true
+
+  class << self
+    def count_for_collection(ids)
+      group(:issue_id).
+        where(issue_id: ids).
+        pluck('issue_id', 'COUNT(*) as count')
+    end
+  end
 end
diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb
index b524ca50ee8..0bbc9451ffd 100644
--- a/app/models/network/graph.rb
+++ b/app/models/network/graph.rb
@@ -188,11 +188,12 @@ module Network
       end
 
       # and mark it as reserved
-      if parent_time.nil?
-        min_time = leaves.first.time
-      else
-        min_time = parent_time + 1
-      end
+      min_time =
+        if parent_time.nil?
+          leaves.first.time
+        else
+          parent_time + 1
+        end
 
       max_time = leaves.last.time
       leaves.last.parents(@map).each do |parent|
diff --git a/app/models/note.rb b/app/models/note.rb
index 029fe667a45..d6d5396afa5 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -72,7 +72,7 @@ class Note < ActiveRecord::Base
   scope :inc_author, ->{ includes(:author) }
   scope :inc_relations_for_view, ->{ includes(:project, :author, :updated_by, :resolved_by, :award_emoji) }
 
-  scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) }
+  scope :diff_notes, ->{ where(type: %w(LegacyDiffNote DiffNote)) }
   scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
 
   scope :with_associations, -> do
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 58f6214bea7..52577bd52ea 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -35,11 +35,11 @@ class NotificationSetting < ActiveRecord::Base
     :merge_merge_request,
     :failed_pipeline,
     :success_pipeline
-  ]
+  ].freeze
 
   EXCLUDED_WATCHER_EVENTS = [
     :success_pipeline
-  ]
+  ].freeze
 
   store :events, accessors: EMAIL_EVENTS, coder: JSON
 
diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index 0b9ebf1ffe2..f2f2fc1e32a 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -2,7 +2,7 @@ class PagesDomain < ActiveRecord::Base
   belongs_to :project
 
   validates :domain, hostname: true
-  validates_uniqueness_of :domain, case_sensitive: false
+  validates :domain, uniqueness: { case_sensitive: false }
   validates :certificate, certificate: true, allow_nil: true, allow_blank: true
   validates :key, certificate_key: true, allow_nil: true, allow_blank: true
 
diff --git a/app/models/project.rb b/app/models/project.rb
index fc5b1a66910..814fd0c0f4f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -22,7 +22,7 @@ class Project < ActiveRecord::Base
   class BoardLimitExceeded < StandardError; end
 
   NUMBER_OF_PERMITTED_BOARDS = 1
-  UNKNOWN_IMPORT_URL = 'http://unknown.git'
+  UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze
 
   cache_markdown_field :description, pipeline: :description
 
@@ -70,8 +70,7 @@ class Project < ActiveRecord::Base
 
   after_validation :check_pending_delete
 
-  ActsAsTaggableOn.strict_case_match = true
-  acts_as_taggable_on :tags
+  acts_as_taggable
 
   attr_accessor :new_default_branch
   attr_accessor :old_path_with_namespace
@@ -172,9 +171,11 @@ class Project < ActiveRecord::Base
   accepts_nested_attributes_for :project_feature
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
+  delegate :count, to: :forks, prefix: true
   delegate :members, to: :team, prefix: true
   delegate :add_user, to: :team
   delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
+  delegate :empty_repo?, to: :repository
 
   # Validations
   validates :creator, presence: true, on: :create
@@ -191,8 +192,8 @@ class Project < ActiveRecord::Base
     format: { with: Gitlab::Regex.project_path_regex,
               message: Gitlab::Regex.project_path_regex_message }
   validates :namespace, presence: true
-  validates_uniqueness_of :name, scope: :namespace_id
-  validates_uniqueness_of :path, scope: :namespace_id
+  validates :name, uniqueness: { scope: :namespace_id }
+  validates :path, uniqueness: { scope: :namespace_id }
   validates :import_url, addressable_url: true, if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
@@ -453,13 +454,14 @@ class Project < ActiveRecord::Base
   end
 
   def add_import_job
-    if forked?
-      job_id = RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path,
-                                                  forked_from_project.path_with_namespace,
-                                                  self.namespace.full_path)
-    else
-      job_id = RepositoryImportWorker.perform_async(self.id)
-    end
+    job_id =
+      if forked?
+        RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path,
+          forked_from_project.path_with_namespace,
+          self.namespace.full_path)
+      else
+        RepositoryImportWorker.perform_async(self.id)
+      end
 
     if job_id
       Rails.logger.info "Import job started for #{path_with_namespace} with job ID #{job_id}"
@@ -552,7 +554,7 @@ class Project < ActiveRecord::Base
   end
 
   def check_limit
-    unless creator.can_create_project? or namespace.kind == 'group'
+    unless creator.can_create_project? || namespace.kind == 'group'
       projects_limit = creator.projects_limit
 
       if projects_limit == 0
@@ -837,10 +839,6 @@ class Project < ActiveRecord::Base
     false
   end
 
-  def empty_repo?
-    repository.empty_repo?
-  end
-
   def repo
     repository.raw
   end
@@ -1028,10 +1026,6 @@ class Project < ActiveRecord::Base
     forked? && project == forked_from_project
   end
 
-  def forks_count
-    forks.count
-  end
-
   def origin_merge_requests
     merge_requests.where(source_project_id: self.id)
   end
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 03194fc2141..e3ef4919b28 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -18,7 +18,7 @@ class ProjectFeature < ActiveRecord::Base
   PRIVATE  = 10
   ENABLED  = 20
 
-  FEATURES = %i(issues merge_requests wiki snippets builds repository)
+  FEATURES = %i(issues merge_requests wiki snippets builds repository).freeze
 
   class << self
     def access_level_attribute(feature)
diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index 0956c4a4ede..5fb95050b83 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -3,7 +3,7 @@ require "addressable/uri"
 class BuildkiteService < CiService
   include ReactiveService
 
-  ENDPOINT = "https://buildkite.com"
+  ENDPOINT = "https://buildkite.com".freeze
 
   prop_accessor :project_url, :token
   boolean_accessor :enable_ssl_verification
diff --git a/app/models/project_services/chat_message/issue_message.rb b/app/models/project_services/chat_message/issue_message.rb
index b96aca47e65..791e5b0cec7 100644
--- a/app/models/project_services/chat_message/issue_message.rb
+++ b/app/models/project_services/chat_message/issue_message.rb
@@ -51,7 +51,8 @@ module ChatMessage
         title: issue_title,
         title_link: issue_url,
         text: format(description),
-        color: "#C95823" }]
+        color: "#C95823"
+      }]
     end
 
     def project_link
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 942ec9371e5..2717c240f05 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -39,7 +39,7 @@ class DroneCiService < CiService
   def commit_status_path(sha, ref)
     url = [drone_url,
            "gitlab/#{project.full_path}/commits/#{sha}",
-           "?branch=#{URI::encode(ref.to_s)}&access_token=#{token}"]
+           "?branch=#{URI.encode(ref.to_s)}&access_token=#{token}"]
 
     URI.join(*url).to_s
   end
@@ -52,7 +52,7 @@ class DroneCiService < CiService
     response = HTTParty.get(commit_status_path(sha, ref), verify: enable_ssl_verification)
 
     status =
-      if response.code == 200 and response['status']
+      if response.code == 200 && response['status']
         case response['status']
         when 'killed'
           :canceled
@@ -74,7 +74,7 @@ class DroneCiService < CiService
   def build_page(sha, ref)
     url = [drone_url,
            "gitlab/#{project.full_path}/redirect/commits/#{sha}",
-           "?branch=#{URI::encode(ref.to_s)}"]
+           "?branch=#{URI.encode(ref.to_s)}"]
 
     URI.join(*url).to_s
   end
@@ -114,7 +114,7 @@ class DroneCiService < CiService
   end
 
   def merge_request_valid?(data)
-    ['opened', 'reopened'].include?(data[:object_attributes][:state]) &&
+    %w(opened reopened).include?(data[:object_attributes][:state]) &&
       data[:object_attributes][:merge_status] == 'unchecked'
   end
 end
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb
index 72da219df28..c4142c38b2f 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -6,7 +6,7 @@ class HipchatService < Service
     a b i strong em br img pre code
     table th tr td caption colgroup col thead tbody tfoot
     ul ol li dl dt dd
-  ]
+  ].freeze
 
   prop_accessor :token, :room, :server, :color, :api_version
   boolean_accessor :notify_only_broken_builds, :notify
@@ -36,7 +36,7 @@ class HipchatService < Service
       { type: 'text', name: 'token',     placeholder: 'Room token' },
       { type: 'text', name: 'room',      placeholder: 'Room name or ID' },
       { type: 'checkbox', name: 'notify' },
-      { type: 'select', name: 'color', choices: ['yellow', 'red', 'green', 'purple', 'gray', 'random'] },
+      { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) },
       { type: 'text', name: 'api_version',
         placeholder: 'Leave blank for default (v2)' },
       { type: 'text', name: 'server',
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index 5d93064f9b3..c62bb4fa120 100644
--- a/app/models/project_services/irker_service.rb
+++ b/app/models/project_services/irker_service.rb
@@ -33,7 +33,8 @@ class IrkerService < Service
   end
 
   def settings
-    { server_host: server_host.present? ? server_host : 'localhost',
+    {
+      server_host: server_host.present? ? server_host : 'localhost',
       server_port: server_port.present? ? server_port : 6659
     }
   end
@@ -96,7 +97,7 @@ class IrkerService < Service
     rescue URI::InvalidURIError
     end
 
-    unless uri.present? and default_irc_uri.nil?
+    unless uri.present? && default_irc_uri.nil?
       begin
         new_recipient = URI.join(default_irc_uri, '/', recipient).to_s
         uri = consider_uri(URI.parse(new_recipient))
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index f2f019c43bb..9819e723fe8 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -3,7 +3,7 @@ class KubernetesService < DeploymentService
   include Gitlab::Kubernetes
   include ReactiveCaching
 
-  self.reactive_cache_key = ->(service) { [ service.class.model_name.singular, service.project_id ] }
+  self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] }
 
   # Namespace defaults to the project path, but can be overridden in case that
   # is an invalid or inappropriate name
@@ -62,23 +62,19 @@ class KubernetesService < DeploymentService
         { type: 'text',
           name: 'namespace',
           title: 'Kubernetes namespace',
-          placeholder: 'Kubernetes namespace',
-        },
+          placeholder: 'Kubernetes namespace' },
         { type: 'text',
           name: 'api_url',
           title: 'API URL',
-          placeholder: 'Kubernetes API URL, like https://kube.example.com/',
-        },
+          placeholder: 'Kubernetes API URL, like https://kube.example.com/' },
         { type: 'text',
           name: 'token',
           title: 'Service token',
-          placeholder: 'Service token',
-        },
+          placeholder: 'Service token' },
         { type: 'textarea',
           name: 'ca_pem',
           title: 'Custom CA bundle',
-          placeholder: 'Certificate Authority bundle (PEM format)',
-        },
+          placeholder: 'Certificate Authority bundle (PEM format)' },
     ]
   end
 
@@ -167,7 +163,7 @@ class KubernetesService < DeploymentService
     url = URI.parse(api_url)
     prefix = url.path.sub(%r{/+\z}, '')
 
-    url.path = [ prefix, *parts ].join("/")
+    url.path = [prefix, *parts].join("/")
 
     url.to_s
   end
diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb
index 9cc642591f4..d86f4f6f448 100644
--- a/app/models/project_services/pivotaltracker_service.rb
+++ b/app/models/project_services/pivotaltracker_service.rb
@@ -1,7 +1,7 @@
 class PivotaltrackerService < Service
   include HTTParty
 
-  API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'
+  API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits'.freeze
 
   prop_accessor :token, :restrict_to_branch
   validates :token, presence: true, if: :activated?
diff --git a/app/models/project_services/pushover_service.rb b/app/models/project_services/pushover_service.rb
index a963d27a376..3e618a8dbf1 100644
--- a/app/models/project_services/pushover_service.rb
+++ b/app/models/project_services/pushover_service.rb
@@ -29,25 +29,24 @@ class PushoverService < Service
           ['Normal Priority', 0],
           ['High Priority', 1]
         ],
-        default_choice: 0
-      },
+        default_choice: 0 },
       { type: 'select', name: 'sound', choices:
         [
           ['Device default sound', nil],
           ['Pushover (default)', 'pushover'],
-          ['Bike', 'bike'],
-          ['Bugle', 'bugle'],
+          %w(Bike bike),
+          %w(Bugle bugle),
           ['Cash Register', 'cashregister'],
-          ['Classical', 'classical'],
-          ['Cosmic', 'cosmic'],
-          ['Falling', 'falling'],
-          ['Gamelan', 'gamelan'],
-          ['Incoming', 'incoming'],
-          ['Intermission', 'intermission'],
-          ['Magic', 'magic'],
-          ['Mechanical', 'mechanical'],
+          %w(Classical classical),
+          %w(Cosmic cosmic),
+          %w(Falling falling),
+          %w(Gamelan gamelan),
+          %w(Incoming incoming),
+          %w(Intermission intermission),
+          %w(Magic magic),
+          %w(Mechanical mechanical),
           ['Piano Bar', 'pianobar'],
-          ['Siren', 'siren'],
+          %w(Siren siren),
           ['Space Alarm', 'spacealarm'],
           ['Tug Boat', 'tugboat'],
           ['Alien Alarm (long)', 'alien'],
@@ -56,8 +55,7 @@ class PushoverService < Service
           ['Pushover Echo (long)', 'echo'],
           ['Up Down (long)', 'updown'],
           ['None (silent)', 'none']
-        ]
-      },
+        ] },
     ]
   end
 
@@ -72,13 +70,14 @@ class PushoverService < Service
     before = data[:before]
     after = data[:after]
 
-    if Gitlab::Git.blank_ref?(before)
-      message = "#{data[:user_name]} pushed new branch \"#{ref}\"."
-    elsif Gitlab::Git.blank_ref?(after)
-      message = "#{data[:user_name]} deleted branch \"#{ref}\"."
-    else
-      message = "#{data[:user_name]} push to branch \"#{ref}\"."
-    end
+    message =
+      if Gitlab::Git.blank_ref?(before)
+        "#{data[:user_name]} pushed new branch \"#{ref}\"."
+      elsif Gitlab::Git.blank_ref?(after)
+        "#{data[:user_name]} deleted branch \"#{ref}\"."
+      else
+        "#{data[:user_name]} push to branch \"#{ref}\"."
+      end
 
     if data[:total_commits_count] > 0
       message << "\nTotal commits count: #{data[:total_commits_count]}"
@@ -97,7 +96,7 @@ class PushoverService < Service
 
     # Sound parameter MUST NOT be sent to API if not selected
     if sound
-      pushover_data.merge!(sound: sound)
+      pushover_data[:sound] = sound
     end
 
     PushoverService.post('/messages.json', body: pushover_data)
diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb
index 9bb456eee24..25b5d777641 100644
--- a/app/models/project_snippet.rb
+++ b/app/models/project_snippet.rb
@@ -9,8 +9,4 @@ class ProjectSnippet < Snippet
 
   participant :author
   participant :notes_with_associations
-
-  def check_for_spam?
-    super && project.public?
-  end
 end
diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb
index 06abd406523..aeaf63abab9 100644
--- a/app/models/project_statistics.rb
+++ b/app/models/project_statistics.rb
@@ -4,7 +4,7 @@ class ProjectStatistics < ActiveRecord::Base
 
   before_save :update_storage_size
 
-  STORAGE_COLUMNS = [:repository_size, :lfs_objects_size, :build_artifacts_size]
+  STORAGE_COLUMNS = [:repository_size, :lfs_objects_size, :build_artifacts_size].freeze
   STATISTICS_COLUMNS = [:commit_count] + STORAGE_COLUMNS
 
   def total_repository_size
diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index d0b991db112..9891f5edf41 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -5,7 +5,7 @@ class ProjectWiki
     'Markdown' => :markdown,
     'RDoc'     => :rdoc,
     'AsciiDoc' => :asciidoc
-  } unless defined?(MARKUPS)
+  }.freeze unless defined?(MARKUPS)
 
   class CouldNotCreateWikiError < StandardError; end
 
@@ -19,6 +19,9 @@ class ProjectWiki
     @user = user
   end
 
+  delegate :empty?, to: :pages
+  delegate :repository_storage_path, to: :project
+
   def path
     @project.path + '.wiki'
   end
@@ -60,10 +63,6 @@ class ProjectWiki
     !!repository.exists?
   end
 
-  def empty?
-    pages.empty?
-  end
-
   # Returns an Array of Gitlab WikiPage instances or an
   # empty Array if this Wiki has no pages.
   def pages
@@ -160,10 +159,6 @@ class ProjectWiki
     }
   end
 
-  def repository_storage_path
-    project.repository_storage_path
-  end
-
   private
 
   def init_repo(path_with_namespace)
diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index 6240912a6e1..39e979ef15b 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -8,8 +8,8 @@ class ProtectedBranch < ActiveRecord::Base
   has_many :merge_access_levels, dependent: :destroy
   has_many :push_access_levels, dependent: :destroy
 
-  validates_length_of :merge_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
-  validates_length_of :push_access_levels, is: 1, message: "are restricted to a single instance per protected branch."
+  validates :merge_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
+  validates :push_access_levels, length: { is: 1, message: "are restricted to a single instance per protected branch." }
 
   accepts_nested_attributes_for :push_access_levels
   accepts_nested_attributes_for :merge_access_levels
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 56c582cd9be..cd2568ad445 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -18,7 +18,7 @@ class Repository
   CACHED_METHODS = %i(size commit_count readme version contribution_guide
                       changelog license_blob license_key gitignore koding_yml
                       gitlab_ci_yml branch_names tag_names branch_count
-                      tag_count avatar exists? empty? root_ref)
+                      tag_count avatar exists? empty? root_ref).freeze
 
   # Certain method caches should be refreshed when certain types of files are
   # changed. This Hash maps file types (as returned by Gitlab::FileDetector) to
@@ -33,7 +33,7 @@ class Repository
     koding: :koding_yml,
     gitlab_ci: :gitlab_ci_yml,
     avatar: :avatar
-  }
+  }.freeze
 
   # Wraps around the given method and caches its output in Redis and an instance
   # variable.
@@ -487,9 +487,7 @@ class Repository
   end
   cache_method :exists?
 
-  def empty?
-    raw_repository.empty?
-  end
+  delegate :empty?, to: :raw_repository
   cache_method :empty?
 
   # The size of this repository in megabytes.
@@ -508,9 +506,7 @@ class Repository
   end
   cache_method :branch_names, fallback: []
 
-  def tag_names
-    raw_repository.tag_names
-  end
+  delegate :tag_names, to: :raw_repository
   cache_method :tag_names, fallback: []
 
   def branch_count
@@ -892,7 +888,7 @@ class Repository
 
   def get_committer_and_author(user, email: nil, name: nil)
     committer = user_to_committer(user)
-    author = Gitlab::Git::committer_hash(email: email, name: name) || committer
+    author = Gitlab::Git.committer_hash(email: email, name: name) || committer
 
     {
       author: author,
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 3dda7948d0b..47789a21133 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -17,7 +17,7 @@ class Todo < ActiveRecord::Base
     APPROVAL_REQUIRED => :approval_required,
     UNMERGEABLE => :unmergeable,
     DIRECTLY_ADDRESSED => :directly_addressed
-  }
+  }.freeze
 
   belongs_to :author, class_name: "User"
   belongs_to :note
diff --git a/app/models/user.rb b/app/models/user.rb
index f614eb66e1f..fada0e567f0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -104,7 +104,7 @@ class User < ActiveRecord::Base
   #
   # Note: devise :validatable above adds validations for :email and :password
   validates :name, presence: true
-  validates_confirmation_of :email
+  validates :email, confirmation: true
   validates :notification_email, presence: true
   validates :notification_email, email: true, if: ->(user) { user.notification_email != user.email }
   validates :public_email, presence: true, uniqueness: true, email: true, allow_blank: true
@@ -580,8 +580,8 @@ class User < ActiveRecord::Base
 
       if project.repository.branch_exists?(event.branch_name)
         merge_requests = MergeRequest.where("created_at >= ?", event.created_at).
-            where(source_project_id: project.id,
-                  source_branch: event.branch_name)
+          where(source_project_id: project.id,
+                source_branch: event.branch_name)
         merge_requests.empty?
       end
     end