Merge branch 'rubycop-improvements' into 'master'

Rubycop improvements

See merge request !1463

.rubocop.yml

 Style/AccessModifierIndentation:
   Description: Check indentation of private/protected visibility modifiers.
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#indent-public-private-protected'
-  Enabled: false
+  Enabled: true
 
 Style/AccessorMethodName:
   Description: Check the naming of accessor methods for get_/set_.
@@ -10,20 +10,20 @@ Style/AccessorMethodName:
 Style/Alias:
   Description: 'Use alias_method instead of alias.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#alias-method'
-  Enabled: false
+  Enabled: true
 
 Style/AlignArray:
   Description: >-
                  Align the elements of an array literal if they span more than
                  one line.
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#align-multiline-arrays'
-  Enabled: false
+  Enabled: true
 
 Style/AlignHash:
   Description: >-
                  Align the elements of a hash literal if they span more than
                  one line.
-  Enabled: false
+  Enabled: true
 
 Style/AlignParameters:
   Description: >-
@@ -45,12 +45,12 @@ Style/ArrayJoin:
 Style/AsciiComments:
   Description: 'Use only ascii symbols in comments.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#english-comments'
-  Enabled: false
+  Enabled: true
 
 Style/AsciiIdentifiers:
   Description: 'Use only ascii symbols in identifiers.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#english-identifiers'
-  Enabled: false
+  Enabled: true
 
 Style/Attr:
   Description: 'Checks for uses of Module#attr.'
@@ -60,7 +60,7 @@ Style/Attr:
 Style/BeginBlock:
   Description: 'Avoid the use of BEGIN blocks.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-BEGIN-blocks'
-  Enabled: false
+  Enabled: true
 
 Style/BarePercentLiterals:
   Description: 'Checks if usage of %() or %Q() matches configuration.'
@@ -74,7 +74,7 @@ Style/BlockComments:
 
 Style/BlockEndNewline:
   Description: 'Put end statement of multiline block on its own line.'
-  Enabled: false
+  Enabled: true
 
 Style/Blocks:
   Description: >-
@@ -82,7 +82,7 @@ Style/Blocks:
                 always ugly).
                 Prefer {...} over do...end for single-line blocks.
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#single-line-blocks'
-  Enabled: false
+  Enabled: true
 
 Style/BracesAroundHashParameters:
   Description: 'Enforce braces style around hash parameters.'
@@ -96,17 +96,17 @@ Style/CaseEquality:
 Style/CaseIndentation:
   Description: 'Indentation of when in a case/when/[else/]end.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#indent-when-to-case'
-  Enabled: false
+  Enabled: true
 
 Style/CharacterLiteral:
   Description: 'Checks for uses of character literals.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-character-literals'
-  Enabled: false
+  Enabled: true
 
 Style/ClassAndModuleCamelCase:
   Description: 'Use CamelCase for classes and modules.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#camelcase-classes'
-  Enabled: false
+  Enabled: true
 
 Style/ClassAndModuleChildren:
   Description: 'Checks style of children classes and modules.'
@@ -124,7 +124,7 @@ Style/ClassMethods:
 Style/ClassVars:
   Description: 'Avoid the use of class variables.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-class-vars'
-  Enabled: false
+  Enabled: true
 
 Style/ColonMethodCall:
   Description: 'Do not use :: for method call.'
@@ -140,12 +140,12 @@ Style/CommentAnnotation:
 
 Style/CommentIndentation:
   Description: 'Indentation of comments.'
-  Enabled: false
+  Enabled: true
 
 Style/ConstantName:
   Description: 'Constants should use SCREAMING_SNAKE_CASE.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#screaming-snake-case'
-  Enabled: false
+  Enabled: true
 
 Style/DefWithParentheses:
   Description: 'Use def with parentheses when there are arguments.'
@@ -177,7 +177,7 @@ Style/EachWithObject:
 
 Style/ElseAlignment:
   Description: 'Align elses and elsifs correctly.'
-  Enabled: false
+  Enabled: true
 
 Style/EmptyElse:
   Description: 'Avoid empty else-clauses.'
@@ -283,12 +283,12 @@ Style/IfWithSemicolon:
 
 Style/IndentationConsistency:
   Description: 'Keep indentation straight.'
-  Enabled: false
+  Enabled: true
 
 Style/IndentationWidth:
   Description: 'Use 2 spaces for indentation.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#spaces-indentation'
-  Enabled: false
+  Enabled: true
 
 Style/IndentArray:
   Description: >-
@@ -657,7 +657,7 @@ Style/Tab:
 Style/TrailingBlankLines:
   Description: 'Checks trailing blank lines and final newline.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#newline-eof'
-  Enabled: false
+  Enabled: true
 
 Style/TrailingComma:
   Description: 'Checks for trailing comma in parameter lists and literals.'
@@ -909,7 +909,7 @@ Lint/StringConversionInInterpolation:
 
 Lint/UnderscorePrefixedVariableName:
   Description: 'Do not use prefix `_` for a variable that is used.'
-  Enabled: false
+  Enabled: true
 
 Lint/UnusedBlockArgument:
   Description: 'Checks for unused block arguments.'
@@ -966,11 +966,11 @@ Rails/Delegate:
 
 Rails/HasAndBelongsToMany:
   Description: 'Prefer has_many :through to has_and_belongs_to_many.'
-  Enabled: false
+  Enabled: true
 
 Rails/Output:
   Description: 'Checks for calls to puts, print, etc.'
-  Enabled: false
+  Enabled: true
 
 Rails/ReadWriteAttribute:
   Description: >-
@@ -991,6 +991,7 @@ Rails/Validation:
 #
 #
 AllCops:
+  RunRailsCops: true
   Exclude:
     - 'spec/**/*'
     - 'features/**/*'
@@ -1000,3 +1001,6 @@ AllCops:
     - 'bin/**/*'
     - 'lib/backup/**/*'
     - 'lib/tasks/**/*'
+    - 'lib/email_validator.rb'
+    - 'lib/gitlab/upgrader.rb'
+    - 'lib/gitlab/seeder.rb'

app/controllers/namespaces_controller.rb

@@ -15,4 +15,3 @@ class NamespacesController < ApplicationController
     end
   end
 end
-

app/controllers/projects/blob_controller.rb

@@ -59,8 +59,7 @@ class Projects::BlobController < Projects::ApplicationController
 
   def preview
     @content = params[:content]
-    diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3',
-                            include_diff_info: true)
+    diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', include_diff_info: true)
     @diff_lines = Gitlab::Diff::Parser.new.parse(diffy.diff.scan(/.*\n/))
 
     render layout: false

app/controllers/projects/raw_controller.rb

@@ -35,4 +35,3 @@ class Projects::RawController < Projects::ApplicationController
     end
   end
 end
-

app/finders/notes_finder.rb

@@ -7,7 +7,8 @@ class NotesFinder
     # Default to 0 to remain compatible with old clients
     last_fetched_at = Time.at(params.fetch(:last_fetched_at, 0).to_i)
 
-    notes = case target_type
+    notes =
+      case target_type
       when "commit"
         project.notes.for_commit_id(target_id).not_inline.fresh
       when "issue"

app/helpers/application_helper.rb

@@ -75,9 +75,9 @@ module ApplicationHelper
     options[:class] ||= ''
     options[:class] << ' identicon'
     bg_key = project.id % 7
+    style = "background-color: ##{ allowed_colors.values[bg_key] }; color: #555"
 
-    content_tag(:div, class: options[:class],
-      style: "background-color: ##{ allowed_colors.values[bg_key] }; color: #555") do
+    content_tag(:div, class: options[:class], style: style) do
       project.name[0, 1].upcase
     end
   end

app/helpers/commits_helper.rb

@@ -112,7 +112,8 @@ module CommitsHelper
     person_name = user.nil? ? source_name : user.name
     person_email = user.nil? ? source_email : user.email
 
-    text = if options[:avatar]
+    text =
+      if options[:avatar]
         avatar = image_tag(avatar_icon(person_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "")
         %Q{#{avatar} <span class="commit-#{options[:source]}-name">#{person_name}</span>}
       else

app/helpers/projects_helper.rb

@@ -254,4 +254,3 @@ module ProjectsHelper
     enabled_oauth_providers.include?(:github)
   end
 end
-

app/models/application_setting.rb

@@ -14,7 +14,8 @@
 #
 
 class ApplicationSetting < ActiveRecord::Base
-  validates :home_page_url, allow_blank: true,
+  validates :home_page_url,
+    allow_blank: true,
     format: { with: URI::regexp(%w(http https)), message: "should be a valid url" },
     if: :home_page_url_column_exist
 

app/models/commit.rb

@@ -88,7 +88,8 @@ class Commit
   # cut off, ellipses (`&hellp;`) are prepended to the commit message.
   def description
     title_end = safe_message.index("\n")
-    @description ||= if (!title_end && safe_message.length > 100) || (title_end && title_end > 100)
+    @description ||=
+      if (!title_end && safe_message.length > 100) || (title_end && title_end > 100)
         "&hellip;".html_safe << safe_message[80..-1]
       else
         safe_message.split("\n", 2)[1].try(:chomp)

app/models/namespace.rb

@@ -20,12 +20,17 @@ class Namespace < ActiveRecord::Base
   belongs_to :owner, class_name: "User"
 
   validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
-  validates :name, presence: true, uniqueness: true,
+  validates :name,
+    presence: true, uniqueness: true,
     length: { within: 0..255 },
     format: { with: Gitlab::Regex.name_regex,
               message: Gitlab::Regex.name_regex_message }
+
   validates :description, length: { within: 0..255 }
-  validates :path, uniqueness: { case_sensitive: false }, presence: true, length: { within: 1..255 },
+  validates :path,
+    uniqueness: { case_sensitive: false },
+    presence: true,
+    length: { within: 1..255 },
     exclusion: { in: Gitlab::Blacklist.path },
     format: { with: Gitlab::Regex.path_regex,
               message: Gitlab::Regex.path_regex_message }

app/models/project.rb

@@ -108,10 +108,14 @@ class Project < ActiveRecord::Base
   # Validations
   validates :creator, presence: true, on: :create
   validates :description, length: { maximum: 2000 }, allow_blank: true
-  validates :name, presence: true, length: { within: 0..255 },
+  validates :name,
+    presence: true,
+    length: { within: 0..255 },
     format: { with: Gitlab::Regex.project_name_regex,
               message: Gitlab::Regex.project_regex_message }
-  validates :path, presence: true, length: { within: 0..255 },
+  validates :path,
+    presence: true,
+    length: { within: 0..255 },
     exclusion: { in: Gitlab::Blacklist.path },
     format: { with: Gitlab::Regex.path_regex,
               message: Gitlab::Regex.path_regex_message }

app/models/project_services/bamboo_service.rb

@@ -17,13 +17,19 @@ class BambooService < CiService
 
   prop_accessor :bamboo_url, :build_key, :username, :password
 
-  validates :bamboo_url, presence: true,
-            format: { with: URI::regexp }, if: :activated?
+  validates :bamboo_url,
+    presence: true,
+    format: { with: URI::regexp },
+    if: :activated?
   validates :build_key, presence: true, if: :activated?
-  validates :username, presence: true,
-            if: ->(service) { service.password? }, if: :activated?
-  validates :password, presence: true,
-            if: ->(service) { service.username? }, if: :activated?
+  validates :username,
+    presence: true,
+    if: ->(service) { service.password? },
+    if: :activated?
+  validates :password,
+    presence: true,
+    if: ->(service) { service.username? },
+    if: :activated?
 
   attr_accessor :response
 

app/models/project_services/teamcity_service.rb

@@ -17,12 +17,15 @@ class TeamcityService < CiService
 
   prop_accessor :teamcity_url, :build_type, :username, :password
 
-  validates :teamcity_url, presence: true,
+  validates :teamcity_url,
+    presence: true,
     format: { with: URI::regexp }, if: :activated?
   validates :build_type, presence: true, if: :activated?
-  validates :username, presence: true,
+  validates :username,
+    presence: true,
     if: ->(service) { service.password? }, if: :activated?
-  validates :password, presence: true,
+  validates :password,
+    presence: true,
     if: ->(service) { service.username? }, if: :activated?
 
   attr_accessor :response

app/models/snippet.rb

@@ -29,7 +29,9 @@ class Snippet < ActiveRecord::Base
 
   validates :author, presence: true
   validates :title, presence: true, length: { within: 0..255 }
-  validates :file_name, presence: true, length: { within: 0..255 },
+  validates :file_name,
+    presence: true,
+    length: { within: 0..255 },
     format: { with: Gitlab::Regex.path_regex,
               message: Gitlab::Regex.path_regex_message }
   validates :content, presence: true

app/models/user.rb

@@ -116,7 +116,9 @@ class User < ActiveRecord::Base
   validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
   validates :bio, length: { maximum: 255 }, allow_blank: true
   validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0 }
-  validates :username, presence: true, uniqueness: { case_sensitive: false },
+  validates :username,
+    presence: true,
+    uniqueness: { case_sensitive: false },
     exclusion: { in: Gitlab::Blacklist.path },
     format: { with: Gitlab::Regex.username_regex,
               message: Gitlab::Regex.username_regex_message }

app/models/wiki_page.rb

@@ -43,7 +43,7 @@ class WikiPage
     @attributes[:slug]
   end
 
-  alias :to_param :slug
+  alias_method :to_param, :slug
 
   # The formatted title of this page.
   def title

app/services/projects/participants_service.rb

@@ -5,7 +5,8 @@ module Projects
     end
 
     def execute(note_type, note_id)
-      participating = if note_type && note_id
+      participating =
+        if note_type && note_id
           participants_in(note_type, note_id)
         else
           []

config/initializers/acts_as_taggable_on_patch.rb

@@ -42,7 +42,8 @@ module ActsAsTaggableOn::Taggable
 
         elsif options.delete(:any)
           # get tags, drop out if nothing returned (we need at least one)
-          tags = if options.delete(:wild)
+          tags =
+            if options.delete(:wild)
               ActsAsTaggableOn::Tag.named_like_any(tag_list)
             else
               ActsAsTaggableOn::Tag.named_any(tag_list)

config/initializers/carrierwave.rb

@@ -12,11 +12,19 @@ if File.exists?(aws_file)
       aws_secret_access_key: AWS_CONFIG['secret_access_key'], # required
       region: AWS_CONFIG['region'],                           # optional, defaults to 'us-east-1'
     }
-    config.fog_directory  = AWS_CONFIG['bucket']                    # required
-    config.fog_public     = false                                   # optional, defaults to true
-    config.fog_attributes = { 'Cache-Control'=>'max-age=315576000' }  # optional, defaults to {}
-    config.fog_authenticated_url_expiration = 1 << 29               # optional time (in seconds) that authenticated urls will be valid.
+
+    # required
+    config.fog_directory = AWS_CONFIG['bucket']
+
+    # optional, defaults to true
+    config.fog_public = false
+
+    # optional, defaults to {}
+    config.fog_attributes = { 'Cache-Control'=>'max-age=315576000' }
+
+    # optional time (in seconds) that authenticated urls will be valid.
     # when fog_public is false and provider is AWS or Google, defaults to 600
+    config.fog_authenticated_url_expiration = 1 << 29
   end
 
   # Mocking Fog requests, based on: https://github.com/carrierwaveuploader/carrierwave/wiki/How-to%3A-Test-Fog-based-uploaders

config/routes.rb

@@ -294,9 +294,7 @@ Gitlab::Application.routes.draw do
         member do
           # tree viewer logs
           get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex }
-          get 'logs_tree/:path' => 'refs#logs_tree',
-            as: :logs_file,
-            constraints: {
+          get 'logs_tree/:path' => 'refs#logs_tree', as: :logs_file, constraints: {
             id: Gitlab::Regex.git_reference_regex,
             path: /.*/
           }

lib/api/api_guard.rb

@@ -47,16 +47,12 @@ module APIGuard
         case validate_access_token(access_token, scopes)
         when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
           raise InsufficientScopeError.new(scopes)
-
         when Oauth2::AccessTokenValidationService::EXPIRED
           raise ExpiredError
-
         when Oauth2::AccessTokenValidationService::REVOKED
           raise RevokedError
-
         when Oauth2::AccessTokenValidationService::VALID
           @current_user = User.find(access_token.resource_owner_id)
-
         end
       end
     end
@@ -120,8 +116,9 @@ module APIGuard
     end
 
     def oauth2_bearer_token_error_handler
-      Proc.new {|e|
-        response = case e
+      Proc.new do |e|
+        response =
+          case e
           when MissingTokenError
             Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new
 
@@ -150,7 +147,7 @@ module APIGuard
           end
 
         response.finish
-      }
+      end
     end
   end
 

lib/api/internal.rb

 module API
   # Internal access API
   class Internal < Grape::API
-    before {
-      authenticate_by_gitlab_shell_token!
-    }
+    before { authenticate_by_gitlab_shell_token! }
 
     namespace 'internal' do
       # Check if git command is allowed to project

lib/api/namespaces.rb

 module API
   # namespaces API
   class Namespaces < Grape::API
-    before {
+    before do
       authenticate!
       authenticated_as_admin!
-    }
+    end
 
     resource :namespaces do
       # Get a namespaces list

lib/api/system_hooks.rb

 module API
   # Hooks API
   class SystemHooks < Grape::API
-    before {
+    before do
       authenticate!
       authenticated_as_admin!
-    }
+    end
 
     resource :hooks do
       # Get the list of system hooks

lib/gitlab/backend/shell_adapter.rb

@@ -9,4 +9,3 @@ module Gitlab
     end
   end
 end
-

lib/gitlab/diff/parser.rb

@@ -4,7 +4,7 @@ module Gitlab
       include Enumerable
 
       def parse(lines)
-        @lines = lines,
+        @lines = lines
         lines_obj = []
         line_obj_index = 0
         line_old = 1

lib/gitlab/force_push_check.rb

@@ -12,4 +12,3 @@ module Gitlab
     end
   end
 end
-

lib/gitlab/git_access.rb

@@ -113,8 +113,8 @@ module Gitlab
       # we dont allow force push to protected branch
       if forced_push?(project, oldrev, newrev)
         :force_push_code_to_protected_branches
-       # and we dont allow remove of protected branch
       elsif newrev == Gitlab::Git::BLANK_SHA
+        # and we dont allow remove of protected branch
         :remove_protected_branches
       elsif project.developers_can_push_to_protected_branch?(branch_name)
         :push_code

lib/gitlab/ldap/adapter.rb

@@ -63,8 +63,10 @@ module Gitlab
       end
 
       def dn_matches_filter?(dn, filter)
-        ldap_search(base: dn, filter: filter,
-          scope: Net::LDAP::SearchScope_BaseObject, attributes: %w{dn}).any?
+        ldap_search(base: dn,
+                    filter: filter,
+                    scope: Net::LDAP::SearchScope_BaseObject,
+                    attributes: %w{dn}).any?
       end
 
       def ldap_search(*args)