(PUP-11767) Enable Style/IfInsideElse

Author: tvpartytonight

.rubocop.yml

@@ -316,3 +316,6 @@ Style/HashTransformValues:
 
 Style/IdenticalConditionalBranches:
   Enabled: true
+
+Style/IfInsideElse:
+  Enabled: true

lib/puppet/face/help.rb

@@ -50,11 +50,9 @@
       if options.has_key? :version
         if options[:version].to_s !~ /^current$/i
           version = options[:version]
-        else
-          if args.length == 0
-            # TRANSLATORS '--version' is a command line option and should not be translated
-            raise ArgumentError, _("Supplying a '--version' only makes sense when a Faces subcommand is given")
-          end
+        elsif args.length == 0
+          raise ArgumentError, _("Supplying a '--version' only makes sense when a Faces subcommand is given")
+          # TRANSLATORS '--version' is a command line option and should not be translated
         end
       end
 
@@ -66,18 +64,16 @@
 
         # legacy apps already emit ronn output
         return render_application_help(facename)
+      elsif options[:ronn]
+        render_face_man(facename || :help)
+      # Calling `puppet help <app> --ronn` normally calls this action with
+      # <app> as the first argument in the `args` array. However, if <app>
+      # happens to match the name of an action, like `puppet help help
+      # --ronn`, then face_base "eats" the argument and `args` will be
+      # empty. Rather than force users to type `puppet help help help
+      # --ronn`, default the facename to `:help`
       else
-        if options[:ronn]
-          # Calling `puppet help <app> --ronn` normally calls this action with
-          # <app> as the first argument in the `args` array. However, if <app>
-          # happens to match the name of an action, like `puppet help help
-          # --ronn`, then face_base "eats" the argument and `args` will be
-          # empty. Rather than force users to type `puppet help help help
-          # --ronn`, default the facename to `:help`
-          render_face_man(facename || :help)
-        else
-          render_face_help(facename, actionname, version)
-        end
+        render_face_help(facename, actionname, version)
       end
     end
   end

lib/puppet/http/service.rb

@@ -107,12 +107,10 @@ def add_puppet_headers(headers)
     Puppet[:http_extra_headers].each do |name, value|
       if modified_headers.keys.find { |key| key.casecmp(name) == 0 }
         Puppet.warning(_('Ignoring extra header "%{name}" as it was previously set.') % { name: name })
+      elsif value.nil? || value.empty?
+        Puppet.warning(_('Ignoring extra header "%{name}" as it has no value.') % { name: name })
       else
-        if value.nil? || value.empty?
-          Puppet.warning(_('Ignoring extra header "%{name}" as it has no value.') % { name: name })
-        else
-          modified_headers[name] = value
-        end
+        modified_headers[name] = value
       end
     end
     modified_headers

lib/puppet/indirector/catalog/compiler.rb

@@ -323,15 +323,13 @@ def compile(node, options)
         benchmark_str = _("Compiled static catalog for %{node} in %%{seconds} seconds") % { node: escaped_node_name }
         profile_str   = _("Compiled static catalog for %{node}") % { node: node.name }
       end
+    elsif node.environment
+      escaped_node_environment = node.environment.to_s.gsub(/%/, '%%')
+      benchmark_str = _("Compiled catalog for %{node} in environment %{environment} in %%{seconds} seconds") % { node: escaped_node_name, environment: escaped_node_environment }
+      profile_str   = _("Compiled catalog for %{node} in environment %{environment}") % { node: node.name, environment: node.environment }
     else
-      if node.environment
-        escaped_node_environment = node.environment.to_s.gsub(/%/, '%%')
-        benchmark_str = _("Compiled catalog for %{node} in environment %{environment} in %%{seconds} seconds") % { node: escaped_node_name, environment: escaped_node_environment }
-        profile_str   = _("Compiled catalog for %{node} in environment %{environment}") % { node: node.name, environment: node.environment }
-      else
-        benchmark_str = _("Compiled catalog for %{node} in %%{seconds} seconds") % { node: escaped_node_name }
-        profile_str   = _("Compiled catalog for %{node}") % { node: node.name }
-      end
+      benchmark_str = _("Compiled catalog for %{node} in %%{seconds} seconds") % { node: escaped_node_name }
+      profile_str   = _("Compiled catalog for %{node}") % { node: node.name }
     end
     config = nil
 

lib/puppet/pal/pal_impl.rb

@@ -79,13 +79,11 @@ def self.with_script_compiler(
 
       # Use the manifest setting
       manifest_file = Puppet[:manifest]
-    else
+    elsif manifest_file.nil? && code_string.nil?
       # An "undef" code_string is the only way to override Puppet[:manifest] & Puppet[:code] settings since an
       # empty string is taken as Puppet[:code] not being set.
       #
-      if manifest_file.nil? && code_string.nil?
-        code_string = 'undef'
-      end
+      code_string = 'undef'
     end
 
     previous_tasks_value = Puppet[:tasks]
@@ -186,13 +184,11 @@ def self.with_catalog_compiler(
 
       # Use the manifest setting
       manifest_file = Puppet[:manifest]
-    else
+    elsif manifest_file.nil? && code_string.nil?
       # An "undef" code_string is the only way to override Puppet[:manifest] & Puppet[:code] settings since an
       # empty string is taken as Puppet[:code] not being set.
       #
-      if manifest_file.nil? && code_string.nil?
-        code_string = 'undef'
-      end
+      code_string = 'undef'
     end
 
     # We need to make sure to set these back when we're done

lib/puppet/parameter/value_collection.rb

@@ -154,8 +154,8 @@ def newvalue(name, options = {}, &block)
       devfail "Cannot use :call value ':none' in combination with a block for property '#{self.class.name}'" if call_opt == :none
       value.block = block
       value.method ||= "set_#{value.name}" if !value.regex?
-    else
-      devfail "Cannot use :call value ':instead' without a block for property '#{self.class.name}'" if call_opt == :instead
+    elsif call_opt == :instead
+      devfail "Cannot use :call value ':instead' without a block for property '#{self.class.name}'"
     end
     value
   end

lib/puppet/parser/resource.rb

@@ -305,14 +305,12 @@ def override_parameter(param)
                 _("Parameter '%{name}' is already set on %{resource} at %{error_location}; cannot redefine") %
                   { name: param.name, resource: ref, error_location: error_location_str }
               end
+            elsif error_location_str.empty?
+              _("Parameter '%{name}' is already set on %{resource} by %{source}; cannot redefine") %
+                { name: param.name, resource: ref, source: current.source.to_s }
             else
-              if error_location_str.empty?
-                _("Parameter '%{name}' is already set on %{resource} by %{source}; cannot redefine") %
-                  { name: param.name, resource: ref, source: current.source.to_s }
-              else
-                _("Parameter '%{name}' is already set on %{resource} by %{source} at %{error_location}; cannot redefine") %
-                  { name: param.name, resource: ref, source: current.source.to_s, error_location: error_location_str }
-              end
+              _("Parameter '%{name}' is already set on %{resource} by %{source} at %{error_location}; cannot redefine") %
+                { name: param.name, resource: ref, source: current.source.to_s, error_location: error_location_str }
             end
       raise Puppet::ParseError.new(msg, param.file, param.line)
     end

lib/puppet/pops/evaluator/access_operator.rb

@@ -349,12 +349,10 @@ def access_PObjectType(o, scope, keys)
     keys.flatten!
     if o.resolved? && !o.name.nil?
       Types::PObjectTypeExtension.create(o, keys)
+    elsif keys.size == 1
+      Types::TypeFactory.object(keys[0])
     else
-      if keys.size == 1
-        Types::TypeFactory.object(keys[0])
-      else
-        fail(Issues::BAD_TYPE_SLICE_ARITY, @semantic, { :base_type => 'Object-Type', :min => 1, :actual => keys.size })
-      end
+      fail(Issues::BAD_TYPE_SLICE_ARITY, @semantic, { :base_type => 'Object-Type', :min => 1, :actual => keys.size })
     end
   end
 

lib/puppet/pops/evaluator/closure.rb

@@ -253,14 +253,12 @@ def combine_values_with_parameters(scope, args)
                 # correct non array default value
                 value = [value]
               end
-            else
+            elsif param_captures
               # not given, does not have default
-              if param_captures
-                # default for captures rest is an empty array
-                value = []
-              else
-                @evaluator.fail(Issues::MISSING_REQUIRED_PARAMETER, parameter, { :param_name => parameter.name })
-              end
+              value = []
+            # default for captures rest is an empty array
+            else
+              @evaluator.fail(Issues::MISSING_REQUIRED_PARAMETER, parameter, { :param_name => parameter.name })
             end
           else
             given_argument = args[index]

lib/puppet/pops/functions/dispatch.rb

@@ -90,15 +90,13 @@ def weave(scope, args)
             else
               raise ArgumentError, _("Unknown injection %{injection_name}") % { injection_name: injection_name }
             end
-        else
+        elsif knit < 0
           # Careful so no new nil arguments are added since they would override default
           # parameter values in the received
-          if knit < 0
-            idx = -knit - 1
-            new_args += args[idx..] if idx < args.size
-          else
-            new_args << args[knit] if knit < args.size
-          end
+          idx = -knit - 1
+          new_args += args[idx..] if idx < args.size
+        elsif knit < args.size
+          new_args << args[knit]
         end
       end
       new_args