Merge pull request #2559 from DavidKang/zero_length_predicate

[ci][webui] Enable Style/ZeroLengthPredicate rubocop cop

src/api/.rubocop.yml

@@ -141,6 +141,9 @@ Style/SpaceInsideBrackets:
 Style/TernaryParentheses:
   Enabled: true
 
+# Checks for receiver.length == 0 predicates and the negated versions receiver.length > 0 and receiver.length != 0.
+Style/ZeroLengthPredicate:
+  Enabled: true
 ##################### Metrics ##################################
 
 # Checks if the length a class exceeds some maximum value

src/api/.rubocop_todo.yml

@@ -970,8 +970,3 @@ Style/VariableName:
 # SupportedStyles: percent, brackets
 Style/WordArray:
   Enabled: false
-
-# Offense count: 28
-# Cop supports --auto-correct.
-Style/ZeroLengthPredicate:
-  Enabled: false

src/api/app/controllers/search_controller.rb

@@ -238,7 +238,7 @@ def search(what, render_all)
       next if xml[item.id]
       xml[item.id] = render_all ? item.to_axml(opts) : item.to_axml_id
       xml[item.id].gsub!(/(..*)/, "  \\1") # indent it by two spaces, if line is not empty
-    end if items.size > 0
+    end unless items.empty?
 
     items.each do |i|
       output << xml[i]

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

@@ -100,7 +100,7 @@ def show
     @my_open_reviews = @req['my_open_reviews']
     @other_open_reviews = @req['other_open_reviews']
     # rubocop:disable Metrics/LineLength
-    @can_add_reviews = %w(new review).include?(@state) && (@is_author || @is_target_maintainer || @my_open_reviews.length > 0) && !User.current.is_nobody?
+    @can_add_reviews = %w(new review).include?(@state) && (@is_author || @is_target_maintainer || !@my_open_reviews.empty?) && !User.current.is_nobody?
     # rubocop:enable Metrics/LineLength
     @can_handle_request = %w(new review declined).include?(@state) && (@is_target_maintainer || @is_author) && !User.current.is_nobody?
 

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

@@ -66,7 +66,7 @@ def search
     end
 
     logger.debug "Searching for the string \"#{@search_text}\" in the #{@search_where}'s of #{@search_what}'s"
-    if @search_where.length < 1 && !@search_attrib_type_id && !@search_issue
+    if @search_where.empty? && !@search_attrib_type_id && !@search_issue
       flash[:error] = "You have to search for #{@search_text} in something. Click the advanced button..."
       return
     end

src/api/app/helpers/webui/project_helper.rb

@@ -55,7 +55,7 @@ def project_bread_crumb(*args)
       prj_parents.each do |name, short_name|
         project_list << link_to(short_name, project_show_path(project: name))
       end
-      @crumb_list << project_list if project_list.length > 0
+      @crumb_list << project_list unless project_list.empty?
     end
     @crumb_list = @crumb_list + args
   end

src/api/app/jobs/consistency_check.rb

@@ -82,7 +82,7 @@ def project_meta_check(project, fix = nil)
     backend_hash['description'] = api_hash['description'] = nil
 
     diff = hash_diff(api_hash, backend_hash)
-    if diff.size > 0
+    unless diff.empty?
       errors << "Project meta is different in backend for #{project.name}\n#{diff}\n"
       if fix
         # Assume that api is right

src/api/app/models/binary_release.rb

@@ -150,7 +150,7 @@ def render_xml
       node = {}
       node[:package] = release_package.name if release_package
       node[:time] = self.binary_releasetime if self.binary_releasetime
-      binary.publish(node) if node.length > 0
+      binary.publish(node) unless node.empty?
 
       binary.build(time: binary_buildtime) if binary_buildtime
       binary.modify(time: modify_time) if modify_time

src/api/app/models/bs_request.rb

@@ -1011,22 +1011,22 @@ def apply_default_reviewers
     # apply reviewers
     reviewers.uniq.each do |r|
       if r.class == User
-        next if reviews.select{|a| a.by_user == r.login}.length > 0
+        next unless reviews.select{|a| a.by_user == r.login}.empty?
         reviews.new(by_user: r.login, state: :new)
       elsif r.class == Group
-        next if reviews.select{|a| a.by_group == r.title}.length > 0
+        next unless reviews.select{|a| a.by_group == r.title}.empty?
         reviews.new(by_group: r.title, state: :new)
       elsif r.class == Project
-        next if reviews.select{|a| a.by_project == r.name && a.by_package.nil? }.length > 0
+        next unless reviews.select{|a| a.by_project == r.name && a.by_package.nil? }.empty?
         reviews.new(by_project: r.name, state: :new)
       elsif r.class == Package
-        next if reviews.select{|a| a.by_project == r.project.name && a.by_package == r.name }.length > 0
+        next unless reviews.select{|a| a.by_project == r.project.name && a.by_package == r.name }.empty?
         reviews.new(by_project: r.project.name, by_package: r.name, state: :new)
       else
         raise 'Unknown review type'
       end
     end
-    self.state = :review if reviews.select{|a| a.state == :new}.length > 0
+    self.state = :review unless reviews.select{|a| a.state == :new}.empty?
   end
 
   def notify

src/api/app/models/bs_request_action_maintenance_release.rb

@@ -79,10 +79,10 @@ def sanity_check!
     # get sure that the releasetarget definition exists or we release without binaries
     prj = Project.get_by_name(source_project)
     prj.repositories.includes(:release_targets).each do |repo|
-      unless repo.release_targets.size > 0
+      if repo.release_targets.empty?
         raise RepositoryWithoutReleaseTarget.new "Release target definition is missing in #{prj.name} / #{repo.name}"
       end
-      unless repo.architectures.size > 0
+      if repo.architectures.empty?
         raise RepositoryWithoutArchitecture.new "Repository has no architecture #{prj.name} / #{repo.name}"
       end
       repo.release_targets.each do |rt|

src/api/app/models/channel.rb

@@ -124,7 +124,7 @@ def update_from_xml(xmlhash)
       end
       hasharray.flatten!
       # no match? either not created or searched in the right way
-      raise "Unable to find binary list #{cbl.project.name} #{cbl.repository.name} #{cbl.architecture.name}" if hasharray.size < 1
+      raise "Unable to find binary list #{cbl.project.name} #{cbl.repository.name} #{cbl.architecture.name}" if hasharray.empty?
       # update...
       _update_from_xml_binaries(cbl, hasharray)
     }
@@ -150,7 +150,7 @@ def is_active?
     # no targets defined, the project has some
     return true if channel_targets.size.zero?
 
-    channel_targets.where(disabled: false).size > 0
+    !channel_targets.where(disabled: false).empty?
   end
 
   def add_channel_repos_to_project(tpkg, mode = nil)

src/api/app/models/owner.rb

@@ -138,7 +138,7 @@ def self.find_assignees(rootproject, binary_name, limit = 1, devel = true, filte
       end
     end
 
-    return instances_without_definition if webui_mode && maintainers.length < 1
+    return instances_without_definition if webui_mode && maintainers.empty?
 
     maintainers << deepest_match if deepest_match
 
@@ -344,7 +344,7 @@ def self._extract_from_container(m, r, sql, objfilter)
     end unless objfilter.class == Group
 
     r.groups.where(groupsql).each do |p|
-      next unless p.group.users.where(state: "confirmed").length > 0
+      next if p.group.users.where(state: "confirmed").empty?
       m.groups ||= {}
       m.groups[p.role.title] ||= []
       m.groups[p.role.title] << p.group.title
@@ -355,7 +355,7 @@ def self._extract_from_container(m, r, sql, objfilter)
   def self._build_rolefilter_sql(rolefilter)
     # construct where condition
     sql = nil
-    if rolefilter.length > 0
+    if !rolefilter.empty?
       rolefilter.each do |rf|
         if sql.nil?
           sql = "( "

src/api/app/models/product.rb

@@ -9,7 +9,7 @@ def self.find_or_create_by_name_and_package( name, package )
     raise Product::NotFoundError.new( "Error: Package not valid." ) unless package.class == Package
     product = find_by_name_and_package name, package
 
-    product = create( name: name, package: package ) unless product.length > 0
+    product = create( name: name, package: package ) if product.empty?
 
     product
   end
@@ -58,7 +58,7 @@ def update_from_xml(xml)
         pd.elements("mediasets") do |ms|
           ms.elements("media") do |m|
             # product depends on others, so it is no standalone operating system
-            swClass = "a" if m.elements("productdependency").length > 0
+            swClass = "a" unless m.elements("productdependency").empty?
           end
         end
         pd.elements('products') do |ps|

src/api/app/models/project.rb

@@ -1225,7 +1225,7 @@ def add_repository_with_targets(repoName, source_repo, add_target_repos = [], op
 
     trigger = nil # no trigger is set by default
     trigger = 'maintenance' if is_maintenance_incident?
-    if add_target_repos.length > 0
+    unless add_target_repos.empty?
       # add repository targets
       add_target_repos.each do |repo|
         trepo.release_targets.create(target_repository: repo, trigger: trigger)
@@ -1808,7 +1808,7 @@ def self.remove_repositories(repositories, opts = {})
       # full remove, otherwise the model will take care of the cleanup
       if opts[:recursive_remove]
         # recursive for INDIRECT linked repositories
-        unless linking_repositories.length < 1
+        unless linking_repositories.empty?
           # FIXME: we would actually need to check for :no_write_to_backend here as well
           #        but the calling code is currently broken and would need the starting
           #        project different

src/api/app/models/repository.rb

@@ -73,7 +73,7 @@ def cleanup_before_destroy
     linking_repositories.each do |lrep|
       lrep.path_elements.includes(:link, :repository).each do |pe|
         next unless pe.link == self # this is not pointing to our repo
-        if lrep.path_elements.where(repository_id: Repository.deleted_instance).size > 0
+        if !lrep.path_elements.where(repository_id: Repository.deleted_instance).empty?
           # repo has already a path element pointing to deleted repository
           pe.destroy
         else
@@ -89,7 +89,7 @@ def cleanup_before_destroy
       lrep.targetlinks.includes(:target_repository, :repository).each do |rt|
         next unless rt.target_repository == self # this is not pointing to our repo
         repo = rt.repository
-        if lrep.targetlinks.where(repository_id: Repository.deleted_instance).size > 0
+        if !lrep.targetlinks.where(repository_id: Repository.deleted_instance).empty?
           # repo has already a path element pointing to deleted repository
           logger.debug "destroy release target #{rt.target_repository.project.name}/#{rt.target_repository.name}"
           rt.destroy

src/api/app/models/status_history_rescaler.rb

@@ -32,7 +32,7 @@ def find_items_for_maxtime(key, maxtimeoffset)
   def find_start_items(allitems, max)
     items = []
 
-    items << allitems.shift while allitems.length > 0 && allitems[0].time < max
+    items << allitems.shift while !allitems.empty? && allitems[0].time < max
     items
   end
 

src/api/app/policies/group_policy.rb

@@ -13,7 +13,7 @@ def update?
     return true if @user.is_admin?
 
     # and also group maintainers
-    @record.group_maintainers.where(user: @user).length > 0
+    !@record.group_maintainers.where(user: @user).empty?
   end
 
   def destroy?

src/api/script/import

@@ -142,7 +142,7 @@ end
 
 run_count = 0
 
-while cache.length > 0
+until cache.empty?
   STDERR.puts "--> trying to store previously unsaved projects, run [#{run_count += 1}]"
   tmp = cache.values.clone
   tmp.each do |p|

src/api/test/functional/webui/spider_test.rb

@@ -88,7 +88,7 @@ def raiseit(message, url)
   end
 
   def crawl
-    while @pages_to_visit.length > 0
+    until @pages_to_visit.empty?
       theone = @pages_to_visit.keys.sort.first
       @pages_visited[theone] = @pages_to_visit[theone]
       @pages_to_visit.delete theone