Merge pull request #10068 from quatran/correct_rubocop_style_ifunless…

…modifier

Correct RuboCop Style/IfUnlessModifier offenses

src/api/.rubocop_todo.yml

@@ -1294,32 +1294,6 @@ Style/IdenticalConditionalBranches:
     - 'app/controllers/build_controller.rb'
     - 'app/controllers/webui/search_controller.rb'
 
-# Offense count: 47
-# Cop supports --auto-correct.
-Style/IfUnlessModifier:
-  Exclude:
-    - 'app/policies/application_policy.rb'
-    - 'app/services/package_controller_service/url_generator.rb'
-    - 'app/views/models/_package.xml.builder'
-    - 'app/views/models/_project.xml.builder'
-    - 'app/views/models/_remote_project.xml.builder'
-    - 'app/views/models/staging/_staging_project.xml.builder'
-    - 'db/migrate/20170704212201_increase_timestamp_precision.rb'
-    - 'lib/tasks/extract.rake'
-    - 'lib/xpath_engine.rb'
-    - 'script/import_database.rb'
-    - 'spec/factories/bs_requests.rb'
-    - 'spec/factories/packages.rb'
-    - 'spec/factories/project.rb'
-    - 'spec/features/webui/packages_spec.rb'
-    - 'spec/rails_helper.rb'
-    - 'test/functional/maintenance_test.rb'
-    - 'test/functional/published_controller_test.rb'
-    - 'test/functional/request_controller_test.rb'
-    - 'test/functional/webui/spider_test.rb'
-    - 'test/test_helper.rb'
-    - 'test/unit/schema_test.rb'
-
 # Offense count: 2
 Style/MissingRespondToMissing:
   Exclude:

src/api/app/policies/application_policy.rb

@@ -2,9 +2,7 @@ class ApplicationPolicy
   attr_reader :user, :record
 
   def initialize(user, record, opts = {})
-    unless user || opts[:user_optional]
-      raise Pundit::NotAuthorizedError, 'must be logged in'
-    end
+    raise Pundit::NotAuthorizedError, 'must be logged in' unless user || opts[:user_optional]
     raise Pundit::NotAuthorizedError, 'record does not exist' unless record
 
     @user = user

src/api/app/services/package_controller_service/url_generator.rb

@@ -34,9 +34,7 @@ def file_available?(url, max_redirects = 5)
       http.open_timeout = 15
       http.read_timeout = 15
       response = http.head uri.path
-      if response.code.to_i == 302 && response['location'] && max_redirects.positive?
-        return file_available?(response['location'], (max_redirects - 1))
-      end
+      return file_available?(response['location'], (max_redirects - 1)) if response.code.to_i == 302 && response['location'] && max_redirects.positive?
 
       response.code.to_i == 200
     rescue Object => e

src/api/app/views/models/_package.xml.builder

@@ -3,9 +3,7 @@ xml.package(name: my_model.name, project: my_model.project.name) do
   xml.description(my_model.description)
   xml.releasename(my_model.releasename) if my_model.releasename
 
-  if my_model.develpackage
-    xml.devel(project: my_model.develpackage.project.name, package: my_model.develpackage.name)
-  end
+  xml.devel(project: my_model.develpackage.project.name, package: my_model.develpackage.name) if my_model.develpackage
 
   my_model.render_relationships(xml)
 

src/api/app/views/models/_project.xml.builder

@@ -48,9 +48,7 @@ xml.project(project_attributes) do
         params[:trigger] = rt.trigger if rt.trigger.present?
         xml_repository.releasetarget(params)
       end
-      if repo.hostsystem
-        xml_repository.hostsystem(project: repo.hostsystem.project.name, repository: repo.hostsystem.name)
-      end
+      xml_repository.hostsystem(project: repo.hostsystem.project.name, repository: repo.hostsystem.name) if repo.hostsystem
       repo.path_elements.includes(:link).each do |pe|
         if pe.link.remote_project_name.present?
           project_name = pe.link.project.name + ':' + pe.link.remote_project_name

src/api/db/migrate/20170704212201_increase_timestamp_precision.rb

@@ -2,19 +2,15 @@ class IncreaseTimestampPrecision < ActiveRecord::Migration[5.1]
   def up
     ActiveRecord::Base.connection.tables.each do |table|
       ActiveRecord::Base.connection.columns(table).each do |column|
-        if column.type == :datetime && column.name == 'updated_at'
-          change_column table, column.name, :datetime, limit: 6
-        end
+        change_column table, column.name, :datetime, limit: 6 if column.type == :datetime && column.name == 'updated_at'
       end
     end
   end
 
   def down
     ActiveRecord::Base.connection.tables.each do |table|
       ActiveRecord::Base.connection.columns(table).each do |column|
-        if column.type == :datetime && column.name == 'updated_at'
-          change_column table, column.name, :datetime
-        end
+        change_column table, column.name, :datetime if column.type == :datetime && column.name == 'updated_at'
       end
     end
   end

src/api/lib/tasks/extract.rake

@@ -126,12 +126,8 @@ namespace :db do
             end
           end
           if table_name == 'taggings'
-            if record['taggable_type'] == 'Project'
-              record['taggable_id'] = ActiveRecord::FixtureSet.identify(Project.find(record['taggable_id']).name.tr(':', '_'))
-            end
-            if record['taggable_type'] == 'Package'
-              record['taggable_id'] = ActiveRecord::FixtureSet.identify(Package.find(record['taggable_id']).fixtures_name)
-            end
+            record['taggable_id'] = ActiveRecord::FixtureSet.identify(Project.find(record['taggable_id']).name.tr(':', '_')) if record['taggable_type'] == 'Project'
+            record['taggable_id'] = ActiveRecord::FixtureSet.identify(Package.find(record['taggable_id']).fixtures_name) if record['taggable_type'] == 'Package'
           end
 
           if table_name == 'distributions'
@@ -143,19 +139,13 @@ namespace :db do
           key = idtokey[id]
           key = nil if key == defaultkey
 
-          if table_name == 'roles_users'
-            defaultkey = "#{record['user']}_#{record['role']}"
-          end
-          if table_name == 'roles_static_permissions'
-            defaultkey = "#{record['role']}_#{record['static_permission']}"
-          end
+          defaultkey = "#{record['user']}_#{record['role']}" if table_name == 'roles_users'
+          defaultkey = "#{record['role']}_#{record['static_permission']}" if table_name == 'roles_static_permissions'
           if table_name == 'projects' || table_name == 'architectures'
             key = record['name'].tr(':', '_')
             record.delete(primary)
           end
-          if ['static_permissions', 'packages'].include?(table_name)
-            key = classname.find(record.delete(primary)).fixtures_name
-          end
+          key = classname.find(record.delete(primary)).fixtures_name if ['static_permissions', 'packages'].include?(table_name)
           defaultkey = record['package'] if table_name == 'backend_packages'
           if ['event_subscriptions', 'ratings', 'package_kinds', 'package_issues',
               'linked_db_projects', 'relationships', 'watched_projects', 'path_elements',

src/api/lib/xpath_engine.rb

@@ -257,13 +257,9 @@ def find(xpath)
     end
     # logger.debug "starting stack: #{@stack.inspect}"
 
-    if @stack.shift != :document
-      raise IllegalXpathError, 'xpath expression has to begin with root node'
-    end
+    raise IllegalXpathError, 'xpath expression has to begin with root node' if @stack.shift != :document
 
-    if @stack.shift != :child
-      raise IllegalXpathError, 'xpath expression has to begin with root node'
-    end
+    raise IllegalXpathError, 'xpath expression has to begin with root node' if @stack.shift != :child
 
     @stack.shift
     @stack.shift
@@ -282,9 +278,7 @@ def find(xpath)
       when :self
         raise IllegalXpathError, "axis '#{token}' not supported"
       when :child
-        if @stack.shift != :qname
-          raise IllegalXpathError, "non :qname token after :child token: #{token.inspect}"
-        end
+        raise IllegalXpathError, "non :qname token after :child token: #{token.inspect}" if @stack.shift != :qname
 
         @stack.shift # namespace
         @stack.shift # node
@@ -359,9 +353,7 @@ def find(xpath)
       logger.debug "strange base table: #{@base_table}"
     end
     cond_ary = nil
-    if @conditions.count.positive?
-      cond_ary = [@conditions.flatten.uniq.join(' AND '), @condition_values].flatten
-    end
+    cond_ary = [@conditions.flatten.uniq.join(' AND '), @condition_values].flatten if @conditions.count.positive?
 
     logger.debug("#{relation.to_sql}.find #{{ joins: @joins.flatten.uniq.join(' '),
                                               conditions: cond_ary }.inspect}")
@@ -382,9 +374,7 @@ def parse_predicate(root, stack)
       when :function
         fname = stack.shift
         fname_int = 'xpath_func_' + fname.tr('-', '_')
-        unless respond_to?(fname_int)
-          raise IllegalXpathError, "unknown xpath function '#{fname}'"
-        end
+        raise IllegalXpathError, "unknown xpath function '#{fname}'" unless respond_to?(fname_int)
 
         __send__(fname_int, root, *stack.shift)
       when :child
@@ -413,9 +403,7 @@ def parse_predicate(root, stack)
       when *@operators
         opname = token.to_s
         opname_int = 'xpath_op_' + opname
-        unless respond_to?(opname_int)
-          raise IllegalXpathError, "unhandled xpath operator '#{opname}'"
-        end
+        raise IllegalXpathError, "unhandled xpath operator '#{opname}'" unless respond_to?(opname_int)
 
         __send__(opname_int, root, *stack)
         stack = []
@@ -449,9 +437,7 @@ def evaluate_expr(expr, root, escape = false)
 
         if @last_key && @attribs[table][@last_key][:split]
           tvalues = value.split(@attribs[table][@last_key][:split])
-          if tvalues.size != 2
-            raise XpathEngine::IllegalXpathError, 'attributes must be $NAMESPACE:$NAME'
-          end
+          raise XpathEngine::IllegalXpathError, 'attributes must be $NAMESPACE:$NAME' if tvalues.size != 2
 
           @condition_values_needed.times { @condition_values << tvalues }
         elsif @last_key && @attribs[table][@last_key][:double]

src/api/script/import_database.rb

@@ -48,9 +48,7 @@ def init
   # There is only the filename given
   abort('No parameters, use --help') if @params.count == 1
 
-  if @params[:all] && (@params[:import] || @params[:load] || @params[:path])
-    abort('The --all parameter is not valid in combination with --import, --load or --filename')
-  end
+  abort('The --all parameter is not valid in combination with --import, --load or --filename') if @params[:all] && (@params[:import] || @params[:load] || @params[:path])
 
   if @params[:load] &&
      @params[:filename]
@@ -68,9 +66,7 @@ def load_dump
   filename = options[environment]['backup_filename']
   port = options[environment]['backup_port']
 
-  if !server || !username || !location || !filename
-    abort('Please specify at least backup_server, backup_user, backup_location and backup_filename in your options.yml')
-  end
+  abort('Please specify at least backup_server, backup_user, backup_location and backup_filename in your options.yml') if !server || !username || !location || !filename
 
   puts 'Downloading database backup ...'
   `scp -v -P #{port || 22} #{username}@#{server}:#{File.join(location, filename)} #{@data_path}`

src/api/spec/factories/bs_requests.rb

@@ -43,13 +43,9 @@
                                group_name: evaluator.group_name,
                                person_name: evaluator.person_name)
 
-      if attribs[:source_project].is_a?(Project)
-        attribs[:source_project] = attribs[:source_project].name
-      end
+      attribs[:source_project] = attribs[:source_project].name if attribs[:source_project].is_a?(Project)
 
-      if attribs[:target_project].is_a?(Project)
-        attribs[:target_project] = attribs[:target_project].name
-      end
+      attribs[:target_project] = attribs[:target_project].name if attribs[:target_project].is_a?(Project)
 
       if attribs[:source_package].is_a?(Package)
         attribs[:source_project] ||= attribs[:source_package].project.name

src/api/spec/factories/packages.rb

@@ -33,9 +33,7 @@
 
       after(:create) do |package, evaluator|
         evaluator.revision_count.times do |i|
-          if CONFIG['global_write_through']
-            Backend::Connection.put("/source/#{package.project}/#{package}/somefile.txt", i.to_s)
-          end
+          Backend::Connection.put("/source/#{package.project}/#{package}/somefile.txt", i.to_s) if CONFIG['global_write_through']
         end
       end
     end
@@ -90,19 +88,15 @@
       after(:create) do |package|
         # NOTE: Enable global write through when writing new VCR cassetes.
         # ensure the backend knows the project
-        if CONFIG['global_write_through']
-          Backend::Connection.put("/source/#{URI.escape(package.project.name)}/#{URI.escape(package.name)}/_service", '<services/>')
-        end
+        Backend::Connection.put("/source/#{URI.escape(package.project.name)}/#{URI.escape(package.name)}/_service", '<services/>') if CONFIG['global_write_through']
       end
     end
 
     factory :package_with_broken_service do
       after(:create) do |package|
         # NOTE: Enable global write through when writing new VCR cassetes.
         # ensure the backend knows the project
-        if CONFIG['global_write_through']
-          Backend::Connection.put("/source/#{URI.escape(package.project.name)}/#{URI.escape(package.name)}/_service", '<service>broken</service>')
-        end
+        Backend::Connection.put("/source/#{URI.escape(package.project.name)}/#{URI.escape(package.name)}/_service", '<service>broken</service>') if CONFIG['global_write_through']
       end
     end
 

src/api/spec/factories/project.rb

@@ -27,13 +27,9 @@
     end
 
     after(:create) do |project, evaluator|
-      if evaluator.link_to
-        LinkedProject.create(project: project, linked_db_project: evaluator.link_to)
-      end
+      LinkedProject.create(project: project, linked_db_project: evaluator.link_to) if evaluator.link_to
 
-      if evaluator.project_config
-        project.config.save({ user: 'factory bot' }, evaluator.project_config)
-      end
+      project.config.save({ user: 'factory bot' }, evaluator.project_config) if evaluator.project_config
 
       project.write_to_backend
     end
@@ -61,9 +57,7 @@
       after(:create) do |project, evaluator|
         new_package = create(:package, { project: project, name: evaluator.package_name }.compact)
         project.packages << new_package
-        if evaluator.create_patchinfo
-          Patchinfo.new.create_patchinfo(project.name, new_package.name, comment: 'Fake comment', force: true)
-        end
+        Patchinfo.new.create_patchinfo(project.name, new_package.name, comment: 'Fake comment', force: true) if evaluator.create_patchinfo
       end
     end
 
@@ -79,19 +73,13 @@
       after(:create) do |project, evaluator|
         evaluator.package_count.times do |index|
           package_title = nil
-          if evaluator.package_title
-            package_title = "#{evaluator.package_title}_#{index}"
-          end
+          package_title = "#{evaluator.package_title}_#{index}" if evaluator.package_title
 
           package_description = nil
-          if evaluator.package_description
-            package_description = "#{evaluator.package_description}_#{index}"
-          end
+          package_description = "#{evaluator.package_description}_#{index}" if evaluator.package_description
 
           package_name = nil
-          if evaluator.package_name
-            package_name = "#{evaluator.package_name}_#{index}"
-          end
+          package_name = "#{evaluator.package_name}_#{index}" if evaluator.package_name
 
           new_package = create(:package, {
             project: project,
@@ -100,9 +88,7 @@
             description: package_description
           }.compact)
           project.packages << new_package
-          if evaluator.create_patchinfo
-            Patchinfo.new.create_patchinfo(project.name, new_package.name, comment: 'Fake comment', force: true)
-          end
+          Patchinfo.new.create_patchinfo(project.name, new_package.name, comment: 'Fake comment', force: true) if evaluator.create_patchinfo
         end
       end
     end

src/api/spec/features/webui/packages_spec.rb

@@ -407,9 +407,7 @@
         login other_user
         visit project_show_path(project: global_project)
         # TODO: Remove `if` once responsive_ux is out of beta.
-        if page.has_link?('Actions')
-          click_link('Actions')
-        end
+        click_link('Actions') if page.has_link?('Actions')
         expect(page).not_to have_link('Create package')
 
         # Use direct path instead

src/api/spec/rails_helper.rb

@@ -6,9 +6,7 @@
   require 'support/coverage'
 end
 # to clean old unused cassettes
-if ENV['CLEAN_UNUSED_CASSETTES']
-  require 'cassette_rewinder'
-end
+require 'cassette_rewinder' if ENV['CLEAN_UNUSED_CASSETTES']
 
 ENV['RAILS_ENV'] ||= 'test'
 require File.expand_path('../config/environment', __dir__)

src/api/test/functional/maintenance_test.rb

@@ -1601,9 +1601,7 @@ def test_create_maintenance_project_and_release_packages
     assert_equal 'old_crap', pac['format']['rpm:obsoletes']['rpm:entry']['name']
     if File.exist?('/var/adm/fillup-templates') || File.exist?('/usr/share/fillup-templates/')
       # seems to be a SUSE system
-      if pac['format']['rpm:suggests'].nil?
-        print 'createrepo seems not to create week dependencies, we want this on SUSE systems'
-      end
+      print 'createrepo seems not to create week dependencies, we want this on SUSE systems' if pac['format']['rpm:suggests'].nil?
       assert_equal 'pure_optional', pac['format']['rpm:suggests']['rpm:entry']['name']
       assert_equal 'would_be_nice', pac['format']['rpm:recommends']['rpm:entry']['name']
       assert_equal 'other_package_likes_it', pac['format']['rpm:supplements']['rpm:entry']['name']

src/api/test/functional/published_controller_test.rb

@@ -133,9 +133,7 @@ def test_rpm_md_formats
         next unless File.exist?('/var/adm/fillup-templates') || File.exist?('/usr/share/fillup-templates/')
 
         # seems to be a SUSE system
-        if p['format']['rpm:suggests'].nil?
-          print 'createrepo seems not to create week dependencies, we need this at least on SUSE systems'
-        end
+        print 'createrepo seems not to create week dependencies, we need this at least on SUSE systems' if p['format']['rpm:suggests'].nil?
         assert_equal 'pure_optional', p['format']['rpm:suggests']['rpm:entry']['name']
         assert_equal 'would_be_nice', p['format']['rpm:recommends']['rpm:entry']['name']
         assert_equal 'other_package_likes_it', p['format']['rpm:supplements']['rpm:entry']['name']

src/api/test/functional/request_controller_test.rb

@@ -304,9 +304,7 @@ def test_submit_request_of_new_package
     assert_equal(node.elements('state').first.value('when'), node.elements('history').last.value('when'), 'Current state is has NOT same time as last history item')
     oldhistory = nil
     node.elements('history') do |h|
-      unless h
-        assert((h.value('when') > oldhistory.value('when')), 'Current history is not newer than the former history')
-      end
+      assert((h.value('when') > oldhistory.value('when')), 'Current history is not newer than the former history') unless h
       oldhistory = h
     end