Merge pull request #4259 from DavidKang/feature/rubocop/style_cop

Rubocop friendly cops part 9

.rubocop_todo.yml

@@ -298,26 +298,12 @@ Rails/SkipsModelValidations:
 Rails/TimeZone:
   Enabled: false
 
-# Offense count: 200
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: braces, no_braces, context_dependent
-Style/BracesAroundHashParameters:
-  Enabled: false
-
 # Offense count: 67
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: nested, compact
 Style/ClassAndModuleChildren:
   Enabled: false
 
-# Offense count: 85
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: is_a?, kind_of?
-Style/ClassCheck:
-  Enabled: false
-
 # Offense count: 35
 Style/ClassVars:
   Exclude:
@@ -330,24 +316,6 @@ Style/ClassVars:
     - 'src/api/test/functional/branch_publish_flag_test.rb'
     - 'src/api/test/test_helper.rb'
 
-# Offense count: 15
-# Cop supports --auto-correct.
-# Configuration parameters: Keywords.
-# Keywords: TODO, FIXME, OPTIMIZE, HACK, REVIEW
-Style/CommentAnnotation:
-  Exclude:
-    - 'src/api/app/controllers/person_controller.rb'
-    - 'src/api/app/controllers/search_controller.rb'
-    - 'src/api/app/controllers/webui/main_controller.rb'
-    - 'src/api/app/controllers/webui/package_controller.rb'
-    - 'src/api/app/models/bs_request_action.rb'
-    - 'src/api/app/models/bs_request_action_group.rb'
-    - 'src/api/app/models/owner.rb'
-    - 'src/api/app/models/product.rb'
-    - 'src/api/test/functional/attributes_test.rb'
-    - 'src/api/test/functional/build_controller_test.rb'
-    - 'src/api/test/functional/request_controller_test.rb'
-
 # Offense count: 8
 Style/CommentedKeyword:
   Exclude:

src/api/app/controllers/application_controller.rb

@@ -89,7 +89,7 @@ def validate_params
     params.each do |key, value|
       next if value.nil?
       next if key == 'xmlhash' # perfectly fine
-      unless value.kind_of? String
+      unless value.is_a? String
         raise InvalidParameterError, "Parameter #{key} has non String class #{value.class}"
       end
     end
@@ -170,7 +170,7 @@ def pass_to_backend(path = nil)
     when :post
       # for form data we don't need to download anything
       if request.form_data?
-        response = Backend::Connection.post(path, '', { 'Content-Type' => 'application/x-www-form-urlencoded' })
+        response = Backend::Connection.post(path, '', 'Content-Type' => 'application/x-www-form-urlencoded')
       else
         file = download_request
         response = Backend::Connection.post(path, file)

src/api/app/controllers/attribute_controller.rb

@@ -248,7 +248,7 @@ def cmd_attribute
       attrib.container = @attribute_container
 
       unless attrib.valid?
-        raise APIException, { message: attrib.errors.full_messages.join('\n'), status: 400 }
+        raise APIException, message: attrib.errors.full_messages.join('\n'), status: 400
       end
 
       authorize attrib, :create?

src/api/app/controllers/build_controller.rb

@@ -56,7 +56,7 @@ def project_index
 
       if !allowed && !params[:package].nil?
         package_names = nil
-        if params[:package].kind_of? Array
+        if params[:package].is_a? Array
           package_names = params[:package]
         else
           package_names = [params[:package]]
@@ -169,7 +169,7 @@ def result_lastsuccess
     required_parameters :package, :pathproject
 
     pkg = Package.get_by_project_and_name(params[:project], params[:package],
-                                          { use_source: false, follow_project_links: true, follow_multibuild: true })
+                                          use_source: false, follow_project_links: true, follow_multibuild: true)
     raise RemoteProjectError, 'The package lifes in a remote project, this is not supported atm' unless pkg
 
     tprj = Project.get_by_name params[:pathproject]

src/api/app/controllers/person_controller.rb

@@ -144,7 +144,7 @@ def grouplist
   end
 
   def register
-    # FIXME 3.0, to be removed
+    # FIXME: 3.0, to be removed
     internal_register
   end
 

src/api/app/controllers/public_controller.rb

@@ -146,7 +146,7 @@ def binary_packages
     end
 
     @binary_links = {}
-    @pkg.project.repositories.includes({ path_elements: { link: :project } }).each do |repo|
+    @pkg.project.repositories.includes(path_elements: { link: :project }).each do |repo|
       repo.path_elements.each do |pe|
         # NOTE: we do not follow indirect path elements here, since most installation handlers
         #       do not support it (exception zypp via ymp files)

src/api/app/controllers/search_controller.rb

@@ -232,7 +232,7 @@ def search(what, render_all)
     end
     relation = relation.includes(includes).references(includes)
 
-    # TODO support sort_by and order parameters?
+    # TODO: support sort_by and order parameters?
 
     relation.each do |item|
       next if xml[item.id]

src/api/app/controllers/source_controller.rb

@@ -136,7 +136,7 @@ def delete_project
     project = Project.get_by_name(params[:project])
 
     # checks
-    unless project.kind_of?(Project) && User.current.can_modify_project?(project)
+    unless project.is_a?(Project) && User.current.can_modify_project?(project)
       logger.debug "No permission to delete project #{project}"
       render_error status: 403, errorcode: 'delete_project_no_permission',
                    message: "Permission denied (delete project #{project})"
@@ -512,7 +512,7 @@ def update_project_meta
         # FIXME3.0: don't modify send data
         project.relationships.build(user: User.current, role: Role.find_by_title!('maintainer'))
       end
-      project.store({ comment: params[:comment] })
+      project.store(comment: params[:comment])
     end
     render_ok
   end
@@ -690,7 +690,7 @@ def update_package_meta
       end
     else
       prj = Project.get_by_name(@project_name)
-      unless prj.kind_of?(Project) && User.current.can_create_package_in?(prj)
+      unless prj.is_a?(Project) && User.current.can_create_package_in?(prj)
         render_error status: 403, errorcode: 'create_package_no_permission',
                      message: "no permission to create a package in project '#{@project_name}'"
         return
@@ -951,7 +951,7 @@ def project_command_modifychannels
     @project.packages.each do |pkg|
       pkg.modify_channel(mode)
     end
-    @project.store({ user: User.current.login })
+    @project.store(user: User.current.login)
 
     render_ok
   end
@@ -993,7 +993,7 @@ def project_command_undelete
   def project_command_release
     params[:user] = User.current.login
 
-    @project = Project.get_by_name params[:project], { includeallpackages: 1 }
+    @project = Project.get_by_name params[:project], includeallpackages: 1
     verify_repos_match!(@project)
 
     if @project.is_a? String # remote project
@@ -1080,7 +1080,7 @@ def project_command_copy
     unless (@project && User.current.can_modify_project?(@project)) || User.current.can_create_project?(project_name)
       raise CmdExecutionNoPermission, "no permission to execute command 'copy'"
     end
-    oprj = Project.get_by_name(params[:oproject], { includeallpackages: 1 })
+    oprj = Project.get_by_name(params[:oproject], includeallpackages: 1)
     if params.has_key?(:makeolder) || params.has_key?(:makeoriginolder)
       unless User.current.can_modify_project?(oprj)
         raise CmdExecutionNoPermission, "no permission to execute command 'copy', requires modification permission in origin project"
@@ -1203,7 +1203,7 @@ def package_command_addchannels
   # POST /source/<project>/<package>?cmd=enablechannel
   def package_command_enablechannel
     @package.modify_channel(:enable_all)
-    @package.project.store({ user: User.current.login })
+    @package.project.store(user: User.current.login)
 
     render_ok
   end
@@ -1258,7 +1258,7 @@ def package_command_collectbuildenv
   # POST /source/<project>/<package>?cmd=instantiate
   def package_command_instantiate
     project = Project.get_by_name(params[:project])
-    opackage = Package.get_by_project_and_name(project.name, params[:package], { check_update_project: true })
+    opackage = Package.get_by_project_and_name(project.name, params[:package], check_update_project: true)
     unless opackage
       raise RemoteProjectError, 'Instantiation from remote project is not supported'
     end
@@ -1286,7 +1286,7 @@ def package_command_undelete
       raise PackageExists, "the package exists already #{@target_project_name} #{@target_package_name}"
     end
     tprj = Project.get_by_name(@target_project_name)
-    unless tprj.kind_of?(Project) && User.current.can_create_package_in?(tprj)
+    unless tprj.is_a?(Project) && User.current.can_create_package_in?(tprj)
       raise CmdExecutionNoPermission, "no permission to create package in project #{@target_project_name}"
     end
 
@@ -1553,7 +1553,7 @@ def verify_can_modify_target!
 
     if Package.exists_by_project_and_name(@target_project_name, @target_package_name, follow_project_links: false)
       verify_can_modify_target_package!
-    elsif !@project.kind_of?(Project) || !User.current.can_create_package_in?(@project)
+    elsif !@project.is_a?(Project) || !User.current.can_create_package_in?(@project)
       raise CmdExecutionNoPermission, "no permission to create package in project #{@target_project_name}"
     end
   end

src/api/app/controllers/webui/main_controller.rb

@@ -94,7 +94,7 @@ def add_news
       flash[:error] = 'Please provide a message and severity'
       redirect_to(action: 'index') && return
     end
-    # TODO - make use of permissions.status_message_create
+    # TODO: make use of permissions.status_message_create
     status_message = StatusMessage.new(message: params[:message], severity: params[:severity], user: User.current)
     if status_message.save
       flash[:notice] = 'Status message was successfully created.'

src/api/app/controllers/webui/package_controller.rb

@@ -365,7 +365,7 @@ def set_file_details
 
       @files = package_files(@srcmd5 || @revision, @expand)
     rescue ActiveXML::Transport::Error => e
-      # TODO crudest hack ever!
+      # TODO: crudest hack ever!
       if e.summary == 'service in progress' && @expand == 1
         @expand = 0
         @service_running = true
@@ -1089,9 +1089,9 @@ def check_build_log_access
     end
 
     begin
-      @package = Package.get_by_project_and_name(@project, params[:package], { use_source:           false,
-                                                                               follow_multibuild:    true,
-                                                                               follow_project_links: true })
+      @package = Package.get_by_project_and_name(@project, params[:package], use_source:           false,
+                                                                             follow_multibuild:    true,
+                                                                             follow_project_links: true)
     rescue Package::UnknownObjectError
       redirect_to project_show_path(@project.to_param),
                   error: "Couldn't find package '#{params[:package]}' in " +

src/api/app/controllers/webui/packages/build_reason_controller.rb

@@ -17,7 +17,7 @@ def index
 
       def set_package
         @package = ::Package.get_by_project_and_name(@project.to_param, params[:package_name],
-                                                     { use_source: false, follow_project_links: true, follow_multibuild: true })
+                                                     use_source: false, follow_project_links: true, follow_multibuild: true)
         @is_link = @package.is_link? || @package.is_local_link?
       rescue APIException
         flash[:error] = "Package \"#{params[:package_name]}\" not found in project \"#{params[:project]}\""

src/api/app/controllers/webui/packages/job_history_controller.rb

@@ -14,7 +14,7 @@ def index
 
       def set_package
         @package = ::Package.get_by_project_and_name(@project.to_param, params[:package_name],
-                                                     { use_source: false, follow_project_links: true, follow_multibuild: true })
+                                                     use_source: false, follow_project_links: true, follow_multibuild: true)
         @is_link = @package.is_link? || @package.is_local_link?
       rescue APIException
         flash[:error] = "Package \"#{params[:package_name]}\" not found in project \"#{params[:project]}\""

src/api/app/controllers/webui/project_controller.rb

@@ -132,7 +132,7 @@ def new_incident_request
         req = BsRequest.new
         req.description = params[:description]
 
-        action = BsRequestActionMaintenanceIncident.new({ source_project: params[:project] })
+        action = BsRequestActionMaintenanceIncident.new(source_project: params[:project])
         req.bs_request_actions << action
         action.bs_request = req
 
@@ -163,7 +163,7 @@ def new_release_request
           req = BsRequest.new
           req.description = params[:description]
 
-          action = BsRequestActionMaintenanceRelease.new({ source_project: params[:project] })
+          action = BsRequestActionMaintenanceRelease.new(source_project: params[:project])
           req.bs_request_actions << action
           action.bs_request = req
 
@@ -240,7 +240,7 @@ def destroy
       if parent
         redirect_to project_show_path(parent)
       else
-        redirect_to({ action: :index })
+        redirect_to(action: :index)
       end
     else
       redirect_to project_show_path(@project), notice: "Project can't be removed: #{@project.errors.full_messages.to_sentence}"

src/api/app/controllers/webui/repositories_controller.rb

@@ -49,10 +49,10 @@ def create
     end
 
     if repository.save
-      @project.store({ comment: "Added #{repository.name} repository" })
+      @project.store(comment: "Added #{repository.name} repository")
       flash[:success] = "Successfully added repository '#{repository.name}'"
       respond_to do |format|
-        format.html { redirect_to({ action: :index, project: @project }) }
+        format.html { redirect_to(action: :index, project: @project) }
         format.js
       end
     else
@@ -72,7 +72,7 @@ def update
     archs = params[:arch].keys.map { |arch| Architecture.find_by_name(arch) } if params[:arch]
     repo.architectures = archs
     repo.save
-    @project.store({ comment: "Modified #{repo.name} repository" })
+    @project.store(comment: "Modified #{repo.name} repository")
 
     # Merge project repo's arch list with currently available arches from API. This needed as you want
     # to keep currently non-working arches in the project meta.
@@ -88,10 +88,10 @@ def destroy
     repository = @project.repositories.find_by(name: params[:target])
     result = repository && @project.repositories.delete(repository)
     if @project.valid? && result
-      @project.store({ comment: "Removed #{repository.name} repository" })
+      @project.store(comment: "Removed #{repository.name} repository")
       respond_to do |format|
         flash[:success] = "Successfully removed repository '#{repository.name}'"
-        format.html { redirect_to({ action: :index, project: @project }) }
+        format.html { redirect_to(action: :index, project: @project) }
         format.js
       end
     else
@@ -160,7 +160,7 @@ def create_image_repository
 
       flash[:success] = 'Successfully added image repository'
       respond_to do |format|
-        format.html { redirect_to({ action: :index, project: @project }) }
+        format.html { redirect_to(action: :index, project: @project) }
         format.js { render 'create' }
       end
     else
@@ -184,7 +184,7 @@ def create_flag
       if @flag.save
         # FIXME: This should happen in Flag or even better in Project
         @main_object.store
-        format.html { redirect_to({ action: :index, controller: :repositories, project: params[:project], package: params[:package] }) }
+        format.html { redirect_to(action: :index, controller: :repositories, project: params[:project], package: params[:package]) }
         format.js { render 'change_flag' }
       else
         format.json { render json: @flag.errors, status: :unprocessable_entity }
@@ -203,7 +203,7 @@ def toggle_flag
       if @flag.save
         # FIXME: This should happen in Flag or even better in Project
         @main_object.store
-        format.html { redirect_to({ action: :index, project: params[:project], package: params[:package] }) }
+        format.html { redirect_to(action: :index, project: params[:project], package: params[:package]) }
         format.js { render 'change_flag' }
       else
         format.json { render json: @flag.errors, status: :unprocessable_entity }
@@ -223,7 +223,7 @@ def remove_flag
     respond_to do |format|
       # FIXME: This should happen in Flag or even better in Project
       @main_object.store
-      format.html { redirect_to({ action: :index, project: params[:project], package: params[:package] }) }
+      format.html { redirect_to(action: :index, project: params[:project], package: params[:package]) }
       format.js { render 'change_flag' }
     end
   end

src/api/app/controllers/webui/request_controller.rb

@@ -302,12 +302,10 @@ def change_devel_request
     begin
       BsRequest.transaction do
         req = BsRequest.new(state: 'new', description: params[:description])
-        action = BsRequestActionChangeDevel.new({
-                                                  target_project: params[:project],
-                                                  target_package: params[:package],
-                                                  source_project: params[:devel_project],
-                                                  source_package: params[:devel_package] || params[:package]
-                                                })
+        action = BsRequestActionChangeDevel.new(target_project: params[:project],
+          target_package: params[:package],
+          source_project: params[:devel_project],
+          source_package: params[:devel_package] || params[:package])
 
         req.bs_request_actions << action
         action.bs_request = req

src/api/app/controllers/webui/webui_controller.rb

@@ -259,7 +259,7 @@ def require_package
     if params[:package].present?
       begin
         @package = Package.get_by_project_and_name(@project.to_param, params[:package],
-                                                   { use_source: false, follow_project_links: true, follow_multibuild: true })
+                                                   use_source: false, follow_project_links: true, follow_multibuild: true)
       rescue APIException # why it's not found is of no concern :)
       end
     end

src/api/app/helpers/flag_helper.rb

@@ -99,7 +99,7 @@ def set_repository_by_product(flag, status, product_name, patchlevel = nil)
     validate_type flag
 
     prj = self
-    prj = project if kind_of? Package
+    prj = project if is_a? Package
     update = nil
 
     # we find all repositories targeted by given products