[api][webui] Enable Style/GuardClause rubocop cop

src/api/.rubocop.yml

@@ -58,6 +58,10 @@ Style/EmptyElse:
 Style/ExtraSpacing:
   Enabled: true
 
+# Use guard clause instead of wrapping the code inside a conditional expression
+Style/GuardClause:
+  Enabled: true
+
 # Checks hash literal syntax
 Style/HashSyntax:
   EnforcedStyle: ruby19_no_mixed_keys

src/api/.rubocop_todo.yml

@@ -515,11 +515,6 @@ Style/GlobalVars:
     - 'test/functional/request_controller_test.rb'
     - 'test/functional/source_controller_test.rb'
 
-# Offense count: 151
-# Configuration parameters: MinBodyLength.
-Style/GuardClause:
-  Enabled: false
-
 # Offense count: 8
 Style/IdenticalConditionalBranches:
   Exclude:

src/api/app/controllers/application_controller.rb

@@ -45,11 +45,8 @@ class AuthenticationRequiredError < APIException
   attr_accessor :auth_method
 
   def pundit_user
-    if User.current.is_nobody?
-      return
-    else
-      return User.current
-    end
+    return if User.current.is_nobody?
+    return User.current
   end
 
   # Method for mapping actions in a controller to (XML) schemas based on request
@@ -421,9 +418,7 @@ def user
   end
 
   def require_parameter!(parameter)
-    unless params.include? parameter.to_s
-      raise MissingParameterError, "Required Parameter #{parameter} missing"
-    end
+    raise MissingParameterError, "Required Parameter #{parameter} missing" unless params.include? parameter.to_s
   end
 
   def required_parameters(*parameters)
@@ -545,24 +540,27 @@ def validate_xml_request(method = nil)
 
   def validate_xml_response
     return if @skip_validation
-    # rubocop:disable Metrics/LineLength
-    if request.format != 'json' && response.status.to_s[0..2] == '200' && response.headers['Content-Type'] !~ /.*\/json/i && response.headers['Content-Disposition'] != 'attachment'
-      opt = params
-      opt[:method] = request.method.to_s
-      opt[:type] = 'response'
-      ms = Benchmark.ms do
-        if response.body.respond_to? :call
-          sio = StringIO.new
-          response.body.call(nil, sio) # send_file can return a block that takes |response, output|
-          str = sio.string
-        else
-          str = response.body
-        end
-        Suse::Validator.validate(opt, str)
+
+    request_format = request.format != 'json'
+    response_status = response.status.to_s[0..2] == '200'
+    response_headers = response.headers['Content-Type'] !~ /.*\/json/i && response.headers['Content-Disposition'] != 'attachment'
+
+    return unless request_format && response_status && response_headers
+
+    opt = params
+    opt[:method] = request.method.to_s
+    opt[:type] = 'response'
+    ms = Benchmark.ms do
+      if response.body.respond_to? :call
+        sio = StringIO.new
+        response.body.call(nil, sio) # send_file can return a block that takes |response, output|
+        str = sio.string
+      else
+        str = response.body
       end
-      logger.debug "Validate XML response: #{response} took #{Integer(ms + 0.5)}ms"
+      Suse::Validator.validate(opt, str)
     end
-    # rubocop:enable Metrics/LineLength
+    logger.debug "Validate XML response: #{response} took #{Integer(ms + 0.5)}ms"
   end
 
   def set_response_format_to_xml

src/api/app/controllers/group_controller.rb

@@ -29,9 +29,7 @@ def index
     else
       @list = Group.all
     end
-    if params[:prefix]
-      @list = @list.find_all { |group| group.title.starts_with? params[:prefix] }
-    end
+    @list = @list.find_all { |group| group.title.starts_with? params[:prefix] } if params[:prefix]
   end
 
   # DELETE for removing it

src/api/app/controllers/message_controller.rb

@@ -8,12 +8,10 @@ class MessageController < ApplicationController
 
   def check_project_and_package
     # get project and package if params are set
-    if params[:project]
-      @project = Project.find_by_name! params[:project]
-      if params[:package]
-        @package = @project.packages.find_by_name! params[:package]
-      end
-    end
+    return unless params[:project]
+
+    @project = Project.find_by_name! params[:project]
+    @package = @project.packages.find_by_name! params[:package] if params[:package]
   end
 
   def list

src/api/app/controllers/public_controller.rb

@@ -186,11 +186,8 @@ def binary_packages
 
   # removes /private prefix from path
   def unshift_public(path)
-    if path =~ %r{/public(.*)}
-      return $1
-    else
-      return path
-    end
+    return $1 if path =~ %r{/public(.*)}
+    return path
   end
 
   def check_package_access(project, package, use_source = true)

src/api/app/controllers/source_controller.rb

@@ -218,11 +218,9 @@ def show_package_issues
   def show_package
     if @deleted_package
       tpkg = Package.find_by_project_and_name(@target_project_name, @target_package_name)
-      if tpkg
-        raise PackageExists.new 'the package is not deleted'
-      else
-        validate_read_access_of_deleted_package(@target_project_name, @target_package_name)
-      end
+      raise PackageExists.new 'the package is not deleted' if tpkg
+
+      validate_read_access_of_deleted_package(@target_project_name, @target_package_name)
     elsif %w(_project _pattern).include? @target_package_name
       Project.get_by_name @target_project_name
     else
@@ -306,12 +304,12 @@ class NoMatchingReleaseTarget < APIException
   end
 
   def verify_can_modify_target_package!
-    unless User.current.can_modify_package?(@package)
-      raise CmdExecutionNoPermission.new "no permission to execute command '#{params[:cmd]}' " +
-                                         "for unspecified package" unless @package.class == Package
-      raise CmdExecutionNoPermission.new "no permission to execute command '#{params[:cmd]}' " +
-                                         "for package #{@package.name} in project #{@package.project.name}"
-    end
+    return if User.current.can_modify_package?(@package)
+
+    raise CmdExecutionNoPermission.new "no permission to execute command '#{params[:cmd]}' " +
+                                       "for unspecified package" unless @package.class == Package
+    raise CmdExecutionNoPermission.new "no permission to execute command '#{params[:cmd]}' " +
+                                       "for package #{@package.name} in project #{@package.project.name}"
   end
 
   # POST /source/:project/:package
@@ -399,9 +397,7 @@ def validate_target_for_package_command_exists!
     end
 
     # check read access rights when the package does not exist anymore
-    if @package.nil? && @deleted_package
-      validate_read_access_of_deleted_package(@target_project_name, @target_package_name)
-    end
+    validate_read_access_of_deleted_package(@target_project_name, @target_package_name) if @package.nil? && @deleted_package
   end
 
   class ChangeProjectNoPermission < APIException
@@ -522,15 +518,10 @@ def update_project_meta
 
   def check_and_remove_repositories!(repositories, opts)
     result = Project.check_repositories(repositories) unless opts[:force]
-    if !opts[:force] && result[:error]
-      raise RepoDependency, result[:error]
-    else
-      result = Project.remove_repositories(repositories, opts)
+    raise RepoDependency, result[:error] if !opts[:force] && result[:error]
 
-      if !opts[:force] && result[:error]
-        raise ChangeProjectNoPermission, result[:error]
-      end
-    end
+    result = Project.remove_repositories(repositories, opts)
+    raise ChangeProjectNoPermission, result[:error] if !opts[:force] && result[:error]
   end
 
   # GET /source/:project/_config
@@ -805,10 +796,9 @@ def update_file
     pass_to_backend @path
 
     # update package timestamp and reindex sources
-    unless params[:rev] == 'repository' || @package_name.in?(["_project", "_pattern"])
-      special_file = params[:filename].in?(["_aggregate", "_constraints", "_link", "_service", "_patchinfo", "_channel"])
-      @pack.sources_changed(wait_for_update: special_file) # wait for indexing for special files
-    end
+    return if params[:rev] == 'repository' || @package_name.in?(["_project", "_pattern"])
+    special_file = params[:filename].in?(["_aggregate", "_constraints", "_link", "_service", "_patchinfo", "_channel"])
+    @pack.sources_changed(wait_for_update: special_file) # wait for indexing for special files
   end
 
   # DELETE /source/:project/:package/:filename
@@ -1051,9 +1041,7 @@ def verify_repos_match!(pro)
         repo_matches = true
       end
     end
-    unless repo_matches
-      raise NoMatchingReleaseTarget.new 'No defined or matching release target'
-    end
+    raise NoMatchingReleaseTarget.new 'No defined or matching release target' unless repo_matches
   end
 
   class RemoteProjectError < APIException
@@ -1387,9 +1375,7 @@ def package_command_commit
     path += build_query_from_hash(params, [:cmd, :user, :comment, :rev, :linkrev, :keeplink, :repairlink])
     pass_to_backend path
 
-    if @package # except in case of _project package
-      @package.sources_changed
-    end
+    @package.sources_changed if @package # except in case of _project package
   end
 
   # POST /source/<project>/<package>?cmd=commitfilelist
@@ -1398,9 +1384,7 @@ def package_command_commitfilelist
     path += build_query_from_hash(params, [:cmd, :user, :comment, :rev, :linkrev, :keeplink, :repairlink])
     answer = pass_to_backend path
 
-    if @package # except in case of _project package
-      @package.sources_changed(dir_xml: answer)
-    end
+    @package.sources_changed(dir_xml: answer) if @package # except in case of _project package
   end
 
   # POST /source/<project>/<package>?cmd=diff
@@ -1458,21 +1442,19 @@ def package_command_copy
 
   def reparse_backend_package(spackage, sproject)
     answer = Suse::Backend.get("/source/#{CGI.escape(sproject)}/#{CGI.escape(spackage)}/_meta")
-    if answer
-      Package.transaction do
-        adata = Xmlhash.parse(answer.body)
-        adata['name'] = params[:package]
-        p = @project.packages.new(name: params[:package])
-        p.update_from_xml(adata)
-        p.remove_all_persons
-        p.remove_all_groups
-        p.develpackage = nil
-        p.store
-      end
-      @package = Package.find_by_project_and_name(params[:project], params[:package])
-    else
-      raise UnknownPackage.new "Unknown package #{spackage} in project #{sproject}"
-    end
+    raise UnknownPackage.new "Unknown package #{spackage} in project #{sproject}" unless answer
+
+    Package.transaction do
+      adata = Xmlhash.parse(answer.body)
+      adata['name'] = params[:package]
+      p = @project.packages.new(name: params[:package])
+      p.update_from_xml(adata)
+      p.remove_all_persons
+      p.remove_all_groups
+      p.develpackage = nil
+      p.store
+    end
+    @package = Package.find_by_project_and_name(params[:project], params[:package])
   end
 
   # POST /source/<project>/<package>?cmd=release

src/api/app/controllers/tag_controller.rb

@@ -130,11 +130,9 @@ def get_tags_by_user_and_project( do_render = true )
     @project = Project.get_by_name(params[:project])
 
     @tags = @project.tags.where("taggings.user_id = ?", user.id).order(:name)
-    if do_render
-      render partial: "tags"
-    else
-      return @tags
-    end
+    return @tags unless do_render
+
+    render partial: "tags"
   end
 
   def get_tags_by_user_and_package( do_render = true )
@@ -146,11 +144,9 @@ def get_tags_by_user_and_package( do_render = true )
     @project = @package.project
 
     @tags = @package.tags.where("taggings.user_id = ?", user.id).order(:name)
-    if do_render
-      render partial: "tags"
-    else
-      return @tags
-    end
+    return @tags unless do_render
+
+    render partial: "tags"
   end
 
   def most_popular_tags

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

@@ -24,11 +24,9 @@ def new
 
   def edit
     authorize @attribute
-    if @attribute.attrib_type.value_count && ( @attribute.attrib_type.value_count > @attribute.values.length )
-      ( @attribute.attrib_type.value_count - @attribute.values.length ).times do
-        @attribute.values.build(attrib: @attribute)
-      end
-    end
+    return unless @attribute.attrib_type.value_count && ( @attribute.attrib_type.value_count > @attribute.values.length )
+
+    (@attribute.attrib_type.value_count - @attribute.values.length).times { @attribute.values.build(attrib: @attribute) }
   end
 
   def create

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

@@ -114,9 +114,7 @@ def binaries
   private
 
   def check_images_repo
-    unless @project.repositories.find_by(name: 'images')
-      flash.now[:alert] = "You need to add an 'images' repository to your project " +
-          'to be able to build a driver update disk image!'
-    end
+    return if @project.repositories.find_by(name: 'images')
+    flash.now[:alert] = "You need to add an 'images' repository to your project to be able to build a driver update disk image!"
   end
 end

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

@@ -70,9 +70,8 @@ def set_group
     @group = Group.find_by_title(params[:title])
 
     # Group.find_by_title! is self implemented and would raise an 500 error
-    unless @group
-      flash[:error] = "Group '#{params[:title]}' does not exist"
-      redirect_back(fallback_location: {controller: 'main', action: 'index'})
-    end
+    return if @group
+    flash[:error] = "Group '#{params[:title]}' does not exist"
+    redirect_back(fallback_location: {controller: 'main', action: 'index'})
   end
 end