(PUP-11993) Style/Next

This commit enables the Style/Next cop and fixes 152 autocorrectable
offenses.

Author: AriaXLi

.rubocop_todo.yml

@@ -629,12 +629,6 @@ Style/MultipleComparison:
 Style/MutableConstant:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, MinBodyLength.
-# SupportedStyles: skip_modifier_ifs, always
-Style/Next:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: predicate, comparison

lib/puppet/face/module/list.rb

@@ -123,22 +123,22 @@ def unmet_dependencies(environment)
         acc[dep[:reason]] << dep
       end
       unmet_grouped.each do |type, deps|
-        unless deps.empty?
-          unmet_grouped[type].sort_by { |dep| dep[:name] }.each do |dep|
-            dep_name           = dep[:name].tr('/', '-')
-            installed_version  = dep[:mod_details][:installed_version]
-            version_constraint = dep[:version_constraint]
-            parent_name        = dep[:parent][:name].tr('/', '-')
-            parent_version     = dep[:parent][:version]
-
-            msg = _("'%{parent_name}' (%{parent_version}) requires '%{dependency_name}' (%{dependency_version})") % { parent_name: parent_name, parent_version: parent_version, dependency_name: dep_name, dependency_version: version_constraint }
-            unmet_deps[type][dep[:name]][:errors] << msg
-            unmet_deps[type][dep[:name]][:parent] = {
-              :name => dep[:parent][:name],
-              :version => parent_version
-            }
-            unmet_deps[type][dep[:name]][:version] = installed_version
-          end
+        next if deps.empty?
+
+        unmet_grouped[type].sort_by { |dep| dep[:name] }.each do |dep|
+          dep_name           = dep[:name].tr('/', '-')
+          installed_version  = dep[:mod_details][:installed_version]
+          version_constraint = dep[:version_constraint]
+          parent_name        = dep[:parent][:name].tr('/', '-')
+          parent_version     = dep[:parent][:version]
+
+          msg = _("'%{parent_name}' (%{parent_version}) requires '%{dependency_name}' (%{dependency_version})") % { parent_name: parent_name, parent_version: parent_version, dependency_name: dep_name, dependency_version: version_constraint }
+          unmet_deps[type][dep[:name]][:errors] << msg
+          unmet_deps[type][dep[:name]][:parent] = {
+            :name => dep[:parent][:name],
+            :version => parent_version
+          }
+          unmet_deps[type][dep[:name]][:version] = installed_version
         end
       end
     end
@@ -151,24 +151,24 @@ def warn_unmet_dependencies(environment)
     # Display unmet dependencies by category.
     error_display_order = [:non_semantic_version, :version_mismatch, :missing]
     error_display_order.each do |type|
-      unless @unmet_deps[type].empty?
-        @unmet_deps[type].keys.sort.each do |dep|
-          name    = dep.tr('/', '-')
-          errors  = @unmet_deps[type][dep][:errors]
-          version = @unmet_deps[type][dep][:version]
-
-          msg = case type
-                when :version_mismatch
-                  _("Module '%{name}' (v%{version}) fails to meet some dependencies:\n") % { name: name, version: version }
-                when :non_semantic_version
-                  _("Non semantic version dependency %{name} (v%{version}):\n") % { name: name, version: version }
-                else
-                  _("Missing dependency '%{name}':\n") % { name: name }
-                end
-
-          errors.each { |error_string| msg << "  #{error_string}\n" }
-          Puppet.warning msg.chomp
-        end
+      next if @unmet_deps[type].empty?
+
+      @unmet_deps[type].keys.sort.each do |dep|
+        name    = dep.tr('/', '-')
+        errors  = @unmet_deps[type][dep][:errors]
+        version = @unmet_deps[type][dep][:version]
+
+        msg = case type
+              when :version_mismatch
+                _("Module '%{name}' (v%{version}) fails to meet some dependencies:\n") % { name: name, version: version }
+              when :non_semantic_version
+                _("Non semantic version dependency %{name} (v%{version}):\n") % { name: name, version: version }
+              else
+                _("Missing dependency '%{name}':\n") % { name: name }
+              end
+
+        errors.each { |error_string| msg << "  #{error_string}\n" }
+        Puppet.warning msg.chomp
       end
     end
   end

lib/puppet/file_serving/fileset.rb

@@ -165,11 +165,11 @@ def perform_recursion
     result = []
 
     while entry = current_dirs.shift # rubocop:disable Lint/AssignmentInCondition
-      if continue_recursion_at?(entry.depth + 1)
-        entry.children.each do |child|
-          result << child.path
-          current_dirs << child
-        end
+      next unless continue_recursion_at?(entry.depth + 1)
+
+      entry.children.each do |child|
+        result << child.path
+        current_dirs << child
       end
     end
 

lib/puppet/graph/relationship_graph.rb

@@ -128,19 +128,19 @@ def traverse(options = {}, &block)
         deferred_resources << resource
       end
 
-      if @ready.empty? and deferred_resources.any?
-        if made_progress
-          enqueue(*deferred_resources)
-        else
-          deferred_resources.each do |res|
-            overly_deferred_resource_handler.call(res)
-            finish(res)
-          end
-        end
+      next unless @ready.empty? and deferred_resources.any?
 
-        made_progress = false
-        deferred_resources = []
+      if made_progress
+        enqueue(*deferred_resources)
+      else
+        deferred_resources.each do |res|
+          overly_deferred_resource_handler.call(res)
+          finish(res)
+        end
       end
+
+      made_progress = false
+      deferred_resources = []
     end
 
     unless continue_while.call()

lib/puppet/indirector/catalog/compiler.rb

@@ -237,32 +237,32 @@ def inline_metadata(catalog, checksum_type)
           source = Puppet::Type.type(:file).attrclass(:source).normalize(source)
 
           list_of_data = Puppet::FileServing::Metadata.indirection.search(source, options)
-          if list_of_data
-            basedir_meta = list_of_data.find { |meta| meta.relative_path == '.' }
-            devfail "FileServing::Metadata search should always return the root search path" if basedir_meta.nil?
-
-            unless inlineable_metadata?(basedir_meta, source, environment_path)
-              # If any source is not in the environment path, skip inlining this resource.
-              log_file_outside_environment
-              sources_in_environment = false
-              break
-            end
+          next unless list_of_data
 
-            base_content_uri = get_content_uri(basedir_meta, source, environment_path)
-            list_of_data.each do |metadata|
-              if metadata.relative_path == '.'
-                metadata.content_uri = base_content_uri
-              else
-                metadata.content_uri = "#{base_content_uri}/#{metadata.relative_path}"
-              end
-            end
+          basedir_meta = list_of_data.find { |meta| meta.relative_path == '.' }
+          devfail "FileServing::Metadata search should always return the root search path" if basedir_meta.nil?
+
+          unless inlineable_metadata?(basedir_meta, source, environment_path)
+            # If any source is not in the environment path, skip inlining this resource.
+            log_file_outside_environment
+            sources_in_environment = false
+            break
+          end
 
-            source_to_metadatas[source] = list_of_data
-            # Optimize for returning less data if sourceselect is first
-            if resource[:sourceselect] == 'first' || resource[:sourceselect].nil?
-              break
+          base_content_uri = get_content_uri(basedir_meta, source, environment_path)
+          list_of_data.each do |metadata|
+            if metadata.relative_path == '.'
+              metadata.content_uri = base_content_uri
+            else
+              metadata.content_uri = "#{base_content_uri}/#{metadata.relative_path}"
             end
           end
+
+          source_to_metadatas[source] = list_of_data
+          # Optimize for returning less data if sourceselect is first
+          if resource[:sourceselect] == 'first' || resource[:sourceselect].nil?
+            break
+          end
         end
 
         if sources_in_environment && !source_to_metadatas.empty?
@@ -282,11 +282,11 @@ def inline_metadata(catalog, checksum_type)
           source = Puppet::Type.type(:file).attrclass(:source).normalize(source)
 
           data = Puppet::FileServing::Metadata.indirection.find(source, options)
-          if data
-            metadata = data
-            metadata.source = source
-            break
-          end
+          next unless data
+
+          metadata = data
+          metadata.source = source
+          break
         end
 
         raise _("Could not get metadata for %{resource}") % { resource: resource[:source] } unless metadata

lib/puppet/indirector/facts/facter.rb

@@ -85,11 +85,11 @@ def self.setup_external_search_paths(request)
     # Add any per-module external fact directories to facter's external search path
     dirs = []
     request.environment.modules.each do |m|
-      if m.has_external_facts?
-        dir = m.plugin_fact_directory
-        Puppet.debug { "Loading external facts from #{dir}" }
-        dirs << dir
-      end
+      next unless m.has_external_facts?
+
+      dir = m.plugin_fact_directory
+      Puppet.debug { "Loading external facts from #{dir}" }
+      dirs << dir
     end
 
     # Add system external fact directory if it exists

lib/puppet/indirector/file_bucket_file/file.rb

@@ -64,14 +64,14 @@ def list(request)
       msg = ''.dup
       # Get all files with mtime between 'from' and 'to'
       Pathname.new(request.options[:bucket_path]).find { |item|
-        if item.file? and item.basename.to_s == "paths"
-          filenames = item.read.strip.split("\n")
-          filestat = Time.parse(item.stat.mtime.to_s)
-          if from <= filestat and filestat <= to
-            filenames.each do |filename|
-              bucket[filename] += [[item.stat.mtime, item.parent.basename]]
-            end
-          end
+        next unless item.file? and item.basename.to_s == "paths"
+
+        filenames = item.read.strip.split("\n")
+        filestat = Time.parse(item.stat.mtime.to_s)
+        next unless from <= filestat and filestat <= to
+
+        filenames.each do |filename|
+          bucket[filename] += [[item.stat.mtime, item.parent.basename]]
         end
       }
       # Sort the results

lib/puppet/module.rb

@@ -429,22 +429,22 @@ def unmet_dependencies
         next
       end
 
-      if version_string
-        begin
-          required_version_semver_range = self.class.parse_range(version_string)
-          actual_version_semver = SemanticPuppet::Version.parse(dep_mod.version)
-        rescue ArgumentError
-          error_details[:reason] = :non_semantic_version
-          unmet_dependencies << error_details
-          next
-        end
+      next unless version_string
 
-        unless required_version_semver_range.include? actual_version_semver
-          error_details[:reason] = :version_mismatch
-          unmet_dependencies << error_details
-          next
-        end
+      begin
+        required_version_semver_range = self.class.parse_range(version_string)
+        actual_version_semver = SemanticPuppet::Version.parse(dep_mod.version)
+      rescue ArgumentError
+        error_details[:reason] = :non_semantic_version
+        unmet_dependencies << error_details
+        next
       end
+
+      next if required_version_semver_range.include? actual_version_semver
+
+      error_details[:reason] = :version_mismatch
+      unmet_dependencies << error_details
+      next
     end
 
     unmet_dependencies