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

5 files changed, 37 insertions, 38 deletions

diff --git a/test/language/functions.rb b/test/language/functions.rb
index 9cd4d24b5..83bfe6e8e 100755
--- a/test/language/functions.rb
+++ b/test/language/functions.rb
@@ -40,7 +40,7 @@ class TestLangFunctions < Test::Unit::TestCase
 
         assert_nothing_raised do
             Puppet::Parser::Functions.newfunction(:fakefunction, :type => :rvalue) do |input|
-                return "output %s" % input[0]
+                return "output #{input[0]}"
             end
         end
 
@@ -80,7 +80,7 @@ class TestLangFunctions < Test::Unit::TestCase
                 val = func.evaluate(scope)
             end
 
-            assert_equal(retval, val, "'tagged' returned %s for %s" % [val, tag])
+            assert_equal(retval, val, "'tagged' returned #{val} for #{tag}")
         end
 
         # Now make sure we correctly get tags.
@@ -118,8 +118,7 @@ class TestLangFunctions < Test::Unit::TestCase
         twop = File.join(Puppet[:templatedir], "two")
 
         File.open(onep, "w") do |f|
-            f.puts "<%- if @one.nil? then raise '@one undefined' end -%>" +
-                "template <%= @one %>"
+            f.puts "<%- if @one.nil? then raise '@one undefined' end -%>template <%= @one %>"
         end
 
         File.open(twop, "w") do |f|
@@ -393,7 +392,7 @@ class TestLangFunctions < Test::Unit::TestCase
                 assert_equal(
                     "template #{value}\n", scope.lookupvar("output"),
 
-                        "%s did not get evaluated correctly" % string.inspect)
+                        "#{string.inspect} did not get evaluated correctly")
         end
     end
 
diff --git a/test/language/parser.rb b/test/language/parser.rb
index b26982d16..a6ce8cf3f 100755
--- a/test/language/parser.rb
+++ b/test/language/parser.rb
@@ -27,7 +27,7 @@ class TestParser < Test::Unit::TestCase
         textfiles { |file|
             Puppet::Node::Environment.clear
             parser = mkparser
-            Puppet.debug("parsing %s" % file) if __FILE__ == $0
+            Puppet.debug("parsing #{file}") if __FILE__ == $0
             assert_nothing_raised() {
                 parser.file = file
                 parser.parse
@@ -38,8 +38,8 @@ class TestParser < Test::Unit::TestCase
     def test_failers
         failers { |file|
             parser = mkparser
-            Puppet.debug("parsing failer %s" % file) if __FILE__ == $0
-            assert_raise(Puppet::ParseError, "Did not fail while parsing %s" % file) {
+            Puppet.debug("parsing failer #{file}") if __FILE__ == $0
+            assert_raise(Puppet::ParseError, "Did not fail while parsing #{file}") {
                 parser.file = file
                 ast = parser.parse
                 config = mkcompiler(parser)
@@ -76,7 +76,7 @@ class TestParser < Test::Unit::TestCase
     end
 
     def mkmanifest(file)
-        name = File.join(tmpdir, "file%s" % rand(100))
+        name = File.join(tmpdir, "file#{rand(100)}")
         @@tmpfiles << name
 
         File.open(file, "w") { |f|
@@ -97,7 +97,7 @@ class TestParser < Test::Unit::TestCase
         }
 
         4.times { |i|
-            path = File.join(basedir, subdir, "subfile%s" % i)
+            path = File.join(basedir, subdir, "subfile#{i}")
             mkmanifest(path)
         }
 
@@ -672,7 +672,7 @@ file { "/tmp/yayness":
         end
 
         [one, two].each do |file|
-            assert(@logs.detect { |l| l.message =~ /importing '#{file}'/}, "did not import %s" % file)
+            assert(@logs.detect { |l| l.message =~ /importing '#{file}'/}, "did not import #{file}")
         end
     end
 
@@ -723,7 +723,7 @@ file { "/tmp/yayness":
                 "one::two::three::four" => ["one::two::three", "four"],
             }.each do |name, ary|
                 result = parser.namesplit(name)
-                assert_equal(ary, result, "%s split to %s" % [name, result])
+                assert_equal(ary, result, "#{name} split to #{result}")
             end
         end
     end
@@ -741,6 +741,6 @@ file { "/tmp/yayness":
         assert_nothing_raised do
             result = parser.newdefine "FunTest"
         end
-        assert_equal(result, parser.find_definition("", "fUntEst"), "%s was not matched" % "fUntEst")
+        assert_equal(result, parser.find_definition("", "fUntEst"), "#{"fUntEst"} was not matched")
     end
 end
diff --git a/test/language/scope.rb b/test/language/scope.rb
index 16149a6be..09bf8a1c5 100755
--- a/test/language/scope.rb
+++ b/test/language/scope.rb
@@ -42,14 +42,14 @@ class TestScope < Test::Unit::TestCase
         # Set a variable in the top and make sure all three can get it
         topscope.setvar("first", "topval")
         scopes.each do |name, scope|
-            assert_equal("topval", scope.lookupvar("first", false), "Could not find var in %s" % name)
+            assert_equal("topval", scope.lookupvar("first", false), "Could not find var in #{name}")
         end
 
         # Now set a var in the midscope and make sure the mid and bottom can see it but not the top
         midscope.setvar("second", "midval")
         assert_equal(:undefined, scopes[:top].lookupvar("second", false), "Found child var in top scope")
         [:mid, :bot].each do |name|
-            assert_equal("midval", scopes[name].lookupvar("second", false), "Could not find var in %s" % name)
+            assert_equal("midval", scopes[name].lookupvar("second", false), "Could not find var in #{name}")
         end
 
         # And set something in the bottom, and make sure we only find it there.
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index 14a267d99..cd8015db7 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -24,21 +24,21 @@ class TestSnippets < Test::Unit::TestCase
 
     def assert_file(path, msg = nil)
         unless file = @catalog.resource(:file, path)
-            msg ||= "Could not find file %s" % path
+            msg ||= "Could not find file #{path}"
             raise msg
         end
     end
 
     def assert_not_file(path, msg = nil)
         if file = @catalog.resource(:file, path)
-            msg ||= "File %s exists!" % path
+            msg ||= "File #{path} exists!"
             raise msg
         end
     end
 
     def assert_mode_equal(mode, path)
         unless file = @catalog.resource(:file, path)
-            raise "Could not find file %s" % path
+            raise "Could not find file #{path}"
         end
 
         unless mode == file.should(:mode)
@@ -145,10 +145,10 @@ class TestSnippets < Test::Unit::TestCase
             [0..1, 0..2].each { |range|
                 params = rands[range]
                 paramstr = params.collect { |param|
-                    "%s => fake" % param
+                    "#{param} => fake"
                 }.join(", ")
 
-                str = "%s { %s }" % [name, paramstr]
+                str = "#{name} { #{paramstr} }"
 
                 scope = nil
                 assert_nothing_raised {
@@ -163,7 +163,7 @@ class TestSnippets < Test::Unit::TestCase
                 p defaults
 
                 params.each { |param|
-                    puts "%s => '%s'" % [name,param]
+                    puts "#{name} => '#{param}'"
                     assert(defaults.include?(param))
                 }
             }
@@ -176,7 +176,7 @@ class TestSnippets < Test::Unit::TestCase
 
     def snippet_filecreate
         %w{a b c d}.each { |letter|
-            path = "/tmp/create%stest" % letter
+            path = "/tmp/create#{letter}test"
             assert_file(path)
             if %w{a b}.include?(letter)
                 assert_mode_equal(0755, path)
@@ -192,7 +192,7 @@ class TestSnippets < Test::Unit::TestCase
 
     def snippet_simpleselector
         files = %w{a b c d}.collect { |letter|
-            path = "/tmp/snippetselect%stest" % letter
+            path = "/tmp/snippetselect#{letter}test"
             assert_file(path)
             assert_mode_equal(0755, path)
         }
@@ -202,7 +202,7 @@ class TestSnippets < Test::Unit::TestCase
         path = "/tmp/classtest"
 
         file = @catalog.resource(:file, path)
-        assert(file, "did not create file %s" % path)
+        assert(file, "did not create file #{path}")
 
         assert_equal( "/Stage[main]/Testing/Mytype[componentname]/File[/tmp/classtest]", file.path)
     end
@@ -233,13 +233,13 @@ class TestSnippets < Test::Unit::TestCase
 
         paths.each { |path|
             file = @catalog.resource(:file, path)
-            assert(file, "File %s is missing" % path)
+            assert(file, "File #{path} is missing")
             assert_mode_equal(0755, path)
         }
     end
 
     def snippet_implicititeration
-        paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
+        paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
 
         paths.each { |path|
             file = @catalog.resource(:file, path)
@@ -249,7 +249,7 @@ class TestSnippets < Test::Unit::TestCase
     end
 
     def snippet_multipleinstances
-        paths = %w{a b c}.collect { |l| "/tmp/multipleinstances%s" % l }
+        paths = %w{a b c}.collect { |l| "/tmp/multipleinstances#{l}" }
 
         paths.each { |path|
             assert_file(path)
@@ -275,7 +275,7 @@ class TestSnippets < Test::Unit::TestCase
     def snippet_selectorvalues
         nums = %w{1 2 3 4 5 6 7}
         files = nums.collect { |n|
-            "/tmp/selectorvalues%s" % n
+            "/tmp/selectorvalues#{n}"
         }
 
         files.each { |f|
@@ -287,7 +287,7 @@ class TestSnippets < Test::Unit::TestCase
     def snippet_singleselector
         nums = %w{1 2 3}
         files = nums.collect { |n|
-            "/tmp/singleselector%s" % n
+            "/tmp/singleselector#{n}"
         }
 
         files.each { |f|
@@ -303,7 +303,7 @@ class TestSnippets < Test::Unit::TestCase
 
     def disabled_snippet_classargtest
         [1,2].each { |num|
-            file = "/tmp/classargtest%s" % num
+            file = "/tmp/classargtest#{num}"
             assert_file(file)
             assert_mode_equal(0755, file)
         }
@@ -311,7 +311,7 @@ class TestSnippets < Test::Unit::TestCase
 
     def snippet_classheirarchy
         [1,2,3].each { |num|
-            file = "/tmp/classheir%s" % num
+            file = "/tmp/classheir#{num}"
             assert_file(file)
             assert_mode_equal(0755, file)
         }
@@ -319,14 +319,14 @@ class TestSnippets < Test::Unit::TestCase
 
     def snippet_singleary
         [1,2,3,4].each { |num|
-            file = "/tmp/singleary%s" % num
+            file = "/tmp/singleary#{num}"
             assert_file(file)
         }
     end
 
     def snippet_classincludes
         [1,2,3].each { |num|
-            file = "/tmp/classincludes%s" % num
+            file = "/tmp/classincludes#{num}"
             assert_file(file)
             assert_mode_equal(0755, file)
         }
@@ -348,7 +348,7 @@ class TestSnippets < Test::Unit::TestCase
         {   1 => 'a $quote',
             2 => 'some "\yayness\"'
         }.each { |count, str|
-            path = "/tmp/singlequote%s" % count
+            path = "/tmp/singlequote#{count}"
             assert_file(path)
             assert_equal(str, @catalog.resource(:file, path).parameter(:content).actual_content)
         }
@@ -378,7 +378,7 @@ class TestSnippets < Test::Unit::TestCase
     def snippet_deepclassheirarchy
         5.times { |i|
             i += 1
-            file = "/tmp/deepclassheir%s" % i
+            file = "/tmp/deepclassheir#{i}"
             assert_file(file)
         }
     end
@@ -487,8 +487,8 @@ class TestSnippets < Test::Unit::TestCase
 
         mname = "snippet_" + file.sub(/\.pp$/, '')
         if self.method_defined?(mname)
-            #eval("alias %s %s" % [testname, mname])
-            testname = ("test_" + mname).intern
+            #eval("alias #{testname} #{mname}")
+            testname = ("test_#{mname}").intern
             self.send(:define_method, testname) {
                 Puppet[:manifest] = snippet(file)
                 facts = {
diff --git a/test/language/transportable.rb b/test/language/transportable.rb
index 247a6eda3..4607c3947 100755
--- a/test/language/transportable.rb
+++ b/test/language/transportable.rb
@@ -75,7 +75,7 @@ class TestTransportable < Test::Unit::TestCase
         }
 
         top.flatten.each do |obj|
-            assert(objects.include?(obj), "Missing obj %s[%s]" % [obj.type, obj.name])
+            assert(objects.include?(obj), "Missing obj #{obj.type}[#{obj.name}]")
         end