(PUP-11993) Style/UnlessElse

This commit enables the Style/UnlessElse cop and fixes 13
autocorrectable offenses.

Author: AriaXLi

.rubocop_todo.yml

@@ -762,18 +762,6 @@ Style/TrailingUnderscoreVariable:
 Style/TrivialAccessors:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/UnlessElse:
-  Exclude:
-    - 'lib/puppet/application/lookup.rb'
-    - 'lib/puppet/pops/evaluator/evaluator_impl.rb'
-    - 'lib/puppet/pops/types/type_mismatch_describer.rb'
-    - 'lib/puppet/provider/package/aix.rb'
-    - 'lib/puppet/provider/package/yum.rb'
-    - 'lib/puppet/ssl/openssl_loader.rb'
-    - 'lib/puppet/type/exec.rb'
-    - 'lib/puppet/util/command_line/trollop.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 Style/UnpackFirst:
   Exclude:

lib/puppet/application/lookup.rb

@@ -369,7 +369,9 @@ def generate_scope
       end
     end
 
-    unless node.is_a?(Puppet::Node) # to allow unit tests to pass a node instance
+    if node.is_a?(Puppet::Node)
+      node.add_extra_facts(given_facts) if given_facts
+    else # to allow unit tests to pass a node instance
       facts = retrieve_node_facts(node, given_facts)
       ni = Puppet::Node.indirection
       tc = ni.terminus_class
@@ -399,8 +401,6 @@ def generate_scope
         node = ni.find(node, facts: facts, environment: Puppet[:environment])
         ni.terminus_class = tc
       end
-    else
-      node.add_extra_facts(given_facts) if given_facts
     end
     node.environment = Puppet[:environment] if Puppet.settings.set_by_cli?(:environment)
     Puppet[:code] = 'undef' unless options[:compile]

lib/puppet/pops/evaluator/evaluator_impl.rb

@@ -1064,10 +1064,10 @@ def eval_IfExpression o, scope
   # Evaluates Puppet DSL `unless`
   def eval_UnlessExpression o, scope
     scope.with_guarded_scope do
-      unless is_true?(evaluate(o.test, scope), o.test)
-        evaluate(o.then_expr, scope)
-      else
+      if is_true?(evaluate(o.test, scope), o.test)
         evaluate(o.else_expr, scope)
+      else
+        evaluate(o.then_expr, scope)
       end
     end
   end

lib/puppet/pops/types/type_mismatch_describer.rb

@@ -954,9 +954,7 @@ def describe_PCallableType(expected, original, actual, path)
           if param_errors.empty?
             this_return_t = expected.return_type || PAnyType::DEFAULT
             that_return_t = actual.return_type || PAnyType::DEFAULT
-            unless this_return_t.assignable?(that_return_t)
-              [TypeMismatch.new(path + [ReturnTypeElement.new], this_return_t, that_return_t)]
-            else
+            if this_return_t.assignable?(that_return_t)
               # names are ignored, they are just information
               # Blocks must be compatible
               this_block_t = expected.block_type || PUndefType::DEFAULT
@@ -966,6 +964,8 @@ def describe_PCallableType(expected, original, actual, path)
               else
                 [TypeMismatch.new(path + [BlockPathElement.new], this_block_t, that_block_t)]
               end
+            else
+              [TypeMismatch.new(path + [ReturnTypeElement.new], this_return_t, that_return_t)]
             end
           else
             param_errors

lib/puppet/provider/package/aix.rb

@@ -152,12 +152,12 @@ def self.instances
   def latest
     upd = latest_info
 
-    unless upd.nil?
-      (upd[:version]).to_s
-    else
+    if upd.nil?
       raise Puppet::DevError, _("Tried to get latest on a missing package") if properties[:ensure] == :absent
 
       properties[:ensure]
+    else
+      (upd[:version]).to_s
     end
   end
 

lib/puppet/provider/package/yum.rb

@@ -333,17 +333,17 @@ def install
   # What's the latest package version available?
   def latest
     upd = self.class.latest_package_version(@resource[:name], disablerepo, enablerepo, disableexcludes)
-    unless upd.nil?
-      # FIXME: there could be more than one update for a package
-      # because of multiarch
-      "#{upd[:epoch]}:#{upd[:version]}-#{upd[:release]}"
-    else
+    if upd.nil?
       # Yum didn't find updates, pretend the current version is the latest
       debug "Yum didn't find updates, current version (#{properties[:ensure]}) is the latest"
       version = properties[:ensure]
       raise Puppet::DevError, _("Tried to get latest on a missing package") if version == :absent || version == :purged
 
       version
+    else
+      # FIXME: there could be more than one update for a package
+      # because of multiarch
+      "#{upd[:epoch]}:#{upd[:version]}-#{upd[:release]}"
     end
   end
 

lib/puppet/ssl/openssl_loader.rb

@@ -7,10 +7,7 @@
 # core Puppet code to load correctly in JRuby environments that do not
 # have a functioning openssl (eg a FIPS enabled one).
 
-unless Puppet::Util::Platform.jruby_fips?
-  require 'openssl'
-  require 'net/https'
-else
+if Puppet::Util::Platform.jruby_fips?
   # Even in JRuby we need to define the constants that are wrapped in
   # Indirections: Puppet::SSL::{Key, Certificate, CertificateRequest}
   module OpenSSL
@@ -23,4 +20,7 @@ class Request; end
       class Certificate; end
     end
   end
+else
+  require 'openssl'
+  require 'net/https'
 end

lib/puppet/type/exec.rb

@@ -163,10 +163,10 @@ def sync
           when :true
             log = @resource[:loglevel]
           when :on_failure
-            unless should.include?(@status.exitstatus.to_s)
-              log = @resource[:loglevel]
-            else
+            if should.include?(@status.exitstatus.to_s)
               log = :false
+            else
+              log = @resource[:loglevel]
             end
           end
           unless log == :false

lib/puppet/util/command_line/trollop.rb

@@ -425,13 +425,13 @@ def parse cmdline = ARGV
         end
 
         if SINGLE_ARG_TYPES.include?(opts[:type])
-          unless opts[:multi]       # single parameter
-            vals[sym] = vals[sym][0][0]
-          else                      # multiple options, each with a single parameter
+          if opts[:multi] # multiple options, each with a single parameter
             vals[sym] = vals[sym].map { |p| p[0] }
+          else # single parameter
+            vals[sym] = vals[sym][0][0]
           end
         elsif MULTI_ARG_TYPES.include?(opts[:type]) && !opts[:multi]
-          vals[sym] = vals[sym][0]  # single option, with multiple parameters
+          vals[sym] = vals[sym][0] # single option, with multiple parameters
         end
         # else: multiple options, with multiple parameters
 
@@ -584,7 +584,10 @@ def each_arg args
           i += 1
         when /^--(\S+)$/ # long argument
           params = collect_argument_parameters(args, i + 1)
-          unless params.empty?
+          if params.empty? # long argument no parameter
+            yield args[i], nil
+            i += 1
+          else
             num_params_taken = yield args[i], params
             unless num_params_taken
               if @stop_on_unknown
@@ -595,16 +598,16 @@ def each_arg args
               end
             end
             i += 1 + num_params_taken
-          else # long argument no parameter
-            yield args[i], nil
-            i += 1
           end
         when /^-(\S+)$/ # one or more short arguments
           shortargs = ::Regexp.last_match(1).split(//)
           shortargs.each_with_index do |a, j|
             if j == (shortargs.length - 1)
               params = collect_argument_parameters(args, i + 1)
-              unless params.empty?
+              if params.empty? # argument no parameter
+                yield "-#{a}", nil
+                i += 1
+              else
                 num_params_taken = yield "-#{a}", params
                 unless num_params_taken
                   if @stop_on_unknown
@@ -615,9 +618,6 @@ def each_arg args
                   end
                 end
                 i += 1 + num_params_taken
-              else # argument no parameter
-                yield "-#{a}", nil
-                i += 1
               end
             else
               yield "-#{a}", nil