Merge pull request #9235 from tvpartytonight/PUP-11767_working

(PUP-11767) enable remaining style cops

Author: joshcooper

.rubocop.yml

@@ -271,3 +271,87 @@ Style/ExpandPathArguments:
 
 Style/FetchEnvVar:
   Enabled: true
+
+Style/FileRead:
+  Enabled: true
+
+Style/FileWrite:
+  Enabled: true
+
+Style/FloatDivision:
+  Enabled: true
+
+Style/For:
+  Enabled: true
+
+Style/FrozenStringLiteralComment:
+  Enabled: true
+
+Style/GlobalStdStream:
+  Enabled: true
+
+Style/GlobalVars:
+  Enabled: true
+
+Style/HashAsLastArrayItem:
+  Enabled: true
+
+Style/HashConversion:
+  Enabled: true
+
+# HashEachMethods does not guarantee the receiver is a Hash, so
+# this will not work since numerous puppet classes define #value
+# not #each_value
+Style/HashEachMethods:
+  Enabled: false
+
+Style/HashLikeCase:
+  Enabled: true
+
+Style/HashTransformKeys:
+  Enabled: true
+
+Style/HashTransformValues:
+  Enabled: true
+
+Style/IdenticalConditionalBranches:
+  Enabled: true
+
+Style/IfInsideElse:
+  Enabled: true
+
+Style/IfUnlessModifierOfIfUnless:
+  Enabled: true
+
+Style/IfWithBooleanLiteralBranches:
+  Enabled: true
+
+Style/IfWithSemicolon:
+  Enabled: true
+
+Style/InfiniteLoop:
+  Enabled: true
+
+Style/InverseMethods:
+  Enabled: true
+
+Style/KeywordParametersOrder:
+  Enabled: true
+
+Style/Lambda:
+  Enabled: true
+
+Style/LambdaCall:
+  Enabled: true
+
+Style/LineEndConcatenation:
+  Enabled: true
+
+Style/MapCompactWithConditionalBlock:
+  Enabled: true
+
+Style/MapToHash:
+  Enabled: true
+
+Style/MapToSet:
+  Enabled: true

.rubocop_todo.yml

@@ -640,6 +640,10 @@ Style/IfWithSemicolon:
   Exclude:
     - 'lib/puppet/pops/parser/evaluating_parser.rb'
 
+# This cop requires significant changes to testing, will require its own effort
+Style/ImplicitRuntimeError:
+  Enabled: false
+
 # This cop supports unsafe auto-correction (--auto-correct-all).
 Style/InfiniteLoop:
   Exclude:

lib/puppet/context/trusted_information.rb

@@ -54,9 +54,9 @@ def self.remote(authenticated, node_name, certificate)
       if certificate.nil?
         Puppet.info(_('TrustedInformation expected a certificate, but none was given.'))
       else
-        extensions = Hash[certificate.custom_extensions.collect do |ext|
+        extensions = certificate.custom_extensions.to_h do |ext|
           [ext['oid'].freeze, ext['value'].freeze]
-        end]
+        end
       end
       new('remote', node_name, extensions, external)
     else

lib/puppet/defaults.rb

@@ -489,7 +489,7 @@ def self.initialize_default_settings!(settings)
     },
     :daemonize => {
         :type     => :boolean,
-        :default  => (Puppet::Util::Platform.windows? ? false : true),
+        :default  => (!Puppet::Util::Platform.windows?),
         :desc     => "Whether to send the process into the background.  This defaults
           to true on POSIX systems, and to false on Windows (where Puppet
           currently cannot daemonize).",

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/face/parser.rb

@@ -86,10 +86,10 @@
       unless errors.empty?
         ignore_error_keys = [:arguments, :environment, :node]
 
-        data = errors.map do |file, error|
+        data = errors.to_h do |file, error|
           file_errors = error.to_h.reject { |k, _| ignore_error_keys.include?(k) }
           [file, file_errors]
-        end.to_h
+        end
 
         puts Puppet::Util::Json.dump(Puppet::Pops::Serialization::ToDataConverter.convert(data, rich_data: false, symbol_as_string: true), :pretty => true)
 

lib/puppet/forge.rb

@@ -146,7 +146,7 @@ def initialize(source, data)
         dependencies = []
       end
 
-      super(source, name, version, Hash[dependencies])
+      super(source, name, version, dependencies.to_h)
     end
 
     def install(dir)

lib/puppet/functions/filter.rb

@@ -96,14 +96,14 @@
   def filter_Hash_1(hash)
     result = hash.select { |x, y| yield([x, y]) }
     # Ruby 1.8.7 returns Array
-    result = Hash[result] unless result.is_a? Hash
+    result = result.to_h unless result.is_a? Hash
     result
   end
 
   def filter_Hash_2(hash)
     result = hash.select { |x, y| yield(x, y) }
     # Ruby 1.8.7 returns Array
-    result = Hash[result] unless result.is_a? Hash
+    result = result.to_h unless result.is_a? Hash
     result
   end
 

lib/puppet/generate/models/type/type.rb

@@ -40,7 +40,7 @@ def initialize(type)
               Property.new(type.paramclass(name))
             end
             sc = Puppet::Pops::Types::StringConverter.singleton
-            @title_patterns = Hash[type.title_patterns.map do |mapping|
+            @title_patterns = type.title_patterns.to_h do |mapping|
               [
                 sc.convert(mapping[0], '%p'),
                 sc.convert(mapping[1].map do |names|
@@ -50,7 +50,7 @@ def initialize(type)
                   names[0].to_s
                 end, '%p')
               ]
-            end]
+            end
             @isomorphic = type.isomorphic?
             # continue to emit capability as false when rendering the ERB
             # template, so that pcore modules generated prior to puppet7 can be

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