Merge pull request #9233 from tvpartytonight/PUP-11767

(PUP-11767) Enable more rubocop styles

Author: joshcooper

.rubocop.yml

@@ -232,3 +232,42 @@ Style/ColonMethodDefinition:
 
 Style/DefWithParentheses:
   Enabled: true
+
+Style/Dir:
+  Enabled: true
+
+Style/DocumentDynamicEvalDefinition:
+  Enabled: true
+
+Style/DoubleCopDisableDirective:
+  Enabled: true
+
+Style/EachForSimpleLoop:
+  Enabled: true
+
+Style/EachWithObject:
+  Enabled: true
+
+Style/EmptyBlockParameter:
+  Enabled: true
+
+Style/EmptyCaseCondition:
+  Enabled: true
+
+Style/EmptyLambdaParameter:
+  Enabled: true
+
+Style/EmptyLiteral:
+  Enabled: true
+
+Style/EvalWithLocation:
+  Enabled: true
+
+Style/EvenOdd:
+  Enabled: true
+
+Style/ExpandPathArguments:
+  Enabled: true
+
+Style/FetchEnvVar:
+  Enabled: true

ext/windows/service/daemon.rb

@@ -18,7 +18,7 @@ class WindowsDaemon < Puppet::Util::Windows::Daemon
   @run_thread = nil
   @LOG_TO_FILE = false
   @loglevel = 0
-  LOG_FILE =  File.expand_path(File.join(ENV['ALLUSERSPROFILE'], 'PuppetLabs', 'puppet', 'var', 'log', 'windows.log'))
+  LOG_FILE =  File.expand_path(File.join(ENV.fetch('ALLUSERSPROFILE', nil), 'PuppetLabs', 'puppet', 'var', 'log', 'windows.log'))
   LEVELS = [:debug, :info, :notice, :warning, :err, :alert, :emerg, :crit]
   LEVELS.each do |level|
     define_method("log_#{level}") do |msg|
@@ -203,10 +203,10 @@ def load_env(base_dir)
       ENV['Path'] = [
         File.join(base_dir, 'puppet', 'bin'),
         File.join(base_dir, 'bin'),
-      ].join(';').tr('/', '\\') + ';' + ENV['Path']
+      ].join(';').tr('/', '\\') + ';' + ENV.fetch('Path', nil)
 
       # ENV that uses forward slashes
-      ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV['RUBYLIB']}"
+      ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV.fetch('RUBYLIB', nil)}"
     rescue => e
       log_exception(e)
     end

lib/puppet/application/resource.rb

@@ -188,7 +188,7 @@ def handle_editing(text)
       tmpfile.puts text
 
       # edit the content
-      system(ENV["EDITOR"] || 'vi', tmpfile.path)
+      system(ENV.fetch("EDITOR", nil) || 'vi', tmpfile.path)
 
       # ...and, now, pass that file to puppet to apply.  Because
       # many editors rename or replace the original file we need to

lib/puppet/confine/exists.rb

@@ -16,6 +16,6 @@ def message(value)
   end
 
   def summary
-    result.zip(values).inject([]) { |array, args| val, f = args; array << f unless val; array }
+    result.zip(values).each_with_object([]) { |args, array| val, f = args; array << f unless val; }
   end
 end

lib/puppet/confine/variable.rb

@@ -12,7 +12,7 @@ class Puppet::Confine::Variable < Puppet::Confine
   # Only returns failed values, not all required values.
   def self.summarize(confines)
     result = Hash.new { |hash, key| hash[key] = [] }
-    confines.inject(result) { |total, confine| total[confine.name] += confine.values unless confine.valid?; total }
+    confines.each_with_object(result) { |confine, total| total[confine.name] += confine.values unless confine.valid?; }
   end
 
   # This is set by ConfineCollection.

lib/puppet/defaults.rb

@@ -49,7 +49,7 @@ def self.default_cadir
   def self.default_basemodulepath
     if Puppet::Util::Platform.windows?
       path = ['$codedir/modules']
-      installdir = ENV["FACTER_env_windows_installdir"]
+      installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
       if installdir
         path << "#{installdir}/puppet/modules"
       end
@@ -61,7 +61,7 @@ def self.default_basemodulepath
 
   def self.default_vendormoduledir
     if Puppet::Util::Platform.windows?
-      installdir = ENV["FACTER_env_windows_installdir"]
+      installdir = ENV.fetch("FACTER_env_windows_installdir", nil)
       if installdir
         "#{installdir}\\puppet\\vendor_modules"
       else
@@ -373,7 +373,7 @@ def self.initialize_default_settings!(settings)
           Puppet::Util::Platform.default_paths.each do |path|
             next if paths.include?(path)
 
-            ENV['PATH'] = ENV['PATH'] + File::PATH_SEPARATOR + path
+            ENV['PATH'] = ENV.fetch('PATH', nil) + File::PATH_SEPARATOR + path
           end
           value
         end

lib/puppet/face/epp.rb

@@ -180,14 +180,13 @@
           raise Puppet::Error, _("No input to parse given on command line or stdin")
         end
       else
-        templates, missing_files = args.reduce([[], []]) do |memo, file|
+        templates, missing_files = args.each_with_object([[], []]) do |file, memo|
           template_file = effective_template(file, compiler.environment)
           if template_file.nil?
             memo[1] << file
           else
             memo[0] << template_file
           end
-          memo
         end
 
         show_filename = templates.count > 1

lib/puppet/face/module/list.rb

@@ -119,9 +119,8 @@ def unmet_dependencies(environment)
     # Prepare the unmet dependencies for display on the console.
     environment.modules.sort_by { |mod| mod.name }.each do |mod|
       unmet_grouped = Hash.new { |h, k| h[k] = [] }
-      unmet_grouped = mod.unmet_dependencies.inject(unmet_grouped) do |acc, dep|
+      unmet_grouped = mod.unmet_dependencies.each_with_object(unmet_grouped) do |dep, acc|
         acc[dep[:reason]] << dep
-        acc
       end
       unmet_grouped.each do |type, deps|
         unless deps.empty?

lib/puppet/file_system/uniquefile.rb

@@ -153,7 +153,7 @@ def try_convert_to_hash(h)
 
   def tmpdir
     tmp = '.'
-    for dir in [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp']
+    for dir in [ENV.fetch('TMPDIR', nil), ENV.fetch('TMP', nil), ENV.fetch('TEMP', nil), @@systmpdir, '/tmp']
       stat = File.stat(dir) if dir
       if stat && stat.directory? && stat.writable?
         tmp = dir

lib/puppet/http/dns.rb

@@ -146,10 +146,9 @@ def expired?(service_name)
     # @yields [[Resolv::DNS::Resource::IN::SRV]] a group of records of
     #         the same priority
     def each_priority(records)
-      pri_hash = records.inject({}) do |groups, element|
+      pri_hash = records.each_with_object({}) do |element, groups|
         groups[element.priority] ||= []
         groups[element.priority] << element
-        groups
       end
 
       pri_hash.keys.sort.each do |key|

lib/puppet/http/proxy.rb

@@ -18,7 +18,7 @@ def self.proxy(uri)
 
   def self.http_proxy_env
     # Returns a URI object if proxy is set, or nil
-    proxy_env = ENV["http_proxy"] || ENV["HTTP_PROXY"]
+    proxy_env = ENV.fetch("http_proxy", nil) || ENV.fetch("HTTP_PROXY", nil)
     begin
       return URI.parse(proxy_env) if proxy_env
     rescue URI::InvalidURIError
@@ -124,7 +124,7 @@ def self.http_proxy_password
   end
 
   def self.no_proxy
-    no_proxy_env = ENV["no_proxy"] || ENV["NO_PROXY"]
+    no_proxy_env = ENV.fetch("no_proxy", nil) || ENV.fetch("NO_PROXY", nil)
 
     if no_proxy_env
       return no_proxy_env

lib/puppet/indirector/node/exec.rb

@@ -54,7 +54,7 @@ def create_node(name, result, facts = nil)
 
   # Translate the yaml string into Ruby objects.
   def translate(name, output)
-    Puppet::Util::Yaml.safe_load(output, [Symbol]).inject({}) do |hash, data|
+    Puppet::Util::Yaml.safe_load(output, [Symbol]).each_with_object({}) do |data, hash|
       case data[0]
       when String
         hash[data[0].intern] = data[1]
@@ -63,8 +63,6 @@ def translate(name, output)
       else
         raise Puppet::Error, _("key is a %{klass}, not a string or symbol") % { klass: data[0].class }
       end
-
-      hash
     end
   rescue => detail
     raise Puppet::Error, _("Could not load external node results for %{name}: %{detail}") % { name: name, detail: detail }, detail.backtrace

lib/puppet/indirector/request.rb

@@ -68,7 +68,7 @@ def initialize(indirection_name, method, key, instance, options = {})
     self.indirection_name = indirection_name
     self.method = method
 
-    options = options.inject({}) { |hash, ary| hash[ary[0].to_sym] = ary[1]; hash }
+    options = options.each_with_object({}) { |ary, hash| hash[ary[0].to_sym] = ary[1]; }
 
     set_attributes(options)
 

lib/puppet/interface/documentation.rb

@@ -34,14 +34,14 @@ def attr_doc(name, &validate)
       # without as methods.  When we are 1.9 only (hah!) you can totally
       # replace this with some up-and-up define_method. --daniel 2011-04-29
       module_eval(<<-EOT, __FILE__, __LINE__ + 1)
-        def #{name}(value = nil)
-          self.#{name} = value unless value.nil?
-          @#{name}
-        end
-
-        def #{name}=(value)
-          @#{name} = Puppet::Interface::DocGen.strip_whitespace(#{get_arg})
-        end
+        def #{name}(value = nil)                 # def attribute(value=nil)
+          self.#{name} = value unless value.nil? #   self.attribute = value unless value.nil?
+          @#{name}                               #   @value
+        end                                      # end
+
+        def #{name}=(value)                                                 # def attribute=(value)
+          @#{name} = Puppet::Interface::DocGen.strip_whitespace(#{get_arg}) #   @value = Puppet::Interface::DocGen.strip_whitespace(#{get_arg})
+        end                                                                 # end
       EOT
     end
   end

lib/puppet/network/http/handler.rb

@@ -171,10 +171,9 @@ def client_cert(request)
   end
 
   def decode_params(params)
-    params.select { |key, _| allowed_parameter?(key) }.inject({}) do |result, ary|
+    params.select { |key, _| allowed_parameter?(key) }.each_with_object({}) do |ary, result|
       param, value = ary
       result[param.to_sym] = parse_parameter_value(param, value)
-      result
     end
   end
 

lib/puppet/node/environment.rb

@@ -554,7 +554,7 @@ def hash
   # not private so it can be called in tests
   def self.extralibs
     if ENV['PUPPETLIB']
-      split_path(ENV['PUPPETLIB'])
+      split_path(ENV.fetch('PUPPETLIB', nil))
     else
       []
     end

lib/puppet/parser/resource.rb

@@ -216,7 +216,7 @@ def set_parameter(param, value = nil)
   alias []= set_parameter
 
   def to_hash
-    parse_title.merge(@parameters.reduce({}) do |result, (_, param)|
+    parse_title.merge(@parameters.each_with_object({}) do |(_, param), result|
       value = param.value
       value = (:undef == value) ? nil : value
 
@@ -231,7 +231,6 @@ def to_hash
           result[param.name] = value
         end
       end
-      result
     end)
   end
 

lib/puppet/pops/evaluator/collectors/abstract_collector.rb

@@ -63,7 +63,7 @@ def evaluate
 
     return false if objects.empty?
 
-    objects.reduce(@collected) { |c, o| c[o.ref] = o; c }
+    objects.each_with_object(@collected) { |o, c| c[o.ref] = o; }
 
     objects
   end

lib/puppet/pops/evaluator/collectors/fixed_set_collector.rb

@@ -16,14 +16,13 @@ def initialize(scope, resources)
   # by the realize function
   def collect
     resolved = []
-    result = @resources.reduce([]) do |memo, ref|
+    result = @resources.each_with_object([]) do |ref, memo|
       res = @scope.findresource(ref.to_s)
       if res
         res.virtual = false
         memo << res
         resolved << ref
       end
-      memo
     end
 
     @resources = @resources - resolved

lib/puppet/pops/evaluator/evaluator_impl.rb

@@ -667,7 +667,7 @@ def eval_LiteralList o, scope
   #
   def eval_LiteralHash o, scope
     # optimized
-    o.entries.reduce({}) { |h, entry| h[evaluate(entry.key, scope)] = evaluate(entry.value, scope); h }
+    o.entries.each_with_object({}) { |entry, h| h[evaluate(entry.key, scope)] = evaluate(entry.value, scope); }
   end
 
   # Evaluates all statements and produces the last evaluated value
@@ -861,7 +861,7 @@ def eval_ResourceExpression(o, scope)
 
       # Store evaluated parameters in a hash associated with the body, but do not yet create resource
       # since the entry containing :defaults may appear later
-      body_to_params[body] = body.operations.reduce({}) do |param_memo, op|
+      body_to_params[body] = body.operations.each_with_object({}) do |op, param_memo|
         params = evaluate(op, scope)
         params = [params] unless params.is_a?(Array)
         params.each do |p|
@@ -871,7 +871,6 @@ def eval_ResourceExpression(o, scope)
 
           param_memo[p.name] = p
         end
-        param_memo
       end
     end
 

lib/puppet/pops/evaluator/json_strict_literal_evaluator.rb

@@ -74,11 +74,10 @@ def literal_LiteralList(o)
   end
 
   def literal_LiteralHash(o)
-    o.entries.reduce({}) do |result, entry|
+    o.entries.each_with_object({}) do |entry, result|
       key = literal(entry.key)
       throw :not_literal unless key.is_a?(String)
       result[key] = literal(entry.value)
-      result
     end
   end
 end

lib/puppet/pops/evaluator/literal_evaluator.rb

@@ -87,9 +87,8 @@ def literal_LiteralList(o)
   end
 
   def literal_LiteralHash(o)
-    o.entries.reduce({}) do |result, entry|
+    o.entries.each_with_object({}) do |entry, result|
       result[literal(entry.key)] = literal(entry.value)
-      result
     end
   end
 end

lib/puppet/pops/loader/dependency_loader.rb

@@ -90,6 +90,6 @@ def loaded_entry_in_dependency(typed_name, check_dependencies)
 
   # An index of module_name to module loader used to speed up lookup of qualified names
   def index
-    @index ||= @dependency_loaders.reduce({}) { |index, loader| index[loader.module_name] = loader; index }
+    @index ||= @dependency_loaders.each_with_object({}) { |loader, index| index[loader.module_name] = loader; }
   end
 end

lib/puppet/pops/model/factory.rb

@@ -1007,7 +1007,7 @@ def initialize(args, name_expr)
   # expression, or expression list.
   #
   def self.transform_calls(expressions)
-    expressions.reduce([]) do |memo, expr|
+    expressions.each_with_object([]) do |expr, memo|
       name = memo[-1]
       if name.instance_of?(Factory) && name.model_class <= QualifiedName && name_is_statement?(name[KEY_VALUE])
         if expr.is_a?(Array)
@@ -1035,7 +1035,6 @@ def self.transform_calls(expressions)
           expr['rval_required'] = false
         end
       end
-      memo
     end
   end
 

lib/puppet/pops/parser/locator.rb

@@ -240,7 +240,7 @@ def initialize(locator, str, leading_line_count, leading_offset, has_margin, mar
       # The last entry is duplicated since there will be  the line "after last line" that would otherwise require
       # conditional logic.
       #
-      @accumulated_margin = margin_per_line.reduce([0]) { |memo, val| memo << memo[-1] + val; memo }
+      @accumulated_margin = margin_per_line.each_with_object([0]) { |val, memo| memo << memo[-1] + val; }
       @accumulated_margin << @accumulated_margin[-1]
     end