Code smell: Use string interpolation

* Replaced 83 occurances of

      (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[])

  with

      \1#{\2}"

  3 Examples:

      The code:
          puts "PUPPET " + status + ": " + process + ", " + state
      becomes:
          puts "PUPPET " + status + ": " + process + ", #{state}"
      The code:
          puts "PUPPET " + status + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      The code:
          }.compact.join( "\n" ) + "\n" + t + "]\n"
      becomes:
          }.compact.join( "\n" ) + "\n#{t}" + "]\n"

* Replaced 21 occurances of (.*)" *[+] *" with \1

  3 Examples:

      The code:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      The code:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      becomes:
          puts "PUPPET #{status}: #{process}, #{state}"
      The code:
          res = self.class.name + ": #{@name}" + "\n"
      becomes:
          res = self.class.name + ": #{@name}\n"

* Don't use string concatenation to split lines unless they would be very long.

  Replaced 11 occurances of

      (.*)(['"]) *[+]
       *(['"])(.*)

  with

  3 Examples:

      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified " +
              "Puppet process is running and the state file is no " +
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
      The code:
          o.separator   "Mandatory arguments to long options are mandatory for " +
          "short options too."
      becomes:
          o.separator   "Mandatory arguments to long options are mandatory for short options too."
      The code:
          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."
      becomes:
          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."

* Replaced no occurances of do (.*?) end with {\1}

* Replaced 1488 occurances of

      "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b)

  with

  20 Examples:

      The code:
          args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",")
      becomes:
          args[0].split(/\./).map do |s| "dc=#{s}" end.join(",")
      The code:
          puts "%s" % Puppet.version
      becomes:
          puts "#{Puppet.version}"
      The code:
          raise "Could not find information for %s" % node
      becomes:
          raise "Could not find information for #{node}"
      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
      The code:
          Puppet.err "Could not run %s: %s" % [client_class, detail]
      becomes:
          Puppet.err "Could not run #{client_class}: #{detail}"
      The code:
          raise "Could not find handler for %s" % arg
      becomes:
          raise "Could not find handler for #{arg}"
      The code:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}"
      The code:
          raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail
      becomes:
          raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
      The code:
          raise "Could not find facts for %s" % Puppet[:certname]
      becomes:
          raise "Could not find facts for #{Puppet[:certname]}"
      The code:
          raise ArgumentError, "%s is not readable" % path
      becomes:
          raise ArgumentError, "#{path} is not readable"
      The code:
          raise ArgumentError, "Invalid handler %s" % name
      becomes:
          raise ArgumentError, "Invalid handler #{name}"
      The code:
          debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str]
      becomes:
          debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
      The code:
          raise Puppet::Error, "unknown cert type '%s'" % hash[:type]
      becomes:
          raise Puppet::Error, "unknown cert type '#{hash[:type]}'"
      The code:
          Puppet.info "Creating a new certificate request for %s" % Puppet[:certname]
      becomes:
          Puppet.info "Creating a new certificate request for #{Puppet[:certname]}"
      The code:
          "Cannot create alias %s: object already exists" % [name]
      becomes:
          "Cannot create alias #{name}: object already exists"
      The code:
          return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum]
      becomes:
          return "replacing from source #{metadata.source} with contents #{metadata.checksum}"
      The code:
          it "should have a %s parameter" % param do
      becomes:
          it "should have a #{param} parameter" do
      The code:
          describe "when registring '%s' messages" % log do
      becomes:
          describe "when registring '#{log}' messages" do
      The code:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
      becomes:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
      The code:
          assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do
      becomes:
          assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do

2 files changed, 22 insertions, 22 deletions

diff --git a/test/network/handler/fileserver.rb b/test/network/handler/fileserver.rb
index 357192716..3f93a74e0 100755
--- a/test/network/handler/fileserver.rb
+++ b/test/network/handler/fileserver.rb
@@ -46,7 +46,7 @@ class TestFileServer < Test::Unit::TestCase
         @@tmpfiles << testdir
         assert_nothing_raised {
             files = %w{a b c d e}.collect { |l|
-                name = File.join(testdir, "file%s" % l)
+                name = File.join(testdir, "file#{l}")
                 File.open(name, "w") { |f|
                     f.puts rand(100)
                 }
@@ -62,9 +62,9 @@ class TestFileServer < Test::Unit::TestCase
         file = File.basename(file)
         assert_nothing_raised {
             desc = server.describe(base + file)
-            assert(desc, "Got no description for %s" % file)
-            assert(desc != "", "Got no description for %s" % file)
-            assert_match(/^\d+/, desc, "Got invalid description %s" % desc)
+            assert(desc, "Got no description for #{file}")
+            assert(desc != "", "Got no description for #{file}")
+            assert_match(/^\d+/, desc, "Got invalid description #{desc}")
         }
     end
 
@@ -82,8 +82,8 @@ class TestFileServer < Test::Unit::TestCase
         }
 
         [" ", "=" "+", "&", "#", "*"].each do |char|
-            assert_raise(Puppet::Network::Handler::FileServerError, "'%s' did not throw a failure in fileserver module names" % char) {
-                server.mount("/tmp", "invalid%sname" % char)
+            assert_raise(Puppet::Network::Handler::FileServerError, "'#{char}' did not throw a failure in fileserver module names") {
+                server.mount("/tmp", "invalid#{char}name")
             }
         end
     end
@@ -250,12 +250,12 @@ class TestFileServer < Test::Unit::TestCase
 
         list = nil
         assert_nothing_raised {
-            list = server.list("/root/" + testdir, :manage, true, false)
+            list = server.list("/root/#{testdir}", :manage, true, false)
         }
 
         assert(list =~ pattern)
         assert_nothing_raised {
-            list = server.list("/root" + testdir, :manage, true, false)
+            list = server.list("/root#{testdir}", :manage, true, false)
         }
 
         assert(list =~ pattern)
@@ -276,10 +276,10 @@ class TestFileServer < Test::Unit::TestCase
 
         # make our deep recursion
         basedir = File.join(tmpdir(), "recurseremotetesting")
-        testdir = "%s/with/some/sub/directories/for/the/purposes/of/testing" % basedir
+        testdir = "#{basedir}/with/some/sub/directories/for/the/purposes/of/testing"
         oldfile = File.join(testdir, "oldfile")
         assert_nothing_raised {
-            system("mkdir -p %s" % testdir)
+            system("mkdir -p #{testdir}")
             File.open(oldfile, "w") { |f|
                 3.times { f.puts rand(100) }
             }
@@ -331,10 +331,10 @@ class TestFileServer < Test::Unit::TestCase
 
         # create a deep dir
         basedir = tempfile()
-        testdir = "%s/with/some/sub/directories/for/testing" % basedir
+        testdir = "#{basedir}/with/some/sub/directories/for/testing"
         oldfile = File.join(testdir, "oldfile")
         assert_nothing_raised {
-            system("mkdir -p %s" % testdir)
+            system("mkdir -p #{testdir}")
             File.open(oldfile, "w") { |f|
                 3.times { f.puts rand(100) }
             }
@@ -457,7 +457,7 @@ class TestFileServer < Test::Unit::TestCase
                 Puppet::Network::Handler::FileServerError,
 
             "Describing non-existent mount did not raise an error") {
-            retval = server.describe("/notmounted/" + "noexisties")
+            retval = server.describe("/notmounted/noexisties")
         }
 
         assert_nil(retval, "Description of non-existent mounts returned a value")
@@ -582,11 +582,11 @@ class TestFileServer < Test::Unit::TestCase
                         assert_raise(
                             Puppet::AuthorizationError,
 
-                            "Host %s, ip %s, allowed %s" % [host, ip, mount]) {
+                            "Host #{host}, ip #{ip}, allowed #{mount}") {
                                 list = server.list(mount, :manage, true, false, host, ip)
                         }
                     when :allow
-                        assert_nothing_raised("Host %s, ip %s, denied %s" % [host, ip, mount]) {
+                        assert_nothing_raised("Host #{host}, ip #{ip}, denied #{mount}") {
                             list = server.list(mount, :manage, true, false, host, ip)
                         }
                     end
@@ -662,7 +662,7 @@ class TestFileServer < Test::Unit::TestCase
                 assert_raise(
                     Puppet::Network::Handler::FileServerError,
 
-                    "Invalid config %s did not raise error" % i) {
+                    "Invalid config #{i} did not raise error") {
 
                         server = Puppet::Network::Handler.fileserver.new(
 
@@ -799,8 +799,8 @@ class TestFileServer < Test::Unit::TestCase
         assert_equal("link", results[:type])
 
         results.each { |p,v|
-            assert(v, "%s has no value" % p)
-            assert(v != "", "%s has no value" % p)
+            assert(v, "#{p} has no value")
+            assert(v != "", "#{p} has no value")
         }
     end
 
@@ -1063,7 +1063,7 @@ allow *
         }.each do |pattern, string|
             file = File.join(dir, string)
             mount = File.join(dir, pattern)
-            File.open(file, "w") do |f| f.puts "yayness: %s" % string end
+            File.open(file, "w") do |f| f.puts "yayness: #{string}" end
             name = "name"
             obj = nil
             assert_nothing_raised {
@@ -1229,7 +1229,7 @@ allow *
         "[valid]\nallow one.two.com\ndeny *.testing something.com", # invalid deny
         ].each do |failer|
             File.open(config, "w") { |f| f.puts failer }
-            assert_raise(Puppet::Network::Handler::FileServerError, "Did not fail on %s" % failer.inspect) {
+            assert_raise(Puppet::Network::Handler::FileServerError, "Did not fail on #{failer.inspect}") {
 
                 server = Puppet::Network::Handler::FileServer.new(
 
diff --git a/test/network/handler/report.rb b/test/network/handler/report.rb
index 914f03450..fa55137bc 100755
--- a/test/network/handler/report.rb
+++ b/test/network/handler/report.rb
@@ -38,7 +38,7 @@ class TestReportServer < Test::Unit::TestCase
         reports = []
         $run = []
         2.times do |i|
-            name = "processtest%s" % i
+            name = "processtest#{i}"
             reports << name
 
             Report.newreport(name) do
@@ -57,7 +57,7 @@ class TestReportServer < Test::Unit::TestCase
         }
 
         reports.each do |name|
-            assert($run.include?(name.intern), "Did not run %s" % name)
+            assert($run.include?(name.intern), "Did not run #{name}")
         end
 
         # Now make sure our server doesn't die on missing reports