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]

10 files changed, 34 insertions, 100 deletions

diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
index fce978d88..43ee683f6 100644
--- a/lib/puppet/application/agent.rb
+++ b/lib/puppet/application/agent.rb
@@ -76,9 +76,7 @@ class Puppet::Application::Agent < Puppet::Application
             Puppet::Util::Log.newdestination(arg)
             options[:setdest] = true
         rescue => detail
-            if Puppet[:debug]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:debug]
             $stderr.puts detail.to_s
         end
     end
@@ -122,9 +120,7 @@ class Puppet::Application::Agent < Puppet::Application
         begin
             report = @agent.run
         rescue => detail
-            if Puppet[:trace]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:trace]
             Puppet.err detail.to_s
         end
 
@@ -167,9 +163,7 @@ class Puppet::Application::Agent < Puppet::Application
             end
         end
 
-        unless options[:setdest]
-            Puppet::Util::Log.newdestination(:syslog)
-        end
+        Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
     end
 
     def enable_disable_client(agent)
@@ -207,14 +201,10 @@ class Puppet::Application::Agent < Puppet::Application
 
         setup_logs
 
-        if Puppet.settings.print_configs?
-            exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+        exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         # If noop is set, then also enable diffs
-        if Puppet[:noop]
-            Puppet[:show_diff] = true
-        end
+        Puppet[:show_diff] = true if Puppet[:noop]
 
         args[:Server] = Puppet[:server]
         if options[:fqdn]
@@ -225,9 +215,7 @@ class Puppet::Application::Agent < Puppet::Application
         if options[:centrallogs]
             logdest = args[:Server]
 
-            if args.include?(:Port)
-                logdest += ":" + args[:Port]
-            end
+            logdest += ":" + args[:Port] if args.include?(:Port)
             Puppet::Util::Log.newdestination(logdest)
         end
 
@@ -262,14 +250,10 @@ class Puppet::Application::Agent < Puppet::Application
 
         # It'd be nice to daemonize later, but we have to daemonize before the
         # waitforcert happens.
-        if Puppet[:daemonize]
-            @daemon.daemonize
-        end
+        @daemon.daemonize if Puppet[:daemonize]
 
         @host = Puppet::SSL::Host.new
-        unless options[:fingerprint]
-            cert = @host.wait_for_cert(options[:waitforcert])
-        end
+        cert = @host.wait_for_cert(options[:waitforcert]) unless options[:fingerprint]
 
         @objects = []
 
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 07ce36736..cd4c75d75 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -45,9 +45,7 @@ class Puppet::Application::Apply < Puppet::Application
 
         begin
             catalog = Puppet::Resource::Catalog.convert_from(Puppet::Resource::Catalog.default_format,text)
-            unless catalog.is_a?(Puppet::Resource::Catalog)
-                catalog = Puppet::Resource::Catalog.pson_create(catalog)
-            end
+            catalog = Puppet::Resource::Catalog.pson_create(catalog) unless catalog.is_a?(Puppet::Resource::Catalog)
         rescue => detail
             raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
         end
@@ -130,7 +128,7 @@ class Puppet::Application::Apply < Puppet::Application
             configurer.execute_postrun_command
 
             status = 0
-            if not Puppet[:noop] and options[:detailed_exitcodes] then
+            if not Puppet[:noop] and options[:detailed_exitcodes]
                 transaction.generate_report
                 exit(transaction.report.exit_status)
             else
@@ -144,18 +142,12 @@ class Puppet::Application::Apply < Puppet::Application
     end
 
     def setup
-        if Puppet.settings.print_configs?
-            exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+        exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         # If noop is set, then also enable diffs
-        if Puppet[:noop]
-            Puppet[:show_diff] = true
-        end
+        Puppet[:show_diff] = true if Puppet[:noop]
 
-        unless options[:logset]
-            Puppet::Util::Log.newdestination(:console)
-        end
+        Puppet::Util::Log.newdestination(:console) unless options[:logset]
         client = nil
         server = nil
 
diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb
index a85f6a02b..af9042993 100644
--- a/lib/puppet/application/cert.rb
+++ b/lib/puppet/application/cert.rb
@@ -64,9 +64,7 @@ class Puppet::Application::Cert < Puppet::Application
     end
 
     def setup
-        if Puppet.settings.print_configs?
-            exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+        exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         Puppet::Util::Log.newdestination :console
 
diff --git a/lib/puppet/application/describe.rb b/lib/puppet/application/describe.rb
index e696f1613..f2621d354 100644
--- a/lib/puppet/application/describe.rb
+++ b/lib/puppet/application/describe.rb
@@ -36,9 +36,7 @@ class Formatter
 
     def scrub(text)
         # For text with no carriage returns, there's nothing to do.
-        if text !~ /\n/
-            return text
-        end
+        return text if text !~ /\n/
         indent = nil
 
         # If we can match an indentation, then just remove that same level of
@@ -125,9 +123,7 @@ class TypeDoc
         docs = {}
         type.allattrs.each do |name|
             kind = type.attrtype(name)
-            if attrs.include?(kind) && name != :provider
-                docs[name] = type.attrclass(name).doc
-            end
+            docs[name] = type.attrclass(name).doc if attrs.include?(kind) && name != :provider
         end
 
         docs.sort { |a,b|
@@ -148,9 +144,7 @@ class TypeDoc
         params = []
         type.allattrs.each do |name|
             kind = type.attrtype(name)
-            if attrs.include?(kind) && name != :provider
-                params << name.to_s
-            end
+            params << name.to_s if attrs.include?(kind) && name != :provider
         end
         puts @format.wrap(params.sort.join(", "), :indent => 4)
     end
@@ -202,12 +196,8 @@ class Puppet::Application::Describe < Puppet::Application
 
     def setup
         options[:types] = command_line.args.dup
-        unless options[:list] || options[:types].size > 0
-            handle_help(nil)
-        end
-        if options[:list] && options[:types].size > 0
-            $stderr.puts "Warning: ignoring types when listing all types"
-        end
+        handle_help(nil) unless options[:list] || options[:types].size > 0
+        $stderr.puts "Warning: ignoring types when listing all types" if options[:list] && options[:types].size > 0
     end
 
 end
diff --git a/lib/puppet/application/doc.rb b/lib/puppet/application/doc.rb
index 8dd116233..e4c247444 100644
--- a/lib/puppet/application/doc.rb
+++ b/lib/puppet/application/doc.rb
@@ -86,9 +86,7 @@ class Puppet::Application::Doc < Puppet::Application
                 Puppet::Util::RDoc.rdoc(options[:outputdir], files, options[:charset])
             end
         rescue => detail
-            if Puppet[:trace]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:trace]
             $stderr.puts "Could not generate documentation: #{detail}"
             exit_code = 1
         end
@@ -99,9 +97,7 @@ class Puppet::Application::Doc < Puppet::Application
         require 'puppet/util/reference'
         options[:references].each do |name|
             section = Puppet::Util::Reference.reference(name) or raise "Could not find section #{name}"
-            unless options[:mode] == :pdf
-                section.trac
-            end
+            section.trac unless options[:mode] == :pdf
         end
     end
 
@@ -154,9 +150,7 @@ class Puppet::Application::Doc < Puppet::Application
             end
         end
 
-        unless with_contents # We've only got one reference
-            text += Puppet::Util::Reference.footer
-        end
+        text += Puppet::Util::Reference.footer unless with_contents # We've only got one reference
 
         # Replace the trac links, since they're invalid everywhere else
         text.gsub!(/`\w+\s+([^`]+)`:trac:/) { |m| $1 }
@@ -193,9 +187,7 @@ class Puppet::Application::Doc < Puppet::Application
             end
         end
 
-        if options[:references].empty?
-            options[:references] << :type
-        end
+        options[:references] << :type if options[:references].empty?
     end
 
     def setup_rdoc(dummy_argument=:work_arround_for_ruby_GC_bug)
diff --git a/lib/puppet/application/filebucket.rb b/lib/puppet/application/filebucket.rb
index 095a413a7..8da2d014d 100644
--- a/lib/puppet/application/filebucket.rb
+++ b/lib/puppet/application/filebucket.rb
@@ -66,9 +66,7 @@ class Puppet::Application::Filebucket < Puppet::Application
         # Now parse the config
         Puppet.parse_config
 
-        if Puppet.settings.print_configs?
-                exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+            exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         require 'puppet/file_bucket/dipper'
         begin
@@ -80,9 +78,7 @@ class Puppet::Application::Filebucket < Puppet::Application
             end
         rescue => detail
             $stderr.puts detail
-            if Puppet[:trace]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:trace]
             exit(1)
         end
     end
diff --git a/lib/puppet/application/kick.rb b/lib/puppet/application/kick.rb
index eaafc0935..6c77e74f2 100644
--- a/lib/puppet/application/kick.rb
+++ b/lib/puppet/application/kick.rb
@@ -77,9 +77,7 @@ class Puppet::Application::Kick < Puppet::Application
                         # Remove our host from the list of children, so the parallelization
                         # continues working.
                         @children.delete(pid)
-                        if $CHILD_STATUS.exitstatus != 0
-                            failures << host
-                        end
+                        failures << host if $CHILD_STATUS.exitstatus != 0
                         print "#{host} finished with exit code #{$CHILD_STATUS.exitstatus}\n"
                     else
                         $stderr.puts "Could not find host for PID #{pid} with status #{$CHILD_STATUS.exitstatus}"
diff --git a/lib/puppet/application/master.rb b/lib/puppet/application/master.rb
index 7ea3f3a83..7485dcb3f 100644
--- a/lib/puppet/application/master.rb
+++ b/lib/puppet/application/master.rb
@@ -20,9 +20,7 @@ class Puppet::Application::Master < Puppet::Application
             Puppet::Util::Log.newdestination(arg)
             options[:setdest] = true
         rescue => detail
-            if Puppet[:debug]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:debug]
             $stderr.puts detail.to_s
         end
     end
@@ -82,18 +80,14 @@ class Puppet::Application::Master < Puppet::Application
 
         xmlrpc_handlers = [:Status, :FileServer, :Master, :Report, :Filebucket]
 
-        if Puppet[:ca]
-            xmlrpc_handlers << :CA
-        end
+        xmlrpc_handlers << :CA if Puppet[:ca]
 
         # Make sure we've got a localhost ssl cert
         Puppet::SSL::Host.localhost
 
         # And now configure our server to *only* hit the CA for data, because that's
         # all it will have write access to.
-        if Puppet::SSL::CertificateAuthority.ca?
-            Puppet::SSL::Host.ca_location = :only
-        end
+        Puppet::SSL::Host.ca_location = :only if Puppet::SSL::CertificateAuthority.ca?
 
         if Puppet.features.root?
             begin
@@ -138,13 +132,9 @@ class Puppet::Application::Master < Puppet::Application
             end
         end
 
-        unless options[:setdest]
-            Puppet::Util::Log.newdestination(:syslog)
-        end
+        Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
 
-        if Puppet.settings.print_configs?
-            exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+        exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         Puppet.settings.use :main, :master, :ssl
 
diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb
index 8e830b39e..6bbff75f9 100644
--- a/lib/puppet/application/queue.rb
+++ b/lib/puppet/application/queue.rb
@@ -75,9 +75,7 @@ class Puppet::Application::Queue < Puppet::Application
 
         setup_logs
 
-        if Puppet.settings.print_configs?
-            exit(Puppet.settings.print_configs ? 0 : 1)
-        end
+        exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
         require 'puppet/resource/catalog'
         Puppet::Resource::Catalog.terminus_class = :active_record
diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb
index a6cc99302..9e1efe2ef 100644
--- a/lib/puppet/application/resource.rb
+++ b/lib/puppet/application/resource.rb
@@ -49,9 +49,7 @@ class Puppet::Application::Resource < Puppet::Application
             end
         end
 
-        if options[:edit] and @host
-            raise "You cannot edit a remote host"
-        end
+        raise "You cannot edit a remote host" if options[:edit] and @host
 
         properties = typeobj.properties.collect { |s| s.name }
 
@@ -63,9 +61,7 @@ class Puppet::Application::Resource < Puppet::Application
                     trans.delete(param)
                 end
 
-                unless properties.include?(param) or @extra_params.include?(param)
-                    trans.delete(param)
-                end
+                trans.delete(param) unless properties.include?(param) or @extra_params.include?(param)
             end
             trans.to_manifest
         }