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, 23 insertions, 23 deletions

diff --git a/test/ral/manager/attributes.rb b/test/ral/manager/attributes.rb
index 4502a3258..7e9f34b52 100755
--- a/test/ral/manager/attributes.rb
+++ b/test/ral/manager/attributes.rb
@@ -42,15 +42,15 @@ class TestTypeAttributes < Test::Unit::TestCase
             end
 
             if param == :property
-                assert(inst.property(param), "did not get obj for %s" % param)
+                assert(inst.property(param), "did not get obj for #{param}")
 
                             assert_equal(
                 true, inst.should(param),
         
                     "should value did not get set")
             else
-                assert_equal(true, inst[param], "did not get correct value for %s from symbol" % param)
-                assert_equal(true, inst[param.to_s], "did not get correct value for %s from string" % param)
+                assert_equal(true, inst[param], "did not get correct value for #{param} from symbol")
+                assert_equal(true, inst[param.to_s], "did not get correct value for #{param} from string")
             end
         end
     end
@@ -88,11 +88,11 @@ class TestTypeAttributes < Test::Unit::TestCase
     def attr_check(type)
         @num ||= 0
         @num += 1
-        name = "name%s" % @num
+        name = "name#{@num}"
         inst = type.new(:name => name)
         [:meta, :param, :prop].each do |name|
             klass = type.attrclass(name)
-            assert(klass, "did not get class for %s" % name)
+            assert(klass, "did not get class for #{name}")
             obj = yield inst, klass
             assert_instance_of(klass, obj, "did not get object back")
 
@@ -112,7 +112,7 @@ class TestTypeAttributes < Test::Unit::TestCase
         {
             :meta => :newmetaparam, :param => :newparam, :prop => :newproperty
         }.each do |name, method|
-            assert_nothing_raised("Could not make %s of type %s" % [name, method]) do
+            assert_nothing_raised("Could not make #{name} of type #{method}") do
                 type.send(method, name) {}
             end
         end
@@ -157,13 +157,13 @@ class TestTypeAttributes < Test::Unit::TestCase
             :four => :two
         }
         aliases.each do |new, old|
-            assert_nothing_raised("Could not create alias parameter %s" % new) do
+            assert_nothing_raised("Could not create alias parameter #{new}") do
                 type.set_attr_alias new => old
             end
         end
 
         aliases.each do |new, old|
-            assert_equal(old, type.attr_alias(new), "did not return alias info for %s" % new)
+            assert_equal(old, type.attr_alias(new), "did not return alias info for #{new}")
         end
 
         assert_nil(type.attr_alias(:name), "got invalid alias info for name")
@@ -172,7 +172,7 @@ class TestTypeAttributes < Test::Unit::TestCase
         assert(inst, "could not create instance")
 
         aliases.each do |new, old|
-            val = "value %s" % new
+            val = "value #{new}"
             assert_nothing_raised do
                 inst[new] = val
             end
@@ -183,12 +183,12 @@ class TestTypeAttributes < Test::Unit::TestCase
                             assert_equal(
                 val, inst[new],
         
-                    "Incorrect alias value for %s in []" % new)
+                    "Incorrect alias value for #{new} in []")
             else
-                assert_equal(val, inst.should(new), "Incorrect alias value for %s in should" % new)
+                assert_equal(val, inst.should(new), "Incorrect alias value for #{new} in should")
             end
-            assert_equal(val, inst.value(new), "Incorrect alias value for %s" % new)
-            assert_equal(val, inst.value(old), "Incorrect orig value for %s" % old)
+            assert_equal(val, inst.value(new), "Incorrect alias value for #{new}")
+            assert_equal(val, inst.value(old), "Incorrect orig value for #{old}")
         end
     end
 
@@ -214,7 +214,7 @@ class TestTypeAttributes < Test::Unit::TestCase
 
         # Now make sure that we get warnings and no properties in those cases where our providers do not support the features requested
         [nope, maybe, yep].each_with_index do |prov, i|
-            resource = type.new(:provider => prov.name, :name => "test%s" % i, :none => "a", :one => "b", :two => "c")
+            resource = type.new(:provider => prov.name, :name => "test#{i}", :none => "a", :one => "b", :two => "c")
 
             case prov.name
             when :nope
@@ -227,7 +227,7 @@ class TestTypeAttributes < Test::Unit::TestCase
                 yes = [:none, :one, :two]
                 no = []
             end
-            yes.each { |a| assert(resource.should(a), "Did not get value for %s in %s" % [a, prov.name]) }
+            yes.each { |a| assert(resource.should(a), "Did not get value for #{a} in #{prov.name}") }
             no.each do |a|
                 # These may or may not get passed to the provider. We shouldn't care.
             end
@@ -245,14 +245,14 @@ class TestTypeAttributes < Test::Unit::TestCase
         inst = klass.new(:resource => resource)
 
         {:property => [:owner, :group], :parameter => [:ignore, :recurse], :metaparam => [:require, :subscribe]}.each do |attrtype, attrs|
-            assert_nothing_raised("Could not set check to a single %s value" % attrtype) do
+            assert_nothing_raised("Could not set check to a single #{attrtype} value") do
                 inst.value = attrs[0]
             end
 
             if attrtype == :property
                 assert(resource.property(attrs[0]), "Check did not create property instance during single check")
             end
-            assert_nothing_raised("Could not set check to multiple %s values" % attrtype) do
+            assert_nothing_raised("Could not set check to multiple #{attrtype} values") do
                 inst.value = attrs
             end
             if attrtype == :property
diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb
index 04ecf95ea..9182dab09 100755
--- a/test/ral/manager/type.rb
+++ b/test/ral/manager/type.rb
@@ -10,11 +10,11 @@ class TestType < Test::Unit::TestCase
     def test_typemethods
         Puppet::Type.eachtype { |type|
             name = nil
-            assert_nothing_raised("Searching for name for %s caused failure" % type.to_s) {
+            assert_nothing_raised("Searching for name for #{type} caused failure") {
                     name = type.name
             }
 
-            assert(name, "Could not find name for %s" % type.to_s)
+            assert(name, "Could not find name for #{type}")
 
 
                         assert_equal(
@@ -22,7 +22,7 @@ class TestType < Test::Unit::TestCase
                 type,
                 Puppet::Type.type(name),
         
-                "Failed to retrieve %s by name" % name
+                "Failed to retrieve #{name} by name"
             )
 
             # Skip types with no parameters or valid properties
@@ -36,7 +36,7 @@ class TestType < Test::Unit::TestCase
                 
                     type.properties,
         
-                    "Properties for %s are nil" % name
+                    "Properties for #{name} are nil"
                 )
 
 
@@ -44,7 +44,7 @@ class TestType < Test::Unit::TestCase
                 
                     type.validproperties,
         
-                    "Valid properties for %s are nil" % name
+                    "Valid properties for #{name} are nil"
                 )
             }
         }
@@ -345,7 +345,7 @@ class TestType < Test::Unit::TestCase
         end
 
         [:path, :owner, :recurse, :loglevel].each do |param|
-            assert(hash[param], "Hash did not include %s" % param)
+            assert(hash[param], "Hash did not include #{param}")
         end
     end