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]

2 files changed, 8 insertions, 24 deletions

diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 98a380e87..12bbffcc5 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -70,9 +70,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     # * Add any aliases that make sense for the resource (e.g., name != title)
     def add_resource(*resources)
         resources.each do |resource|
-            unless resource.respond_to?(:ref)
-                raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}"
-            end
+            raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref)
         end.each { |resource| fail_on_duplicate_type_and_title(resource) }.each do |resource|
             title_key = title_key_for_ref(resource.ref)
 
@@ -89,9 +87,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
 
             add_vertex(resource)
 
-            if @relationship_graph
-                @relationship_graph.add_vertex(resource)
-            end
+            @relationship_graph.add_vertex(resource) if @relationship_graph
 
             yield(resource) if block_given?
         end
@@ -237,16 +233,12 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
             end
             bucket = tmp || bucket
             if child = target.to_trans
-                unless bucket
-                    raise "No bucket created for #{source}"
-                end
+                raise "No bucket created for #{source}" unless bucket
                 bucket.push child
 
                 # It's important that we keep a reference to any TransBuckets we've created, so
                 # we don't create multiple buckets for children.
-                unless target.builtin?
-                    buckets[target.to_s] = child
-                end
+                buckets[target.to_s] = child unless target.builtin?
             end
         end
 
@@ -518,13 +510,9 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
         # If we've gotten this far, it's a real conflict
         msg = "Duplicate definition: #{resource.ref} is already defined"
 
-        if existing_resource.file and existing_resource.line
-            msg << " in file #{existing_resource.file} at line #{existing_resource.line}"
-        end
+        msg << " in file #{existing_resource.file} at line #{existing_resource.line}" if existing_resource.file and existing_resource.line
 
-        if resource.line or resource.file
-            msg << "; cannot redefine"
-        end
+        msg << "; cannot redefine" if resource.line or resource.file
 
         raise DuplicateResourceError.new(msg)
     end
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index a6cab8b8b..684d2c2f9 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -129,9 +129,7 @@ class Puppet::Resource::Type
         end
 
         array_class = Puppet::Parser::AST::ASTArray
-        unless self.code.is_a?(array_class)
-            self.code = array_class.new(:children => [self.code])
-        end
+        self.code = array_class.new(:children => [self.code]) unless self.code.is_a?(array_class)
 
         if other.code.is_a?(array_class)
             code.children += other.code.children
@@ -148,9 +146,7 @@ class Puppet::Resource::Type
         resource_type = type == :hostclass ? :class : :node
 
         # Make sure our parent class has been evaluated, if we have one.
-        if parent and ! scope.catalog.resource(resource_type, parent)
-            parent_type.mk_plain_resource(scope)
-        end
+        parent_type.mk_plain_resource(scope) if parent and ! scope.catalog.resource(resource_type, parent)
 
         # Do nothing if the resource already exists; this makes sure we don't
         # get multiple copies of the class resource, which helps provide the