From 9cd470a0df890d5f7f097fd9f3aa3ecd82390066 Mon Sep 17 00:00:00 2001
From: Kylo Ginsberg <kylo@puppetlabs.com>
Date: Mon, 15 Dec 2014 09:18:07 -0800
Subject: [PATCH] (maint) Enable rubocop checking for shadowed variables

Previously, we allowed variables to be shadowed by variables
of the same name at local scope. This can make for confusing
reading to someone new to the code (and can potentially indicate
logic errors).

So this commit enables rubocop checking for shadow variables. It
also fixes (by renaming) all extant shadowed variables. Naming
is hard, but a best effort was made.
---
 .rubocop.yml                                  |  3 +--
 ext/nagios/check_puppet.rb                    |  2 +-
 lib/puppet/application/face_base.rb           |  6 ++---
 lib/puppet/external/nagios/base.rb            |  4 ++--
 lib/puppet/file_bucket/dipper.rb              |  3 +--
 lib/puppet/file_serving/mount/pluginfacts.rb  |  2 +-
 lib/puppet/file_serving/mount/plugins.rb      |  2 +-
 lib/puppet/graph/relationship_graph.rb        |  6 ++---
 .../indirector/catalog/static_compiler.rb     |  6 ++---
 lib/puppet/indirector/rest.rb                 | 24 +++++++++----------
 lib/puppet/interface/face_collection.rb       |  4 ++--
 .../module_tool/applications/installer.rb     | 14 +++++------
 .../module_tool/applications/upgrader.rb      | 10 ++++----
 .../module_tool/contents_description.rb       |  8 +++----
 lib/puppet/module_tool/installed_modules.rb   |  6 ++---
 lib/puppet/network/auth_config_parser.rb      |  4 ++--
 lib/puppet/network/format_handler.rb          | 14 +++++------
 lib/puppet/network/http/handler.rb            |  2 +-
 lib/puppet/network/resolver.rb                |  4 ++--
 lib/puppet/network/rights.rb                  |  4 ++--
 lib/puppet/parameter.rb                       |  6 ++---
 lib/puppet/pops/evaluator/access_operator.rb  |  8 +++----
 .../evaluator/callable_mismatch_describer.rb  |  6 ++---
 lib/puppet/pops/evaluator/closure.rb          |  6 ++---
 .../pops/evaluator/relationship_operator.rb   |  2 +-
 lib/puppet/pops/loader/dependency_loader.rb   |  4 ++--
 lib/puppet/provider/group/aix.rb              |  2 +-
 lib/puppet/provider/package/pkgin.rb          |  2 +-
 lib/puppet/provider/service/upstart.rb        |  4 ++--
 lib/puppet/provider/user/aix.rb               |  2 +-
 lib/puppet/reference/type.rb                  | 11 ++++-----
 lib/puppet/resource/type_collection.rb        |  6 ++---
 lib/puppet/settings.rb                        | 10 ++++----
 lib/puppet/transaction.rb                     |  2 +-
 lib/puppet/transaction/report.rb              |  8 +++----
 lib/puppet/type.rb                            | 12 +++++-----
 .../util/network_device/cisco/interface.rb    |  4 ++--
 .../util/network_device/transport/ssh.rb      |  4 ++--
 lib/puppet/util/rdoc/code_objects.rb          |  6 ++---
 .../util/rdoc/generators/puppet_generator.rb  |  2 +-
 .../util/rdoc/parser/puppet_parser_core.rb    |  6 ++---
 41 files changed, 119 insertions(+), 122 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index e7c696dbafc..0813e2e4ca1 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -37,9 +37,8 @@ Lint/HandleExceptions:
 Lint/LiteralInCondition:
   Enabled: false
 
-# MAYBE useful - but too many instances
 Lint/ShadowingOuterLocalVariable:
-  Enabled: false
+  Enabled: true
 
 # Can catch complicated strings.
 Lint/LiteralInInterpolation:
diff --git a/ext/nagios/check_puppet.rb b/ext/nagios/check_puppet.rb
index 36d8fa79cb7..a7570f868a1 100755
--- a/ext/nagios/check_puppet.rb
+++ b/ext/nagios/check_puppet.rb
@@ -16,7 +16,7 @@ class CheckPuppet
     :interval  => 30,
   }
 
-  o = OptionParser.new do |o|
+  OptionParser.new do |o|
     o.set_summary_indent('  ')
     o.banner =    "Usage: #{script_name} [OPTIONS]"
     o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval."
diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index c6e4a2828c5..e767805c7cd 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -121,9 +121,9 @@ def parse_options
 
     # Now we can interact with the default option code to build behaviour
     # around the full set of options we now know we support.
-    @action.options.each do |option|
-      option = @action.get_option(option) # make it the object.
-      self.class.option(*option.optparse) # ...and make the CLI parse it.
+    @action.options.each do |o|
+      o = @action.get_option(o) # make it the object.
+      self.class.option(*o.optparse) # ...and make the CLI parse it.
     end
 
     # ...and invoke our parent to parse all the command line options.
diff --git a/lib/puppet/external/nagios/base.rb b/lib/puppet/external/nagios/base.rb
index 06f6987abd3..14c7a558fbb 100644
--- a/lib/puppet/external/nagios/base.rb
+++ b/lib/puppet/external/nagios/base.rb
@@ -283,8 +283,8 @@ def to_ldif
     oc.sub!(/Nagios/,'nagios')
     oc.sub!(/::/,'')
     ocs.push oc
-    ocs.each { |oc|
-      str += "objectclass: #{oc}\n"
+    ocs.each { |objclass|
+      str += "objectclass: #{objclass}\n"
     }
     @parameters.each { |name,value|
       next if self.class.suppress.include?(name)
diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb
index b627ce02dd7..e521b0a2e22 100644
--- a/lib/puppet/file_bucket/dipper.rb
+++ b/lib/puppet/file_bucket/dipper.rb
@@ -94,10 +94,9 @@ def restore(file, sum)
         end
         ::File.open(file, ::File::WRONLY|::File::TRUNC|::File::CREAT) { |of|
           of.binmode
-          source_stream = newcontents.stream do |source_stream|
+          newcontents.stream do |source_stream|
             FileUtils.copy_stream(source_stream, of)
           end
-          #of.print(newcontents)
         }
         ::File.chmod(changed, file) if changed
       else
diff --git a/lib/puppet/file_serving/mount/pluginfacts.rb b/lib/puppet/file_serving/mount/pluginfacts.rb
index 5ba8f7146a3..eec00d087e2 100644
--- a/lib/puppet/file_serving/mount/pluginfacts.rb
+++ b/lib/puppet/file_serving/mount/pluginfacts.rb
@@ -6,7 +6,7 @@
 class Puppet::FileServing::Mount::PluginFacts < Puppet::FileServing::Mount
   # Return an instance of the appropriate class.
   def find(relative_path, request)
-    return nil unless mod = request.environment.modules.find { |mod|  mod.pluginfact(relative_path) }
+    return nil unless mod = request.environment.modules.find { |m|  m.pluginfact(relative_path) }
 
     path = mod.pluginfact(relative_path)
 
diff --git a/lib/puppet/file_serving/mount/plugins.rb b/lib/puppet/file_serving/mount/plugins.rb
index 1e5dcefdd7c..60eb003c092 100644
--- a/lib/puppet/file_serving/mount/plugins.rb
+++ b/lib/puppet/file_serving/mount/plugins.rb
@@ -6,7 +6,7 @@
 class Puppet::FileServing::Mount::Plugins < Puppet::FileServing::Mount
   # Return an instance of the appropriate class.
   def find(relative_path, request)
-    return nil unless mod = request.environment.modules.find { |mod|  mod.plugin(relative_path) }
+    return nil unless mod = request.environment.modules.find { |m|  m.plugin(relative_path) }
 
     path = mod.plugin(relative_path)
 
diff --git a/lib/puppet/graph/relationship_graph.rb b/lib/puppet/graph/relationship_graph.rb
index 3860a5e4c8e..83ee3ab65cb 100644
--- a/lib/puppet/graph/relationship_graph.rb
+++ b/lib/puppet/graph/relationship_graph.rb
@@ -126,9 +126,9 @@ def traverse(options = {}, &block)
         if made_progress
           enqueue(*deferred_resources)
         else
-          deferred_resources.each do |resource|
-            overly_deferred_resource_handler.call(resource)
-            finish(resource)
+          deferred_resources.each do |res|
+            overly_deferred_resource_handler.call(res)
+            finish(res)
           end
         end
 
diff --git a/lib/puppet/indirector/catalog/static_compiler.rb b/lib/puppet/indirector/catalog/static_compiler.rb
index 8841bfd2276..ff95d30dde4 100644
--- a/lib/puppet/indirector/catalog/static_compiler.rb
+++ b/lib/puppet/indirector/catalog/static_compiler.rb
@@ -140,10 +140,10 @@ def get_child_resources(host, catalog, resource, file)
     source = resource[:source]
 
     # This is largely a copy of recurse_remote in File
-    total = file[:source].collect do |source|
-      next unless result = file.perform_recursion(source)
+    total = file[:source].collect do |src|
+      next unless result = file.perform_recursion(src)
       return if top = result.find { |r| r.relative_path == "." } and top.ftype != "directory"
-      result.each { |data| data.source = "#{source}/#{data.relative_path}" }
+      result.each { |data| data.source = "#{src}/#{data.relative_path}" }
       break result if result and ! result.empty? and sourceselect == :first
       result
     end.flatten.compact
diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index 464f68ab1bf..979577957aa 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -85,13 +85,13 @@ def find(request)
     uri, body = Puppet::Network::HTTP::API::V1.request_to_uri_and_body(request)
     uri_with_query_string = "#{uri}?#{body}"
 
-    response = do_request(request) do |request|
+    response = do_request(request) do |req|
       # WEBrick in Ruby 1.9.1 only supports up to 1024 character lines in an HTTP request
       # http://redmine.ruby-lang.org/issues/show/3991
       if "GET #{uri_with_query_string} HTTP/1.1\r\n".length > 1024
-        http_post(request, uri, body, headers)
+        http_post(req, uri, body, headers)
       else
-        http_get(request, uri_with_query_string, headers)
+        http_get(req, uri_with_query_string, headers)
       end
     end
 
@@ -119,8 +119,8 @@ def find(request)
   end
 
   def head(request)
-    response = do_request(request) do |request|
-      http_head(request, Puppet::Network::HTTP::API::V1.request_to_uri(request), headers)
+    response = do_request(request) do |req|
+      http_head(req, Puppet::Network::HTTP::API::V1.request_to_uri(req), headers)
     end
 
     if is_http_200?(response)
@@ -131,8 +131,8 @@ def head(request)
   end
 
   def search(request)
-    response = do_request(request) do |request|
-      http_get(request, Puppet::Network::HTTP::API::V1.request_to_uri(request), headers)
+    response = do_request(request) do |req|
+      http_get(req, Puppet::Network::HTTP::API::V1.request_to_uri(req), headers)
     end
 
     if is_http_200?(response)
@@ -146,8 +146,8 @@ def search(request)
   def destroy(request)
     raise ArgumentError, "DELETE does not accept options" unless request.options.empty?
 
-    response = do_request(request) do |request|
-      http_delete(request, Puppet::Network::HTTP::API::V1.request_to_uri(request), headers)
+    response = do_request(request) do |req|
+      http_delete(req, Puppet::Network::HTTP::API::V1.request_to_uri(req), headers)
     end
 
     if is_http_200?(response)
@@ -161,8 +161,8 @@ def destroy(request)
   def save(request)
     raise ArgumentError, "PUT does not accept options" unless request.options.empty?
 
-    response = do_request(request) do |request|
-      http_put(request, Puppet::Network::HTTP::API::V1.request_to_uri(request), request.instance.render, headers.merge({ "Content-Type" => request.instance.mime }))
+    response = do_request(request) do |req|
+      http_put(req, Puppet::Network::HTTP::API::V1.request_to_uri(req), req.instance.render, headers.merge({ "Content-Type" => req.instance.mime }))
     end
 
     if is_http_200?(response)
@@ -180,7 +180,7 @@ def save(request)
   # to request.do_request from here, thus if we change what we pass or how we
   # get it, we only need to change it here.
   def do_request(request)
-    request.do_request(self.class.srv_service, self.class.server, self.class.port) { |request| yield(request) }
+    request.do_request(self.class.srv_service, self.class.server, self.class.port) { |req| yield(req) }
   end
 
   def validate_key(request)
diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb
index 0c95f50b328..ae11e7e468e 100644
--- a/lib/puppet/interface/face_collection.rb
+++ b/lib/puppet/interface/face_collection.rb
@@ -27,8 +27,8 @@ def self.get_action_for_face(name, action_name, version)
       # ...we need to search for it bound to an o{lder,ther} version.  Since
       # we load all actions when the face is first references, this will be in
       # memory in the known set of versions of the face.
-      (@faces[name].keys - [ :current ]).sort.reverse.each do |version|
-        break if action = @faces[name][version].get_action(action_name)
+      (@faces[name].keys - [ :current ]).sort.reverse.each do |vers|
+        break if action = @faces[name][vers].get_action(action_name)
       end
     end
 
diff --git a/lib/puppet/module_tool/applications/installer.rb b/lib/puppet/module_tool/applications/installer.rb
index 92aa6c02d2f..7ce53307324 100644
--- a/lib/puppet/module_tool/applications/installer.rb
+++ b/lib/puppet/module_tool/applications/installer.rb
@@ -57,11 +57,11 @@ def run
         results = { :action => :install, :module_name => name, :module_version => version }
 
         begin
-          if mod = installed_modules[name]
+          if installed_module = installed_modules[name]
             unless forced?
-              if Semantic::VersionRange.parse(version).include? mod.version
+              if Semantic::VersionRange.parse(version).include? installed_module.version
                 results[:result] = :noop
-                results[:version] = mod.version
+                results[:version] = installed_module.version
                 return results
               else
                 changes = Checksummer.run(installed_modules[name].mod.path) rescue []
@@ -135,8 +135,8 @@ def run
           unless forced?
             # Check for module name conflicts.
             releases.each do |rel|
-              if mod = installed_modules_source.by_name[rel.name.split('-').last]
-                next if mod.has_metadata? && mod.forge_name.tr('/', '-') == rel.name
+              if installed_module = installed_modules_source.by_name[rel.name.split('-').last]
+                next if installed_module.has_metadata? && installed_module.forge_name.tr('/', '-') == rel.name
 
                 if rel.name != name
                   dependency = {
@@ -149,8 +149,8 @@ def run
                   :requested_module  => name,
                   :requested_version => options[:version] || 'latest',
                   :dependency        => dependency,
-                  :directory         => mod.path,
-                  :metadata          => mod.metadata
+                  :directory         => installed_module.path,
+                  :metadata          => installed_module.metadata
               end
             end
           end
diff --git a/lib/puppet/module_tool/applications/upgrader.rb b/lib/puppet/module_tool/applications/upgrader.rb
index d901f86a836..d1c4a953240 100644
--- a/lib/puppet/module_tool/applications/upgrader.rb
+++ b/lib/puppet/module_tool/applications/upgrader.rb
@@ -107,9 +107,9 @@ def installed_release.priority
             add_module_name_constraints_to_graph(graph)
           end
 
-          installed_modules.each do |mod, release|
-            mod = mod.tr('/', '-')
-            next if mod == name
+          installed_modules.each do |installed_module, release|
+            installed_module = installed_module.tr('/', '-')
+            next if installed_module == name
 
             version = release.version
 
@@ -118,7 +118,7 @@ def installed_release.priority
               # we'll place constraints on the graph for each installed
               # module, locking it to upgrades within the same major version.
               ">=#{version} #{version.major}.x".tap do |range|
-                graph.add_constraint('installed', mod, range) do |node|
+                graph.add_constraint('installed', installed_module, range) do |node|
                   Semantic::VersionRange.parse(range).include? node.version
                 end
               end
@@ -127,7 +127,7 @@ def installed_release.priority
                 dep_name = dep['name'].tr('/', '-')
 
                 dep['version_requirement'].tap do |range|
-                  graph.add_constraint("#{mod} constraint", dep_name, range) do |node|
+                  graph.add_constraint("#{installed_module} constraint", dep_name, range) do |node|
                     Semantic::VersionRange.parse(range).include? node.version
                   end
                 end
diff --git a/lib/puppet/module_tool/contents_description.rb b/lib/puppet/module_tool/contents_description.rb
index 7fd6505f4d7..ca67930d81a 100644
--- a/lib/puppet/module_tool/contents_description.rb
+++ b/lib/puppet/module_tool/contents_description.rb
@@ -42,9 +42,9 @@ def data
           end
         end
 
-        type_names.each do |type_name|
-          if type = Puppet::Type.type(type_name.to_sym)
-            type_hash = {:name => type_name, :doc => type.doc}
+        type_names.each do |name|
+          if type = Puppet::Type.type(name.to_sym)
+            type_hash = {:name => name, :doc => type.doc}
             type_hash[:properties] = attr_doc(type, :property)
             type_hash[:parameters] = attr_doc(type, :param)
             if type.providers.size > 0
@@ -52,7 +52,7 @@ def data
             end
             @data << type_hash
           else
-            Puppet.warning "Could not find/load type: #{type_name}"
+            Puppet.warning "Could not find/load type: #{name}"
           end
         end
       end
diff --git a/lib/puppet/module_tool/installed_modules.rb b/lib/puppet/module_tool/installed_modules.rb
index de9421132c5..8042f09b86f 100644
--- a/lib/puppet/module_tool/installed_modules.rb
+++ b/lib/puppet/module_tool/installed_modules.rb
@@ -69,11 +69,11 @@ def initialize(source, mod)
         super(source, name, version, {})
 
         if mod.dependencies
-          mod.dependencies.each do |dep|
-            results = Puppet::ModuleTool.parse_module_dependency(release, dep)
+          mod.dependencies.each do |dependency|
+            results = Puppet::ModuleTool.parse_module_dependency(release, dependency)
             dep_name, parsed_range, range = results
 
-            dep.tap do |dep|
+            dependency.tap do |dep|
               add_constraint('initialize', dep_name, range.to_s) do |node|
                 parsed_range === node.version
               end
diff --git a/lib/puppet/network/auth_config_parser.rb b/lib/puppet/network/auth_config_parser.rb
index cb7199ee302..01a17ae1107 100644
--- a/lib/puppet/network/auth_config_parser.rb
+++ b/lib/puppet/network/auth_config_parser.rb
@@ -40,8 +40,8 @@ def parse_rights
     # Verify each of the rights are valid.
     # We let the check raise an error, so that it can raise an error
     # pointing to the specific problem.
-    rights.each { |name, right|
-      right.valid?
+    rights.each { |name, r|
+      r.valid?
     }
     rights
   end
diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb
index d341b433520..9667ee4e5f9 100644
--- a/lib/puppet/network/format_handler.rb
+++ b/lib/puppet/network/format_handler.rb
@@ -77,16 +77,16 @@ def self.format_to_canonical_name(format)
   # @return [Puppet::Network::Format, nil] the most suitable format
   # @api private
   def self.most_suitable_format_for(accepted, supported)
-    format_name = accepted.collect do |accepted|
-      accepted.to_s.sub(/;q=.*$/, '')
-    end.collect do |accepted|
-      if accepted == ALL_MEDIA_TYPES
+    format_name = accepted.collect do |format|
+      format.to_s.sub(/;q=.*$/, '')
+    end.collect do |format|
+      if format == ALL_MEDIA_TYPES
         supported.first
       else
-        format_to_canonical_name_or_nil(accepted)
+        format_to_canonical_name_or_nil(format)
       end
-    end.find do |accepted|
-      supported.include?(accepted)
+    end.find do |format|
+      supported.include?(format)
     end
 
     if format_name
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index e26973e7651..7851c3aaef4 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -57,7 +57,7 @@ def process(request, response)
     profiler = configure_profiler(request_headers, request_params)
 
     Puppet::Util::Profiler.profile("Processed request #{request_method} #{request_path}", [:http, request_method, request_path]) do
-      if route = @routes.find { |route| route.matches?(new_request) }
+      if route = @routes.find { |r| r.matches?(new_request) }
         route.process(new_request, new_response)
       else
         raise Puppet::Network::HTTP::Error::HTTPNotFoundError.new("No route for #{new_request.method} #{new_request.path}", HANDLER_NOT_FOUND)
diff --git a/lib/puppet/network/resolver.rb b/lib/puppet/network/resolver.rb
index 98554eb8b79..54e27f9e58a 100644
--- a/lib/puppet/network/resolver.rb
+++ b/lib/puppet/network/resolver.rb
@@ -31,8 +31,8 @@ def self.each_srv_record(domain, service_name = :puppet, &block)
       # for the specific service.
       each_srv_record(domain, :puppet, &block)
     else
-      each_priority(records) do |priority, records|
-        while next_rr = records.delete(find_weighted_server(records))
+      each_priority(records) do |priority, recs|
+        while next_rr = recs.delete(find_weighted_server(recs))
           Puppet.debug "Yielding next server of #{next_rr.target.to_s}:#{next_rr.port}"
           yield next_rr.target.to_s, next_rr.port
         end
diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb
index d7275babfe0..bfd25aca0de 100644
--- a/lib/puppet/network/rights.rb
+++ b/lib/puppet/network/rights.rb
@@ -20,8 +20,8 @@ def is_request_forbidden_and_why?(method, path, params)
                        else
                          [method]
                        end
-    authorization_failure_exceptions = methods_to_check.map do |method|
-      is_forbidden_and_why?(path, params.merge({:method => method}))
+    authorization_failure_exceptions = methods_to_check.map do |m|
+      is_forbidden_and_why?(path, params.merge({:method => m}))
     end
     if authorization_failure_exceptions.include? nil
       # One of the methods we checked is ok, therefore this request is ok.
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 77907ffd915..c52b0666466 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -108,8 +108,8 @@ def doc
           @doc << "\n\n#{vals}"
         end
 
-        if f = self.required_features
-          @doc << "\n\nRequires features #{f.flatten.collect { |f| f.to_s }.join(" ")}."
+        if features = self.required_features
+          @doc << "\n\nRequires features #{features.flatten.collect { |f| f.to_s }.join(" ")}."
         end
         @addeddocvals = true
       end
@@ -542,7 +542,7 @@ def to_s
   #
   def self.format_value_for_display(value)
     if value.is_a? Array
-      formatted_values = value.collect {|value| format_value_for_display(value)}.join(', ')
+      formatted_values = value.collect {|v| format_value_for_display(v)}.join(', ')
       "[#{formatted_values}]"
     elsif value.is_a? Hash
       # Sorting the hash keys for display is largely for having stable
diff --git a/lib/puppet/pops/evaluator/access_operator.rb b/lib/puppet/pops/evaluator/access_operator.rb
index c05b2b2b236..a95e7897d83 100644
--- a/lib/puppet/pops/evaluator/access_operator.rb
+++ b/lib/puppet/pops/evaluator/access_operator.rb
@@ -89,11 +89,11 @@ def access_Array(o, scope, keys)
     when 0
       fail(Puppet::Pops::Issues::BAD_ARRAY_SLICE_ARITY, @semantic.left_expr, {:actual => keys.size})
     when 1
-      k = coerce_numeric(keys[0], @semantic.keys[0], scope)
-      unless k.is_a?(Integer)
-        bad_access_key_type(o, 0, k, Integer)
+      key = coerce_numeric(keys[0], @semantic.keys[0], scope)
+      unless key.is_a?(Integer)
+        bad_access_key_type(o, 0, key, Integer)
       end
-      o[k]
+      o[key]
     when 2
       # A slice [from, to] with support for -1 to mean start, or end respectively.
       k1 = coerce_numeric(keys[0], @semantic.keys[0], scope)
diff --git a/lib/puppet/pops/evaluator/callable_mismatch_describer.rb b/lib/puppet/pops/evaluator/callable_mismatch_describer.rb
index c1d9c38961c..b4394134ad7 100644
--- a/lib/puppet/pops/evaluator/callable_mismatch_describer.rb
+++ b/lib/puppet/pops/evaluator/callable_mismatch_describer.rb
@@ -17,9 +17,9 @@ def self.diff_string(name, args_type, supported_signatures)
       result << "expected:\n  #{name}(#{signature_string(signature)}) - #{arg_count_string(signature.type)}"
     else
       result << "expected one of:\n"
-      result << supported_signatures.map do |signature|
-        params_type = signature.type.param_types
-        "  #{name}(#{signature_string(signature)}) - #{arg_count_string(signature.type)}"
+      result << supported_signatures.map do |sig|
+        params_type = sig.type.param_types
+        "  #{name}(#{signature_string(sig)}) - #{arg_count_string(sig.type)}"
       end.join("\n")
     end
 
diff --git a/lib/puppet/pops/evaluator/closure.rb b/lib/puppet/pops/evaluator/closure.rb
index 7decc5a4c85..7ab5a6e04c3 100644
--- a/lib/puppet/pops/evaluator/closure.rb
+++ b/lib/puppet/pops/evaluator/closure.rb
@@ -35,11 +35,11 @@ def call(*args)
     variable_bindings = combine_values_with_parameters(args)
 
     tc = Puppet::Pops::Types::TypeCalculator
-    final_args = tc.infer_set(parameters.inject([]) do |final_args, param|
+    final_args = tc.infer_set(parameters.inject([]) do |tmp_args, param|
       if param.captures_rest
-        final_args.concat(variable_bindings[param.name])
+        tmp_args.concat(variable_bindings[param.name])
       else
-        final_args << variable_bindings[param.name]
+        tmp_args << variable_bindings[param.name]
       end
     end)
 
diff --git a/lib/puppet/pops/evaluator/relationship_operator.rb b/lib/puppet/pops/evaluator/relationship_operator.rb
index 1731870af1a..8282d37a773 100644
--- a/lib/puppet/pops/evaluator/relationship_operator.rb
+++ b/lib/puppet/pops/evaluator/relationship_operator.rb
@@ -134,7 +134,7 @@ def evaluate (left_right_evaluated, relationship_expression, scope)
       # into an array, and thus the resulting left and right must be flattened individually
       # Once flattened, the operands should be sets (to remove duplicate entries)
       #
-      real = left_right_evaluated.collect {|x| [x].flatten.collect {|x| transform(x, scope) }}
+      real = left_right_evaluated.collect {|x| [x].flatten.collect {|y| transform(y, scope) }}
       real[0].flatten!
       real[1].flatten!
       real[0].uniq!
diff --git a/lib/puppet/pops/loader/dependency_loader.rb b/lib/puppet/pops/loader/dependency_loader.rb
index dbeda418f23..79525d2cce6 100644
--- a/lib/puppet/pops/loader/dependency_loader.rb
+++ b/lib/puppet/pops/loader/dependency_loader.rb
@@ -27,8 +27,8 @@ def initialize(parent_loader, name, dependency_loaders)
   #
   def find(typed_name)
     if typed_name.qualified
-      if loader = index()[typed_name.name_parts[0]]
-        loader.load_typed(typed_name)
+      if l = index()[typed_name.name_parts[0]]
+        l.load_typed(typed_name)
       else
         # no module entered as dependency with name matching first segment of wanted name
         nil
diff --git a/lib/puppet/provider/group/aix.rb b/lib/puppet/provider/group/aix.rb
index 1722748d043..c16b9e49606 100644
--- a/lib/puppet/provider/group/aix.rb
+++ b/lib/puppet/provider/group/aix.rb
@@ -111,7 +111,7 @@ def filter_attributes(hash)
         |k,v| !self.class.attribute_mapping_from.include?(k) and
                 !self.class.attribute_ignore.include?(k)
       }.inject({}) {
-        |hash, array| hash[array[0]] = array[1]; hash
+        |h, array| h[array[0]] = array[1]; h
       }
   end
 
diff --git a/lib/puppet/provider/package/pkgin.rb b/lib/puppet/provider/package/pkgin.rb
index 5cdb858d5a4..c3bdc62c782 100644
--- a/lib/puppet/provider/package/pkgin.rb
+++ b/lib/puppet/provider/package/pkgin.rb
@@ -75,7 +75,7 @@ def uninstall
   end
 
   def latest
-    package = parse_pkgsearch_line.detect{ |package| package[:status] == '<' }
+    package = parse_pkgsearch_line.detect{ |p| p[:status] == '<' }
     return properties[:ensure] if not package
     return package[:ensure]
   end
diff --git a/lib/puppet/provider/service/upstart.rb b/lib/puppet/provider/service/upstart.rb
index 548341a895a..191b04774ff 100644
--- a/lib/puppet/provider/service/upstart.rb
+++ b/lib/puppet/provider/service/upstart.rb
@@ -354,8 +354,8 @@ def read_script_from(filename)
   end
 
   def write_script_to(file, text)
-    Puppet::Util.replace_file(file, 0644) do |file|
-      file.write(text)
+    Puppet::Util.replace_file(file, 0644) do |f|
+      f.write(text)
     end
   end
 end
diff --git a/lib/puppet/provider/user/aix.rb b/lib/puppet/provider/user/aix.rb
index e75763a0dd3..2114971b5ed 100644
--- a/lib/puppet/provider/user/aix.rb
+++ b/lib/puppet/provider/user/aix.rb
@@ -294,7 +294,7 @@ def filter_attributes(hash)
     results = hash.select {
         |k,v| should_include?(k, managed_keys)
       }.inject({}) {
-        |hash, array| hash[array[0]] = array[1]; hash
+        |h, array| h[array[0]] = array[1]; h
       }
     results
   end
diff --git a/lib/puppet/reference/type.rb b/lib/puppet/reference/type.rb
index ed3e4507d61..0159ac99e0c 100644
--- a/lib/puppet/reference/type.rb
+++ b/lib/puppet/reference/type.rb
@@ -94,19 +94,18 @@
     str << markdown_header("Parameters", 4) + "\n"
     type.parameters.sort { |a,b|
       a.to_s <=> b.to_s
-    }.each { |name,param|
-      #docs[name] = indent(scrub(type.paramdoc(name)), $tab)
-      docs[name] = scrub(type.paramdoc(name))
+    }.each { |type_name, param|
+      docs[type_name] = scrub(type.paramdoc(type_name))
     }
 
     additional_key_attributes = type.key_attributes - [:name]
     docs.sort { |a, b|
       a[0].to_s <=> b[0].to_s
-    }.each { |name, doc|
-      if additional_key_attributes.include?(name)
+    }.each { |type_name, doc|
+      if additional_key_attributes.include?(type_name)
         doc = "(**Namevar:** If omitted, this parameter's value defaults to the resource's title.)\n\n" + doc
       end
-      str << markdown_definitionlist(name, doc)
+      str << markdown_definitionlist(type_name, doc)
     }
     str << "\n"
   }
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 5d46807ca45..97a1fb93d7a 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -83,9 +83,9 @@ def node(name)
       return node
     end
 
-    @node_list.each do |node|
-      next unless node.name_is_regex?
-      return node if node.match(name)
+    @node_list.each do |n|
+      next unless n.name_is_regex?
+      return n if n.match(name)
     end
     nil
   end
diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb
index 46be3c07caa..fae03e34080 100644
--- a/lib/puppet/settings.rb
+++ b/lib/puppet/settings.rb
@@ -446,8 +446,8 @@ def print_config_options
         val = val.inspect if val == ""
         hash[name] = val
       end
-      hash.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, val|
-        puts "#{name} = #{val}"
+      hash.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, v|
+        puts "#{name} = #{v}"
       end
     else
       val.split(/\s*,\s*/).sort.each do |v|
@@ -1106,7 +1106,7 @@ def parse_file(file, allowed_sections = [])
 
   DEPRECATION_REFS = {
     # intentionally empty. This could be repopulated if we deprecate more settings
-    # and have reference links to associate with them 
+    # and have reference links to associate with them
   }.freeze
 
   def screen_non_puppet_conf_settings(puppet_conf)
@@ -1281,8 +1281,8 @@ def initialize(mode, environment, value_sets, defaults)
     # @return [Object] The configuration setting value or nil if the setting is not known
     # @api public
     def lookup(name)
-      set = @value_sets.find do |set|
-        set.include?(name)
+      set = @value_sets.find do |value_set|
+        value_set.include?(name)
       end
       if set
         value = set.lookup(name)
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 2809b65cc73..ddeba65d828 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -284,7 +284,7 @@ def ignore_tags?
 
   def resources_by_provider(type_name, provider_name)
     unless @resources_by_provider
-      @resources_by_provider = Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = {} } }
+      @resources_by_provider = Hash.new { |h, k| h[k] = Hash.new { |h1, k1| h1[k1] = {} } }
 
       @catalog.vertices.each do |resource|
         if resource.class.attrclass(:provider)
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index a95b4f32bde..e2e452846ba 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -122,8 +122,8 @@ def add_times(name, value)
   def add_metric(name, hash)
     metric = Puppet::Util::Metric.new(name)
 
-    hash.each do |name, value|
-      metric.newvalue(name, value)
+    hash.each do |metric_name, value|
+      metric.newvalue(metric_name, value)
     end
 
     @metrics[metric.name] = metric
@@ -281,8 +281,8 @@ def raw_summary
     @metrics.each do |name, metric|
       key = metric.name.to_s
       report[key] = {}
-      metric.values.each do |name, label, value|
-        report[key][name.to_s] = value
+      metric.values.each do |metric_name, label, value|
+        report[key][metric_name.to_s] = value
       end
       report[key]["total"] = 0 unless key == "time" or report[key].include?("total")
     end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index c4893d3f282..4dea86cc37f 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1006,15 +1006,15 @@ def insync?(is)
       end
     end
 
-    properties.each { |property|
-      unless is.include? property
+    properties.each { |prop|
+      unless is.include? prop
         raise Puppet::DevError,
-          "The is value is not in the is array for '#{property.name}'"
+          "The is value is not in the is array for '#{prop.name}'"
       end
 
-      propis = is[property]
-      unless property.safe_insync?(propis)
-        property.debug("Not in sync: #{propis.inspect} vs #{property.should.inspect}")
+      propis = is[prop]
+      unless prop.safe_insync?(propis)
+        prop.debug("Not in sync: #{propis.inspect} vs #{prop.should.inspect}")
         insync = false
       #else
       #    property.debug("In sync")
diff --git a/lib/puppet/util/network_device/cisco/interface.rb b/lib/puppet/util/network_device/cisco/interface.rb
index d184b9c3cc8..14067e4edd1 100644
--- a/lib/puppet/util/network_device/cisco/interface.rb
+++ b/lib/puppet/util/network_device/cisco/interface.rb
@@ -68,8 +68,8 @@ def execute(property, value, prefix='')
       command(prefix + COMMANDS[property][1] % value)
     when Proc
       value = [value] unless value.is_a?(Array)
-      value.each do |value|
-        command(prefix + COMMANDS[property][1].call(*value))
+      value.each do |v|
+        command(prefix + COMMANDS[property][1].call(*v))
       end
     end
   end
diff --git a/lib/puppet/util/network_device/transport/ssh.rb b/lib/puppet/util/network_device/transport/ssh.rb
index 1c20eb822e3..370457d48b4 100644
--- a/lib/puppet/util/network_device/transport/ssh.rb
+++ b/lib/puppet/util/network_device/transport/ssh.rb
@@ -49,8 +49,8 @@ def connect(&block)
       channel.send_channel_request("shell") do |ch, success|
         raise "failed to open ssh shell channel" unless success
 
-        ch.on_data { |ch,data| @buf << data }
-        ch.on_extended_data { |ch,type,data|  @buf << data if type == 1 }
+        ch.on_data { |_,data| @buf << data }
+        ch.on_extended_data { |_,type,data|  @buf << data if type == 1 }
         ch.on_close { @eof = true }
 
         @channel = ch
diff --git a/lib/puppet/util/rdoc/code_objects.rb b/lib/puppet/util/rdoc/code_objects.rb
index 800b3690dd1..b67480e8e24 100644
--- a/lib/puppet/util/rdoc/code_objects.rb
+++ b/lib/puppet/util/rdoc/code_objects.rb
@@ -151,10 +151,10 @@ def find_symbol(symbol, method=nil)
           if result
             last_name = ""
             previous = nil
-            modules.each do |module_name|
+            modules.each do |mod|
               previous = result
-              last_name = module_name
-              result = result.find_module_named(module_name)
+              last_name = mod
+              result = result.find_module_named(mod)
               break unless result
             end
             unless result
diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index 2ad15c264b9..914c570679f 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -416,7 +416,7 @@ def value_hash
       @values["resources"] = rl unless rl.empty?
 
       @context.sections.each do |section|
-        secdata = @values["sections"].select { |secdata| secdata["secsequence"] == section.sequence }
+        secdata = @values["sections"].select { |s| s["secsequence"] == section.sequence }
         if secdata.size == 1
           secdata = secdata[0]
 
diff --git a/lib/puppet/util/rdoc/parser/puppet_parser_core.rb b/lib/puppet/util/rdoc/parser/puppet_parser_core.rb
index 9cb5d9efa1b..eeb26448cf8 100644
--- a/lib/puppet/util/rdoc/parser/puppet_parser_core.rb
+++ b/lib/puppet/util/rdoc/parser/puppet_parser_core.rb
@@ -52,10 +52,10 @@ def get_class_or_module(container, name)
     names = name.split('::')
 
     final_name = names.pop
-    names.each do |name|
+    names.each do |n|
       prev_container = container
-      container = find_object_named(container, name)
-      container ||= prev_container.add_class(RDoc::PuppetClass, name, nil)
+      container = find_object_named(container, n)
+      container ||= prev_container.add_class(RDoc::PuppetClass, n, nil)
     end
     [container, final_name]
   end