Code smell: Line modifiers are preferred to one-line blocks.

* Replaced 6 occurances of (while .*?) *do$ with

  The do is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          while line = f.gets do
      becomes:
          while line = f.gets
      The code:
          while line = shadow.gets do
      becomes:
          while line = shadow.gets
      The code:
          while wrapper = zeros.pop do
      becomes:
          while wrapper = zeros.pop

* Replaced 19 occurances of ((if|unless) .*?) *then$ with

  The then is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then
      becomes:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
      The code:
          unless defined?(@spec_command) then
      becomes:
          unless defined?(@spec_command)
      The code:
          if c == ?\n then
      becomes:
          if c == ?\n

* Replaced 758 occurances of

      ((?:if|unless|while|until) .*)
          (.*)
      end

  with

  The one-line form is preferable provided:

      * The condition is not used to assign a variable
      * The body line is not already modified
      * The resulting line is not too long

  3 Examples:

      The code:
          if Puppet.features.libshadow?
              has_feature :manages_passwords
          end
      becomes:
          has_feature :manages_passwords if Puppet.features.libshadow?
      The code:
          unless (defined?(@current_pool) and @current_pool)
              @current_pool = process_zpool_data(get_pool_data)
          end
      becomes:
          @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool)
      The code:
          if Puppet[:trace]
              puts detail.backtrace
          end
      becomes:
          puts detail.backtrace if Puppet[:trace]

5 files changed, 18 insertions, 54 deletions

diff --git a/lib/puppet/network/handler/ca.rb b/lib/puppet/network/handler/ca.rb
index 8bafb5f9b..c72171d5d 100644
--- a/lib/puppet/network/handler/ca.rb
+++ b/lib/puppet/network/handler/ca.rb
@@ -60,9 +60,7 @@ class Puppet::Network::Handler
 
         def initialize(hash = {})
             Puppet.settings.use(:main, :ssl, :ca)
-            if hash.include? :autosign
-                @autosign = hash[:autosign]
-            end
+            @autosign = hash[:autosign] if hash.include? :autosign
 
             @ca = Puppet::SSLCertificates::CA.new(hash)
         end
@@ -109,9 +107,7 @@ class Puppet::Network::Handler
                 return [cert.to_pem, cacert.to_pem]
             elsif @ca
                 if self.autosign?(hostname) or client.nil?
-                    if client.nil?
-                        Puppet.info "Signing certificate for CA server"
-                    end
+                    Puppet.info "Signing certificate for CA server" if client.nil?
                     # 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
diff --git a/lib/puppet/network/handler/filebucket.rb b/lib/puppet/network/handler/filebucket.rb
index bea1c85f5..13fee1661 100755
--- a/lib/puppet/network/handler/filebucket.rb
+++ b/lib/puppet/network/handler/filebucket.rb
@@ -26,9 +26,7 @@ class Puppet::Network::Handler # :nodoc:
         # Accept a file from a client and store it by md5 sum, returning
         # the sum.
         def addfile(contents, path, client = nil, clientip = nil)
-            if client
-                contents = Base64.decode64(contents)
-            end
+            contents = Base64.decode64(contents) if client
             bucket = Puppet::FileBucket::File.new(contents)
             return bucket.save
         end
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index 756e74909..cb54ca4e4 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -98,22 +98,16 @@ class Puppet::Network::Handler
                 @local = false
             end
 
-            if hash[:Config] == false
-                @noreadconfig = true
-            end
+            @noreadconfig = true if hash[:Config] == false
 
             @passed_configuration_path = hash[:Config]
 
             if hash.include?(:Mount)
                 @passedconfig = true
-                unless hash[:Mount].is_a?(Hash)
-                    raise Puppet::DevError, "Invalid mount hash #{hash[:Mount].inspect}"
-                end
+                raise Puppet::DevError, "Invalid mount hash #{hash[:Mount].inspect}" unless hash[:Mount].is_a?(Hash)
 
                 hash[:Mount].each { |dir, name|
-                    if FileTest.exists?(dir)
-                        self.mount(dir, name)
-                    end
+                    self.mount(dir, name) if FileTest.exists?(dir)
                 }
                 self.mount(nil, MODULES)
                 self.mount(nil, PLUGINS)
@@ -179,9 +173,7 @@ class Puppet::Network::Handler
 
             mount, path = convert(url, client, clientip)
 
-            if client
-                mount.info "Sending #{url} to #{client}"
-            end
+            mount.info "Sending #{url} to #{client}" if client
 
             unless mount.path_exists?(path, client)
                 mount.debug "#{mount} reported that #{path} does not exist"
@@ -264,9 +256,7 @@ class Puppet::Network::Handler
 
             return unless configuration
 
-            if check and ! @configuration.changed?
-                return
-            end
+            return if check and ! @configuration.changed?
 
             newmounts = {}
             begin
@@ -279,9 +269,7 @@ class Puppet::Network::Handler
                         when /^\s*$/; next # skip blank lines
                         when /\[([-\w]+)\]/
                             name = $1
-                            if newmounts.include?(name)
-                                raise FileServerError, "#{newmounts[name]} is already mounted as #{name} in #{@configuration.file}"
-                            end
+                            raise FileServerError, "#{newmounts[name]} is already mounted as #{name} in #{@configuration.file}" if newmounts.include?(name)
                             mount = Mount.new(name)
                             newmounts[name] = mount
                         when /^\s*(\w+)\s+(.+)$/
@@ -379,9 +367,7 @@ class Puppet::Network::Handler
             # We let the check raise an error, so that it can raise an error
             # pointing to the specific problem.
             newmounts.each { |name, mount|
-                unless mount.valid?
-                    raise FileServerError, "Invalid mount #{name}"
-                end
+                raise FileServerError, "Invalid mount #{name}" unless mount.valid?
             }
             @mounts = newmounts
         end
@@ -529,9 +515,7 @@ class Puppet::Network::Handler
                 end
 
                 # This, ah, might be completely redundant
-                unless obj[:links] == links
-                    obj[:links] = links
-                end
+                obj[:links] = links unless obj[:links] == links
 
                 return obj
             end
@@ -572,15 +556,9 @@ class Puppet::Network::Handler
                     # Mark that we're expandable.
                     @expandable = true
                 else
-                    unless FileTest.exists?(path)
-                        raise FileServerError, "#{path} does not exist"
-                    end
-                    unless FileTest.directory?(path)
-                        raise FileServerError, "#{path} is not a directory"
-                    end
-                    unless FileTest.readable?(path)
-                        raise FileServerError, "#{path} is not readable"
-                    end
+                    raise FileServerError, "#{path} does not exist" unless FileTest.exists?(path)
+                    raise FileServerError, "#{path} is not a directory" unless FileTest.directory?(path)
+                    raise FileServerError, "#{path} is not readable" unless FileTest.readable?(path)
                     @expandable = false
                 end
                 @path = path
diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb
index beb6e4c57..d55046b5b 100644
--- a/lib/puppet/network/handler/master.rb
+++ b/lib/puppet/network/handler/master.rb
@@ -45,9 +45,7 @@ class Puppet::Network::Handler
 
             # This is only used by the cfengine module, or if --loadclasses was
             # specified in +puppet+.
-            if hash.include?(:Classes)
-                args[:Classes] = hash[:Classes]
-            end
+            args[:Classes] = hash[:Classes] if hash.include?(:Classes)
         end
 
         # Call our various handlers; this handler is getting deprecated.
diff --git a/lib/puppet/network/handler/report.rb b/lib/puppet/network/handler/report.rb
index 9f885518e..50e2fbfea 100755
--- a/lib/puppet/network/handler/report.rb
+++ b/lib/puppet/network/handler/report.rb
@@ -24,18 +24,14 @@ class Puppet::Network::Handler
         # Accept a report from a client.
         def report(report, client = nil, clientip = nil)
             # Unescape the report
-            unless @local
-                report = CGI.unescape(report)
-            end
+            report = CGI.unescape(report) unless @local
 
             Puppet.info "Processing reports #{reports().join(", ")} for #{client}"
             begin
                 process(report)
             rescue => detail
                 Puppet.err "Could not process report for #{client}: #{detail}"
-                if Puppet[:trace]
-                    puts detail.backtrace
-                end
+                puts detail.backtrace if Puppet[:trace]
             end
         end
 
@@ -65,9 +61,7 @@ class Puppet::Network::Handler
                         newrep.extend(mod)
                         newrep.process
                     rescue => detail
-                        if Puppet[:trace]
-                            puts detail.backtrace
-                        end
+                        puts detail.backtrace if Puppet[:trace]
                         Puppet.err "Report #{name} failed: #{detail}"
                     end
                 else