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]

31 files changed, 111 insertions, 327 deletions

diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index cdfc194da..21f99e4c0 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -50,9 +50,7 @@ class Puppet::Util::Autoload
 
     def initialize(obj, path, options = {})
         @path = path.to_s
-        if @path !~ /^\w/
-            raise ArgumentError, "Autoload paths cannot be fully qualified"
-        end
+        raise ArgumentError, "Autoload paths cannot be fully qualified" if @path !~ /^\w/
         @object = obj
 
         self.class[obj] = self
@@ -66,9 +64,7 @@ class Puppet::Util::Autoload
             end
         end
 
-        unless defined?(@wrap)
-            @wrap = true
-        end
+        @wrap = true unless defined?(@wrap)
     end
 
     # Load a single plugin by name.  We use 'load' here so we can reload a
diff --git a/lib/puppet/util/autoload/file_cache.rb b/lib/puppet/util/autoload/file_cache.rb
index 881e08637..873dc8fb7 100644
--- a/lib/puppet/util/autoload/file_cache.rb
+++ b/lib/puppet/util/autoload/file_cache.rb
@@ -48,9 +48,7 @@ module Puppet::Util::Autoload::FileCache
 
     def found_file?(path, type = nil)
         if data = found_files[path] and ! data_expired?(data[:time])
-            if type and ! data[:stat].send(type)
-                return false
-            end
+            return false if type and ! data[:stat].send(type)
             return true
         else
             return false
diff --git a/lib/puppet/util/cacher.rb b/lib/puppet/util/cacher.rb
index 3a75dc88d..bcd4c1bc1 100644
--- a/lib/puppet/util/cacher.rb
+++ b/lib/puppet/util/cacher.rb
@@ -88,9 +88,7 @@ module Puppet::Util::Cacher
         private
 
         def cache_timestamp
-            unless defined?(@cache_timestamp)
-                @cache_timestamp = Time.now
-            end
+            @cache_timestamp = Time.now unless defined?(@cache_timestamp)
             @cache_timestamp
         end
 
@@ -102,9 +100,7 @@ module Puppet::Util::Cacher
             elsif expired_by_ttl?(name)
                 value_cache.delete(name)
             end
-            unless value_cache.include?(name)
-                value_cache[name] = send("init_#{name}")
-            end
+            value_cache[name] = send("init_#{name}") unless value_cache.include?(name)
             value_cache[name]
         end
 
@@ -126,9 +122,7 @@ module Puppet::Util::Cacher
         end
 
         def value_cache
-            unless defined?(@value_cache) and @value_cache
-                @value_cache = {}
-            end
+            @value_cache = {} unless defined?(@value_cache) and @value_cache
             @value_cache
         end
     end
diff --git a/lib/puppet/util/classgen.rb b/lib/puppet/util/classgen.rb
index d9022bbdc..d4c693e1e 100644
--- a/lib/puppet/util/classgen.rb
+++ b/lib/puppet/util/classgen.rb
@@ -114,13 +114,9 @@ module Puppet::Util::ClassGen
 
         # Evaluate the passed block if there is one.  This should usually
         # define all of the work.
-        if block
-            klass.send(evalmethod, &block)
-        end
+        klass.send(evalmethod, &block) if block
 
-        if klass.respond_to? :postinit
-            klass.postinit
-        end
+        klass.postinit if klass.respond_to? :postinit
 
         # Store the class in hashes or arrays or whatever.
         storeclass(klass, name, options)
@@ -148,16 +144,12 @@ module Puppet::Util::ClassGen
 
     # Perform the initializations on the class.
     def initclass(klass, options)
-        if klass.respond_to? :initvars
-            klass.initvars
-        end
+        klass.initvars if klass.respond_to? :initvars
 
         if attrs = options[:attributes]
             attrs.each do |param, value|
                 method = param.to_s + "="
-                if klass.respond_to? method
-                    klass.send(method, value)
-                end
+                klass.send(method, value) if klass.respond_to? method
             end
         end
 
@@ -170,9 +162,7 @@ module Puppet::Util::ClassGen
             end
         end
 
-        if klass.respond_to? :preinit
-            klass.preinit
-        end
+        klass.preinit if klass.respond_to? :preinit
     end
 
     # Convert our name to a constant.
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index fa1b08b70..568d52c47 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -54,9 +54,7 @@ module Puppet
                     require_application subcommand_name
                     Puppet::Application.find(subcommand_name).new(self).run
                 else
-                    unless execute_external_subcommand
-                        abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}"
-                    end
+                    abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand
                 end
             end
 
diff --git a/lib/puppet/util/docs.rb b/lib/puppet/util/docs.rb
index d73550d9f..beed4bb0b 100644
--- a/lib/puppet/util/docs.rb
+++ b/lib/puppet/util/docs.rb
@@ -84,9 +84,7 @@ module Puppet::Util::Docs
         # Stupid markdown
         #text = text.gsub("<%=", "&lt;%=")
         # 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
diff --git a/lib/puppet/util/errors.rb b/lib/puppet/util/errors.rb
index d78064227..3aa65d06c 100644
--- a/lib/puppet/util/errors.rb
+++ b/lib/puppet/util/errors.rb
@@ -10,9 +10,7 @@ module Puppet::Util::Errors
         error.line ||= self.line if self.respond_to?(:line) and self.line
         error.file ||= self.file if self.respond_to?(:file) and self.file
 
-        if other and other.respond_to?(:backtrace)
-            error.set_backtrace other.backtrace
-        end
+        error.set_backtrace other.backtrace if other and other.respond_to?(:backtrace)
 
         return error
     end
diff --git a/lib/puppet/util/feature.rb b/lib/puppet/util/feature.rb
index 40ecfe447..30422ce4c 100644
--- a/lib/puppet/util/feature.rb
+++ b/lib/puppet/util/feature.rb
@@ -13,9 +13,7 @@ class Puppet::Util::Feature
     # successfully.
     def add(name, options = {})
         method = name.to_s + "?"
-        if self.class.respond_to?(method)
-            raise ArgumentError, "Feature #{name} is already defined"
-        end
+        raise ArgumentError, "Feature #{name} is already defined" if self.class.respond_to?(method)
 
         if block_given?
             begin
@@ -28,9 +26,7 @@ class Puppet::Util::Feature
         end
 
         meta_def(method) do
-            unless @results.include?(name)
-                @results[name] = test(name, options)
-            end
+            @results[name] = test(name, options) unless @results.include?(name)
             @results[name]
         end
     end
@@ -77,9 +73,7 @@ class Puppet::Util::Feature
     private
 
     def load_library(lib, name)
-        unless lib.is_a?(String)
-            raise ArgumentError, "Libraries must be passed as strings not #{lib.class}"
-        end
+        raise ArgumentError, "Libraries must be passed as strings not #{lib.class}" unless lib.is_a?(String)
 
         begin
             require lib
diff --git a/lib/puppet/util/file_locking.rb b/lib/puppet/util/file_locking.rb
index ab43e2353..cfac5ba16 100644
--- a/lib/puppet/util/file_locking.rb
+++ b/lib/puppet/util/file_locking.rb
@@ -16,9 +16,7 @@ module Puppet::Util::FileLocking
     # Create an exclusive lock for writing, and do the writing in a
     # tmp file.
     def writelock(file, mode = nil)
-        unless FileTest.directory?(File.dirname(file))
-            raise Puppet::DevError, "Cannot create #{file}; directory #{File.dirname(file)} does not exist"
-        end
+        raise Puppet::DevError, "Cannot create #{file}; directory #{File.dirname(file)} does not exist" unless FileTest.directory?(File.dirname(file))
         raise ArgumentError, "#{file} is not a file" unless !File.exists?(file) or File.file?(file)
         tmpfile = file + ".tmp"
 
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index 974d908b8..7348b8946 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -43,18 +43,14 @@ module Puppet::Util::FileParsing
         def fields=(fields)
             @fields = fields.collect do |field|
                 r = symbolize(field)
-                if INVALID_FIELDS.include?(r)
-                    raise ArgumentError.new("Cannot have fields named #{r}")
-                end
+                raise ArgumentError.new("Cannot have fields named #{r}") if INVALID_FIELDS.include?(r)
                 r
             end
         end
 
         def initialize(type, options = {}, &block)
             @type = symbolize(type)
-            unless [:record, :text].include?(@type)
-                raise ArgumentError, "Invalid record type #{@type}"
-            end
+            raise ArgumentError, "Invalid record type #{@type}" unless [:record, :text].include?(@type)
 
             set_options(options)
 
@@ -64,9 +60,7 @@ module Puppet::Util::FileParsing
                 self.separator ||= /\s+/
                 self.joiner ||= " "
                 self.optional ||= []
-                unless defined?(@rollup)
-                    @rollup = true
-                end
+                @rollup = true unless defined?(@rollup)
             end
 
             if block_given?
@@ -213,9 +207,7 @@ module Puppet::Util::FileParsing
     end
 
     def line_separator
-        unless defined?(@line_separator)
-            @line_separator = "\n"
-        end
+        @line_separator = "\n" unless defined?(@line_separator)
 
         @line_separator
     end
@@ -244,18 +236,14 @@ module Puppet::Util::FileParsing
 
     # Handle parsing a single line.
     def parse_line(line)
-        unless records?
-            raise Puppet::DevError, "No record types defined; cannot parse lines"
-        end
+        raise Puppet::DevError, "No record types defined; cannot parse lines" unless records?
 
         @record_order.each do |record|
             # These are basically either text or record lines.
             method = "handle_#{record.type}_line"
             if respond_to?(method)
                 if result = send(method, line, record)
-                    if record.respond_to?(:post_parse)
-                        record.send(:post_parse, result)
-                    end
+                    record.send(:post_parse, result) if record.respond_to?(:post_parse)
                     return result
                 end
             else
@@ -282,9 +270,7 @@ module Puppet::Util::FileParsing
     #   the regex will be removed.
     # * <tt>:separator</tt>: The record separator.  Defaults to /\s+/.
     def record_line(name, options, &block)
-        unless options.include?(:fields)
-            raise ArgumentError, "Must include a list of fields"
-        end
+        raise ArgumentError, "Must include a list of fields" unless options.include?(:fields)
 
         record = FileRecord.new(:record, options, &block)
         record.name = symbolize(name)
@@ -299,9 +285,7 @@ module Puppet::Util::FileParsing
 
     # Define a new type of text record.
     def text_line(name, options, &block)
-        unless options.include?(:match)
-            raise ArgumentError, "You must provide a :match regex for text lines"
-        end
+        raise ArgumentError, "You must provide a :match regex for text lines" unless options.include?(:match)
 
         record = FileRecord.new(:text, options, &block)
         record.name = symbolize(name)
@@ -313,9 +297,7 @@ module Puppet::Util::FileParsing
     def to_file(records)
         text = records.collect { |record| to_line(record) }.join(line_separator)
 
-        if trailing_separator
-            text += line_separator
-        end
+        text += line_separator if trailing_separator
 
         return text
     end
@@ -334,9 +316,7 @@ module Puppet::Util::FileParsing
         case record.type
         when :text; return details[:line]
         else
-            if record.respond_to?(:to_line)
-                return record.to_line(details)
-            end
+            return record.to_line(details) if record.respond_to?(:to_line)
 
             line = record.join(details)
 
@@ -381,9 +361,7 @@ module Puppet::Util::FileParsing
         @record_types ||= {}
         @record_order ||= []
 
-        if @record_types.include?(record.name)
-            raise ArgumentError, "Line type #{record.name} is already defined"
-        end
+        raise ArgumentError, "Line type #{record.name} is already defined" if @record_types.include?(record.name)
 
         @record_types[record.name] = record
         @record_order << record
diff --git a/lib/puppet/util/filetype.rb b/lib/puppet/util/filetype.rb
index 3df4fdee0..4d7da67eb 100755
--- a/lib/puppet/util/filetype.rb
+++ b/lib/puppet/util/filetype.rb
@@ -44,9 +44,7 @@ class Puppet::Util::FileType
                 rescue Puppet::Error => detail
                     raise
                 rescue => detail
-                    if Puppet[:trace]
-                        puts detail.backtrace
-                    end
+                    puts detail.backtrace if Puppet[:trace]
                     raise Puppet::Error, "#{self.class} could not read #{@path}: #{detail}"
                 end
             end
@@ -61,9 +59,7 @@ class Puppet::Util::FileType
                 rescue Puppet::Error => detail
                     raise
                 rescue => detail
-                    if Puppet[:debug]
-                        puts detail.backtrace
-                    end
+                    puts detail.backtrace if Puppet[:debug]
                     raise Puppet::Error, "#{self.class} could not write #{@path}: #{detail}"
                 end
             end
@@ -76,9 +72,7 @@ class Puppet::Util::FileType
 
     # Pick or create a filebucket to use.
     def bucket
-        unless defined?(@bucket)
-            @bucket = Puppet::Type.type(:filebucket).mkdefaultbucket.bucket
-        end
+        @bucket = Puppet::Type.type(:filebucket).mkdefaultbucket.bucket unless defined?(@bucket)
         @bucket
     end
 
@@ -105,9 +99,7 @@ class Puppet::Util::FileType
 
         # Remove the file.
         def remove
-            if File.exist?(@path)
-                File.unlink(@path)
-            end
+            File.unlink(@path) if File.exist?(@path)
         end
 
         # Overwrite the file.
@@ -260,9 +252,7 @@ class Puppet::Util::FileType
         def read
             begin
                 output = Puppet::Util.execute(%w{crontab -l}, :uid => @path)
-                if output.include?("You are not authorized to use the cron command")
-                    raise Puppet::Error, "User #{@path} not authorized to use cron"
-                end
+                raise Puppet::Error, "User #{@path} not authorized to use cron" if output.include?("You are not authorized to use the cron command")
                 return output
             rescue => detail
                 raise Puppet::Error, "Could not read crontab for #{@path}: #{detail}"
diff --git a/lib/puppet/util/graph.rb b/lib/puppet/util/graph.rb
index a052bf8f1..5dd55e951 100644
--- a/lib/puppet/util/graph.rb
+++ b/lib/puppet/util/graph.rb
@@ -18,9 +18,7 @@ module Puppet::Util::Graph
             unless block_given? and ! yield(child)
                 graph.add_edge(self, child)
 
-                if child.respond_to?(:to_graph)
-                    child.to_graph(graph, &block)
-                end
+                child.to_graph(graph, &block) if child.respond_to?(:to_graph)
             end
         end
 
diff --git a/lib/puppet/util/inifile.rb b/lib/puppet/util/inifile.rb
index 38bc1c574..b90134835 100644
--- a/lib/puppet/util/inifile.rb
+++ b/lib/puppet/util/inifile.rb
@@ -56,9 +56,7 @@ module Puppet::Util::IniConfig
         # exists, return nil
         def [](key)
             entry = find_entry(key)
-            if entry.nil?
-                return nil
-            end
+            return nil if entry.nil?
             return entry[1]
         end
 
@@ -69,9 +67,7 @@ module Puppet::Util::IniConfig
             @entries.each do |entry|
                 if entry.is_a?(Array)
                     key, value = entry
-                    unless value.nil?
-                        text << "#{key}=#{value}\n"
-                    end
+                    text << "#{key}=#{value}\n" unless value.nil?
                 else
                     text << entry
                 end
@@ -82,9 +78,7 @@ module Puppet::Util::IniConfig
         private
         def find_entry(key)
             @entries.each do |entry|
-                if entry.is_a?(Array) && entry[0] == key
-                    return entry
-                end
+                return entry if entry.is_a?(Array) && entry[0] == key
             end
             return nil
         end
@@ -102,9 +96,7 @@ module Puppet::Util::IniConfig
         # already existing sections
         def read(file)
             text = Puppet::Util::FileType.filetype(:flat).new(file).read
-            if text.nil?
-                raise "Could not find #{file}"
-            end
+            raise "Could not find #{file}" if text.nil?
 
             section = nil   # The name of the current section
             optname = nil   # The name of the last option in section
@@ -172,9 +164,7 @@ module Puppet::Util::IniConfig
         def each_section(&block)
             @files.each do |file, list|
                 list.each do |entry|
-                    if entry.is_a?(Section)
-                        yield(entry)
-                    end
+                    yield(entry) if entry.is_a?(Section)
                 end
             end
         end
@@ -203,9 +193,7 @@ module Puppet::Util::IniConfig
 
         # Add a section to be stored in FILE when store is called
         def add_section(name, file)
-            if include?(name)
-                raise "A section with name #{name} already exists"
-            end
+            raise "A section with name #{name} already exists" if include?(name)
             result = Section.new(name, file)
             @files[file] ||= []
             @files[file] << result
diff --git a/lib/puppet/util/inline_docs.rb b/lib/puppet/util/inline_docs.rb
index 405b53945..695b8e8df 100644
--- a/lib/puppet/util/inline_docs.rb
+++ b/lib/puppet/util/inline_docs.rb
@@ -15,9 +15,7 @@ module Puppet::Util::InlineDocs
         attr_writer :doc
 
         def doc
-            unless defined?(@doc) and @doc
-                @doc = ""
-            end
+            @doc = "" unless defined?(@doc) and @doc
             @doc
         end
 
diff --git a/lib/puppet/util/loadedfile.rb b/lib/puppet/util/loadedfile.rb
index c91fb847f..3dd8151a4 100755
--- a/lib/puppet/util/loadedfile.rb
+++ b/lib/puppet/util/loadedfile.rb
@@ -16,9 +16,7 @@ module Puppet
         # be reparsed.
         def changed?
             # Allow the timeout to be disabled entirely.
-            if Puppet[:filetimeout] < 0
-                return true
-            end
+            return true if Puppet[:filetimeout] < 0
             tmp = stamp()
 
             # We use a different internal variable than the stamp method
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index ac1992526..5dea6d67f 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -47,12 +47,8 @@ class Puppet::Util::Log
     # undefs other objects.
     def Log.close(destination)
         if @destinations.include?(destination)
-            if @destinations[destination].respond_to?(:flush)
-                @destinations[destination].flush
-            end
-            if @destinations[destination].respond_to?(:close)
-                @destinations[destination].close
-            end
+            @destinations[destination].flush if @destinations[destination].respond_to?(:flush)
+            @destinations[destination].close if @destinations[destination].respond_to?(:close)
             @destinations.delete(destination)
         end
     end
@@ -66,21 +62,15 @@ class Puppet::Util::Log
     # Flush any log destinations that support such operations.
     def Log.flush
         @destinations.each { |type, dest|
-            if dest.respond_to?(:flush)
-                dest.flush
-            end
+            dest.flush if dest.respond_to?(:flush)
         }
     end
 
     # Create a new log message.  The primary role of this method is to
     # avoid creating log messages below the loglevel.
     def Log.create(hash)
-        unless hash.include?(:level)
-            raise Puppet::DevError, "Logs require a level"
-        end
-        unless @levels.index(hash[:level])
-            raise Puppet::DevError, "Invalid log level #{hash[:level]}"
-        end
+        raise Puppet::DevError, "Logs require a level" unless hash.include?(:level)
+        raise Puppet::DevError, "Invalid log level #{hash[:level]}" unless @levels.index(hash[:level])
         if @levels.index(hash[:level]) >= @loglevel
             return Puppet::Util::Log.new(hash)
         else
@@ -104,13 +94,9 @@ class Puppet::Util::Log
 
     # Set the current log level.
     def Log.level=(level)
-        unless level.is_a?(Symbol)
-            level = level.intern
-        end
+        level = level.intern unless level.is_a?(Symbol)
 
-        unless @levels.include?(level)
-            raise Puppet::DevError, "Invalid loglevel #{level}"
-        end
+        raise Puppet::DevError, "Invalid loglevel #{level}" unless @levels.include?(level)
 
         @loglevel = @levels.index(level)
     end
@@ -130,9 +116,7 @@ class Puppet::Util::Log
             klass.match?(dest)
         end
 
-        unless type
-            raise Puppet::DevError, "Unknown destination type #{dest}"
-        end
+        raise Puppet::DevError, "Unknown destination type #{dest}" unless type
 
         begin
             if type.instance_method(:initialize).arity == 1
@@ -143,14 +127,10 @@ class Puppet::Util::Log
             flushqueue
             @destinations[dest]
         rescue => detail
-            if Puppet[:debug]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:debug]
 
             # If this was our only destination, then add the console back in.
-            if @destinations.empty? and (dest != :console and dest != "console")
-                newdestination(:console)
-            end
+            newdestination(:console) if @destinations.empty? and (dest != :console and dest != "console")
         end
     end
 
@@ -159,9 +139,7 @@ class Puppet::Util::Log
     # a potential for a loop here, if the machine somehow gets the destination set as
     # itself.
     def Log.newmessage(msg)
-        if @levels.index(msg.level) < @loglevel
-            return
-        end
+        return if @levels.index(msg.level) < @loglevel
 
         queuemessage(msg) if @destinations.length == 0
 
@@ -193,9 +171,7 @@ class Puppet::Util::Log
         Puppet.notice "Reopening log files"
         types = @destinations.keys
         @destinations.each { |type, dest|
-            if dest.respond_to?(:close)
-                dest.close
-            end
+            dest.close if dest.respond_to?(:close)
         }
         @destinations.clear
         # We need to make sure we always end up with some kind of destination
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 37e6d1ae9..6c94247ea 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -4,9 +4,7 @@ Puppet::Util::Log.newdesttype :syslog do
     end
 
     def initialize
-        if Syslog.opened?
-            Syslog.close
-        end
+        Syslog.close if Syslog.opened?
         name = Puppet[:name]
         name = "puppet-#{name}" unless name =~ /puppet/
 
@@ -49,9 +47,7 @@ Puppet::Util::Log.newdesttype :file do
     end
 
     def flush
-        if defined?(@file)
-            @file.flush
-        end
+        @file.flush if defined?(@file)
     end
 
     def initialize(path)
@@ -158,14 +154,10 @@ Puppet::Util::Log.newdesttype :host do
 
     def handle(msg)
         unless msg.is_a?(String) or msg.remote
-            unless defined?(@hostname)
-                @hostname = Facter["hostname"].value
-            end
+            @hostname = Facter["hostname"].value unless defined?(@hostname)
             unless defined?(@domain)
                 @domain = Facter["domain"].value
-                if @domain
-                    @hostname += ".#{@domain}"
-                end
+                @hostname += ".#{@domain}" if @domain
             end
             if msg.source =~ /^\//
                 msg.source = @hostname + ":#{msg.source}"
@@ -187,9 +179,7 @@ Puppet::Util::Log.newdesttype :host do
                 # Add the hostname to the source
                 @driver.addlog(tmp)
             rescue => detail
-                if Puppet[:trace]
-                    puts detail.backtrace
-                end
+                puts detail.backtrace if Puppet[:trace]
                 Puppet.err detail
                 Puppet::Util::Log.close(self)
             end
diff --git a/lib/puppet/util/log_paths.rb b/lib/puppet/util/log_paths.rb
index b5bdc50de..3132238e2 100644
--- a/lib/puppet/util/log_paths.rb
+++ b/lib/puppet/util/log_paths.rb
@@ -5,9 +5,7 @@ module Puppet::Util::LogPaths
     # return the full path to us, for logging and rollback
     # some classes (e.g., FileTypeRecords) will have to override this
     def path
-        unless defined?(@path)
-            @path = pathbuilder
-        end
+        @path = pathbuilder unless defined?(@path)
 
         return "/" + @path.join("/")
     end
diff --git a/lib/puppet/util/logging.rb b/lib/puppet/util/logging.rb
index 3af698062..fa50efa0a 100644
--- a/lib/puppet/util/logging.rb
+++ b/lib/puppet/util/logging.rb
@@ -10,9 +10,7 @@ module Puppet::Util::Logging
     # Create a method for each log level.
     Puppet::Util::Log.eachlevel do |level|
         define_method(level) do |args|
-            if args.is_a?(Array)
-                args = args.join(" ")
-            end
+            args = args.join(" ") if args.is_a?(Array)
             send_log(level, args)
         end
     end
diff --git a/lib/puppet/util/methodhelper.rb b/lib/puppet/util/methodhelper.rb
index 771d0e648..f8dc2cedc 100644
--- a/lib/puppet/util/methodhelper.rb
+++ b/lib/puppet/util/methodhelper.rb
@@ -2,9 +2,7 @@
 module Puppet::Util::MethodHelper
     def requiredopts(*names)
         names.each do |name|
-            if self.send(name).nil?
-                devfail("#{name} is a required option for #{self.class}")
-            end
+            devfail("#{name} is a required option for #{self.class}") if self.send(name).nil?
         end
     end
 
diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb
index e8de54cf2..6f5b2cf9b 100644
--- a/lib/puppet/util/metric.rb
+++ b/lib/puppet/util/metric.rb
@@ -118,9 +118,7 @@ class Puppet::Util::Metric
             Puppet.warning "RRD library is missing; cannot store metrics"
             return
         end
-        unless FileTest.exists?(self.path)
-            self.create(time - 5)
-        end
+        self.create(time - 5) unless FileTest.exists?(self.path)
 
         @rrd ||= RRDtool.new(self.path)
 
diff --git a/lib/puppet/util/package.rb b/lib/puppet/util/package.rb
index 874e782ca..24df1c727 100644
--- a/lib/puppet/util/package.rb
+++ b/lib/puppet/util/package.rb
@@ -4,7 +4,7 @@ module Puppet::Util::Package
         ax = version_a.scan(vre)
         bx = version_b.scan(vre)
 
-        while (ax.length>0 && bx.length>0) do
+        while (ax.length>0 && bx.length>0)
             a = ax.shift
             b = bx.shift
 
diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index f73b18dce..9d41a71ef 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -232,17 +232,13 @@ module Generators
             res1 = []
             collection['classes'].sort.each do |f|
                 if f.document_self
-                    unless f.context.is_module?
-                        res1 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.index_name) }
-                    end
+                    res1 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.index_name) } unless f.context.is_module?
                 end
             end
 
             res2 = []
             collection['methods'].sort.each do |f|
-                if f.document_self
-                    res2 << { "href" => "../#{f.path}", "name" => f.index_name.sub(/\(.*\)$/,'') }
-                end
+                res2 << { "href" => "../#{f.path}", "name" => f.index_name.sub(/\(.*\)$/,'') } if f.document_self
             end
 
             module_name = []
@@ -264,9 +260,7 @@ module Generators
 
             res5 = []
             collection['nodes'].sort.each do |f|
-                if f.document_self
-                    res5 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.name) }
-                end
+                res5 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.name) } if f.document_self
             end
 
             values = {
@@ -467,9 +461,7 @@ module Generators
         # which is also its url
         def http_url(full_name, prefix)
             path = full_name.dup
-            if path['<<']
-                path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
-            end
+            path.gsub!(/<<\s*(\w*)/) { "from-#$1" } if path['<<']
             File.join(prefix, path.split("::").collect { |p| Digest::MD5.hexdigest(p) }) + ".html"
         end
 
@@ -581,9 +573,7 @@ module Generators
             c = @context
             c = c.parent while c and !c.diagram
 
-            if c && c.diagram
-                @values["diagram"] = diagram_reference(c.diagram)
-            end
+            @values["diagram"] = diagram_reference(c.diagram) if c && c.diagram
 
             @values["full_name"] = h_name
 
@@ -599,9 +589,7 @@ module Generators
                 end
                 lookup = "NODE(#{lookup})"
                 parent_url = AllReferences[lookup] || AllReferences[parent_class]
-                if parent_url and parent_url.document_self
-                    @values["par_url"] = aref_to(parent_url.path)
-                end
+                @values["par_url"] = aref_to(parent_url.path) if parent_url and parent_url.document_self
             end
 
             files = []
@@ -612,9 +600,7 @@ module Generators
                 res["full_path"]     = full_path
                 res["full_path_url"] = aref_to(f.viewer.path) if f.document_self
 
-                if @options.webcvs
-                    res["cvsurl"] = cvs_url( @options.webcvs, full_path )
-                end
+                res["cvsurl"] = cvs_url( @options.webcvs, full_path ) if @options.webcvs
 
                 files << res
             end
@@ -723,9 +709,7 @@ module Generators
         # which is also its url
         def http_url(full_name, prefix)
             path = full_name.dup
-            if path['<<']
-                path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
-            end
+            path.gsub!(/<<\s*(\w*)/) { "from-#$1" } if path['<<']
             File.join(prefix, path.split("::")) + ".html"
         end
 
@@ -802,9 +786,7 @@ module Generators
                 res["full_path"]     = full_path
                 res["full_path_url"] = aref_to(f.viewer.path) if f.document_self
 
-                if @options.webcvs
-                    res["cvsurl"] = cvs_url( @options.webcvs, full_path )
-                end
+                res["cvsurl"] = cvs_url( @options.webcvs, full_path ) if @options.webcvs
 
                 files << res
             end
@@ -895,9 +877,7 @@ module Generators
 
         def find_symbol(symbol, method=nil)
             res = @context.parent.find_symbol(symbol, method)
-            if res
-                res = res.viewer
-            end
+            res = res.viewer if res
             res
         end
 
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index 9d14ae6eb..cc767fa05 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -67,9 +67,7 @@ class Parser
         names.each do |name|
             prev_container = container
             container = find_object_named(container, name)
-            unless container
-                container = prev_container.add_class(PuppetClass, name, nil)
-            end
+            container = prev_container.add_class(PuppetClass, name, nil) unless container
         end
         return [container, final_name]
     end
diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb
index c5bfe4722..314f9c136 100644
--- a/lib/puppet/util/reference.rb
+++ b/lib/puppet/util/reference.rb
@@ -147,9 +147,7 @@ class Puppet::Util::Reference
         options[:level] ||= 5
         #str = "#{name} : "
         str = h(name, options[:level])
-        if options[:namevar]
-            str += "- **namevar**\n\n"
-        end
+        str += "- **namevar**\n\n" if options[:namevar]
         str += text
         #str += text.gsub(/\n/, "\n    ")
 
@@ -170,17 +168,13 @@ class Puppet::Util::Reference
         # First the header
         text = h(@title, 1)
         text += "\n\n**This page is autogenerated; any changes will get overwritten** *(last generated on #{Time.now.to_s})*\n\n"
-        if withcontents
-            text +=  ".. contents:: :depth: #{@depth}\n\n"
-        end
+        text +=  ".. contents:: :depth: #{@depth}\n\n" if withcontents
 
         text += @header
 
         text += generate()
 
-        if withcontents
-            text += self.class.footer
-        end
+        text += self.class.footer if withcontents
 
         return text
     end
diff --git a/lib/puppet/util/selinux.rb b/lib/puppet/util/selinux.rb
index fad15d74e..7c6177f81 100644
--- a/lib/puppet/util/selinux.rb
+++ b/lib/puppet/util/selinux.rb
@@ -14,9 +14,7 @@ require 'pathname'
 module Puppet::Util::SELinux
 
     def selinux_support?
-        unless defined?(Selinux)
-            return false
-        end
+        return false unless defined?(Selinux)
         if Selinux.is_selinux_enabled == 1
             return true
         end
@@ -26,9 +24,7 @@ module Puppet::Util::SELinux
     # Retrieve and return the full context of the file.  If we don't have
     # SELinux support or if the SELinux call fails then return nil.
     def get_selinux_current_context(file)
-        unless selinux_support?
-            return nil
-        end
+        return nil unless selinux_support?
         retval = Selinux.lgetfilecon(file)
         if retval == -1
             return nil
@@ -39,14 +35,10 @@ module Puppet::Util::SELinux
     # Retrieve and return the default context of the file.  If we don't have
     # SELinux support or if the SELinux call fails to file a default then return nil.
     def get_selinux_default_context(file)
-        unless selinux_support?
-            return nil
-        end
+        return nil unless selinux_support?
         # If the filesystem has no support for SELinux labels, return a default of nil
         # instead of what matchpathcon would return
-        unless selinux_label_support?(file)
-            return nil
-        end
+        return nil unless selinux_label_support?(file)
         # If the file exists we should pass the mode to matchpathcon for the most specific
         # matching.  If not, we can pass a mode of 0.
         begin
@@ -89,9 +81,7 @@ module Puppet::Util::SELinux
     # I believe that the OS should always provide at least a fall-through context
     # though on any well-running system.
     def set_selinux_context(file, value, component = false)
-        unless selinux_support? && selinux_label_support?(file)
-            return nil
-        end
+        return nil unless selinux_support? && selinux_label_support?(file)
 
         if component
             # Must first get existing context to replace a single component
@@ -137,9 +127,7 @@ module Puppet::Util::SELinux
     # the file.
     def set_selinux_default_context(file)
         new_context = get_selinux_default_context(file)
-        unless new_context
-            return nil
-        end
+        return nil unless new_context
         cur_context = get_selinux_current_context(file)
         if new_context != cur_context
             set_selinux_context(file, new_context)
@@ -207,10 +195,8 @@ module Puppet::Util::SELinux
         # Remove the last slash and everything after it,
         #   and repeat with that as the file for the next loop through.
         path = realpath(path)
-        while not path.empty? do
-            if mnts.has_key?(path)
-                return mnts[path]
-            end
+        while not path.empty?
+            return mnts[path] if mnts.has_key?(path)
             path = parent_directory(path)
         end
         return mnts['/']
@@ -222,9 +208,7 @@ module Puppet::Util::SELinux
     # false if not.
     def selinux_label_support?(file)
         fstype = find_fs(file)
-        if fstype.nil?
-            return false
-        end
+        return false if fstype.nil?
         filesystems = ['ext2', 'ext3', 'ext4', 'gfs', 'gfs2', 'xfs', 'jfs']
         return filesystems.include?(fstype)
     end
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index afec53a8a..b8f4effba 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -76,9 +76,7 @@ class Puppet::Util::Settings
 
         # Don't clear the 'used' in this case, since it's a config file reparse,
         # and we want to retain this info.
-        unless exceptcli
-            @used = []
-        end
+        @used = [] unless exceptcli
 
         @cache.clear
     end
@@ -377,9 +375,7 @@ class Puppet::Util::Settings
     # a default or a value, so we can't actually assign it.
     def newsetting(hash)
         klass = nil
-        if hash[:section]
-            hash[:section] = hash[:section].to_sym
-        end
+        hash[:section] = hash[:section].to_sym if hash[:section]
         if type = hash[:type]
             unless klass = {:setting => Setting, :file => FileSetting, :boolean => BooleanSetting}[type]
                 raise ArgumentError, "Invalid setting type '#{type}'"
@@ -457,9 +453,7 @@ class Puppet::Util::Settings
         self.each { |name, obj|
             section = obj.section || "puppet"
             sections[section] ||= []
-            unless sectionlist.include?(section)
-                sectionlist << section
-            end
+            sectionlist << section unless sectionlist.include?(section)
             sections[section] << obj
         }
 
@@ -477,7 +471,7 @@ class Puppet::Util::Settings
     end
 
     def legacy_to_mode(type, param)
-        if not defined?(@app_names) then
+        if not defined?(@app_names)
             require 'puppet/util/command_line'
             command_line = Puppet::Util::CommandLine.new
             @app_names = Puppet::Util::CommandLine::LegacyName.inject({}) do |hash, pair|
@@ -504,12 +498,8 @@ class Puppet::Util::Settings
                     "Attempt to assign a value to unknown configuration parameter #{param.inspect}"
             end
         end
-        if setting.respond_to?(:munge)
-            value = setting.munge(value)
-        end
-        if setting.respond_to?(:handle) and not options[:dont_trigger_handles]
-            setting.handle(value)
-        end
+        value = setting.munge(value) if setting.respond_to?(:munge)
+        setting.handle(value) if setting.respond_to?(:handle) and not options[:dont_trigger_handles]
         if ReadOnly.include? param and type != :mutable_defaults
             raise ArgumentError,
                 "You're attempting to set configuration parameter $#{param}, which is read-only."
@@ -549,9 +539,7 @@ class Puppet::Util::Settings
             name = name.to_sym
             hash[:name] = name
             hash[:section] = section
-            if @config.include?(name)
-                raise ArgumentError, "Parameter #{name} is already defined"
-            end
+            raise ArgumentError, "Parameter #{name} is already defined" if @config.include?(name)
             tryconfig = newsetting(hash)
             if short = tryconfig.short
                 if other = @shortnames[short]
@@ -682,9 +670,7 @@ if @config.include?(:run_mode)
                 # Look for the value.  We have to test the hash for whether
                 # it exists, because the value might be false.
                 @sync.synchronize do
-                    if @values[source].include?(param)
-                        throw :foundval, @values[source][param]
-                    end
+                    throw :foundval, @values[source][param] if @values[source].include?(param)
                 end
             end
             throw :foundval, nil
@@ -749,9 +735,7 @@ if @config.include?(:run_mode)
 
         Puppet::Util::SUIDManager.asuser(*chown) do
             mode = obj.mode || 0640
-            if args.empty?
-                args << "w"
-            end
+            args << "w" if args.empty?
 
             args << mode
 
@@ -768,9 +752,7 @@ if @config.include?(:run_mode)
         file = value(get_config_file_default(default).name)
         tmpfile = file + ".tmp"
         sync = Sync.new
-        unless FileTest.directory?(File.dirname(tmpfile))
-            raise Puppet::DevError, "Cannot create #{file}; directory #{File.dirname(file)} does not exist"
-        end
+        raise Puppet::DevError, "Cannot create #{file}; directory #{File.dirname(file)} does not exist" unless FileTest.directory?(File.dirname(tmpfile))
 
         sync.synchronize(Sync::EX) do
             File.open(file, ::File::CREAT|::File::RDWR, 0600) do |rf|
@@ -806,9 +788,7 @@ if @config.include?(:run_mode)
             raise ArgumentError, "Unknown default #{default}"
         end
 
-        unless obj.is_a? FileSetting
-            raise ArgumentError, "Default #{default} is not a file"
-        end
+        raise ArgumentError, "Default #{default} is not a file" unless obj.is_a? FileSetting
 
         return obj
     end
@@ -824,9 +804,7 @@ if @config.include?(:run_mode)
 
             if user = setting.owner and user != "root" and catalog.resource(:user, user).nil?
                 resource = Puppet::Resource.new(:user, user, :parameters => {:ensure => :present})
-                if self[:group]
-                    resource[:gid] = self[:group]
-                end
+                resource[:gid] = self[:group] if self[:group]
                 catalog.add_resource resource
             end
             if group = setting.group and ! %w{root wheel}.include?(group) and catalog.resource(:group, group).nil?
@@ -859,9 +837,7 @@ if @config.include?(:run_mode)
                 if str =~ /^\s*(\w+)\s*=\s*([\w\d]+)\s*$/
                     param, value = $1.intern, $2
                     result[param] = value
-                    unless [:owner, :mode, :group].include?(param)
-                        raise ArgumentError, "Invalid file option '#{param}'"
-                    end
+                    raise ArgumentError, "Invalid file option '#{param}'" unless [:owner, :mode, :group].include?(param)
 
                     if param == :mode and value !~ /^\d+$/
                         raise ArgumentError, "File modes must be numbers"
diff --git a/lib/puppet/util/settings/setting.rb b/lib/puppet/util/settings/setting.rb
index 6f8e39209..c3a037102 100644
--- a/lib/puppet/util/settings/setting.rb
+++ b/lib/puppet/util/settings/setting.rb
@@ -37,16 +37,12 @@ class Puppet::Util::Settings::Setting
 
         args.each do |param, value|
             method = param.to_s + "="
-            unless self.respond_to? method
-                raise ArgumentError, "#{self.class} does not accept #{param}"
-            end
+            raise ArgumentError, "#{self.class} does not accept #{param}" unless self.respond_to? method
 
             self.send(method, value)
         end
 
-        unless self.desc
-            raise ArgumentError, "You must provide a description for the #{self.name} config option"
-        end
+        raise ArgumentError, "You must provide a description for the #{self.name} config option" unless self.desc
     end
 
     def iscreated
@@ -71,9 +67,7 @@ class Puppet::Util::Settings::Setting
 
     # short name for the celement
     def short=(value)
-        if value.to_s.length != 1
-            raise ArgumentError, "Short names can only be one character."
-        end
+        raise ArgumentError, "Short names can only be one character." if value.to_s.length != 1
         @short = value.to_s
     end
 
@@ -82,9 +76,7 @@ class Puppet::Util::Settings::Setting
         str = @desc.gsub(/^/, "# ") + "\n"
 
         # Add in a statement about the default.
-        if defined?(@default) and @default
-            str += "# The default value is '#{@default}'.\n"
-        end
+        str += "# The default value is '#{@default}'.\n" if defined?(@default) and @default
 
         # If the value has not been overridden, then print it out commented
         # and unconverted, so it's clear that that's the default and how it
diff --git a/lib/puppet/util/storage.rb b/lib/puppet/util/storage.rb
index 974d56f56..8ca1881ed 100644
--- a/lib/puppet/util/storage.rb
+++ b/lib/puppet/util/storage.rb
@@ -47,9 +47,7 @@ class Puppet::Util::Storage
         Puppet.settings.use(:main) unless FileTest.directory?(Puppet[:statedir])
 
         unless File.exists?(Puppet[:statefile])
-            unless defined?(@@state) and ! @@state.nil?
-                self.init
-            end
+            self.init unless defined?(@@state) and ! @@state.nil?
             return
         end
         unless File.file?(Puppet[:statefile])
@@ -87,9 +85,7 @@ class Puppet::Util::Storage
     def self.store
         Puppet.debug "Storing state"
 
-        unless FileTest.exist?(Puppet[:statefile])
-            Puppet.info "Creating state file #{Puppet[:statefile]}"
-        end
+        Puppet.info "Creating state file #{Puppet[:statefile]}" unless FileTest.exist?(Puppet[:statefile])
 
         Puppet::Util.benchmark(:debug, "Stored state") do
             Puppet::Util::FileLocking.writelock(Puppet[:statefile], 0660) do |file|
diff --git a/lib/puppet/util/subclass_loader.rb b/lib/puppet/util/subclass_loader.rb
index 80a3672c9..505c4b0a5 100644
--- a/lib/puppet/util/subclass_loader.rb
+++ b/lib/puppet/util/subclass_loader.rb
@@ -15,9 +15,7 @@ module Puppet::Util::SubclassLoader
     # The hook method that sets up subclass loading.  We need the name
     # of the method to create and the path in which to look for them.
     def handle_subclasses(name, path)
-        unless self.is_a?(Class)
-            raise ArgumentError, "Must be a class to use SubclassLoader"
-        end
+        raise ArgumentError, "Must be a class to use SubclassLoader" unless self.is_a?(Class)
         @subclasses = []
 
                     @loader = Puppet::Util::Autoload.new(
@@ -41,9 +39,7 @@ module Puppet::Util::SubclassLoader
 
                 # Now make the method that returns this subclass.  This way we
                 # normally avoid the method_missing method.
-                if c and ! respond_to?(subname)
-                    define_method(subname) { c }
-                end
+                define_method(subname) { c } if c and ! respond_to?(subname)
             end
             return c
         end
@@ -76,9 +72,7 @@ module Puppet::Util::SubclassLoader
 
     # Retrieve or calculate a name.
     def name(dummy_argument=:work_arround_for_ruby_GC_bug)
-        unless defined?(@name)
-            @name = self.to_s.sub(/.+::/, '').intern
-        end
+        @name = self.to_s.sub(/.+::/, '').intern unless defined?(@name)
 
         return @name
     end
diff --git a/lib/puppet/util/tagging.rb b/lib/puppet/util/tagging.rb
index 51cff9520..1f185e42c 100644
--- a/lib/puppet/util/tagging.rb
+++ b/lib/puppet/util/tagging.rb
@@ -36,9 +36,7 @@ module Puppet::Util::Tagging
 
         return if tags.nil? or tags == ""
 
-        if tags.is_a?(String)
-            tags = tags.strip.split(/\s*,\s*/)
-        end
+        tags = tags.strip.split(/\s*,\s*/) if tags.is_a?(String)
 
         tags.each do |t|
             tag(t)