Merge pull request #6643 from jhelwig/PUP-8463-warning-cleanup-and-th…

…e-goblet-of-fire

(PUP-8463) Remove "ambiguous first argument", and "assigned but unused variable" warnings

lib/puppet/pops/evaluator/runtime3_support.rb

@@ -301,10 +301,13 @@ def external_call_function(name, args, scope, &block)
   def call_function(name, args, o, scope, &block)
     file, line = extract_file_line(o)
     loader = Adapters::LoaderAdapter.loader_for_model_object(o, file)
-    if loader && func = loader.load(:function, name)
+    # 'ruby -wc' thinks that _func is unused, because the only reference to it
+    # is inside of the Kernel.eval string below. By prefixing it with the
+    # underscore, we let Ruby know to not worry about whether it's unused or not.
+    if loader && _func = loader.load(:function, name)
       Puppet::Util::Profiler.profile(name, [:functions, name]) do
         # Add stack frame when calling. See Puppet::Pops::PuppetStack
-        return Kernel.eval('func.call(scope, *args, &block)', Kernel.binding, file || '', line)
+        return Kernel.eval('_func.call(scope, *args, &block)', Kernel.binding, file || '', line)
       end
     end
     # Call via 3x API if function exists there

lib/puppet/provider/package/pkgdmg.rb

@@ -95,7 +95,7 @@ def self.installpkgdmg(source, name)
             Puppet.debug "Success: curl transferred [#{name}] (via: curl #{args.join(" ")})"
         rescue Puppet::ExecutionFailure
           Puppet.debug "curl #{args.join(" ")} did not transfer [#{name}].  Falling back to local file." # This used to fall back to open-uri. -NF
-            cached_source = source
+          cached_source = source
         end
       end
 

lib/puppet/type/user.rb

@@ -411,7 +411,7 @@ def insync?(current)
            a zero-padded YYYY-MM-DD format -- for example, 2010-02-19."
 
       newvalues :absent
-      newvalues /^\d{4}-\d{2}-\d{2}$/
+      newvalues(/^\d{4}-\d{2}-\d{2}$/)
 
       validate do |value|
         if value.intern != :absent and value !~ /^\d{4}-\d{2}-\d{2}$/

lib/puppet/util/network_device/cisco/facts.rb

@@ -63,7 +63,7 @@ def ios_major_version(version)
   end
 
   def uptime_to_seconds(uptime)
-    captures = (uptime.match /^(?:(\d+) years?,)?\s*(?:(\d+) weeks?,)?\s*(?:(\d+) days?,)?\s*(?:(\d+) hours?,)?\s*(\d+) minutes?$/).captures
+    captures = (uptime.match(/^(?:(\d+) years?,)?\s*(?:(\d+) weeks?,)?\s*(?:(\d+) days?,)?\s*(?:(\d+) hours?,)?\s*(\d+) minutes?$/)).captures
     captures.zip([31536000, 604800, 86400, 3600, 60]).inject(0) do |total, (x,y)|
       total + (x.nil? ? 0 : x.to_i * y)
     end

spec/fixtures/unit/pops/binder/bindings_composer/ok/modules/awesome2/lib/puppet_x/awesome2/echo_scheme_handler.rb

@@ -11,7 +11,7 @@ def contributed_bindings(uri, scope, composer)
         factory = ::Puppet::Pops::Binder::BindingsFactory
         bindings = factory.named_bindings("echo")
         bindings.bind.name(uri.path.gsub(/\//, '::')).to("echo: #{uri.path.gsub(/\//, ' ').strip!}")
-        result = factory.contributed_bindings("echo", bindings.model) ### , nil)
+        factory.contributed_bindings("echo", bindings.model) ### , nil)
       end
     end
   end

spec/integration/application/apply_spec.rb

@@ -493,7 +493,7 @@ def bogus()
 
       it 'will log a warning that a value of unknown type is converted into a string' do
         logs = []
-        json = Puppet::Util::Log.with_destination(Puppet::Test::LogCollector.new(logs)) do
+        Puppet::Util::Log.with_destination(Puppet::Test::LogCollector.new(logs)) do
           compile_to_catalog('include amod::bad_type', node).to_json
         end
         logs = logs.select { |log| log.level == :warning }.map { |log| log.message }

spec/integration/parser/catalog_spec.rb

@@ -90,7 +90,7 @@ class real {
   end
 
   def master_catalog_for(manifest)
-    master_catalog = Puppet::Resource::Catalog::Compiler.new.filter(compile_to_catalog(manifest, node))
+    Puppet::Resource::Catalog::Compiler.new.filter(compile_to_catalog(manifest, node))
   end
 
   def master_and_agent_catalogs_for(manifest)

spec/integration/parser/collection_spec.rb

@@ -180,7 +180,7 @@ def expect_the_message_to_be(expected_messages, code, node = Puppet::Node.new('t
     it "does not collect classes" do
       node = Puppet::Node.new('the node')
       expect do
-        catalog = compile_to_catalog(<<-MANIFEST, node)
+        compile_to_catalog(<<-MANIFEST, node)
           class theclass {
             @notify { "testing": message => "good message" }
           }
@@ -192,7 +192,7 @@ class theclass {
     it "does not collect resources that don't exist" do
       node = Puppet::Node.new('the node')
       expect do
-        catalog = compile_to_catalog(<<-MANIFEST, node)
+        compile_to_catalog(<<-MANIFEST, node)
           class theclass {
             @notify { "testing": message => "good message" }
           }

spec/integration/parser/compiler_spec.rb

@@ -259,7 +259,7 @@ class foo::bar::example($x = 100)  {
     it "should not create duplicate resources when a class is referenced both directly and indirectly by the node classifier (4792)" do
       node = Puppet::Node.new("testnodex")
       node.classes = ['foo', 'bar']
-      catalog = compile_to_catalog(<<-PP, node)
+      compile_to_catalog(<<-PP, node)
         class foo
         {
           notify { foo_notify: }
@@ -401,7 +401,7 @@ class baz {
 
       it "should not favor local name scope" do
         expect {
-          catalog = compile_to_catalog(<<-PP)
+          compile_to_catalog(<<-PP)
             class experiment {
               class baz {
               }
@@ -683,7 +683,7 @@ class a { $_a = 10}
 
       it 'an initial underscore in not ok if elsewhere than last segment' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class a { $_a = 10}
             include a
             notify { 'test': message => $_a::_a }
@@ -769,7 +769,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'accepts anything when parameters are untyped' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
           define foo($a, $b, $c) { }
           foo { 'test': a => String, b=>10, c=>undef }
           MANIFEST
@@ -778,7 +778,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'denies non type compliant arguments' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(Integer $x) { }
             foo { 'test': x =>'say friend' }
           MANIFEST
@@ -787,7 +787,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'denies undef for a non-optional type' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(Integer $x) { }
             foo { 'test': x => undef }
           MANIFEST
@@ -796,7 +796,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'denies non type compliant default argument' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(Integer $x = 'pow') { }
             foo { 'test':  }
           MANIFEST
@@ -805,7 +805,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'denies undef as the default for a non-optional type' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(Integer $x = undef) { }
             foo { 'test':  }
           MANIFEST
@@ -826,7 +826,7 @@ class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
 
       it 'uses infer_set when reporting type mismatch' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(Struct[{b => Integer, d=>String}] $a) { }
             foo{ bar: a => {b => 5, c => 'stuff'}}
           MANIFEST
@@ -855,7 +855,7 @@ class { 'foo': }
 
       it 'accepts anything when parameters are untyped' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo($a, $b, $c) { }
             class { 'foo': a => String, b=>10, c=>undef }
           MANIFEST
@@ -864,7 +864,7 @@ class { 'foo': a => String, b=>10, c=>undef }
 
       it 'denies non type compliant arguments' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo(Integer $x) { }
             class { 'foo': x =>'say friend' }
           MANIFEST
@@ -873,7 +873,7 @@ class { 'foo': x =>'say friend' }
 
       it 'denies undef for a non-optional type' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo(Integer $x) { }
             class { 'foo': x => undef }
           MANIFEST
@@ -882,7 +882,7 @@ class { 'foo': x => undef }
 
       it 'denies non type compliant default argument' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo(Integer $x = 'pow') { }
             class { 'foo':  }
           MANIFEST
@@ -891,7 +891,7 @@ class { 'foo':  }
 
       it 'denies undef as the default for a non-optional type' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo(Integer $x = undef) { }
             class { 'foo':  }
           MANIFEST
@@ -900,7 +900,7 @@ class { 'foo':  }
 
       it 'denies a regexp (rich data) argument given to class String parameter (even if later encoding of it is a string)' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             class foo(String $x) { }
             class { 'foo':  x => /I am a regexp and I don't want to be a String/}
           MANIFEST
@@ -909,7 +909,7 @@ class { 'foo':  x => /I am a regexp and I don't want to be a String/}
 
       it 'denies a regexp (rich data) argument given to define String parameter (even if later encoding of it is a string)' do
         expect do
-          catalog = compile_to_catalog(<<-MANIFEST)
+          compile_to_catalog(<<-MANIFEST)
             define foo(String $x) { }
             foo { 'foo':  x => /I am a regexp and I don't want to be a String/}
           MANIFEST
@@ -1162,7 +1162,7 @@ def assert_created_relationships(catalog, type_name, expectations)
     it 'errors when an alias cannot be found when relationship is formed with -> operator' do
       node = Puppet::Node.new("testnodex")
       expect {
-        catalog = compile_to_catalog(<<-PP, node)
+        compile_to_catalog(<<-PP, node)
           notify { 'actual_2':  }
           notify { 'actual_1': alias => 'alias_1' }
           Notify[actual_2] -> Notify[alias_2]
@@ -1190,7 +1190,6 @@ class foo {
         include foo
         include bar
       MANIFEST
-      package = catalog.resource('Package', 'pip')
       expect(catalog.resource('Package', 'pip')[:require].to_s).to eql('Package[python]')
     end
 

spec/integration/parser/pcore_resource_spec.rb

@@ -227,7 +227,7 @@ module Puppet
       pending "assertion of parameter types not yet implemented"
       genface.types
       expect {
-      catalog = compile_to_catalog(<<-MANIFEST)
+      compile_to_catalog(<<-MANIFEST)
         test2 { 'b':
           color => 'white is not a color'
         }
@@ -237,7 +237,7 @@ module Puppet
   end
 
   def find_resource_type(scope, name)
-    t1 = Puppet::Pops::Evaluator::Runtime3ResourceSupport.find_resource_type(scope, name)
+    Puppet::Pops::Evaluator::Runtime3ResourceSupport.find_resource_type(scope, name)
   end
 
   def generate_and_in_a_compilers_context(&block)

spec/integration/parser/scope_spec.rb

@@ -20,7 +20,7 @@ def expect_the_message_not_to_be(message, node = Puppet::Node.new('the node'))
   describe "using unsupported operators" do
     it "issues an error for +=" do
       expect do
-        catalog = compile_to_catalog(<<-MANIFEST)
+        compile_to_catalog(<<-MANIFEST)
               $var = ["top_msg"]
               node default {
                 $var += ["override"]
@@ -31,7 +31,7 @@ def expect_the_message_not_to_be(message, node = Puppet::Node.new('the node'))
 
     it "issues an error for -=" do
       expect do
-        catalog = compile_to_catalog(<<-MANIFEST)
+        compile_to_catalog(<<-MANIFEST)
               $var = ["top_msg"]
               node default {
                 $var -= ["top_msg"]

spec/integration/ssl/certificate_authority_spec.rb

@@ -36,16 +36,16 @@
 
   describe "when signing certificates" do
     it "should save the signed certificate" do
-      host = certificate_request_for("luke.madstop.com")
+      certificate_request_for("luke.madstop.com")
 
       ca.sign("luke.madstop.com")
 
       expect(Puppet::SSL::Certificate.indirection.find("luke.madstop.com")).to be_instance_of(Puppet::SSL::Certificate)
     end
 
     it "should be able to sign multiple certificates" do
-      host = certificate_request_for("luke.madstop.com")
-      other = certificate_request_for("other.madstop.com")
+      certificate_request_for("luke.madstop.com")
+      certificate_request_for("other.madstop.com")
 
       ca.sign("luke.madstop.com")
       ca.sign("other.madstop.com")
@@ -55,7 +55,7 @@
     end
 
     it "should save the signed certificate to the :signeddir" do
-      host = certificate_request_for("luke.madstop.com")
+      certificate_request_for("luke.madstop.com")
 
       ca.sign("luke.madstop.com")
 
@@ -64,16 +64,16 @@
     end
 
     it "should save valid certificates" do
-      host = certificate_request_for("luke.madstop.com")
+      certificate_request_for("luke.madstop.com")
 
       ca.sign("luke.madstop.com")
 
-      unless ssl = Puppet::Util::which('openssl')
+      unless Puppet::Util::which('openssl')
         pending "No ssl available"
       else
         ca_cert = Puppet[:cacert]
         client_cert = File.join(Puppet[:signeddir], "luke.madstop.com.pem")
-        output = %x{openssl verify -CAfile #{ca_cert} #{client_cert}}
+        %x{openssl verify -CAfile #{ca_cert} #{client_cert}}
         expect($CHILD_STATUS).to eq(0)
       end
     end

spec/integration/ssl/certificate_revocation_list_spec.rb

@@ -24,7 +24,7 @@
   }
 
   it "should be able to read in written out CRLs with no revoked certificates" do
-    ca = Puppet::SSL::CertificateAuthority.new
+    Puppet::SSL::CertificateAuthority.new
 
     raise "CRL not created" unless Puppet::FileSystem.exist?(Puppet[:hostcrl])
 

spec/integration/ssl/key_spec.rb

@@ -84,7 +84,7 @@
       # note incorrect password is an error
       expect do
         Puppet::FileSystem.open(pem_path, nil, 'r:ASCII') do |f|
-          reloaded_key = OpenSSL::PKey::RSA.new(f.read, 'invalid_password')
+          OpenSSL::PKey::RSA.new(f.read, 'invalid_password')
         end
       end.to raise_error(OpenSSL::PKey::RSAError)
 

spec/integration/transaction/report_spec.rb

@@ -87,7 +87,7 @@ def new_catalog(resources = [])
     end
 
     def get_cc_count(report)
-      cc = report.metrics["resources"].values.each do |v|
+      report.metrics["resources"].values.each do |v|
         if v[0] == "corrective_change"
           return v[2]
         end

spec/integration/transaction_spec.rb

@@ -146,7 +146,6 @@ def touch(path)
   # Verify that one component requiring another causes the contained
   # resources in the requiring component to get refreshed.
   it "should propagate events from a contained resource through its container to its dependent container's contained resources" do
-    transaction = nil
     file = Puppet::Type.type(:file).new :path => tmpfile("event_propagation"), :ensure => :present
     execfile = File.join(tmpdir("exec_event"), "exectestingness2")
     exec = Puppet::Type.type(:exec).new :command => touch(execfile), :path => ENV['PATH']

spec/integration/type/file_spec.rb

@@ -542,8 +542,6 @@ def get_aces_for_path_by_sid(path, sid)
         File.open(dest1, "w") { |f| f.puts "whatever" }
         Puppet::FileSystem.symlink(dest1, link)
 
-        d = filebucket_digest.call(File.read(file[:path]))
-
         catalog.apply
 
         expect(Puppet::FileSystem.readlink(link)).to eq(dest2)
@@ -1031,8 +1029,8 @@ def build_path(dir)
     before do
       source = tmpdir("generating_in_catalog_source")
 
-      s1 = file_in_dir_with_contents(source, "one", "uno")
-      s2 = file_in_dir_with_contents(source, "two", "dos")
+      file_in_dir_with_contents(source, "one", "uno")
+      file_in_dir_with_contents(source, "two", "dos")
 
       @file = described_class.new(
         :name => path,

spec/integration/type_spec.rb

@@ -21,8 +21,7 @@
 
   it "should not lose its provider parameter when it is reloaded" do
     type = Puppet::Type.newtype(:reload_test_type)
-
-    provider = type.provide(:test_provider)
+    type.provide(:test_provider)
 
     # reload it
     type = Puppet::Type.newtype(:reload_test_type)