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

4 files changed, 44 insertions, 44 deletions

diff --git a/lib/puppet/network/handler/ca.rb b/lib/puppet/network/handler/ca.rb
index 20d971007..8bafb5f9b 100644
--- a/lib/puppet/network/handler/ca.rb
+++ b/lib/puppet/network/handler/ca.rb
@@ -35,13 +35,13 @@ class Puppet::Network::Handler
 
             # we only otherwise know how to handle files
             unless autosign =~ /^\//
-                raise Puppet::Error, "Invalid autosign value %s" % autosign.inspect
+                raise Puppet::Error, "Invalid autosign value #{autosign.inspect}"
             end
 
             unless FileTest.exists?(autosign)
                 unless defined?(@@warnedonautosign)
                     @@warnedonautosign = true
-                    Puppet.info "Autosign is enabled but %s is missing" % autosign
+                    Puppet.info "Autosign is enabled but #{autosign} is missing"
                 end
                 return false
             end
@@ -89,7 +89,7 @@ class Puppet::Network::Handler
             hostname = nameary[1]
 
             unless @ca
-                Puppet.notice "Host %s asked for signing from non-CA master" % hostname
+                Puppet.notice "Host #{hostname} asked for signing from non-CA master"
                 return ""
             end
 
@@ -102,9 +102,9 @@ class Puppet::Network::Handler
             # first check to see if we already have a signed cert for the host
             cert, cacert = ca.getclientcert(hostname)
             if cert and cacert
-                Puppet.info "Retrieving existing certificate for %s" % hostname
+                Puppet.info "Retrieving existing certificate for #{hostname}"
                 unless csr.public_key.to_s == cert.public_key.to_s
-                    raise Puppet::Error, "Certificate request does not match existing certificate; run 'puppetca --clean %s'." % hostname
+                    raise Puppet::Error, "Certificate request does not match existing certificate; run 'puppetca --clean #{hostname}'."
                 end
                 return [cert.to_pem, cacert.to_pem]
             elsif @ca
@@ -115,15 +115,15 @@ class Puppet::Network::Handler
                     # okay, we don't have a signed cert
                     # if we're a CA and autosign is turned on, then go ahead and sign
                     # the csr and return the results
-                    Puppet.info "Signing certificate for %s" % hostname
+                    Puppet.info "Signing certificate for #{hostname}"
                     cert, cacert = @ca.sign(csr)
-                    #Puppet.info "Cert: %s; Cacert: %s" % [cert.class, cacert.class]
+                    #Puppet.info "Cert: #{cert.class}; Cacert: #{cacert.class}"
                     return [cert.to_pem, cacert.to_pem]
                 else # just write out the csr for later signing
                     if @ca.getclientcsr(hostname)
-                        Puppet.info "Not replacing existing request from %s" % hostname
+                        Puppet.info "Not replacing existing request from #{hostname}"
                     else
-                        Puppet.notice "Host %s has a waiting certificate request" % hostname
+                        Puppet.notice "Host #{hostname} has a waiting certificate request"
                         @ca.storeclientcsr(csr)
                     end
                     return ["", ""]
@@ -142,7 +142,7 @@ class Puppet::Network::Handler
             if FileTest.exists?(pkeyfile)
                 currentkey = File.open(pkeyfile) { |k| k.read }
                 unless currentkey == public_key.to_s
-                    raise Puppet::Error, "public keys for %s differ" % hostname
+                    raise Puppet::Error, "public keys for #{hostname} differ"
                 end
             else
                 File.open(pkeyfile, "w", 0644) { |f|
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index a48d05d91..756e74909 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -67,7 +67,7 @@ class Puppet::Network::Handler
 
             mount, path = convert(url, client, clientip)
 
-            mount.debug("Describing %s for %s" % [url, client]) if client
+            mount.debug("Describing #{url} for #{client}") if client
 
             # use the mount to resolve the path for us.
             return "" unless full_path = mount.file_path(path, client)
@@ -107,7 +107,7 @@ class Puppet::Network::Handler
             if hash.include?(:Mount)
                 @passedconfig = true
                 unless hash[:Mount].is_a?(Hash)
-                    raise Puppet::DevError, "Invalid mount hash %s" % hash[:Mount].inspect
+                    raise Puppet::DevError, "Invalid mount hash #{hash[:Mount].inspect}"
                 end
 
                 hash[:Mount].each { |dir, name|
@@ -131,14 +131,14 @@ class Puppet::Network::Handler
         def list(url, links = :ignore, recurse = false, ignore = false, client = nil, clientip = nil)
             mount, path = convert(url, client, clientip)
 
-            mount.debug "Listing %s for %s" % [url, client] if client
+            mount.debug "Listing #{url} for #{client}" if client
 
             return "" unless mount.path_exists?(path, client)
 
             desc = mount.list(path, recurse, ignore, client)
 
             if desc.length == 0
-                mount.notice "Got no information on //%s/%s" % [mount, path]
+                mount.notice "Got no information on //#{mount}/#{path}"
                 return ""
             end
 
@@ -158,7 +158,7 @@ class Puppet::Network::Handler
         def mount(path, name)
             if @mounts.include?(name)
                 if @mounts[name] != path
-                    raise FileServerError, "%s is already mounted at %s" % [@mounts[name].path, name]
+                    raise FileServerError, "#{@mounts[name].path} is already mounted at #{name}"
                 else
                     # it's already mounted; no problem
                     return
@@ -167,7 +167,7 @@ class Puppet::Network::Handler
 
             # Let the mounts do their own error-checking.
             @mounts[name] = Mount.new(name, path)
-            @mounts[name].info "Mounted %s" % path
+            @mounts[name].info "Mounted #{path}"
 
             return @mounts[name]
         end
@@ -180,7 +180,7 @@ class Puppet::Network::Handler
             mount, path = convert(url, client, clientip)
 
             if client
-                mount.info "Sending %s to %s" % [url, client]
+                mount.info "Sending #{url} to #{client}"
             end
 
             unless mount.path_exists?(path, client)
@@ -217,8 +217,8 @@ class Puppet::Network::Handler
                 clientip = nil
             end
             unless mount.allowed?(client, clientip)
-                mount.warning "%s cannot access %s" % [client, file]
-                raise Puppet::AuthorizationError, "Cannot access %s" % mount
+                mount.warning "#{client} cannot access #{file}"
+                raise Puppet::AuthorizationError, "Cannot access #{mount}"
             end
         end
 
@@ -280,7 +280,7 @@ class Puppet::Network::Handler
                         when /\[([-\w]+)\]/
                             name = $1
                             if newmounts.include?(name)
-                                raise FileServerError, "%s is already mounted as %s in %s" % [newmounts[name], name, @configuration.file]
+                                raise FileServerError, "#{newmounts[name]} is already mounted as #{name} in #{@configuration.file}"
                             end
                             mount = Mount.new(name)
                             newmounts[name] = mount
@@ -295,14 +295,14 @@ class Puppet::Network::Handler
                                     begin
                                         mount.path = value
                                     rescue FileServerError => detail
-                                        Puppet.err "Removing mount %s: %s" % [mount.name, detail]
+                                        Puppet.err "Removing mount #{mount.name}: #{detail}"
                                         newmounts.delete(mount.name)
                                     end
                                 end
                             when "allow"
                                 value.split(/\s*,\s*/).each { |val|
                                     begin
-                                        mount.info "allowing %s access" % val
+                                        mount.info "allowing #{val} access"
                                         mount.allow(val)
                                     rescue AuthStoreError => detail
                                         puts detail.backtrace if Puppet[:trace]
@@ -316,7 +316,7 @@ class Puppet::Network::Handler
                             when "deny"
                                 value.split(/\s*,\s*/).each { |val|
                                     begin
-                                        mount.info "denying %s access" % val
+                                        mount.info "denying #{val} access"
                                         mount.deny(val)
                                     rescue AuthStoreError => detail
 
@@ -327,19 +327,19 @@ class Puppet::Network::Handler
                                     end
                                 }
                             else
-                                raise FileServerError.new("Invalid argument '%s'" % var, count, @configuration.file)
+                                raise FileServerError.new("Invalid argument '#{var}'", count, @configuration.file)
                             end
                         else
-                            raise FileServerError.new("Invalid line '%s'" % line.chomp, count, @configuration.file)
+                            raise FileServerError.new("Invalid line '#{line.chomp}'", count, @configuration.file)
                         end
                         count += 1
                     }
                 }
             rescue Errno::EACCES => detail
-                Puppet.err "FileServer error: Cannot read %s; cannot serve" % @configuration
-                #raise Puppet::Error, "Cannot read %s" % @configuration
+                Puppet.err "FileServer error: Cannot read #{@configuration}; cannot serve"
+                #raise Puppet::Error, "Cannot read #{@configuration}"
             rescue Errno::ENOENT => detail
-                Puppet.err "FileServer error: '%s' does not exist; cannot serve" % @configuration
+                Puppet.err "FileServer error: '#{@configuration}' does not exist; cannot serve"
             end
 
             unless newmounts[MODULES]
@@ -380,7 +380,7 @@ class Puppet::Network::Handler
             # pointing to the specific problem.
             newmounts.each { |name, mount|
                 unless mount.valid?
-                    raise FileServerError, "Invalid mount %s" % name
+                    raise FileServerError, "Invalid mount #{name}"
                 end
             }
             @mounts = newmounts
@@ -398,11 +398,11 @@ class Puppet::Network::Handler
 
                 unless mount = modules_mount(mount_name, client)
                     unless mount = @mounts[mount_name]
-                        raise FileServerError, "Fileserver module '%s' not mounted" % mount_name
+                        raise FileServerError, "Fileserver module '#{mount_name}' not mounted"
                     end
                 end
             else
-                raise FileServerError, "Fileserver error: Invalid path '%s'" % dir
+                raise FileServerError, "Fileserver error: Invalid path '#{dir}'"
             end
 
             if path.nil? or path == ''
@@ -448,7 +448,7 @@ class Puppet::Network::Handler
                 if client
                     map = clientmap(client)
                 else
-                    Puppet.notice "No client; expanding '%s' with local host" % path
+                    Puppet.notice "No client; expanding '#{path}' with local host"
                     # Else, use the local information
                     map = localmap()
                 end
@@ -490,7 +490,7 @@ class Puppet::Network::Handler
             # Create out object.  It must have a name.
             def initialize(name, path = nil)
                 unless name =~ %r{^[-\w]+$}
-                    raise FileServerError, "Invalid name format '%s'" % name
+                    raise FileServerError, "Invalid name format '#{name}'"
                 end
                 @name = name
 
@@ -573,13 +573,13 @@ class Puppet::Network::Handler
                     @expandable = true
                 else
                     unless FileTest.exists?(path)
-                        raise FileServerError, "%s does not exist" % path
+                        raise FileServerError, "#{path} does not exist"
                     end
                     unless FileTest.directory?(path)
-                        raise FileServerError, "%s is not a directory" % path
+                        raise FileServerError, "#{path} is not a directory"
                     end
                     unless FileTest.readable?(path)
-                        raise FileServerError, "%s is not readable" % path
+                        raise FileServerError, "#{path} is not readable"
                     end
                     @expandable = false
                 end
@@ -617,7 +617,7 @@ class Puppet::Network::Handler
             end
 
             def to_s
-                "mount[%s]" % @name
+                "mount[#{@name}]"
             end
 
             # Verify our configuration is valid.  This should really check to
diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
index d16733523..beb6e4c57 100644
--- a/lib/puppet/network/handler/master.rb
+++ b/lib/puppet/network/handler/master.rb
@@ -67,7 +67,7 @@ class Puppet::Network::Handler
             when "marshal"
                 return CGI.escape(Marshal.dump(catalog.extract))
             else
-                raise "Invalid markup format '%s'" % format
+                raise "Invalid markup format '#{format}'"
             end
         end
 
diff --git a/lib/puppet/network/handler/report.rb b/lib/puppet/network/handler/report.rb
index 960b65f73..9f885518e 100755
--- a/lib/puppet/network/handler/report.rb
+++ b/lib/puppet/network/handler/report.rb
@@ -12,7 +12,7 @@ class Puppet::Network::Handler
 
         # Add a new report type.
         def self.newreport(name, options = {}, &block)
-            Puppet.warning "The interface for registering report types has changed; use Puppet::Reports.register_report for report type %s" % name
+            Puppet.warning "The interface for registering report types has changed; use Puppet::Reports.register_report for report type #{name}"
             Puppet::Reports.register_report(name, options, &block)
         end
 
@@ -28,11 +28,11 @@ class Puppet::Network::Handler
                 report = CGI.unescape(report)
             end
 
-            Puppet.info "Processing reports %s for %s" % [reports().join(", "), client]
+            Puppet.info "Processing reports #{reports().join(", ")} for #{client}"
             begin
                 process(report)
             rescue => detail
-                Puppet.err "Could not process report for %s: %s" % [client, detail]
+                Puppet.err "Could not process report for #{client}: #{detail}"
                 if Puppet[:trace]
                     puts detail.backtrace
                 end
@@ -49,7 +49,7 @@ class Puppet::Network::Handler
             begin
                 report = YAML.load(yaml)
             rescue => detail
-                Puppet.warning "Could not load report: %s" % detail
+                Puppet.warning "Could not load report: #{detail}"
                 return
             end
 
@@ -68,10 +68,10 @@ class Puppet::Network::Handler
                         if Puppet[:trace]
                             puts detail.backtrace
                         end
-                        Puppet.err "Report %s failed: %s" % [name, detail]
+                        Puppet.err "Report #{name} failed: #{detail}"
                     end
                 else
-                    Puppet.warning "No report named '%s'" % name
+                    Puppet.warning "No report named '#{name}'"
                 end
             end
         end